[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