Skip to content
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.

Sign up for GitHub

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

[llvm-gsymutil] Fix flaky test #123814

Merged
merged 1 commit into from
Jan 21, 2025
Merged

[llvm-gsymutil] Fix flaky test #123814

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 6 additions & 4 deletions llvm/test/tools/llvm-gsymutil/ARM_AArch64/macho-gsym-merged-callsites-dsym.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
## Test that reconstructs a dSYM file from YAML and generates a gsym from it. The gsym has callsite info and merged functions.

# TODO: Add line numbers instead of ':{{.}}' below - after introducing accurate DIE-based lookups: https://github.com/llvm/llvm-project/pull/123391

# RUN: split-file %s %t
# RUN: yaml2obj %t/merged_callsites.dSYM.yaml -o %t/merged_callsites.dSYM

Expand Down Expand Up @@ -61,12 +63,12 @@

# RUN: llvm-gsymutil %t/dwarf_call_sites_dSYM.gsym --merged-functions --address=0x000000010000035c --merged-functions-filter="function3_copy1" | FileCheck --check-prefix=CHECK-C3 %s
# CHECK-C3: Found 1 function at address 0x000000010000035c:
# CHECK-C3-NEXT: 0x000000010000035c: function3_copy1 + 16 @ /tmp/tst{{[/\\]}}out/merged_funcs_test.cpp:28
# CHECK-C3-NEXT: 0x000000010000035c: function3_copy1 + 16 @ /tmp/tst{{[/\\]}}out/merged_funcs_test.cpp:{{.}}
# CHECK-C3-NEXT: CallSites: function4_copy1

# RUN: llvm-gsymutil %t/dwarf_call_sites_dSYM.gsym --merged-functions --address=0x0000000100000340 --merged-functions-filter="function4_copy1" | FileCheck --check-prefix=CHECK-C4 %s
# CHECK-C4: Found 1 function at address 0x0000000100000340:
# CHECK-C4-NEXT: 0x0000000100000340: function4_copy1 + 8 @ /tmp/tst{{[/\\]}}out/merged_funcs_test.cpp:14
# CHECK-C4-NEXT: 0x0000000100000340: function4_copy1 + 8 @ /tmp/tst{{[/\\]}}out/merged_funcs_test.cpp:{{.}}

### ----------------------------------------------------------------------------------------------------------------------------------
### Resolve the 2nd call stack - the 2nd and 3rd addresses are the same but they resolve to a different function because of the filter
Expand All @@ -78,12 +80,12 @@

# RUN: llvm-gsymutil %t/dwarf_call_sites_dSYM.gsym --merged-functions --address=0x000000010000035c --merged-functions-filter="function3_copy2" | FileCheck --check-prefix=CHECK-C6 %s
# CHECK-C6: Found 1 function at address 0x000000010000035c:
# CHECK-C6-NEXT: 0x000000010000035c: function3_copy2 + 16 @ /tmp/tst{{[/\\]}}out/merged_funcs_test.cpp:28
# CHECK-C6-NEXT: 0x000000010000035c: function3_copy2 + 16 @ /tmp/tst{{[/\\]}}out/merged_funcs_test.cpp:{{.}}
# CHECK-C6-NEXT: CallSites: function4_copy2

# RUN: llvm-gsymutil %t/dwarf_call_sites_dSYM.gsym --merged-functions --merged-functions-filter="function4_copy2" --address=0x0000000100000340 | FileCheck --check-prefix=CHECK-C7 %s
# CHECK-C7: Found 1 function at address 0x0000000100000340:
# CHECK-C7-NEXT: 0x0000000100000340: function4_copy2 + 8 @ /tmp/tst{{[/\\]}}out/merged_funcs_test.cpp:14
# CHECK-C7-NEXT: 0x0000000100000340: function4_copy2 + 8 @ /tmp/tst{{[/\\]}}out/merged_funcs_test.cpp:{{.}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Hopefully we can eventually use [[#@LINE+N]] and --leading-lines to test these line numbers. Also, where does the /tmp/tst file path prefix come from?

https://llvm.org/docs/TestingGuide.html#extra-files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both the paths and line numbers come from merged_callsites.dSYM.yaml - which is binary encoded. So the input is hard-coded in binary format effectively.
[[#@LINE+N]] won't work in this case - we can update the lines in the test but the line numbers to test won't change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but presumably those line numbers come from merged_funcs_test.cpp? Then we can in fact use the [[#@LINE+N]] trick if we make sure to use --leading-lines. We may have to regenerate the files so they are correct, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

And if we move #--- merged_funcs_test.cpp towards the beginning of the file it makes it less likely that the line numbers will change when we update the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see - one issue is that this test isn't really easily manually updatable because of hardcoded values in callsites.yaml. So using [[#@LINE+N]] we could have that part auto-match but we still have to manually edit callsites.yaml.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any line numbers in callsites.yaml. What would you have to update? Is there anyway it could be generated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The callsites.yaml has return_offset: [0x10] which is a binary offset of a callsite into the function. There's no way to generate this right now - it's supposed to be test data generated from looking at the binary.

Copy link
Contributor

Choose a reason for hiding this comment

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

So it sounds like callsites.yaml needs to be changed every time we regenerate the input files, which seems to be an orthogonal problem to using @LINE+N.



#--- merged_funcs_test.cpp
Expand Down
Loading