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

[James Henderson via Phabricator via llvm-commits]

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

LGTM, with one request.



================
Comment at: llvm/lib/Object/Archive.cpp:1189-1192
+  if (RawOffset.getAsInteger(10, GlobSymOffset))
+    Err = malformedError(
+        "malformed AIX big archive: global symbol table offset \"" + RawOffset +
+        "\" is not a number");
----------------
DiggerLin wrote:
> jhenderson wrote:
> > DiggerLin wrote:
> > > jhenderson wrote:
> > > > Test case?
> > > I do not think we can add a test case for the malformed in the test case. 
> > > since we do not support yaml2obj  for big archive, we can not generate malformed big archive.  there is https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/llvm-objdump/malformed-archives.test#L182 
> > > as in patch https://reviews.llvm.org/D111889#change-i2aT5QfHJFnJ
> > > 
> > One option might be to use python to binary edit an archive created by llvm-ar. You could probably assume that the offset of the field is stable, so can just do something like:
> > # use llvm-ar to create archive.a
> > # read archive.a into byte array
> > # modify byte at offset x
> > # write byte array to disk as new file (bad.a)
> > # run llvm-ar on bad.a
> My suggestion is that we do not have a test case for the these code in the patch.  @jhenderson 
> 
> I will create a new patch to add test case for all malformedError test including the test case in https://reviews.llvm.org/D111889.  the [AIX] Support of Big archive (read) ,if you suggestion work. and I think I need to write a python script and run the script.
Thanks. That sounds reasonable. Please add a "TODO: add test case." comment to the bits of code that are missing test cases before landing this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124865