[PATCH] D95158: Use DataExtractor to decode SLEB128 in android_relas.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 25 00:17:20 PST 2021


jhenderson added reviewers: MaskRay, grimar.
jhenderson added a comment.

So, if the previous build bot failure was due to an unhandled `Error`, that means that this error path doesn't have any testing. Basically, there needs to be a test for each individual `getSLEB128` call, in case the section happens to be corrupted at that specific point (e.g. a truncated SLEB or one that goes outside the uint64_t range). Given that this is in the Object library, it would also probably make most sense for those tests to be gtest unit tests rather than lit stuff.

If I'm following things correctly, I don't think you've made the testing situation any worse, so if you'd prefer to defer that to a separate change, that's fine by me.



================
Comment at: llvm/lib/Object/ELF.cpp:381
+  DataExtractor Data(Content, isLE(), ELFT::Is64Bits ? 8 : 4);
+  DataExtractor::Cursor Cur(4);
 
----------------
Add a comment saying what the `4` here represents, i.e. something like `DataExtractor::Cursor Cur(/*Offset=*/4);`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95158/new/

https://reviews.llvm.org/D95158



More information about the llvm-commits mailing list