lewisje / address-sanitizer

Automatically exported from code.google.com/p/address-sanitizer
0 stars 0 forks source link

Clang driver incorrectly handles -Wl,-r flag #344

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Moving from Go repo as it seems to be clang issue (and gcc works fine):
https://code.google.com/p/go/issues/detail?id=8788

go version go1.3 linux/amd64
clang version 3.6.0 (217929)

The program is:
package main
// void foo() {}
import "C"
func main() {
    C.foo()
}

$ CC="clang -fsanitize=address" go run src.go

/var/tmp/go-link-FimrBB/000000.o:(.data._ZN11__sanitizer15IOCTL_KDGKBSENTE+0x0):
 multiple definition of `__sanitizer::IOCTL_KDGKBSENT'
/ssd/src/llvm/build/bin/../lib/clang/3.6.0/lib/linux/libclang_rt.asan-x86_64.a(s
anitizer_platform_limits_posix.cc.o):(.data._ZN11__sanitizer15IOCTL_KDGKBSENTE+0
x0): first defined here
/var/tmp/go-link-FimrBB/000000.o: In function `__interceptor_xdr_float':

While gcc 4.8.2 works fine.

gcc uses shared asan runtime, and so it tolerates some issues in build process:

$ ldd src
    libasan.so.0 => /usr/lib/x86_64-linux-gnu/libasan.so.0 (0x00007fae89644000)

while clang links asan runtime statically, and some issue in cgo build process 
causes multiple symbol errors.

The issue is in clang driver, it incorrectly handles -r linker flag. One of the 
objects is build as:

clang -I . -fsanitize=address -fPIC -m64 -pthread -fno-caret-diagnostics 
-Qunused-arguments -fmessage-length=0 -o 
$WORK/command-line-arguments/_obj/_all.o 
$WORK/command-line-arguments/_obj/_cgo_export.o 
$WORK/command-line-arguments/_obj/src.cgo2.o -g -O2 -Wl,-r -nostdlib 
/usr/lib/gcc/x86_64-linux-gnu/4.8/libgcc.a

It's kind of linking, but this object is later linked again:

pack r $WORK/command-line-arguments.a 
$WORK/command-line-arguments/_obj/_cgo_import.6 
$WORK/command-line-arguments/_obj/_cgo_defun.6 
$WORK/command-line-arguments/_obj/_all.o # internal
cd .
/usr/lib/google-golang/pkg/tool/linux_amd64/6l -o 
$WORK/command-line-arguments/_obj/exe/src -L $WORK -w -extld=clang 
-extldflags=-fsanitize=address $WORK/command-line-arguments.a

So one asan runtime comes from _all.o (incorrectly) and another from implicitly 
added by driver -lasan.

Driver must not link in asan runtime if -Wl,-r is present.

Original issue reported on code.google.com by dvyu...@google.com on 22 Sep 2014 at 6:11

GoogleCodeExporter commented 9 years ago
I am not sure if the driver should parse the linker flags. I'd prefer it not to 
do that. 

Original comment by konstant...@gmail.com on 22 Sep 2014 at 5:33

GoogleCodeExporter commented 9 years ago
How do you propose to resolve this?
The idea is that a user needs to add -fsanitize=foo to all driver invocations 
and that will magically do the right thing. It does not work in this case.

Original comment by dvyu...@google.com on 22 Sep 2014 at 5:46

GoogleCodeExporter commented 9 years ago
Isn't it easier to just not pass -fsanitize=address when you invoke ld with 
"-relocatable" option? We may state that "-fsanitize=address" should be passed 
if:
1) you're compiling the source code into object file
2) you're linking an executable.

Original comment by samso...@google.com on 22 Sep 2014 at 6:12

GoogleCodeExporter commented 9 years ago
This level of control is generally impossible in an arbitrary C/C++ project out 
there. One better pray that the build system at least properly supports 
CC/CFLAGS/LDFLAGS and pass it to all invocations of a compiler.
We don't link asan runtime into dynamic libraries for this reason. But you can 
easily say -- just don't pass -fsanitize=foo when linking dynamic libraries. 
It's easy to say but usually near impossible to do...

Original comment by dvyu...@google.com on 22 Sep 2014 at 6:25

