r205012 - Link in profile library on Linux using --whole-archive

[Duncan P. N. Exon Smith]

This is the wrong change.  -whole-archive will pull in way too much.

In particular, this pulls in two runtimes that are unrelated and in separate object
files:  GCDAProfiling and InstrProfiling.  InstrProfiling (at least) has static
initializers that should only be invoked if -fprofile-instr-generate is on, and
that have nothing at all to do with -fcoverage, etc.

I don’t know much about GCDAProfiling, but I expect it pulls in things that should
not be on without the appropriate flags.

Please revert.

On the other hand, there needs to be some way pull in the right bits of the runtime
on Linux.  I’m researching now.

Does anyone know why the -u option doesn’t do what I expect?

  - There’s an int in InstrProfilingRuntime.cpp called __llvm_profile_runtime.

  - On Darwin, -u ___llvm_profile_runtime causes that object file
    (InstrProfilingRuntime.o) to get pulled in by the linker.

  - That object file has a C++ static initializer that adds a function to the
    atexit() queue, which should pull in everything else.

Not having a Linux system handy, it’s hard for me to guess what part of that is
failing on Linux.

I’m going to try to move this logic to IRGen; maybe the -u option is interpreted
differently between the platforms’ linkers.

On 2014 Mar 28, at 08:39, Alexey Samsonov <samsonov at google.com> wrote:

> Author: samsonov
> Date: Fri Mar 28 10:39:08 2014
> New Revision: 205012
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=205012&view=rev
> Log:
> Link in profile library on Linux using --whole-archive
> 
> Modified:
>    cfe/trunk/lib/Driver/Tools.cpp
>    cfe/trunk/test/Driver/coverage-ld.c
> 
> Modified: cfe/trunk/lib/Driver/Tools.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=205012&r1=205011&r2=205012&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Driver/Tools.cpp (original)
> +++ cfe/trunk/lib/Driver/Tools.cpp Fri Mar 28 10:39:08 2014
> @@ -1817,18 +1817,16 @@ static void addProfileRT(
>         Args.hasArg(options::OPT_coverage)))
>     return;
> 
> -  // Pull in runtime for -fprofile-inst-generate.  This is required since there
> -  // are no calls to the runtime in the code.
> -  if (Args.hasArg(options::OPT_fprofile_instr_generate)) {
> -    CmdArgs.push_back("-u");
> -    CmdArgs.push_back("___llvm_profile_runtime");
> -  }
> -
>   SmallString<128> LibProfile = getCompilerRTLibDir(TC);
>   llvm::sys::path::append(LibProfile,
>       Twine("libclang_rt.profile-") + getArchNameForCompilerRTLib(TC) + ".a");
> 
> -  CmdArgs.push_back(Args.MakeArgString(LibProfile));
> +  SmallVector<const char *, 3> LibProfileArgs;
> +  LibProfileArgs.push_back("-whole-archive");
> +  LibProfileArgs.push_back(Args.MakeArgString(LibProfile));
> +  LibProfileArgs.push_back("-no-whole-archive");
> +
> +  CmdArgs.insert(CmdArgs.end(), LibProfileArgs.begin(), LibProfileArgs.end());

CmdArgs is a vector; why not push back to it directly?

> }
> 
> static void addSanitizerRTLinkFlags(
> 
> Modified: cfe/trunk/test/Driver/coverage-ld.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/coverage-ld.c?rev=205012&r1=205011&r2=205012&view=diff
> ==============================================================================
> --- cfe/trunk/test/Driver/coverage-ld.c (original)
> +++ cfe/trunk/test/Driver/coverage-ld.c Fri Mar 28 10:39:08 2014
> @@ -16,8 +16,11 @@
> // RUN:   | FileCheck --check-prefix=CHECK-LINUX-X86-64 %s
> //
> // CHECK-LINUX-X86-64: "{{(.*[^-.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
> -// CHECK-LINUX-X86-64: "{{.*}}/Inputs/resource_dir{{/|\\\\}}lib{{/|\\\\}}linux{{/|\\\\}}libclang_rt.profile-x86_64.a" {{.*}} "-lc"
> -//
> +// CHECK-LINUX-X86-64: "-whole-archive"
> +// CHECK-LINUX-X86-64: "{{.*}}/Inputs/resource_dir{{/|\\\\}}lib{{/|\\\\}}linux{{/|\\\\}}libclang_rt.profile-x86_64.a"
> +// CHECK-LINUX-X86-64: "-no-whole-archive"
> +// CHECK-LINUX-X86-64: "-lc"
> +
> // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
> // RUN:     -target x86_64-unknown-freebsd --coverage \
> // RUN:     -resource-dir=%S/Inputs/resource_dir \
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits