[PATCH] D111889: [AIX] Support of Big archive (read)

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 14 00:46:08 PST 2022


jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

I'm giving this an LGTM on the basis that the "normal" functionality looks good, and it's not worth blocking it further, awaiting yaml2obj support. However, before you commit this, I'd like you to add the following comment to all the places I've highlighted with this batch of inline comments: `TODO: Add testing`, as there are numerous parts that are untested, primarily to do with invalid/malformed archives.



================
Comment at: llvm/lib/Object/Archive.cpp:207
+  if (!NameLenOrErr)
+    return NameLenOrErr.takeError();
+  uint64_t NameLen = NameLenOrErr.get();
----------------
Add TODO, as noted out-of-line.


================
Comment at: llvm/lib/Object/Archive.cpp:220
+        Parent->getData().data();
+    return malformedError(
+        "name does not have name terminator \"`\\n\" for archive member"
----------------
Add TODO, as noted out-of-line.


================
Comment at: llvm/lib/Object/Archive.cpp:472
 
-  // Check to see if this is at the end of the archive.
-  if (NextLoc == Parent->Data.getBufferEnd())
----------------
Let's readd this comment.


================
Comment at: llvm/lib/Object/Archive.cpp:476
 
-  // Check to see if this is past the end of the archive.
   if (NextLoc > Parent->Data.getBufferEnd()) {
----------------
Let's readd this comment (sorry if I asked for it to be removed earlier...)


================
Comment at: llvm/lib/Object/Archive.cpp:1150
+  if (RawOffset.getAsInteger(10, FirstChildOffset))
+    Err = malformedError("malformed AIX big archive: first member offset \"" +
+                         RawOffset + "\" is not a number");
----------------
Add TODO, as noted out-of-line.


================
Comment at: llvm/lib/Object/Archive.cpp:1155
+  if (RawOffset.getAsInteger(10, LastChildOffset))
+    Err = malformedError("malformed AIX big archive: last member offset \"" +
+                         RawOffset + "\" is not a number");
----------------
Add TODO, as noted out-of-line.


================
Comment at: llvm/test/tools/llvm-objdump/malformed-archives.test:180
 
-# BOGUS13B: error: truncated or malformed archive (characters in LastModified field in archive header are not all decimal numbers: '1foobar273' for the archive member header at offset 8)
+# BOGUS13B: error: truncated or malformed archive (characters in LastModified field in archive member header are not all decimal numbers: '1foobar273' for the archive member header at offset 8)
----------------
Add `## TODO: add testing for AIX Big archive` to the end of this file (with a blank line between it and the previous line).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111889



More information about the llvm-commits mailing list