Skip to content

[lldb][test] Fix flaky DIL array subscript test by reducing array indexes #141738

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 28, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ def expect_var_path(self, expr, compare_to_framevar=False, value=None, type=None
self.runCmd("settings set target.experimental.use-DIL true")
self.assertEqual(value_dil.GetValue(), value_frv.GetValue())

# int_arr[100] sometimes points to above the stack region, fix coming soon.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does frame var suffer from the same problem? If not, should DIL handle things differently?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean it should be able to say:

int_arr[100] = <unavailable>;

Like registers can do?

I'll check what frame var does.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tbh I'm not even sure what DIL is :) So feel free to educate me.

Copy link
Member

@Michael137 Michael137 May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summoning @labath @kuilpd who've been much more involved in DIL

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the point of this test is to check that the expression really allows you to go beyond the array bounds, so fixing this by ensuring the memory at that address exists is the right thing to do. However, I think the index 100 is unnecessarily big. I think we could just change the test to use indexes 3 and 10 (a "one past" and "reasonably beyond" value), and then you don't need to write the very long comment.

Printing <unavailable> or similar when this indexing lands you into unaddressable memory would be nice (maybe it happens already -- I don't know), but that's a different test.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh but the long comments are the most fun :)

I agree though, I'll find a smaller out of bounds that is consistently within the stack.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does frame var suffer from the same problem?

(lldb) frame var int_arr[200]
(int) int_arr[200] = <read memory from 0xc812500160 failed (0 of 4 bytes read)>

With DIL on:

(lldb) settings set target.experimental.use-DIL true
(lldb) frame var int_arr[200]
(int) int_arr[200] = <read memory from 0xc812500160 failed (0 of 4 bytes read)>

Which I think means both are reporting it the same way, and that way seems fine to me.

@skipIfWindows
def test_subscript(self):
self.build()
lldbutil.run_to_source_breakpoint(
Expand Down Expand Up @@ -53,9 +51,10 @@ def test_subscript(self):
# Both typedefs and refs
self.expect("frame var 'td_int_arr_ref[td_int_idx_1_ref]'", error=True)

# Test for index out of bounds.
self.expect_var_path("int_arr[42]", True, type="int")
self.expect_var_path("int_arr[100]", True, type="int")
# Test for index out of bounds. 1 beyond the end.
self.expect_var_path("int_arr[3]", True, type="int")
# Far beyond the end (but not far enough to be off the top of the stack).
self.expect_var_path("int_arr[10]", True, type="int")

# Test address-of of the subscripted value.
self.expect_var_path("*(&int_arr[1])", value="2")
Expand Down
Loading