This patch helps add support for emitting the .debug_str_offsets section
to yaml2elf.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/ObjectYAML/DWARFEmitter.cpp | ||
---|---|---|
434 | 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. | |
llvm/test/tools/yaml2obj/ELF/DWARF/debug-str-offsets.yaml | ||
47–48 | I'd explicitly use the full 4 bytes here (and 8 bytes in the DWARF64 version). | |
99 | Perhaps omit Offsets entirely here. | |
121 | Perhaps it would be worth a test case for there being no tables at all, i.e: DWARF: 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. |
Address comments.
llvm/lib/ObjectYAML/DWARFEmitter.cpp | ||
---|---|---|
434 |
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. | |
llvm/test/tools/yaml2obj/ELF/DWARF/debug-str-offsets.yaml | ||
121 | Yes, this test is missing in other tests. I'll add them. Thanks! |
llvm/lib/ObjectYAML/DWARFEmitter.cpp | ||
---|---|---|
434 | 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. | |
llvm/test/tools/yaml2obj/ELF/DWARF/debug-str-offsets.yaml | ||
47–48 | I meant something like 0x12345678 and 0x87654321, i.e. so that all the bytes contain non-zero. | |
122 | Hmmm... I would expect the section header to be emitted, but for the section to be empty in this context. |
Address comments.
llvm/test/tools/yaml2obj/ELF/DWARF/debug-str-offsets.yaml | ||
---|---|---|
122 | 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. |
llvm/test/tools/yaml2obj/ELF/DWARF/debug-str-offsets.yaml | ||
---|---|---|
125–133 | 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. |
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.