[Bug sanitizer/59063] New: [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[Bug sanitizer/59063] New: [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[Joost.VandeVondele at mat dot ethz.ch]

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

            Bug ID: 59063
           Summary: [4.9 Regression] ASAN: segfault in
                    __interceptor_clock_gettime
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: sanitizer
          Assignee: unassigned at gcc dot gnu.org
          Reporter: Joost.VandeVondele at mat dot ethz.ch
                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

with -fsanitize=address the following code works on 4.8 branch bug segfaults
with 4.9

> cat test.f90 
  INTEGER, SAVE  :: count_max, count_rate
  CALL SYSTEM_CLOCK(count_rate=count_rate,count_max=count_max)
END 

> gfortran -fsanitize=address test.f90 ; ./a.out
ASAN:SIGSEGV
=================================================================
==48940==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc
0x000000000000 sp 0x7fff97430008 bp 0x7fff97430040 T0)
AddressSanitizer can not provide additional info.                               
==48940==ABORTING

Under gdb the bt is:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000000000 in ?? ()
(gdb) bt
#0  0x0000000000000000 in ?? ()
#1  0x00007ffff6b76278 in __interceptor_clock_gettime (clk_id=<optimized out>,
tp=0x7fffffffd6e0)
    at
../../../../gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:740
#2  0x00007ffff6b1efbc in gf_gettime_mono (tck=<synthetic pointer>,
fracsecs=<synthetic pointer>, secs=<synthetic pointer>)
    at ../../../gcc/libgfortran/intrinsics/system_clock.c:98
#3  _gfortran_system_clock_4 (count=0x0, count_rate=0x600f60 <count_rate.1893>,
count_max=0x600fa0 <count_max.1892>)
    at ../../../gcc/libgfortran/intrinsics/system_clock.c:147
#4  0x000000000040089d in MAIN__ ()
#5  0x00000000004008d3 in main ()

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[y.gribov at samsung dot com]

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

Yury Gribov <y.gribov at samsung dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |y.gribov at samsung dot com