GoogleCodeExporter commented 9 years ago
I haven't seen lots of C++ projects, but I suspect that they don't usually 
build objects with "-Wl,-r". On the contrary (correct me if I'm wrong), I guess 
that Go build system somehow does it explicitly.

Original comment by samso...@google.com on 22 Sep 2014 at 6:55

GoogleCodeExporter commented 9 years ago
All compiler/linker flags are added explicitly by build systems.
If you feel adventurous, you can check out gcc and try to modify its build 
system to not pass -fsanitize=address with it links dynamic libraries.

FWIW I've mailed https://codereview.appspot.com/142470043 to do this in Go, but 
that fixes only the concrete case.

Original comment by dvyu...@google.com on 23 Sep 2014 at 4:09

GoogleCodeExporter commented 9 years ago
We probably should not link asan runtime with -nostdlib, regardless of -Wl,-r.

Original comment by euge...@google.com on 23 Sep 2014 at 9:39

GoogleCodeExporter commented 9 years ago
-nostdlib is a driver flag and it is a great idea to look at it. 
Looking at -Wl,* is bad

Original comment by konstant...@gmail.com on 23 Sep 2014 at 4:28

GoogleCodeExporter commented 9 years ago
BTW at the moment if user passes -nostdlib we link in static ASan runtime, but 
don't add -lpthread, -lrt etc. We can change this to just ignore 
-fsanitize=address in this case.

Original comment by samso...@google.com on 23 Sep 2014 at 7:17

GoogleCodeExporter commented 9 years ago
Yes, this is exactly what we should do. ASan runtime here is exactly an 
"stdlib" the same as pthread, rt, c, gcc.

Without -nostdlib (and without asan) the build in #1 will still break in the 
same way due to symbols from other standard libraries.

Original comment by euge...@google.com on 24 Sep 2014 at 9:08

GoogleCodeExporter commented 9 years ago
Well, ASan build with -nostdlib work, if user provides his own versions of 
libpthread/librt/libc etc. in the command line. But probably dropping ASan 
runtime from linker invocation is still the right thing to do. I will implement 
this.

Original comment by samso...@google.com on 25 Sep 2014 at 2:57

GoogleCodeExporter commented 9 years ago
> This level of control is generally impossible in an arbitrary C/C++ project 
out
> there. One better pray that the build system at least properly supports
> CC/CFLAGS/LDFLAGS and pass it to all invocations of a compiler.

FYI we worked around this by replacing gcc/g++ with shell scripts that would 
force-enable sanitizer flags. This is super-ugly but was proved to be very 
efficient.

Original comment by tetra2...@gmail.com on 25 Sep 2014 at 8:54

GoogleCodeExporter commented 9 years ago
We did wrappers a lot as well. It's probably the most practical solution.
But it does not look like something that end user must be doing.

Original comment by dvyu...@google.com on 25 Sep 2014 at 3:56

GoogleCodeExporter commented 9 years ago
FWIW, I've submitted r218541, so that now sanitizer runtimes won't be passed to 
the linker if user provides -nostdlib flag. Please reopen if you think we need 
to smth. else here.

Original comment by samso...@google.com on 26 Sep 2014 at 9:33

GoogleCodeExporter commented 9 years ago
FTR, r218541 was reverted in r220455 - there are legitimate use cases when user 
want to pass -nostdlib and still link with ASan runtime (e.g. if one links in 
its own implementation of standard libraries). See review threads for more 
discussion on that.

Original comment by samso...@google.com on 13 Mar 2015 at 10:11

GoogleCodeExporter commented 9 years ago
gcc still has the old behavior (i.e. -nostdlib negates -fsanitize=address). 
This is causing a bit of trouble when compiling ASan-instrumented glibc. 
Alexey, could you please fix this in gcc as well?

Original comment by earth...@chromium.org on 30 Mar 2015 at 2:47

GoogleCodeExporter commented 9 years ago
Can you open a GCC bug and describe glibc use case there? Maybe someone (e.g. 
Roland) would want to help us with that change there.

Original comment by samso...@google.com on 30 Mar 2015 at 8:31

GoogleCodeExporter commented 9 years ago
Adding Project:AddressSanitizer as part of GitHub migration.

Original comment by ramosian.glider@gmail.com on 30 Jul 2015 at 9:14