[PATCH] D54451: [libObject] Fix getDesc for Elf_Note_Impl
Jake Ehrlich via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 12 15:42:26 PST 2018
jakehehrlich added a comment.
In https://reviews.llvm.org/D54451#1296292, @Bigcheese wrote:
> Overall this looks fine, but can you add a test of something that this fixes? I'm rather surprised that no tests fail now.
Yeah, I think I spotted bug that could have been spotted in an integration test.
As far as I am aware there are no unit tests for libObject. The reason no tests fail makes sense as well. llvm-readobj was the only user of this and everywhere it used it it did something like the following
std::string(reinterpret_cast<const char *>(Words.data()),
Words.size())};
Notice that `Words.size()` is still 16 so this conversion actually works and fixes the faulty bug.
I think the one spot where an issue could have been noticed is in `getGNUAbiTag` where despite the size being say 1-3 Elf_Word's the size in bytes would have been between 4-12 all of which would not have triggered the error. I think checking for that error in an integration test would suffice but the code could change in a way that made that test stale.
Repository:
rL LLVM
https://reviews.llvm.org/D54451
More information about the llvm-commits
mailing list