since the commit did the same change , I closed the patch

In D139864#4650667, @MaskRay wrote:

Thanks for the update. Doing the following looks good to me.

bool CanHaveLeadingDot = true;
if (StripUnderscore)
  if (DecoratedStr[0].consume_front("_")) // simplified
    CanHaveLeadingDot = false;

std::string Result;
if (nonMicrosoftDemangle(DecoratedStr, Result, CanHaveLeadingDot))

rebase the code and address James' comment.

address comment

In D139864#4649934, @jhenderson wrote:

Could you not just modify nonMicrosoftDemangle to handle the dot prefix? That would mean this works for all tools, including llvm-cxxfilt, rather than needing this and the separate D159539 patch.

rebase the code

In D159539#4649765, @MaskRay wrote:

llvm::demangle is used by a lot of ELF tools to assume ELF style mangling where there is no extra prefix. I think many don't expect demangling ._Zxxxx symbols.

address comment

In D139864#4641608, @jhenderson wrote:

My opinion hasn't changed that it's a mistake to put the dot demangling in llvm-cxxfilt specifically. That will mean you'll need to make the same change in llvm-nm, llvm-readelf, llvm-objdump and probably other tools too. Code duplication is bad. The next tool to be added might not realise that they can't just use the main demangler library as-is to do their name demangling.

If there is tool code that conflicts with that behaviour (such as the llvm-nm mach-o code), perhaps it's that tool code that should change instead, or perhaps we need to enhance the demangler library a little further, for example by adding an optional "strip underscore" behaviour to it. Adding @MaskRay as a reviewer in case he's got a different opinion.

In D139864#4641608, @jhenderson wrote:

My opinion hasn't changed that it's a mistake to put the dot demangling in llvm-cxxfilt specifically. That will mean you'll need to make the same change in llvm-nm, llvm-readelf, llvm-objdump and probably other tools too. Code duplication is bad. The next tool to be added might not realise that they can't just use the main demangler library as-is to do their name demangling.

If there is tool code that conflicts with that behaviour (such as the llvm-nm mach-o code), perhaps it's that tool code that should change instead, or perhaps we need to enhance the demangler library a little further, for example by adding an optional "strip underscore" behaviour to it. Adding @MaskRay as a reviewer in case he's got a different opinion.

gentle ping, @jhenderson

address comment

delete an empty line

address comment

added more test scenario and address comment.

LGTM.

address comment

LGTM

@jhenderson , I would greatly appreciate your thoughts on the possibility of using the implement proposed in https://reviews.llvm.org/D158004?id=552000 for the patch.

add new test scenario

what is preventing the change to clang to not call llvm-nm with import files?

rebase the patch.

implement a new class AIXLinkerImportFile

@jhenderson ,May I kindly ask if you have any further comments? If not, would it be acceptable for me to move forward with committing the patch? Your valuable input would be sincerely appreciated.

@MaskRay, would you have any additional comments to share? If not, would it be appropriate for me to proceed with committing the patch? Your input would be greatly appreciated.

address minor comment.

address comment

yes, your understand is correct. but I need to clarify following.

In D158004#4592989, @stephenpeckham wrote:

I think the changes look okay. (In your patch description, I would specify a link to the 7.3 AIX documentation rather than the 7.2 version.)

added new test scenarios .

rebase the patch

In D144872#4579160, @jhenderson wrote:

I'll be honest, I'm not convinced the test coverage looks to be anywhere near comprehensive enough. However, I also haven't attempted to review it versus the code changes to check how much of the new code is actually covered. I'd suggest you consider using a code coverage tool to see how much coverage there is of your new code.

according to https://www.ibm.com/docs/en/aix/7.2?topic=program-using-thread-local-storage

address comment

In D144872#4549482, @jhenderson wrote:

Just as a heads-up, I'm off for the rest of the week. Hopefully I'll be able to get to any final points either before the end of my workday or later next week.

I'd still rather the invalid OBJECT_MODE value be read and rejected upfront before even parsing the -X option.

address comment

rebase code

addressed comment and let llvm-ranlib supports -Xany option

I do see the appeal of a single enum parameter in the meantime. I would suggest it should be called SymtabWritingMode and have values NoSymtab, NormalSymtab, BigArchive64Bit, BigArchive32Bit, and BigArchiveBoth. You could optionally drop one of the BigArchive* values, and treat NormalSymtab as that value for BigArchive (as long as "Normal" is clear as to its meaning to someone familiar with BigArchive). All existing WriteSymtab false values would be replaced with NoSymtab and true with NormalSymtab (except for BigArchive cases).

As an alternative (but I think adds unnecessary complexity, due to needing an extra variable), you could have both tools read the environment variable into a string variable, then, if the -X option is present, overwrite that variable, and finally feed that string into the parsing code that converts into a BitMode value. If the string is invalid, the parsing code could report an error along the lines of "invalid OBJECT_MODE or -X option value".

if I do not think it is better than my current implement, If I implement as your suggestion, I need another variable to recoded the where the string come from(from OBJECT_MODE  or -X option value). otherwise when the invalid value of string , how can I report it is an invalid OBJECT_MODE"  or "invalid -X option value"

addressed partial comments, after deciding whether to delete the bool NeedSymbols parameter from functions API,(writeArchiveToStream etc), I will upload a update patch.

1. rebase the code
2. for the env OBJECT_MODE= or is env OBJECT_MODE="" , the ranlib and ar in AIX OS , have different behaviors(ranlib looks as -X32 , but ar and nm output error ),

I change llvm-ranlib as the same behavior as ar and llvm-ar, output an error(I think output error is more reasonable). and look env OBJECT_MODE unset as default 32-BIT for llvm-ranlib as AIX OS command ranlib , nm ,ar.

address comment