[AIX] Add '-bcdtors:all:0:s' to linker to gather static init functions

[stevewan]

Summary

On AIX, add '-bcdtors:all:0:s' to the linker implicitly through the driver so that we can collect all static constructor and destructor functions.

[ZarkoCA]

clang/test/Driver/aix-ld.cpp

1 ↗

(On Diff #265631)

s/Arguemnt/Argument/

7 ↗

(On Diff #265631)

Add a line between the run steps and check, and can you align things vertically.

Eg.

CHECK-LD32-ARG-ORDER-NOT: warning:
CHECK-LD32-ARG-ORDER:     {{.*}}clang++" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"

[ZarkoCA]

clang/test/Driver/aix-ld.cpp

3 ↗

(On Diff #265631)

formatting, align with the line above

[hubert.reinterpretcast]

clang/test/Driver/aix-ld.cpp

1 ↗

(On Diff #265631)

Lowercase "argument".

2 ↗

(On Diff #265631)

Let's not use a .o suffix for the link step output. It seems using no -o is fine for tests with -###.

[stevewan]

Address comments and fix Windows compatibility issues with the test case.

[hubert.reinterpretcast]

clang/test/Driver/aix-ld.cpp

9 ↗

(On Diff #265765)

I am not sure I've seen this used in conjunction with %clangxx.
"{{.*}}clang{{.*}}" seems appropriate based on existing usage.

[hubert.reinterpretcast]

clang/test/Driver/aix-ld.cpp

18 ↗

(On Diff #265765)

The main point of the test being that -bnocdtors from the user is not overriden by the implied -bcdtors:all:0:s, I think we should drop these last two lines and add a CHECK-LD32-ARG-ORDER-NOT against -bcdtors:all:0:s.

[stevewan]

Address furthur comments on the test case.

[hubert.reinterpretcast]

clang/test/Driver/aix-ld.cpp

2 ↗

(On Diff #265781)

I am wondering if it makes sense to put this into aix-ld.c. It helps to demonstrate that the link step behaviour is not determined by the source file name.

For example, in hexagon-toolchain-elf.c:

182 // RUN: %clangxx -### -target hexagon-unknown-elf \
183 // RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
184 // RUN:   -mcpu=hexagonv60 \
185 // RUN:   %s 2>&1 \
186 // RUN:   | FileCheck -check-prefix=CHECK031 %s

[stevewan]

clang/test/Driver/aix-ld.cpp

2 ↗

(On Diff #265781)

Yes, it makes sense to show that, and that was my original design. I moved this to a separate file to avoid getting a warning because of the inherited CHECK-LD32-ARG-ORDER-NOT: warning:. I'm now thinking of two ways to incorporate this into aix-ld.c,

1. Simply remove the warning check like what's done in the example you've showed.
2. Change the line CHECK-LD32-ARG-ORDER-NOT: warning: into something like,

// CHECK-LD32-ARG-ORDER-NOT: warning:
// CHECK-LD32-ARG-ORDER:     warning: treating 'c' input as 'c++' when in C++ mode, this behavior is deprecated [-Wdeprecated]
// CHECK-LD32-ARG-ORDER-NOT: warning:

Do we have a preference for one way over another?

[hubert.reinterpretcast]

clang/test/Driver/aix-ld.cpp

2 ↗

(On Diff #265781)

I think not adding something extra is better than adding something extra that's not particularly relevant. The intended behaviour for what we're testing is the same even if the deprecated behaviour is finally actually removed (and maybe with the removal of or a change to the warning message).

[stevewan]

Incorporate the test case into 'aix-ld.c'.

[stevewan]

I'm planning to post an NFC patch after this to fix the formatting for existing cases in 'aix-ld.c' so that everything is consistent.

[hubert.reinterpretcast]

Thanks. LGTM with minor nit.

In D80415#2051772, @stevewan wrote:

I'm planning to post an NFC patch after this to fix the formatting for existing cases in 'aix-ld.c' so that everything is consistent.

I'm not sure how NFC it is, but I would suggest removing the -o %t.o from the other cases as well.

clang/test/Driver/aix-ld.c

184

Minor nit: Maybe the prefix should be CHECK-LD32-CXX-ARG-ORDER.

[stevewan]

Update check prefix based on comments.

clang/lib/Driver/ToolChains/AIX.cpp

void aix::Assembler::ConstructJob(Compilation &C, const JobAction &JA,
   C.addCommand(std::make_unique<Command>(JA, *this, Exec, CmdArgs, Inputs));
 }
 
 void aix::Linker::ConstructJob(Compilation &C, const JobAction &JA,
                                const InputInfo &Output,
                                const InputInfoList &Inputs, const ArgList &Args,
                                const char *LinkingOutput) const {
   const AIX &ToolChain = static_cast<const AIX &>(getToolChain());
+  const Driver &D = ToolChain.getDriver();
   ArgStringList CmdArgs;
 
   const bool IsArch32Bit = ToolChain.getTriple().isArch32Bit();
   const bool IsArch64Bit = ToolChain.getTriple().isArch64Bit();
   // Only support 32 and 64 bit.
   if (!(IsArch32Bit || IsArch64Bit))
     llvm_unreachable("Unsupported bit width value.");
 
else
       return IsArch32Bit ? "crt0.o" : "crt0_64.o";
   };
 
   if (!Args.hasArg(options::OPT_nostdlib)) {
     CmdArgs.push_back(
         Args.MakeArgString(ToolChain.GetFilePath(getCrt0Basename())));
   }
 
+  // Collect all static constructor and destructor functions in CXX mode. This
+  // has to come before AddLinkerInputs as the implied option needs to precede
+  // any other '-bcdtors' settings or '-bnocdtors' that '-Wl' might forward.
+  if (D.CCCIsCXX())
+    CmdArgs.push_back("-bcdtors:all:0:s");
+
   // Specify linker input file(s).
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
 
   // Add directory to library search path.
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {

clang/test/Driver/aix-ld.c

 // CHECK-LD64-NO-DEFAULT-LIBS: "{{.*}}ld{{(.exe)?}}" 
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-bnso"
 // CHECK-LD64-NO-DEFAULT-LIBS: "-b64" 
 // CHECK-LD64-NO-DEFAULT-LIBS: "-bpT:0x100000000" "-bpD:0x110000000" 
 // CHECK-LD64-NO-DEFAULT-LIBS: "[[SYSROOT]]/usr/lib{{/|\\\\}}crt0_64.o"
 // CHECK-LD64-NO-DEFAULT-LIBS: "-L[[SYSROOT]]/usr/lib" 
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lpthreads"
 // CHECK-LD64-NO-DEFAULT-LIBS-NOT: "-lc"
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. 'bcdtors' and argument order.
+// RUN: %clangxx -no-canonical-prefixes %s 2>&1 -### \
+// RUN:          -Wl,-bnocdtors \
+// RUN:          -target powerpc-ibm-aix7.1.0.0 \
+// RUN:          --sysroot %S/Inputs/aix_ppc_tree \
+// RUN: | FileCheck --check-prefix=CHECK-LD32-CXX-ARG-ORDER %s
+
+// CHECK-LD32-CXX-ARG-ORDER:     {{.*}}clang{{.*}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32-CXX-ARG-ORDER:     "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LD32-CXX-ARG-ORDER:     "{{.*}}ld{{(.exe)?}}"
+// CHECK-LD32-CXX-ARG-ORDER-NOT: "-bnso"
+// CHECK-LD32-CXX-ARG-ORDER:     "-b32"
+// CHECK-LD32-CXX-ARG-ORDER:     "-bpT:0x10000000" "-bpD:0x20000000"
+// CHECK-LD32-CXX-ARG-ORDER:     "[[SYSROOT]]/usr/lib{{/|\\\\}}crt0.o"
+// CHECK-LD32-CXX-ARG-ORDER:     "-bcdtors:all:0:s"
+// CHECK-LD32-CXX-ARG-ORDER:     "-bnocdtors"
+// CHECK-LD32-CXX-ARG-ORDER-NOT: "-bcdtors:all:0:s"