[PATCH] D64281: [Object] Create MutableELFObject Class for Doing Mutations on ELFObjectFiles [Part 1]

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/include/llvm/Object/MutableELFObject.h:113
+    NewType &makeMutable(uint64_t Index, Args &&... Arguments) {
+      assert(Index < Mappings.size() && "Out of bounds");
+      if (Mappings[Index].Type == MappingType::New)
----------------
jhenderson wrote:
> MaskRay wrote:
> > I think such `Out of bounds` assertions are probably not very necessary. They will certainly segfault if the user does have out-of-bounds accesses.
> Won't that entirely depend on how the memory is laid out, compiler-generated checks etc? A segmentation fault hardly provides any useful information, whereas at least the assertion shows what's going wrong in builds with assertions enabled...
I think if the user of this library makes an out-of-bounds access, the program will very quickly segfault with a clear stack trace (symbolization can happen with a symbol table, even if debug info is absent) that the problem is related to the use of MutableELFObject.

There are lots of ways to detect errors: -D_GLIBCXX_DEBUG / asan / ...

We can add some assert() in some tricky places (they also serve as documentation of some invariants) but here I think it is probably not very necessary.


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

https://reviews.llvm.org/D64281