--- Comment #1 from Yury Gribov <y.gribov at samsung dot com> ---
(In reply to Joost VandeVondele from comment #0)
> with -fsanitize=address the following code works on 4.8 branch bug segfaults
> with 4.9

I can confirm. Here is what's happening: clock_gettime is defined in librt.so
which is _not_ linked to the executable. On the other hand, libasan.so defines
this symbol in sanitizer_common_interceptors.inc. This interceptor will try to
call the original clock_gettime. The symbol will not be found which will result
in jump to NULL.

I'm not sure how to address this, perhaps we should link libasan against
librt.so on Linux?

I'd also suggest to make libasan issue warning in case "real" symbol obtained
in GetRealFunctionAddress is NULL. This would allow us to detect errors like
this faster.

-Y

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[eugeni.stepanov at gmail dot com]

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

--- Comment #2 from Evgeniy Stepanov <eugeni.stepanov at gmail dot com> ---
Weak symbols. Ouch.
We should link with libasan with librt. In Clang, we add -lrt to the linker
flags when we see -fsanitize=(address|thread|memory).

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I think in newer versions of glibc, clock_gettime is in libc.so now too.

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[y.gribov at samsung dot com]

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

--- Comment #4 from Yury Gribov <y.gribov at samsung dot com> ---
(In reply to Andrew Pinski from comment #3)
> I think in newer versions of glibc, clock_gettime is in libc.so now too.

I think Andrew is right - clock_gettime is in libc nowadays. Should we wontfix
this bug? A trivial WAR for users would be to add -lrt to cmdline.

(In reply to Evgeniy Stepanov from comment #2)
> We should link with libasan with librt. In Clang, we add -lrt to the linker
> flags when we see -fsanitize=(address|thread|memory).

Evgeniy, what about a warning in GetRealFunctionAddress if real symbol is NULL?

-Y

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[y.gribov at samsung dot com]

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

--- Comment #5 from Yury Gribov <y.gribov at samsung dot com> ---
(In reply to Yury Gribov from comment #4)
> I think Andrew is right - clock_gettime is in libc nowadays. Should we
> wontfix this bug? A trivial WAR for users would be to add -lrt to cmdline.

A patch to add librt would be trivial though...

-Y

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-11-11
     Ever confirmed|0                           |1

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Yury Gribov from comment #4)
> (In reply to Andrew Pinski from comment #3)
> > I think in newer versions of glibc, clock_gettime is in libc.so now too.
> 
> I think Andrew is right - clock_gettime is in libc nowadays. Should we
> wontfix this bug? A trivial WAR for users would be to add -lrt to cmdline.

It was just done for glibc 2.18 which was only released in the last year.  Not
everyone has the opportunity to upgrade to 2.18 yet or in some cases won't for
years to come.

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[y.gribov at samsung dot com]

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

--- Comment #7 from Yury Gribov <y.gribov at samsung dot com> ---
Created attachment 31192
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=31192&action=edit
Proposed patch

Does this patch make sense?

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Yury Gribov from comment #7)
> Created attachment 31192 [details]
> Proposed patch
> 
> Does this patch make sense?

No.  Just add it to STATIC_LIBASAN_LIBS (dunno about tsan, does it require the
same?), and make sure libasan is linked against librt.

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[y.gribov at samsung dot com]

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

--- Comment #9 from Yury Gribov <y.gribov at samsung dot com> ---
(In reply to Jakub Jelinek from comment #8)
> No.  Just add it to STATIC_LIBASAN_LIBS and make sure libasan is linked against librt.

Hm, I was under impression that these only get activated when user has
specified -static-libasan on cmdline...

> (dunno about tsan, does it require the same?), 

I don't think so, clock_* API are only intercepted by libasan.so.

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[jakub at gcc dot gnu.org]

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

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Yury Gribov from comment #9)
> (In reply to Jakub Jelinek from comment #8)
> > No.  Just add it to STATIC_LIBASAN_LIBS and make sure libasan is linked against librt.
> 
> Hm, I was under impression that these only get activated when user has
> specified -static-libasan on cmdline...

Sure.  But for dynamically linked libasan.so you just add it to
libasan_la_LDFLAGS
and it will DTRT.

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[eugeni.stepanov at gmail dot com]

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

--- Comment #11 from Evgeniy Stepanov <eugeni.stepanov at gmail dot com> ---
(In reply to Yury Gribov from comment #4)
> (In reply to Andrew Pinski from comment #3)
> > I think in newer versions of glibc, clock_gettime is in libc.so now too.
> 
> I think Andrew is right - clock_gettime is in libc nowadays. Should we
> wontfix this bug? A trivial WAR for users would be to add -lrt to cmdline.
> 
> (In reply to Evgeniy Stepanov from comment #2)
> > We should link with libasan with librt. In Clang, we add -lrt to the linker
> > flags when we see -fsanitize=(address|thread|memory).
> 
> Evgeniy, what about a warning in GetRealFunctionAddress if real symbol is
> NULL?

Warning sounds like a good idea.

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[y.gribov at samsung dot com]

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

Yury Gribov <y.gribov at samsung dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #31192|0                           |1
        is obsolete|                            |

--- Comment #12 from Yury Gribov <y.gribov at samsung dot com> ---
Created attachment 31193
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=31193&action=edit
Second-try patch

(In reply to Jakub Jelinek from comment #10)
> > Hm, I was under impression that these only get activated when user has
> > specified -static-libasan on cmdline...
> 
> Sure.  But for dynamically linked libasan.so you just add it to
> libasan_la_LDFLAGS
> and it will DTRT.

Makes sense. Attaching new patch.

I had to add the ugly -lstdc++ in pr59063-2.c because we do not autolink
libsupc++ when '-fsanitize=address -static-libasan' is specified. This may be a
bug.

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[y.gribov at samsung dot com]

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

--- Comment #13 from Yury Gribov <y.gribov at samsung dot com> ---
(In reply to Evgeniy Stepanov from comment #11)
> (In reply to Yury Gribov from comment #4)
> > Evgeniy, what about a warning in GetRealFunctionAddress if real symbol is
> > NULL?
> 
> Warning sounds like a good idea.

Cool. Should I file an issue at
http://code.google.com/p/address-sanitizer/issues ?

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[y.gribov at samsung dot com]

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

--- Comment #14 from Yury Gribov <y.gribov at samsung dot com> ---
(In reply to Yury Gribov from comment #12)
> Makes sense. Attaching new patch.

Forgot to mention - tested on x64_64.

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[eugeni.stepanov at gmail dot com]

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

--- Comment #15 from Evgeniy Stepanov <eugeni.stepanov at gmail dot com> ---
(In reply to Yury Gribov from comment #13)
> (In reply to Evgeniy Stepanov from comment #11)
> > (In reply to Yury Gribov from comment #4)
> > > Evgeniy, what about a warning in GetRealFunctionAddress if real symbol is
> > > NULL?
> > 
> > Warning sounds like a good idea.
> 
> Cool. Should I file an issue at
> http://code.google.com/p/address-sanitizer/issues ?

No need, I'm fixing it now.

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.9.0

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[y.gribov at samsung dot com]

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

--- Comment #16 from Yury Gribov <y.gribov at samsung dot com> ---
(In reply to Evgeniy Stepanov from comment #15)
> No need, I'm fixing it now.

Sorry for bothering but is this in compiler-rt trunk yet?

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[eugeni.stepanov at gmail dot com]

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

--- Comment #17 from Evgeniy Stepanov <eugeni.stepanov at gmail dot com> ---
(In reply to Yury Gribov from comment #16)
> (In reply to Evgeniy Stepanov from comment #15)
> > No need, I'm fixing it now.
> 
> Sorry for bothering but is this in compiler-rt trunk yet?

Sorry, I forgot to mention this. Should be done in r194372.

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[y.gribov at samsung dot com]

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

--- Comment #18 from Yury Gribov <y.gribov at samsung dot com> ---
(In reply to Evgeniy Stepanov from comment #17)
> Sorry, I forgot to mention this. Should be done in r194372.

Thanks! A pity this isn't going to help in gcc case that much because of bug
58680.

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[Joost.VandeVondele at mat dot ethz.ch]

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

Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |http://gcc.gnu.org/ml/gcc-p
                   |                            |atches/2013-11/msg01389.htm
                   |                            |l
                 CC|                            |Joost.VandeVondele at mat dot ethz
                   |                            |.ch

--- Comment #19 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
posted patch:

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

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[kcc at gcc dot gnu.org]

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

--- Comment #20 from Kostya Serebryany <kcc at gcc dot gnu.org> ---
This is fixed in llvm trunk, right? 
I am planing another merge to gcc in ~1 week.

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[ygribov at gcc dot gnu.org]

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

--- Comment #21 from ygribov at gcc dot gnu.org ---
Author: ygribov
Date: Fri Nov 29 12:30:36 2013
New Revision: 205524

URL: http://gcc.gnu.org/viewcvs?rev=205524&root=gcc&view=rev
Log:
gcc/
2013-11-29  Jakub Jelinek  <jakub@redhat.com>
            Yury Gribov  <y.gribov@samsung.com>

        PR sanitizer/59063
        * config/gnu-user.h: Removed old code for setting up sanitizer
        libs.
        * gcc.c: Using libsanitizer spec instead of explicit libs.

gcc/testsuite/
2013-11-29  Jakub Jelinek  <jakub@redhat.com>
            Yury Gribov  <y.gribov@samsung.com>

        PR sanitizer/59063
        * c-c++-common/asan/pr59063-1.c: New test.
        * c-c++-common/asan/pr59063-2.c: Likewise.
        * lib/asan-dg.exp: Add path to libsanitizer.spec to cflags.
        * lib/ubsan-dg.exp: Likewise.

libsanitizer/
2013-11-29  Jakub Jelinek  <jakub@redhat.com>
            Yury Gribov  <y.gribov@samsung.com>

        PR sanitizer/59063
        * libsanitizer.spec.in: Add spec file to hold link flags for
        various sanitizer libs.
        * configure.ac: Check whether clock_* routines come from librt.
        * asan/Makefile.am (libasan_la_LDFLAGS): Libs now come from
        configure.ac.
        * tsan/Makefile.am (libtsan_la_LDFLAGS): Likewise.
        * ubsan/Makefile.am (libubsan_la_LDFLAGS): Likewise.
        * lsan/Makefile.am (liblsan_la_LDFLAGS): Likewise.
        * asan/Makefile.in: Regenerate.
        * interception/Makefile.in: Regenerate.
        * lsan/Makefile.in: Regenerate.
        * sanitizer_common/Makefile.in: Regenerate.
        * tsan/Makefile.in: Regenerate.
        * ubsan/Makefile.in: Regenerate.
        * Makefile.in: Regenerate.
        * configure: Regenerate.


Added:
    trunk/gcc/testsuite/c-c++-common/asan/pr59063-1.c
    trunk/gcc/testsuite/c-c++-common/asan/pr59063-2.c
    trunk/libsanitizer/libsanitizer.spec.in
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/gnu-user.h
    trunk/gcc/gcc.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/lib/asan-dg.exp
    trunk/gcc/testsuite/lib/ubsan-dg.exp
    trunk/libsanitizer/ChangeLog
    trunk/libsanitizer/Makefile.am
    trunk/libsanitizer/Makefile.in
    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.in
    trunk/libsanitizer/tsan/Makefile.am
    trunk/libsanitizer/tsan/Makefile.in
    trunk/libsanitizer/ubsan/Makefile.am
    trunk/libsanitizer/ubsan/Makefile.in

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[y.gribov at samsung dot com]

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

--- Comment #22 from Yury Gribov <y.gribov at samsung dot com> ---
Joost, could you check and close bug if trunk works for you?

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[Joost.VandeVondele at mat dot ethz.ch]

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

Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> changed:

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

--- Comment #23 from Joost VandeVondele <Joost.VandeVondele at mat dot ethz.ch> ---
Thanks, fixed!

[Bug sanitizer/59063] [4.9 Regression] ASAN: segfault in __interceptor_clock_gettime

[jakub at gcc dot gnu.org]

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

--- Comment #24 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Tue Dec  3 16:01:13 2013
New Revision: 205639

URL: http://gcc.gnu.org/viewcvs?rev=205639&root=gcc&view=rev
Log:
    PR sanitizer/59063
    * lib/asan-dg.exp: Don't add anything to flags if libsanitizer
    has not been found.
    * lib/ubsan-dg.exp: Likewise.  Append to flags also
    -B${gccpath}/libsanitizer/.

Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/lib/asan-dg.exp
    trunk/gcc/testsuite/lib/ubsan-dg.exp