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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D124865#3601518 <https://reviews.llvm.org/D124865#3601518>, @DiggerLin wrote:

> In D124865#3601036 <https://reviews.llvm.org/D124865#3601036>, @jhenderson wrote:
>
>> I feel like this code is missing testing for the non-error cases. In particular, I'd expect to see some sort of test that shows that the archive symbol table can be read successfully. I think you can do that using `llvm-nm --print-armap`. I'd be surprised if there aren't already equivalent test cases for other formats that do this, so you may be able to copy/modify those to cover big archives too.
>
> There is already has "llvm-nm --print-armap" in the llvm/test/tools/llvm-ar/delete.test , the test case is disable in AIX OS, it enable in AIX in the patch by deleting  the "# XFAIL: system-aix"

`delete.test` is a test that shows that llvm-ar can delete symbols when its members are deleted. It's hardly comprehensive coverage of the archive symbol table code. However, there are many other symbol table tests both in the Object and tools/llvm-ar test directories. It's not clear to me why the changes you've made didn't cause a test to start passing/failing when it wasn't before, at least on AIX OS. You need to investigate, and ensure there is coverage of the new code you've added in a test dedicated to symbol table reading etc.



================
Comment at: llvm/test/tools/llvm-ar/malform-global-symbol-table-bigarchive.test:1
+## Test malformed global symbal table of big archive.
+
----------------
Test-case name should be "malformed-..." (not "malform-...")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124865