[Bug sanitizer/65749] New: sanitizer stack trace pc off by 1

[Bug sanitizer/65749] New: sanitizer stack trace pc off by 1

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65749

            Bug ID: 65749
           Summary: sanitizer stack trace pc off by 1
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: sanitizer
          Assignee: unassigned at gcc dot gnu.org
          Reporter: msebor 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

The PC printed in sanitizer stack traces is consistently off by 1.  Either off
by 1 byte, or by 1 instruction, depending on the target.  The problem can be
seen in any stack trace, including the one below.

The StackTrace::Print() function in
libsanitizer/sanitizer_common/sanitizer_stacktrace_libcdep.cc computes the PC
to include in the backtrace like so:

    // PCs in stack traces are actually the return addresses, that is,
    // addresses of the next instructions after the call.
    uptr pc = GetPreviousInstructionPc(trace[i]);

GetPreviousInstructionPc() simply returns its argument decremented by the width
of the smallest instruction on a target, or 4 on powerpc and 1 on x86_64.

This computation is incorrect because it results in PC values that don't match
up with either the faulting address in the case of the active frame, or with
the address of either the subsequent instruction that would have been executed
had the faulting function returned to the caller or with the call instruction
(on x86_64).

As a side effect of this defect the sanitizer stack trace also sometimes lists
different line numbers (for programs compiled with -g) than GDB (and than is
recorded in the DWARF line program).

$ cat -n t.c && ./gcc/xgcc -Bgcc
-L$PWD/x86_64-unknown-linux-gnu/libsanitizer/asan/.libs -O2 -fno-builtin-memset
-fsanitize=address -g t.c &&
LD_LIBRARY_PATH=$PWD/x86_64-unknown-linux-gnu/libsanitizer/asan/.libs ./a.out;
LD_LIBRARY_PATH=$PWD/x86_64-unknown-linux-gnu/libsanitizer/asan/.libs gdb
-batch -ex r -ex 'x/1i $pc' -ex bt -quiet a.out
     1    void __attribute__ ((weak)) foo (int *p)
     2    {
     3        *p = 0xdeadbeef;
     4    }
     5    
     6    void __attribute__ ((weak)) bar (int *p)
     7    {
     8        int a;
     9        foo (&a);
    10    
    11        foo (p);
    12    }
    13    
    14    int main (void)
    15    {
    16        bar (0);
    17    
    18        return 0;
    19    }
    20    
ASAN:SIGSEGV
=================================================================
==7883==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc
0x0000004007ef bp 0x0ffffffffbc4 sp 0x7fffffffde18 T0)
    #0 0x4007ee in foo /build/gcc-65479/t.c:2
    #1 0x400869 in bar /build/gcc-65479/t.c:11
    #2 0x4006ba in main /build/gcc-65479/t.c:16
    #3 0x7ffff6ad3fdf in __libc_start_main (/lib64/libc.so.6+0x1ffdf)
    #4 0x4006fd  (/home/msebor/build/gcc-65479/a.out+0x4006fd)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /build/gcc-65479/t.c:2 foo
==7883==ABORTING
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
0x00000000004007ef in foo (p=p@entry=0x0) at t.c:3
3        *p = 0xdeadbeef;
=> 0x4007ef <foo+31>:    movl   $0xdeadbeef,(%rdi)
#0  0x00000000004007ef in foo (p=p@entry=0x0) at t.c:3
#1  0x000000000040086a in bar (p=p@entry=0x0) at t.c:11
#2  0x00000000004006bb in main () at t.c:16

[Bug sanitizer/65749] sanitizer stack trace pc off by 1

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65749

--- Comment #1 from Martin Sebor <msebor at gcc dot gnu.org> ---
Replacing the computation:
    uptr pc = GetPreviousInstructionPc(trace[i]);
with the assignment:
    uptr pc = trace[i];
makes the PC in active frame in the stack trace consistent with the one
reported on the ERROR: AddressSanitizer line, and the (return) addresses in
frames #1 and beyond consistent with those printed by GDB.  This patch however
causes regressions in a number of asan tests due to changes in the line
numbers.  An example is the stack-overflow-1.c test which is reported as
failing due to the change in the faulting line number in the stack trace from
the expected 16 to 13:

FAIL: c-c++-common/asan/stack-overflow-1.c   -O2  output pattern test, is
=================================================================
==14778==ERROR: AddressSanitizer: stack-buffer-overflow on address
0x7fff29383d3a at pc 0x000000400966 bp 0x7fff29383d00 sp 0x7fff29383cf8
READ of size 1 at 0x7fff29383d3a thread T0
    #0 0x400966 in main c-c++-common/asan/stack-overflow-1.c:13
    #1 0x7f1ad7283fe0 in __libc_start_main (/lib64/libc.so.6+0x1ffe0)
    #2 0x4009f5 
(/home/msebor/build/gcc-65479/gcc/testsuite/gcc/stack-overflow-1.exe+0x4009f5)

The line number reported by the patched sanitizer matches the line number
reported by tools like addr2line and readelf --debug-dump=decodedline:

$ addr2line 0x400966
/home/msebor/fsf/gcc-git/gcc/testsuite/c-c++-common/asan/stack-overflow-1.c:13

The complete line program for the test looks like this:

$ readelf  --debug-dump=decodedline a.out
Decoded dump of debug contents of section .debug_line:

CU:
/home/msebor/fsf/gcc-git/libsanitizer/sanitizer_common/sanitizer_stacktrace.h:
File name                            Line number    Starting address

CU:
/home/msebor/fsf/gcc-git/gcc/testsuite/c-c++-common/asan/stack-overflow-1.c:
File name                            Line number    Starting address
stack-overflow-1.c                            13            0x4008f6

stack-overflow-1.c                            15            0x40096e
stack-overflow-1.c                            16            0x400984
stack-overflow-1.c                            17            0x4009d7
stack-overflow-1.c                            13            0x4009dd
stack-overflow-1.c                            18            0x400a21
stack-overflow-1.c                            18            0x400a2e
stack-overflow-1.c                            18            0x400a32
stack-overflow-1.c                            18            0x400a43
stack-overflow-1.c                            18            0x400a47

[Bug sanitizer/65749] sanitizer stack trace pc off by 1

[y.gribov at samsung dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65749

--- Comment #3 from Yury Gribov <y.gribov at samsung dot com> ---
@Kostya: I suggest to mention this in ASan FAQ.

[Bug sanitizer/65749] sanitizer stack trace pc off by 1

[y.gribov at samsung dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65749

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

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

--- Comment #2 from Yury Gribov <y.gribov at samsung dot com> ---
This is not a bug but rather a design choice - it is hard to compute exact size
of preceding instruction on CISC platforms. ASan just decrements 1 because this
is enough for tools like addr2line or gdb to symbolize addresses. Replacing by
trace[i] would indeed cause invalid symbolization as you already noticed.

[Bug sanitizer/65749] sanitizer stack trace pc off by 1

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65749

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
For the purpose of looking up the address in line table etc. IMNSHO the
subtraction of 1 is needed (that is what gcc unwinder does too, except for
signal frames where the pc must be on the faulting or asynchronously
interrupted insn)).  But if the addresses are printed, supposedly it should
match what the debugger does, and at least gdb prints the address after the
call instruction, not address after the call instruction - 1.