[Bug sanitizer/59136] New: llvm-symbolizer shouldn't be started always

[Bug sanitizer/59136] New: llvm-symbolizer shouldn't be started always

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59136

            Bug ID: 59136
           Summary: llvm-symbolizer shouldn't be started always
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: sanitizer
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jakub at gcc dot gnu.org
                CC: dodji at gcc dot gnu.org, dvyukov at gcc dot gnu.org,
                    jakub at gcc dot gnu.org, kcc at gcc dot gnu.org

I've noticed that libasan/liblsan now start external llvm-symbolizer for all
programs just in case they would be buggy, that looks like a very bad idea to
me.  It is quite a big overhead, and the usual case for sanitization should be
that no problems are reported, so IMNSHO if you really need external
symbolizer, at least start it on the first diagnostic output that should be
symbolized.
Furthermore, it doesn't have stderr redirected to /dev/null and passes by
default options e.g. llvm 3.3 llvm-symbolizer doesn't grok, so pretty much for
everything it emits ugly error messages.
And, when I've tried to LD_PRELOAD=./liblsan.so.0.0.0 ls -l
it because of the starting llvm-symbolizer just in case created a beatiful
fork-bomb.

[Bug sanitizer/59136] llvm-symbolizer shouldn't be started always

[kcc at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59136

Kostya Serebryany <kcc at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |glider at google dot com,
                   |                            |samsonov at google dot com

--- Comment #1 from Kostya Serebryany <kcc at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #0)
> I've noticed that libasan/liblsan now start external llvm-symbolizer for all
> programs just in case they would be buggy, that looks like a very bad idea
> to me.  It is quite a big overhead, and the usual case for sanitization
> should be that no problems are reported, so IMNSHO if you really need
> external symbolizer, at least start it on the first diagnostic output that
> should be symbolized.

Let Alexey and Alexander speak here.
This complexity should be gone completely once we make the in-process
symbolizer. 

> Furthermore, it doesn't have stderr redirected to /dev/null and passes by
> default options e.g. llvm 3.3 llvm-symbolizer doesn't grok, so pretty much
> for everything it emits ugly error messages.
> And, when I've tried to LD_PRELOAD=./liblsan.so.0.0.0 ls -l
> it because of the starting llvm-symbolizer just in case created a beatiful
> fork-bomb.

[Bug sanitizer/59136] llvm-symbolizer shouldn't be started always

[samsonov at google dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59136

--- Comment #2 from Alexey Samsonov <samsonov at google dot com> ---
(In reply to Jakub Jelinek from comment #0)
> I've noticed that libasan/liblsan now start external llvm-symbolizer for all
> programs just in case they would be buggy, that looks like a very bad idea
> to me.  It is quite a big overhead, and the usual case for sanitization
> should be that no problems are reported, so IMNSHO if you really need
> external symbolizer, at least start it on the first diagnostic output that
> should be symbolized.

We found it convenient to run fork+exec early at program startup. It can also
be slightly dangerous to call fork() in multi-threaded program (even though we
call exec soon after fork). Also, you've mentioned that starting a symbolizer
introduces some overhead, and it would be nice to start printing error message
ASAP (although, symbolizing the stack frame probably takes much more time).
That said, I think your suggestion makes sense. I'll try to experiment with
starting llvm-symbolizer lazily.

> Furthermore, it doesn't have stderr redirected to /dev/null and passes by
> default options e.g. llvm 3.3 llvm-symbolizer doesn't grok, so pretty much
> for everything it emits ugly error messages.

Not sure I understand it. Do you mean that when ASan runtime calls old version
of llvm-symbolizer, it passes command-line options which are not recognized,
and therefore fill the stderr with garbage?

> And, when I've tried to LD_PRELOAD=./liblsan.so.0.0.0 ls -l
> it because of the starting llvm-symbolizer just in case created a beatiful
> fork-bomb.

[Bug sanitizer/59136] llvm-symbolizer shouldn't be started always

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59136

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Alexey Samsonov from comment #2)
> We found it convenient to run fork+exec early at program startup. It can
> also be slightly dangerous to call fork() in multi-threaded program (even
> though we call exec soon after fork). Also, you've mentioned that starting a

Well, POSIX is clear on what functions you can and what you can't call after
fork in multi-threaded program.  You aren't calling exactly those functions,
but functions that do the same thing as close/dup2/execl/_exit, that is fine,
getdtablesize is not, but it just calls getrlimit syscall under the hood,
which, while also not officially async-signal-safe, is safe in practice.

> symbolizer introduces some overhead, and it would be nice to start printing
> error message ASAP (although, symbolizing the stack frame probably takes
> much more time). That said, I think your suggestion makes sense. I'll try to
> experiment with starting llvm-symbolizer lazily.

It makes sense to start early only if you expect every program to be buggy and
assume you will actually need to print something.

> > Furthermore, it doesn't have stderr redirected to /dev/null and passes by
> > default options e.g. llvm 3.3 llvm-symbolizer doesn't grok, so pretty much
> > for everything it emits ugly error messages.
> 
> Not sure I understand it. Do you mean that when ASan runtime calls old
> version of llvm-symbolizer, it passes command-line options which are not
> recognized, and therefore fill the stderr with garbage?

Yeah, I'm getting:
llvm-symbolizer: Unknown command line argument '--default-arch=x86_64'.  Try:
'/usr/bin/llvm-symbolizer -help'
llvm-symbolizer: Did you mean '-disable-ssc=x86_64'?
on stderr for all programs linked against libasan or liblsan.

> > And, when I've tried to LD_PRELOAD=./liblsan.so.0.0.0 ls -l
> > it because of the starting llvm-symbolizer just in case created a beatiful
> > fork-bomb.

And in the case of the fork-bomb it would just keep the above mentioned text
forever,
because with LD_PRELOAD=./liblsan.so.0.0.0 in the environment also
llvm-symbolizer itself
was linked against liblsan and when it tried to run llvm-symbolizer eagerly, it
would do that forever.
If it started lazily, that would happen only if llvm-symbolizer was buggy and
had to print something.

[Bug sanitizer/59136] llvm-symbolizer shouldn't be started always

[samsonov at google dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59136

--- Comment #4 from Alexey Samsonov <samsonov at google dot com> ---
(In reply to Jakub Jelinek from comment #3)
> (In reply to Alexey Samsonov from comment #2)
> > We found it convenient to run fork+exec early at program startup. It can
> > also be slightly dangerous to call fork() in multi-threaded program (even
> > though we call exec soon after fork). Also, you've mentioned that starting a
> 
> Well, POSIX is clear on what functions you can and what you can't call after
> fork in multi-threaded program.  You aren't calling exactly those functions,
> but functions that do the same thing as close/dup2/execl/_exit, that is
> fine, getdtablesize is not, but it just calls getrlimit syscall under the
> hood, which, while also not officially async-signal-safe, is safe in
> practice.
> 
> > symbolizer introduces some overhead, and it would be nice to start printing
> > error message ASAP (although, symbolizing the stack frame probably takes
> > much more time). That said, I think your suggestion makes sense. I'll try to
> > experiment with starting llvm-symbolizer lazily.
> 
> It makes sense to start early only if you expect every program to be buggy
> and assume you will actually need to print something.
> 
> > > Furthermore, it doesn't have stderr redirected to /dev/null and passes by
> > > default options e.g. llvm 3.3 llvm-symbolizer doesn't grok, so pretty much
> > > for everything it emits ugly error messages.
> > 
> > Not sure I understand it. Do you mean that when ASan runtime calls old
> > version of llvm-symbolizer, it passes command-line options which are not
> > recognized, and therefore fill the stderr with garbage?
> 
> Yeah, I'm getting:
> llvm-symbolizer: Unknown command line argument '--default-arch=x86_64'. 
> Try: '/usr/bin/llvm-symbolizer -help'
> llvm-symbolizer: Did you mean '-disable-ssc=x86_64'?
> on stderr for all programs linked against libasan or liblsan.

We may fix this specific garbage output (by redirecting stderr to /dev/null, as
you proposed), but in general we can't provide a guarantee that ASan will work
with older versions of llvm-symbolizer, sorry. For example, our runtime code
may depend on new features of llvm-symbolizer which are only implemented in
LLVM trunk. E.g. w/o the --default-arch support we won't be able to symbolize
universal binaries (including ASan runtime itself) on Mac.

> 
> > > And, when I've tried to LD_PRELOAD=./liblsan.so.0.0.0 ls -l
> > > it because of the starting llvm-symbolizer just in case created a beatiful
> > > fork-bomb.
> 
> And in the case of the fork-bomb it would just keep the above mentioned text
> forever,
> because with LD_PRELOAD=./liblsan.so.0.0.0 in the environment also
> llvm-symbolizer itself
> was linked against liblsan and when it tried to run llvm-symbolizer eagerly,
> it would do that forever.
> If it started lazily, that would happen only if llvm-symbolizer was buggy
> and had to print something.

[Bug sanitizer/59136] [4.9 Regression] llvm-symbolizer shouldn't be started always

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59136

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1
   Target Milestone|---                         |4.9.0
            Summary|llvm-symbolizer shouldn't   |[4.9 Regression]
                   |be started always           |llvm-symbolizer shouldn't
                   |                            |be started always

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
No processes should be launched at all.  Blocks the release - please make it
at least configurable to be able to turn it off.

[Bug sanitizer/59136] [4.9 Regression] llvm-symbolizer shouldn't be started always

[samsonov at google dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59136

--- Comment #6 from Alexey Samsonov <samsonov at google dot com> ---
(In reply to Richard Biener from comment #5)
> No processes should be launched at all.  Blocks the release - please make it
> at least configurable to be able to turn it off.

External process will only be launched if a user has llvm-symbolizer binary in
his PATH. This behaivor can also be turned off with a runtime option
(ASAN_OPTIONS=symbolize=false).

[Bug sanitizer/59136] [4.9 Regression] llvm-symbolizer shouldn't be started always

[burnus at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59136

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |burnus at gcc dot gnu.org

--- Comment #7 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Alexey Samsonov from comment #6)
> External process will only be launched if a user has llvm-symbolizer binary
> in his PATH. This behaivor can also be turned off with a runtime option
> (ASAN_OPTIONS=symbolize=false).

Can the ASAN_OPTIONS be documented somewhere in GCC? (e.g. along -fsanitize= in
invoke.texi). I know that they are somewhere hidden in the wiki, but having
some offline documentation would be useful. (Especially as the Wiki is rather
Clang/LLVM centric.)

[Bug sanitizer/59136] [4.9 Regression] llvm-symbolizer shouldn't be started always

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59136

--- Comment #8 from H.J. Lu <hjl.tools at gmail dot com> ---
(In reply to Alexey Samsonov from comment #6)
> External process will only be launched if a user has llvm-symbolizer binary
> in his PATH. This behaivor can also be turned off with a runtime option
> (ASAN_OPTIONS=symbolize=false).

This makes GCC compiled with ASAN totally unusable since
every configure fails with

llvm-symbolizer: Unknown command line argument '--default-arch=x86_64'.  Try:
'/usr/bin/llvm-symbolizer -help'
llvm-symbolizer: Did you mean '-disable-ssc=x86_64'?

ASAN_OPTIONS=symbolize=false should be the default when
built within GCC.

[Bug sanitizer/59136] [4.9 Regression] llvm-symbolizer shouldn't be started always

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59136

--- Comment #9 from H.J. Lu <hjl.tools at gmail dot com> ---
A patch is posted at

http://gcc.gnu.org/ml/gcc-patches/2013-11/msg03118.html

[Bug sanitizer/59136] [4.9 Regression] llvm-symbolizer shouldn't be started always

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59136

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Patches have been posted, but haven't been reviewed yet.
http://gcc.gnu.org/ml/gcc-patches/2013-12/msg00558.html
http://gcc.gnu.org/ml/gcc-patches/2013-12/msg00964.html

Various tsan testcases right now fail because it isn't in.

[Bug sanitizer/59136] [4.9 Regression] llvm-symbolizer shouldn't be started always

[glider at google dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59136

--- Comment #12 from Alexander Potapenko <glider at google dot com> ---
I wonder if https://code.google.com/p/address-sanitizer/issues/detail?id=253 is
relevant here. In the case TSan tests do fork() (which I'm not expecting from
them, however) the parent and the child will share the same symbolizer, which
will just fail to multiplex their queries.

[Bug sanitizer/59136] [4.9 Regression] llvm-symbolizer shouldn't be started always

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59136

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Thu Jan  9 18:13:39 2014
New Revision: 206475

URL: http://gcc.gnu.org/viewcvs?rev=206475&root=gcc&view=rev
Log:
    PR sanitizer/59136
libsanitizer/
    * sanitizer_common/Makefile.am (AM_CXXFLAGS): If
    LIBBACKTRACE_SUPPORTED add -DSANITIZER_LIBBACKTRACE
    and -I/-include flags.
    * lsan/Makefile.am (liblsan_la_LIBADD): Add
    libsanitizer_libbacktrace.la if LIBBACKTRACE_SUPPORTED.
    * tsan/Makefile.am (libtsan_la_LIBADD): Likewise.
    * ubsan/Makefile.am (libubsan_la_LIBADD): Likewise.
    * asan/Makefile.am (libasan_la_LIBADD): Likewise.
    * Makefile.am (SUBDIRS): If LIBBACKTRACE_SUPPORTED add
    libbacktrace.
    * README.gcc: Document that also lsan and ubsan are
    maintained in compiler-rt upstream.
    * libbacktrace/Makefile.am: New file.
    * libbacktrace/backtrace-rename.h: New file.
    * libbacktrace/backtrace-supported.h.in: New file.
    * libbacktrace/bridge.cc: New file.
    * configure.ac: Add tests needed for libbacktrace build
    within libsanitizer.
    * sanitizer_common/Makefile.in: Regenerated.
    * lsan/Makefile.in: Regenerated.
    * tsan/Makefile.in: Regenerated.
    * ubsan/Makefile.in: Regenerated.
    * libbacktrace/Makefile.in: Generated.
    * config.h.in: Regenerated.
    * configure: Regenerated.
    * Makefile.in: Regenerated.
    * interception/Makefile.in: Regenerated.
    * asan/Makefile.in: Regenerated.
    * aclocal.m4: Regenerated.
testsuite/
    * c-c++-common/asan/strip-path-prefix-1.c: Allow also the
    filename:line instead of (modulename+offset) form with stripped
    initial / from the filename.

Added:
    trunk/libsanitizer/config.h.in
    trunk/libsanitizer/libbacktrace/
    trunk/libsanitizer/libbacktrace/Makefile.am
    trunk/libsanitizer/libbacktrace/Makefile.in
    trunk/libsanitizer/libbacktrace/backtrace-rename.h
    trunk/libsanitizer/libbacktrace/backtrace-supported.h.in
    trunk/libsanitizer/libbacktrace/bridge.cc
Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/c-c++-common/asan/strip-path-prefix-1.c
    trunk/libsanitizer/ChangeLog
    trunk/libsanitizer/Makefile.am
    trunk/libsanitizer/Makefile.in
    trunk/libsanitizer/README.gcc
    trunk/libsanitizer/aclocal.m4
    trunk/libsanitizer/asan/Makefile.am
    trunk/libsanitizer/asan/Makefile.in
    trunk/libsanitizer/configure
    trunk/libsanitizer/configure.ac
    trunk/libsanitizer/interception/Makefile.in
    trunk/libsanitizer/lsan/Makefile.am
    trunk/libsanitizer/lsan/Makefile.in
    trunk/libsanitizer/sanitizer_common/Makefile.am
    trunk/libsanitizer/sanitizer_common/Makefile.in
    trunk/libsanitizer/tsan/Makefile.am
    trunk/libsanitizer/tsan/Makefile.in
    trunk/libsanitizer/ubsan/Makefile.am
    trunk/libsanitizer/ubsan/Makefile.in

[Bug sanitizer/59136] [4.9 Regression] llvm-symbolizer shouldn't be started always

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59136

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Should be fixed now.