This is an archive of the discontinued LLVM Phabricator instance.

[DWARFYAML] Make the unit_length and header_length fields optional.

Authored by Higuoxing on Aug 25 2020, 11:57 PM.



This patch makes the unit_length and header_length fields of line tables
optional. yaml2obj is able to infer them for us.

Diff Detail

Event Timeline

Higuoxing created this revision.Aug 25 2020, 11:57 PM
Higuoxing requested review of this revision.Aug 25 2020, 11:57 PM

The added test case is verified using llvm-dwarfdump.

Fix failed test.

jhenderson added inline comments.Aug 26 2020, 12:43 AM

Could you add a TODO somewhere in this function saying that this doesn't support DWARF v5 yet, please?


I'd get rid of the auto here and below in the for loops. I think it's quite important we know what type they are because of how we write things.


It may be worth having a DWARF64 version of this case too (i.e. where the format is explicitly specified as such). You could add it as a separate subsequent table to this same case, I think (which also then shows that the lengths are calculated independently of one another).


The unit_length field looks incorrect. It is currently apparently 0x39, yet the end of the things you check is at offset 0x28, so I'd expect a value of 0x24. The header_length looks right though.


Never mind you fixed it whilst I was looking.


Nit: delete the extra DWARF:


I would suggest a non-standard number of lengths, to show that the header length isn't hard-coded.


Similarly, I'd add an include directory here, to show that the length properly includes this field.

Higuoxing updated this revision to Diff 287864.Aug 26 2020, 1:36 AM
Higuoxing marked 9 inline comments as done.

Address review comments.

jhenderson accepted this revision.Aug 26 2020, 2:07 AM

LGTM, with nit.


No need for the cast, since OpcodeLength is uint8_t. Does that apply to any of the other writeInteger calls elsewhere in this function too?

This revision is now accepted and ready to land.Aug 26 2020, 2:07 AM
Higuoxing updated this revision to Diff 287893.Aug 26 2020, 3:05 AM
Higuoxing marked an inline comment as done.

Address comments.

Thanks for reviewing!

jhenderson accepted this revision.Aug 26 2020, 3:06 AM