[Bug c++/62017] New: AddressSanitizer reports *-buffer-overflow in destructor when multiple virtual inheritance is used

[Bug c++/62017] New: AddressSanitizer reports *-buffer-overflow in destructor when multiple virtual inheritance is used

[bezkrovatki at gmail dot com]

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

            Bug ID: 62017
           Summary: AddressSanitizer reports *-buffer-overflow in
                    destructor when multiple virtual inheritance is used
           Product: gcc
           Version: 4.9.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: bezkrovatki at gmail dot com

Consider the following sample code (test.cpp):
====
struct IA
{
    virtual ~IA() {}
};

struct IB
{
    virtual ~IB() {}
};

struct IC: virtual IA, virtual IB {};

struct CA : virtual IA {};

struct CB: virtual IB {};

struct CC: virtual IC, CA, CB {};

int main()
{
    CC c;
    return 0;
}
====
Compile it with g++ 4.9.1 (Debian sid amd64):
g++ -o test.asan -g -O0 -fno-omit-frame-pointer -fsanitize=address test.cpp
Running it gives the following report:

==3591==ERROR: AddressSanitizer: stack-buffer-overflow on address
0x7fff92d37da0 at pc 0x400f58 bp 0x7fff92d37d20 sp 0x7fff92d37d18
WRITE of size 16 at 0x7fff92d37da0 thread T0
    #0 0x400f57 in IC::~IC() test.cpp:11
    #1 0x401675 in CC::~CC() test.cpp:17
    #2 0x400a20 in main test.cpp:22
    #3 0x7fd0c55a6b44 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x21b44)
    #4 0x4008b8 (test.asan+0x4008b8)

Address 0x7fff92d37da0 is located in stack of thread T0 at offset 48 in frame
    #0 0x400995 in main test.cpp:20

  This frame has 1 object(s):
    [32, 56) 'c' <== Memory access at offset 48 partially overflows this
variable
HINT: this may be a false positive if your program uses some custom stack
unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow test.cpp:11 IC::~IC()
Shadow bytes around the buggy address:
  0x10007259ef60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007259ef70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007259ef80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007259ef90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007259efa0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1
=>0x10007259efb0: f1 f1 00 00[00]f4 f3 f3 f3 f3 00 00 00 00 00 00
  0x10007259efc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007259efd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007259efe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007259eff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007259f000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Contiguous container OOB:fc
  ASan internal:           fe
==3591==ABORTING

When an object of type CC is allocated on heap the error is reported as well.
If an object of type CC is aggregated into another type and followed by another
field (e.g. the type of variable 'c' from the sample is
std::pair<CC,std::nullptr_t>) then the error no is reported.

No error is observed when clang++ 3.4.2 or g++ 4.8.3 is used.

[Bug c++/62017] AddressSanitizer reports *-buffer-overflow in destructor when multiple virtual inheritance is used

[y.gribov at samsung dot com]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dodji at gcc dot gnu.org,
                   |                            |jakub at redhat dot com,
                   |                            |kcc at gcc dot gnu.org,
                   |                            |y.gribov at samsung dot com

--- Comment #1 from Yury Gribov <y.gribov at samsung dot com> ---
Adding sanitizer folks. It seems that size of standalone IC class is 16 but
this shortens to 8 (due to virtual inheritance magic) when IC inherited in CC.
GCC still emits 16 byte check which causes false positive.

[Bug c++/62017] AddressSanitizer reports *-buffer-overflow in destructor when multiple virtual inheritance is used

[chefmax at gcc dot gnu.org]

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

Maxim Ostapenko <chefmax at gcc dot gnu.org> changed:

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

--- Comment #2 from Maxim Ostapenko <chefmax at gcc dot gnu.org> ---
Here another reprocase (diamond inheritance):
======================

struct A {
  int x;
  virtual ~A() {}
};

struct B: public virtual A {
};

struct C: public virtual A {
};

struct D: public B, virtual public C {
};

int main() {
  D d;
}


==24761==ERROR: AddressSanitizer: stack-buffer-overflow on address
0x7ffff1d86bf8 at pc 0x400e1b bp 0x7ffff1d86b70 sp 0x7ffff1d86b68
WRITE of size 24 at 0x7ffff1d86bf8 thread T0
    #0 0x400e1a in C::~C() (/home/max/build/gcc-upstream-x86_64/a.out+0x400e1a)
    #1 0x401257 in D::~D() (/home/max/build/gcc-upstream-x86_64/a.out+0x401257)
    #2 0x400a8b in main (/home/max/build/gcc-upstream-x86_64/a.out+0x400a8b)
    #3 0x7f7487a1676c in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x2176c)
    #4 0x400918 (/home/max/build/gcc-upstream-x86_64/a.out+0x400918)

Address 0x7ffff1d86bf8 is located in stack of thread T0 at offset 56 in frame
    #0 0x400a17 in main (/home/max/build/gcc-upstream-x86_64/a.out+0x400a17)

  This frame has 1 object(s):
    [32, 64) 'd' <== Memory access at offset 56 partially overflows this
variable
HINT: this may be a false positive if your program uses some custom stack
unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow ??:0 C::~C()
Shadow bytes around the buggy address:
  0x10007e3a8d20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007e3a8d30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007e3a8d40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007e3a8d50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007e3a8d60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x10007e3a8d70: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00[00]
  0x10007e3a8d80: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007e3a8d90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007e3a8da0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007e3a8db0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007e3a8dc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  ASan internal:           fe
==24761==ABORTING

