[PATCH] D124865: [AIX] support read global symbol of big archive

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/Object/Archive.cpp:1096-1098
+  } else if (kind() == K_AIXBIG)
+    buf = getStringTable().begin();
+  else {
----------------
Once you start using braces for an if/else sequence, use them consistently throughout that sequence.


================
Comment at: llvm/lib/Object/Archive.cpp:1178
+    Err = malformedError(
+        "malformed AIX big archive: global symbol tables offset \"" +
+        RawOffset + "\" is not a number");
----------------



================
Comment at: llvm/lib/Object/Archive.cpp:1181-1183
+    const char *GlobSymTblLoc = Data.getBufferStart() + GlobSymOffset;
+    const BigArMemHdrType *GlobalSymHdr =
+        reinterpret_cast<const BigArMemHdrType *>(GlobSymTblLoc);
----------------
You need to check that the offset falls within the file, and that the file is big enough to hold the header and table or you risk reading invalid memory.


================
Comment at: llvm/lib/Object/Archive.cpp:1188
+      Err = malformedError(
+          "malformed AIX big archive: global symbol tables size \"" +
+          RawOffset + "\" is not a number");
----------------



================
Comment at: llvm/lib/Object/Archive.cpp:1192
+    unsigned SymNum = getNumberOfSymbols();
+    uint64_t SymbolTableStringSize = Size - 8 * (SymNum + 1);
+    StringTable =
----------------
Could you factor out `8 * (SymNum + 1)` into a separate variable, please, rather than repeat it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124865