This patch makes the unit_length and header_length fields of line tables
optional. yaml2obj is able to infer them for us.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/ObjectYAML/DWARFEmitter.cpp | ||
---|---|---|
508 | Could you add a TODO somewhere in this function saying that this doesn't support DWARF v5 yet, please? | |
510 | ||
517–518 | 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. | |
llvm/test/tools/yaml2obj/ELF/DWARF/debug-line.yaml | ||
483 | 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). | |
489 | 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. | |
489 | Never mind you fixed it whilst I was looking. | |
518 | Nit: delete the extra DWARF: | |
528 | I would suggest a non-standard number of lengths, to show that the header length isn't hard-coded. | |
529 | Similarly, I'd add an include directory here, to show that the length properly includes this field. |
LGTM, with nit.
llvm/lib/ObjectYAML/DWARFEmitter.cpp | ||
---|---|---|
526 | 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? |
Could you add a TODO somewhere in this function saying that this doesn't support DWARF v5 yet, please?