[llvm] [llvm][ELF]Add Shdr check for getBuildID (PR #126537)
James Henderson via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 21 01:03:50 PST 2025
https://github.com/jh7370 commented:
Having sat and looked at the test and the original issue, I'm not certain whether a lit test is particularly useful. I could see a place for it, but I think more useful would be a gtest unit test that directly tests the `getBuildID` function instead.
As things stand, the lit test you've written will cover the original issue (though it needs some improvements, I've commented on them anyway), but as I understand it, there's no particular reason why `-D` should trigger the `getBuildID` function and it's just an implementation detail of how the object file is read internally.
As `BuildID.cpp` doesn't currently have a corresponding test file in `llvm/test/unittests/Object`, I'd add a `BuildIDTest.cpp` file to that project, with just some tests to show the behaviour of `getBuildID`. I don't think (in this PR) you necessarily need to test every code path in that function, but you do need to test the ones you've modified. There are examples of how to use YAML ELF inputs, like the one in the lit test you've written, in `ELFObjectFileTest.cpp`, so those are probably good starting points on how to write these tests. Essentially, the behaviours you need to cover, in my opinion, are:
1) That the BuildID can be looked up from a section header, if there is no program header.
2) That the code handles a malformed program header that points at data outside the file.
3) That the code handles a malformed section header that points at data outside the file.
https://github.com/llvm/llvm-project/pull/126537
More information about the llvm-commits
mailing list