This is an archive of the discontinued LLVM Phabricator instance.

[DWARFYAML] Implement the .debug_str_offsets section.

Authored by Higuoxing on Jul 15 2020, 12:21 AM.



This patch helps add support for emitting the .debug_str_offsets section
to yaml2elf.

Diff Detail

Event Timeline

Higuoxing created this revision.Jul 15 2020, 12:21 AM
jhenderson added inline comments.Jul 15 2020, 12:49 AM

A thought I've just had: what do you think should happen if a user specifies a 64-bit-using value for DWARF32? This may apply in one or two other places too.

I don't have any particular opinions here. Just wondering if you've considered it at all.


I'd explicitly use the full 4 bytes here (and 8 bytes in the DWARF64 version).


Perhaps omit Offsets entirely here.


Perhaps it would be worth a test case for there being no tables at all, i.e:

  debug_str_offsets: []

I'd expect an empty section to be emitted in this case.

This might also be a missing test case in some (most?) of the other sections.

Higuoxing updated this revision to Diff 278104.Jul 15 2020, 1:35 AM
Higuoxing marked 5 inline comments as done.

Address comments.


Just wondering if you've considered it at all.

Yes, I've considered it before.

At first, It seemed to me that the format should be inferred from the content. When the length is greater than UINT32_MAX or the offset is greater than UINT32_MAX, the DWARF64 should be specified automatically. However, this approach makes our implementation complicated. Besides, DWARF32 is the most commonly used format. I think it's not a good approach.

Another approach is to check whether the integer is able to be encoded using the given size and warn the user about it.


Yes, this test is missing in other tests. I'll add them. Thanks!

jhenderson added inline comments.Jul 15 2020, 1:42 AM

My instinct is to do nothing at all for now. If people find it causing problems, it's functionality that could be added later. I wouldn't bother with the warning - in most cases, people will never see the warning, due to yaml2obj being run as part of lit tests.


I meant something like 0x12345678 and 0x87654321, i.e. so that all the bytes contain non-zero.


Hmmm... I would expect the section header to be emitted, but for the section to be empty in this context.

Higuoxing updated this revision to Diff 278111.Jul 15 2020, 2:08 AM
Higuoxing marked 3 inline comments as done.

Address comments.


Oh, I misunderstood it. We can wrap std::vector<StringOffsetsTable> with an additional Optional layer (Optional<std::vector<StringOffsetsTable>>). We can add it for other DWARF sections as well.

jhenderson added inline comments.Jul 15 2020, 3:01 AM

I don't think you need to check the whole section header table in this case. Probably sufficient is just checking the .debug_str_offsets line. It might make more sense to use llvm-readobj style in that case.

Higuoxing updated this revision to Diff 278123.Jul 15 2020, 3:15 AM
Higuoxing marked an inline comment as done.

Address comments.

Thanks for reviewing!

jhenderson accepted this revision.Jul 15 2020, 3:19 AM



Nice solution!

This revision is now accepted and ready to land.Jul 15 2020, 3:19 AM
MaskRay accepted this revision.Jul 15 2020, 9:53 PM
This revision was automatically updated to reflect the committed changes.