gentle ping. if you have time , would you like to take sometime to review @jhenderson

add a redzone boundary test case

In D146071#4194372, @stephenpeckham wrote:

Are there still tests where IsBackChainStored is true?

fixed part of comment.

gentle ping.

In D142484#4080202, @Jake-Egan wrote:

In D142484#4079909, @jhuber6 wrote:

In D142484#4079869, @Jake-Egan wrote:

Hi, this new test fails on AIX. Could you take a look?
https://lab.llvm.org/buildbot/#/builders/214/builds/5477/steps/6/logs/FAIL__Clang__linker-wrapper-libs_c

I might just put that this test requires Linux. Because I don't think I can debug it without access to the system. We don't really support offloading on anything but Linux anyway.

I think that makes sense. We'd need time to investigate it on AIX anyways.

LGTM

In D123949#3517524, @bd1976llvm wrote:

Hi @DiggerLin. Currently the --help text states --format=aix but llvm-ar accepts --format=bigarchive. Could you fix that please.

fixed a bug

In my previous implementation, It will only work on big archives with an empty global symbol table. if there is already a global symbol table (regardless of whether it's 32-bit or 64-bit) in the big archive, then llvm-ranlib -X will not do anything. (For example, if a big archive has a 64-bit global symbol table, then llvm-ranlib -X32 will not do anything). This is different from the AIX ranlib, where if there is only a 64-bit global symbol table, then -X32 will generate a 32-bit global symbol table, resulting in both the 32-bit and 64-bit global symbol tables being present in the big archive at the same time. I added the new functionality as AIX OS "ranlib"

In D142660#4090174, @jhenderson wrote:

I'm not sure I understand why the logic for llvm-ranlib seems to be significantly different to how the option is implemented for llvm-ar? Could you explain this, please?

address comment and add bit code into test case.

address comment .

In D142479#4090280, @jhenderson wrote:

I'm out of time for now. Please take a look at my inline comments and I'll come back to this at some point after you've addressed them (I have a big backlog to catch up on, so it may be a while).

run git format

LGTM.

when using CMake versions >= 3.22 we'll pickup the built llvm-ranlib by default if it's in the path (which it is when doing a sub build via llvm_ExternalProject_Add for the runtimes),

Gentle Ping.

LGTM

added --traceback-table imply --disassemble-all and addressed comments.

thanks for adding a new malform empty archive test case.

LGTM

LGTM , but wait for several days to see whether other reviewer has comment or not.

In D136787#3977618, @Esme wrote:

Btw. Do you have any plans to implement an option like "--loader-section" to embrace all this loader section related information?

added decode size of relocation type in --expand-relocs

added a new no expand relocation format.

In D136787#3977618, @Esme wrote:

Btw. Do you have any plans to implement an option like "--loader-section" to embrace all this loader section related information?

In D138982#3974293, @Esme wrote:

Hi @DiggerLin, thank you for your review. I'm also confused by the description from the doc, if my understanding is correct, according to the doc, the FixedValue will ignore the value of Fixup.getOffset(), because the address of the csect containing the branch instruction to be modified equals to SectionMap[ParentSec]->Address + Layout.getFragmentOffset(Fragment). This is obviously incorrect. I have tested LNT and bootstrap for this patch under the options of -fintegrated-as -ffunction-sections, and both are clean.

In D136787#3965951, @jhenderson wrote:

Essentially looks reasonable to me, although I haven't verified that all the call paths are properly covered by tests. @Esme or another XCOFF developer should take a look at this before landing it.

FYI, my last day working before Christmas is next Friday, and I may have limited reviewing time next week. I won't be back and able to review things further after that until late January.

address James' comment. thanks James.

added a new test case to test invalid symbol name for relocation.

address James' comment

In D137819#3939790, @Esme wrote:

In D137819#3925955, @DiggerLin wrote:

change the description to 'support the relocation overflow section."

As the doc says, the overflow section is required when either of the relocation entries or the line number entries exceeds 65,534. As long as one of the counts exceeds 65,534, the other one will also be set to 65535 in the primary section header, and then the overflow section will record actual counts for both relocation entries and line number entries, where the s_paddr for relocation entries and s_vaddr for line number entries.
Therefore I reckon there's no need to identify whether it's a relocation overflow section or line number overflow section.

addressed comment , rebased the patch , changed the test case from using canned xcoff object file to using yaml2obj.

address James's comment. thanks @jhenderson

In D135887#3939182, @vitalybuka wrote:

there is a use of uninitialized value:
https://lab.llvm.org/buildbot/#/builders/5/builds/29299

In D135887#3939235, @MaskRay wrote:

(Note, a typo in the commit [XCOFF] llvvm-readobj ...)

change the description to 'support the relocation overflow section."

LGTM, please wait for several days and Hubert maybe has comment on it. I do not think we need to check so many symbol items and relocation items in the test case, but I can not give any suggestion on it.

address James' comment

address comment