DWARF 5 uses a 0-based index while previous versions use a 1-based index. Fix the verifier and add a test.
Details
Diff Detail
Unit Tests
Time | Test | |
---|---|---|
60,100 ms | x64 debian > MLIR.Examples/standalone::test.toy |
Event Timeline
llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | ||
---|---|---|
935 | Could we tie the valid values here to the values used to initialise FileIndex somehow, so that we only do this check once? uint32_t MinFileIndex = isDWARF5 ? 0 : 1; uint32_t FileIndex = MinFileIndex; ... << " (valid values are [" << MinFileIndex | |
llvm/test/tools/llvm-dwarfdump/X86/verify_dwarf5_debug_line.s | ||
1 ↗ | (On Diff #509562) | I'm not too familiar with the DWARF verifier, but does it require the full debug information? The reason I'm asking is because this file has a lot of stuff in it that is completely unrelated to the thing under test, and it would be nice to simplify it somehow (could the yaml2obj DWARF support be used, for example, if we only need the .debug_line section?). One idea, other than using yaml2obj, that I had was to use llvm-mc's debug data generation, so you'd only need an instruction or two and then use llvm-mc -g to generate the debug data. Caveat: I don't know much about how this works and whether you can generate the kind of data you need for this test. |
Address James' feedback:
- Reduce test case and use yaml2obj
- Avoid code duplication with MinFileIndex
Hmm, not quite what I had in mind when I suggested using yaml2obj. It appears that obj2yaml doesn't have proper support for debug_line, so you'd need to craft it by hand. My hope would be though that you'd be able to omit 95% of the rest of the YAML to look something like the examples in https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/yaml2obj/ELF/DWARF/debug-line.yaml. However, it turns out that the DWARF support in yaml2obj doesn't yet cover DWARF 5, so that's not going to work as intended. I think it would be useful for tasks like this, but I don't have the bandwidth to do that work myself unfortunately.
Apologies for the misdirection. A simple YAML block generated by obj2yaml like you've done doesn't really improve anything, so you were probably better off with the asm. Ideally, I'd like to see a hand-written .debug_line section in the asm, and no other DWARF secion (since they aren't needed for the test, as I understand it). However, if you'd prefer not to do that, I'd ask you to include instructions on how to create the asm (or YAML) that you do choose to use, along with a clear description of what is important about it, so that the test can be maintained easily going forwards.
Apologies for the misdirection. A simple YAML block generated by obj2yaml like you've done doesn't really improve anything, so you were probably better off with the asm. Ideally, I'd like to see a hand-written .debug_line section in the asm, and no other DWARF secion (since they aren't needed for the test, as I understand it). However, if you'd prefer not to do that, I'd ask you to include instructions on how to create the asm (or YAML) that you do choose to use, along with a clear description of what is important about it, so that the test can be maintained easily going forwards.
Gotcha, that all makes sense. Unfortunately I'm in the same boat as to bandwidth, but I've added DWARF 5 line table support to my mental list of fun weekend projects. I reduced the assembly test case further to take out all the other sections.
Could we tie the valid values here to the values used to initialise FileIndex somehow, so that we only do this check once?
Example: