[PATCH] D55629: [elfabi] Add support for reading DT_SONAME from binaries

[Jake Ehrlich via Phabricator via llvm-commits]

jakehehrlich added inline comments.


================
Comment at: llvm/tools/llvm-elfabi/ELFObjHandler.cpp:68-75
+  if (!Dyn.StrTabOffset.hasValue()) {
+    return createError(
+        "Couldn't locate dynamic string table (no DT_STRTAB entry)");
+  }
+  if (!Dyn.StrSize.hasValue()) {
+    return createError(
+        "Couldn't determine dynamic string table size (no DT_STRSZ entry)");
----------------
If they're never supposed to be missing, does it make sense for them to be Optional?


================
Comment at: llvm/tools/llvm-elfabi/ELFObjHandler.cpp:111
+  if (DynEnt.SONameOffset.hasValue()) {
+    DestStub->SoName = std::string(DynStrPtr + *DynEnt.SONameOffset);
+  }
----------------
nit: Don't fret over this too much but It would be nice if there were a way to ensure that when looking for the null terminator this didn't fall off the end of the string table. As is if the string table happens to be erroneous and not contain a null terminator, this will segfault. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55629