Compiled with recent GCC version:
GNU C++ (GCC) version 5.0.0 20140916 (experimental) (x86_64-unknown-linux-gnu)

[Bug c++/62017] AddressSanitizer reports *-buffer-overflow in destructor when multiple virtual inheritance is used

[chefmax at gcc dot gnu.org]

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

--- Comment #3 from Maxim Ostapenko <chefmax at gcc dot gnu.org> ---
Test fails only if compile on -O0 and -Os optimization levels:

$ ~/install/master/bin/g++ diamond_virt.cc -fsanitize=address -Os

On -O1, -O2 and -O3 ASan does not report any error.

[Bug c++/62017] AddressSanitizer reports *-buffer-overflow in destructor when multiple virtual inheritance is used

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
CCing Jason whether the type on the MEM_REF in the clobber stmt in C::~C is
appropriate though.

[Bug c++/62017] AddressSanitizer reports *-buffer-overflow in destructor when multiple virtual inheritance is used

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Thu Sep 18 14:08:28 2014
New Revision: 215354

URL: https://gcc.gnu.org/viewcvs?rev=215354&root=gcc&view=rev
Log:
    PR c++/62017
    * asan.c (transform_statements): Don't instrument clobber statements.

    * g++.dg/asan/pr62017.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/asan/pr62017.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/asan.c
    trunk/gcc/testsuite/ChangeLog

[Bug c++/62017] AddressSanitizer reports *-buffer-overflow in destructor when multiple virtual inheritance is used

[jakub at gcc dot gnu.org]

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Thu Sep 18 14:09:57 2014
New Revision: 215355

URL: https://gcc.gnu.org/viewcvs?rev=215355&root=gcc&view=rev
Log:
    PR c++/62017
    * asan.c (transform_statements): Don't instrument clobber statements.

    * g++.dg/asan/pr62017.C: New test.

Added:
    branches/gcc-4_9-branch/gcc/testsuite/g++.dg/asan/pr62017.C
Modified:
    branches/gcc-4_9-branch/gcc/ChangeLog
    branches/gcc-4_9-branch/gcc/asan.c
    branches/gcc-4_9-branch/gcc/testsuite/ChangeLog

[Bug c++/62017] AddressSanitizer reports *-buffer-overflow in destructor when multiple virtual inheritance is used

[jason at gcc dot gnu.org]

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

--- Comment #8 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #5)
> CCing Jason whether the type on the MEM_REF in the clobber stmt in C::~C is
> appropriate though.

What MEM_REF?  Compiling with -fdump-tree-gimple the only clobber I see is

        *this = {CLOBBER};

are you suggesting that we should only clobber the CLASSTYPE_AS_BASE subset of
*this?  That makes sense.

[Bug c++/62017] AddressSanitizer reports *-buffer-overflow in destructor when multiple virtual inheritance is used

[jakub at gcc dot gnu.org]

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Jason Merrill from comment #8)
> (In reply to Jakub Jelinek from comment #5)
> > CCing Jason whether the type on the MEM_REF in the clobber stmt in C::~C is
> > appropriate though.
> 
> What MEM_REF?  Compiling with -fdump-tree-gimple the only clobber I see is
> 
>         *this = {CLOBBER};
> 
> are you suggesting that we should only clobber the CLASSTYPE_AS_BASE subset
> of *this?  That makes sense.

*this is actually MEM_REF<this, 0> with some type, and yeah, I mean that
clobber.
Don't know how exactly CLASSTYPE_AS_BASE differs from the type of *this to know
what should be clobbered, the clobber should say as accurately as possible what
becomes undefined after the dtor ends, so that e.g. stores to fields in the
class can be DSEd if nothing uses them until the destructor.

[Bug c++/62017] AddressSanitizer reports *-buffer-overflow in destructor when multiple virtual inheritance is used

[jason at gcc dot gnu.org]

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

--- Comment #10 from Jason Merrill <jason at gcc dot gnu.org> ---
Created attachment 33511
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33511&action=edit
patch to dtor clobbers

Like this?

[Bug c++/62017] AddressSanitizer reports *-buffer-overflow in destructor when multiple virtual inheritance is used

[jason at gcc dot gnu.org]

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

--- Comment #11 from Jason Merrill <jason at gcc dot gnu.org> ---
Author: jason
Date: Sun Sep 21 02:42:40 2014
New Revision: 215427

URL: https://gcc.gnu.org/viewcvs?rev=215427&root=gcc&view=rev
Log:
    PR c++/62017
    * decl.c (begin_destructor_body): Only clobber the as-base part of
    *this.

Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/decl.c

[Bug c++/62017] AddressSanitizer reports *-buffer-overflow in destructor when multiple virtual inheritance is used

[jason at gcc dot gnu.org]

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

--- Comment #12 from Jason Merrill <jason at gcc dot gnu.org> ---
Author: jason
Date: Fri Feb 13 22:14:07 2015
New Revision: 220699

URL: https://gcc.gnu.org/viewcvs?rev=220699&root=gcc&view=rev
Log:
    PR c++/62017
    * decl.c (begin_destructor_body): Only clobber the as-base part of
    *this.

Modified:
    branches/gcc-4_9-branch/gcc/cp/ChangeLog
    branches/gcc-4_9-branch/gcc/cp/decl.c

[Bug c++/62017] AddressSanitizer reports *-buffer-overflow in destructor when multiple virtual inheritance is used

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|---                         |4.9.3

--- Comment #13 from Jason Merrill <jason at gcc dot gnu.org> ---
Fixed.