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

[Digger Lin via Phabricator via llvm-commits]

DiggerLin 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");
----------------
jhenderson wrote:
> 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.
thanks


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


================
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:
> > jhenderson wrote:
> > > 
> > Not addressed?
> STILL not addressed...
thanks.


================
Comment at: llvm/test/tools/llvm-ar/malform-global-symbol-table-bigarchive.test:16
+
+# CHECK:  error: unable to load '[[FILE]]': truncated or malformed archive (global symbol table header at offset 0x20e and size 114 goes past the end of file)
+# CHECK2: error: unable to load '[[FILE]]': truncated or malformed archive (global symbol table content at offset 0x280 and size 37 goes past the end of file)
----------------
jhenderson wrote:
> @MaskRay, I'm in two minds about this, so would like another opinion: would you prefer to have sizes in hex or decimal? On the one hand, decimal is more "natural" when speaking of sizes typically. On the other hand, it's much easier to do maths if the two things are in the same base, so my instinct would be to say that the sizes need to be hex too.
I can change to using both hex and decimal. 
truncated or malformed archive (global symbol table header at offset 0x20e and size 114(0x72) goes past the end of file) 
@jhenderson 


================
Comment at: llvm/test/tools/llvm-ar/malform-global-symbol-table-bigarchive.test:9
+## Truncate the file to end before the global symbol table header ends.
+# RUN: %python -c "with open('%/t.a', 'r+b') as input: input.truncate(656)"
+## Truncate the file to end before the global symbol table ends.
----------------
MaskRay wrote:
> It may be useful to do `rm -rf %t && cd %t` at the top once you are using tricky lit feature like `%/t.a`... Then you can use regular filenames without the `%t` prefix.
thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124865