This is an archive of the discontinued LLVM Phabricator instance.

[dsymutil] Relocate DW_TAG_label
ClosedPublic

Authored by JDevlieghere on Mar 29 2021, 12:37 PM.

Details

Summary

dsymutil is not relocating the DW_AT_low_pc for a DW_TAG_label. This
patch fixes that and adds a test.

Diff Detail

Event Timeline

JDevlieghere created this revision.Mar 29 2021, 12:37 PM
JDevlieghere requested review of this revision.Mar 29 2021, 12:37 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 29 2021, 12:37 PM
aprantl accepted this revision.Mar 29 2021, 2:59 PM

Seems legit.

This revision is now accepted and ready to land.Mar 29 2021, 2:59 PM
This revision was landed with ongoing or failed builds.Mar 29 2021, 3:53 PM
This revision was automatically updated to reflect the committed changes.

Hi Jonas,

The label2.test failed on Windows in the line 19:

CHECK-NEXT:   DW_AT_decl_file       ("/tmp/label/label.c")

The generated DW_AT_decl_file On Windows is:

CHECK-NEXT:   DW_AT_decl_file       ("/tmp/label\label.c")   <--- Windows path separator

Could you please look at the test and make it pass on Windows as well?

For example, changing the Check line to:

CHECK-NEXT:   DW_AT_decl_file       ("/tmp/label{{[/\\]}}label.c")

Thanks,
Maggie

thakis added a subscriber: thakis.Mar 30 2021, 4:44 AM

Looks like this breaks tests on windows: http://45.33.8.238/win/36016/step_11.txt

Please take a look, and revert for now if it takes a while to fix.

Hi Jonas and Nico,

I (with Doug’s help) have committed the patch (
https://github.com/llvm/llvm-project/commit/c5109d3c7936d9c78216e0cc722c1ce0ea9246ec)
since the test failure blocked the local build. Please modify or revert the
patch if there is a better fix.

Thanks,

Apologies, the Windows issue must've been buried among the dwarfdump vs llvm-dwarfdump build failures. Thank you for fixing that!

Any uses of low_pc other than the CU's low_pc that you know of that shouldn't be handled in this way? Maybe the positive list (inlined, lexical, label) should be removed?

(what about generalizing further to any address class variable? In DWARFv5 that's as easy as DW_FORM_addr and DW_FORM_addrx* forms, I think?)