[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:1190-1191
+      Err = malformedError("global symbol table header at offset " +
+                           Twine(GlobSymOffset) + " and size " +
+                           Twine(sizeof(BigArMemHdrType)) +
+                           " is past the end of file");
----------------
These will print in decimal, but offsets are usually printed in hex. Use `utohexstr` instead (you may need to explicitly add a 0x prefix too, but I don't know). Also "is" -> "goes" in the message, because the thing in question may start within range, but not finish. "is" implies that ot starts past the end of the file.


================
Comment at: llvm/lib/Object/Archive.cpp:1207-1210
+    if (GlobalSymTblContentOffset + Size > BufferSize) {
+      Err = malformedError("global symbol table content at offset " +
+                           Twine(GlobalSymTblContentOffset) + " and size " +
+                           Twine(Size) + " is past the end of file");
----------------
Same comments as above.


================
Comment at: llvm/lib/Object/Archive.cpp:1208
+    unsigned SymNum = getNumberOfSymbols();
+    unsigned SizeOfSymOffSets = 8 * (SymNum + 1);
+    uint64_t SymbolTableStringSize = Size - SizeOfSymOffSets;
----------------
DiggerLin wrote:
> jhenderson wrote:
> > This is a shorter name (plus the "S" in "Offsets" shouldn't be capitalized).
> thanks
Not addressed?


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124865