Add support for XCOFF DWARF for llvm-dwarfdump tool.
The patch is made by @hubert.reinterpretcast
Differential D97186
[XCOFF][llvm-dwarfdump] support llvm-dwarfdump for XCOFF DWARF shchenz on Feb 22 2021, 12:55 AM. Authored by
Details
Add support for XCOFF DWARF for llvm-dwarfdump tool. The patch is made by @hubert.reinterpretcast
Diff Detail
Unit Tests Event TimelineComment Actions I think what we usually do for test case is to upstream some pre-compiled binaries for testing purpose. But that's not the way community preferred anyway because those binaries are baked, and might be hard to reproduce later. Given this patch is small and pretty self-explanatory, I'm okay with skipping the test and let D97184 to "test" it. Comment Actions I would prefer you use a checked in binary to test. I think it's reasonable to create a test case based on the dwarf emission support once you're convinced that's correct and go from there. For updates you can specify what the testcase is as a comment in the check. This is similar to a number of other tests FWIW :) Comment Actions @Higuoxing added more support to yaml2obj last year for DWARF emission via YAML. Maybe it would be best to wait on basic XCOFF yaml2obj support to be finished, add DWARF emission support to that, and then use yaml2obj to generate the DWARF output as required? Also, the pre-merge checks claim your tests are failing. Finally, how does this actually add support for llvm-dwarfdump + XCOFF? It's not particularly obvious to me, but I'm not all that familiar about what llvm-dwarfdump requires for this support.
Comment Actions I'm very happy to help implement DWARF support for yaml2xcoff (if needed), but I'm a little busy these days. I think I'm able to work on it in the next few months :-) Comment Actions
I totally agree with your suggestions, committing an object directly is not so good.
llvm-dwarfdump calls object::createBinary( to recognize a specific form object, we already added XCOFF support in object::createBinary()-> identify_magic(), so in this patch, we only need to override some functions called by DWARFContext class. llvm-dwarfdump calls DWARFContext to dump DWARF info.
Thanks for your kindly help. If possible, could you please help to review the XCOFF yam2obj support patch D95505? Thanks @Higuoxing
Comment Actions Thanks for review @echristo
Done
Yeah, sort of auto-generating. But do we need to loosen up the checks? This DWARF info should be settled as the result of the tests comes from object files.
Comment Actions Yeah, it's tough. Right now if anything changes even slightly in the output the test will fail, but isn't necessarily everything you need to be checking if that makes sense? That said, it's probably on the small order of update probability so it's fine if you'd like to leave it this way and we can update it later if it becomes an issue. Accepting (though fix any other feedback first). -eric Comment Actions Yeah. Agree, at least there is no need for the requirement of the DIE attributes' order and the fixed offset in the .dwinfo sections. This will be too tough if we regenerate the object file for every new commit. But for this case, since we directly test the object files, so I think it should be ok. Yeah, we may still have some changes in llvm-dwarfdump tool which makes the output different, but that should be not often. Thanks for your review
|
This list is missing DWARFv5 section names. Does XCOFF not support those sections?