Collecting address ranges in DWARFUnit::collectAddressRanges.

​Hi !

I have a question about code used for collecting ranges. I was trying to fix PR35199.

Its issue that it calls unreachable when calls DWARFObject::getFileName().

We do not implement this method in LLD and it fails.

Issue appears when we start to provide .debug_str

section to DWARF parser. And it is relative to following code (lines 335-339):

https://github.com/llvm-mirror/llvm/blob/master/lib/DebugInfo/DWARF/DWARFUnit.cpp#L335

Object I used for debugging parser is following:

t.ii:
int a;

clang++ -gsplit-dwarf -c t.ii -o t.o

When parser tries to read this object it attemps to extract address ranges from dwo. 

When we don't give it .debug_str it fails to extract dwo name, exits early and "works" fine, but with

.debug_str provided it tries to take ranges from there, tries to get DWO name to open it and fails. 

Though I did not find why it tries to scan .dwo. I tried to comment these lines (335-339) and no tests

were failing for me, at least I tried running tests from \llvm\test\DebugInfo and

running DebugInfoDWARFTests. It seems to me these lines are either untested or useless (?).

So my question is probably next: what is the case when we need to scan .dwo for extracting ranges,

should not address data be accessible from main object always ?

I wonder if these lines really do useful job ?​

​Hi !

I have a question about code used for collecting ranges. I was trying to fix PR35199.

Its issue that it calls unreachable when calls DWARFObject::getFileName().

We do not implement this method in LLD and it fails.

Issue appears when we start to provide .debug_str

section to DWARF parser. And it is relative to following code (lines 335-339):

https://github.com/llvm-mirror/llvm/blob/master/lib/DebugInfo/DWARF/DWARFUnit.cpp#L335

Object I used for debugging parser is following:

t.ii:
int a;

clang++ -gsplit-dwarf -c t.ii -o t.o

When parser tries to read this object it attemps to extract address ranges from dwo. 

When we don't give it .debug_str it fails to extract dwo name, exits early and "works" fine, but with

.debug_str provided it tries to take ranges from there, tries to get DWO name to open it and fails. 

Though I did not find why it tries to scan .dwo. I tried to comment these lines (335-339) and no tests

were failing for me, at least I tried running tests from \llvm\test\DebugInfo and

running DebugInfoDWARFTests. It seems to me these lines are either untested or useless (?).

So my question is probably next: what is the case when we need to scan .dwo for extracting ranges,

should not address data be accessible from main object always ?

I wonder if these lines really do useful job ?​

There’s no requirement that DW_AT_ranges (or high/low_pc) appear on the skeleton CU rather than the DWO CU. So it’s quite possible that to get the address ranges covered by the CU one would need to look in the DWO, I think?

Is that not correct/have I misunderstood something there?

Though, admittedly - the presence of debug_str shouldn’t be the deciding factor that leads to an error. If it’s an error not to find the .dwo file, it seems it should be an error not to find the debug_str that’s needed to get there… but I haven’t looked at the code to check - maybe this does make sense/perhaps there are aspects I haven’t considered.

  • Dave

There’s no requirement that DW_AT_ranges (or high/low_pc) appear on the skeleton CU rather than the DWO CU. So it’s quite possible that to get the address ranges covered by the CU one would need to look in the DWO, I think?

Is that not correct/have I misunderstood something there?

The DWO isn’t supposed to contain addresses (because it isn’t supposed to contain relocations). In DWARF 5 the DWO can have FORM_addrx references to the .debug_addr section in the main .o file, which allows the DWO to contain DIEs/attributes that should have address values, because the actual address values are still in the .o file; but before that anything that’s an address really can’t go into the DWO.

Unless I am also misunderstanding something…

–paulr

There’s no requirement that DW_AT_ranges (or high/low_pc) appear on the skeleton CU rather than the DWO CU. So it’s quite possible that to get the address ranges covered by the CU one would need to look in the DWO, I think?

Is that not correct/have I misunderstood something there?

The DWO isn’t supposed to contain addresses (because it isn’t supposed to contain relocations). In DWARF 5 the DWO can have FORM_addrx references to the .debug_addr section in the main .o file, which allows the DWO to contain DIEs/attributes that should have address values, because the actual address values are still in the .o file; but before that anything that’s an address really can’t go into the DWO.

DWOs didn’t exist (in a standard form) before DWARF 5, right? Insofar as they did exist (in a non-standard form) they have always supported FORM_addr_index to reference addresses in .debug_addr in the main .o.

But the low_pc/high_pc/ranges attributes would appear in the .dwo, using a FORM_addr_index/FORM_addrx - so if you want to collect the address range of a CU you might need to load the .dwo to do so. That’s the crux of what I was getting at: To get the address range a CU covers, you may need to read the .dwo.

Ah right, the GNU forms. Yes, I did forget about those. So, reading the DWO units can be necessary.

–paulr

David Blaikie via llvm-dev <llvm-dev@lists.llvm.org> writes:

t.diff (716 Bytes)

Totally fair call, sorry it took me a while to come back around to this - added in r324702

Totally fair call, sorry it took me a while to come back around to this - added in r324702

Thanks ! Have a question: does it need “requres shell” ?

I tried without and it worked for me under windows (had to change check to CHECK: .{{\|/}}test.cpp:2:51 though).

And I see nothing special probably that might require shell I think. We use cd/rm/cp in LLD testcases

without requiring shell.

(also noticed other tests do that, so it seems consistent, though did not look deeply)

George.

Nah, doesn’t look like it. Removed it (& the place I copied it from) in: r324738

Thanks!

I expect r324738 will break on windows,

as I mentioned I had to change

CHECK: ./test.cpp:2:51

to

CHECK: {\|/}}test.cpp:2:51

Ah, sorry, thanks - removed the “./” prefix entirely. Hope that works - if you happen to test it & find it doesn’t, please let me know :slight_smile:

Ah, sorry, thanks - removed the “./” prefix entirely. Hope that works - if you happen to test it & find it doesn’t, please let me know :slight_smile:

Thanks ! Both tests pass fine for me now.

(I was unsure if “./” is a part of testcase that is useful or not to remove it) :slight_smile:

George.