[Bug sanitizer/97294] New: ASAN "dynamic-stack-buffer-overflow" false positive with OpenMP reduction to std::vector

[Bug sanitizer/97294] New: ASAN "dynamic-stack-buffer-overflow" false positive with OpenMP reduction to std::vector

[paulg-b at web dot de]

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

            Bug ID: 97294
           Summary: ASAN "dynamic-stack-buffer-overflow" false positive
                    with OpenMP reduction to std::vector
           Product: gcc
           Version: 10.2.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: sanitizer
          Assignee: unassigned at gcc dot gnu.org
          Reporter: paulg-b at web dot de
                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, marxin at gcc dot gnu.org
  Target Milestone: ---

Created attachment 49308
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49308&action=edit
Minimal example

When the attached minimal example is compiled with

g++ -fsanitize=address -fopenmp -o asan_omp_test asan_omp_test.cpp

and executed, ASAN reports a dynamic-stack-buffer-overflow. I tested gcc 10.2.1
and 8.2 on different systems (one being the login node of a cluster, the other
being my personal computer with a Fedora 32 OS). 

ASAN seems to report on the copy of the array-section invoked by the omp clause
reduction(+ : ptr[:v.size()])
where ptr is a copy of v.data() and v is the std::vector.

This does *not* happen when the std::vector is replaced by std::array. 

The same code does *not* report anything when compiled and ASAN-instrumented
with clang 10.0.1. I first filed a issue
https://github.com/google/sanitizers/issues/1326 and was sent here.

The problem is also *not* reproducible in C when replacing the std::vector with
dynamic memory allocated via calloc.

[Bug sanitizer/97294] ASAN "dynamic-stack-buffer-overflow" false positive with OpenMP reduction to std::vector

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |openmp
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2020-10-06

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Confirmed, it used to work with GCC 7. I bet it's related to a change in vector
class.
@Jakub: Will you take a look please?

[Bug sanitizer/97294] ASAN "dynamic-stack-buffer-overflow" false positive with OpenMP reduction to std::vector

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 49323
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49323&action=edit
gcc11-pr97294.patch

Untested fix.
Self-contained testcase with -O0 -fsanitize=address -fopenmp could be e.g.
__attribute__((noipa)) void
foo (int *p, int n)
{
  int i;
  #pragma omp parallel for num_threads(2) reduction(+:p[:n])
  for (i = 0; i < 10; i++)
    {
      p[0]++;
      p[n - 1] += 2;
    }
}

__attribute__((noipa)) void
bar (void)
{
  unsigned char buf[1024];
  int i;
  asm volatile ("" : : "r" (&buf[0]) : "memory");
  for (i = 0; i < 1024; i++)
    buf[i] = i;
  asm volatile ("" : : "r" (&buf[0]) : "memory");
}

int
main ()
{
  int p[50], i;
  for (i = 0; i < 50; i++)
    p[i] = 0;
  foo (p, 50);
  bar ();
  if (p[0] != 10 || p[49] != 20)
    __builtin_abort ();
  return 0;
}
The problem was that nothing set cfun->calls_alloca flag in the child omp
function and thus the asan code wouldn't add __asan_allocas_unpoison call at
the end of the function.  Normally when optimizing, DCE clears those flags and
recomputes them again, so this was only problematic with -O0.

[Bug sanitizer/97294] ASAN "dynamic-stack-buffer-overflow" false positive with OpenMP reduction to std::vector

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Note I'm afraid we can't put the testcase into GCC testsuite easily, because we
have testsuite for libgomp and testsuite for asan, but don't have *.exp to link
against both libraries at their build locations at once.

[Bug sanitizer/97294] ASAN "dynamic-stack-buffer-overflow" false positive with OpenMP reduction to std::vector

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:273b30c8e93f043f65a04e0ff2ec305b311e98fa

commit r11-3716-g273b30c8e93f043f65a04e0ff2ec305b311e98fa
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Oct 8 11:10:34 2020 +0200

    openmp: Set cfun->calls_alloca when needed in OpenMP outlined regions
[PR97294]

    The following testcase FAILs, because we don't mark the child OpenMP
function
    as cfun->calls_alloca when it does call alloca.  When optimizing, during
DCE we
    reset those flags and recompute them again, but with -O0 DCE is not
performed.

    Fixed by calling notice_special_calls when moving insns to the child
function.

    cfun->calls_alloca is normally set during gimplification and most of the
    alloca calls omp-low.c does go through the gimplifier, but one spot didn't
    and built the gcall directly, so that one needs to set calls_alloca too.

    2020-10-08  Jakub Jelinek  <jakub@redhat.com>

            PR sanitizer/97294
            * tree-cfg.c (move_block_to_fn): Call notice_special_calls on
            call stmts being moved into dest_cfun.
            * omp-low.c (lower_rec_input_clauses): Set cfun->calls_alloca when
            adding __builtin_alloca_with_align call without gimplification.

            * gcc.dg/asan/pr97294.c: New test.

[Bug sanitizer/97294] ASAN "dynamic-stack-buffer-overflow" false positive with OpenMP reduction to std::vector

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 11+ so far.

[Bug sanitizer/97294] ASAN "dynamic-stack-buffer-overflow" false positive with OpenMP reduction to std::vector

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:a30fcfb3b848e4b895cb47378b4a250184af3afe

commit r10-8901-ga30fcfb3b848e4b895cb47378b4a250184af3afe
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Oct 8 11:10:34 2020 +0200

    openmp: Set cfun->calls_alloca when needed in OpenMP outlined regions
[PR97294]

    The following testcase FAILs, because we don't mark the child OpenMP
function
    as cfun->calls_alloca when it does call alloca.  When optimizing, during
DCE we
    reset those flags and recompute them again, but with -O0 DCE is not
performed.

    Fixed by calling notice_special_calls when moving insns to the child
function.

    cfun->calls_alloca is normally set during gimplification and most of the
    alloca calls omp-low.c does go through the gimplifier, but one spot didn't
    and built the gcall directly, so that one needs to set calls_alloca too.

    2020-10-08  Jakub Jelinek  <jakub@redhat.com>

            PR sanitizer/97294
            * tree-cfg.c (move_block_to_fn): Call notice_special_calls on
            call stmts being moved into dest_cfun.
            * omp-low.c (lower_rec_input_clauses): Set cfun->calls_alloca when
            adding __builtin_alloca_with_align call without gimplification.

            * gcc.dg/asan/pr97294.c: New test.

    (cherry picked from commit 273b30c8e93f043f65a04e0ff2ec305b311e98fa)

[Bug sanitizer/97294] ASAN "dynamic-stack-buffer-overflow" false positive with OpenMP reduction to std::vector

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:2913a8f35b7100e8632d2c10dc4126a636cbc9d9

commit r9-9389-g2913a8f35b7100e8632d2c10dc4126a636cbc9d9
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Oct 8 11:10:34 2020 +0200

    openmp: Set cfun->calls_alloca when needed in OpenMP outlined regions
[PR97294]

    The following testcase FAILs, because we don't mark the child OpenMP
function
    as cfun->calls_alloca when it does call alloca.  When optimizing, during
DCE we
    reset those flags and recompute them again, but with -O0 DCE is not
performed.

    Fixed by calling notice_special_calls when moving insns to the child
function.

    cfun->calls_alloca is normally set during gimplification and most of the
    alloca calls omp-low.c does go through the gimplifier, but one spot didn't
    and built the gcall directly, so that one needs to set calls_alloca too.

    2020-10-08  Jakub Jelinek  <jakub@redhat.com>

            PR sanitizer/97294
            * tree-cfg.c (move_block_to_fn): Call notice_special_calls on
            call stmts being moved into dest_cfun.
            * omp-low.c (lower_rec_input_clauses): Set cfun->calls_alloca when
            adding __builtin_alloca_with_align call without gimplification.

            * gcc.dg/asan/pr97294.c: New test.

    (cherry picked from commit a30fcfb3b848e4b895cb47378b4a250184af3afe)

[Bug sanitizer/97294] ASAN "dynamic-stack-buffer-overflow" false positive with OpenMP reduction to std::vector

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-8 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:129f0dfb1f08afc7029d9559aa77befa545fb666

commit r8-10859-g129f0dfb1f08afc7029d9559aa77befa545fb666
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Thu Oct 8 11:10:34 2020 +0200

    openmp: Set cfun->calls_alloca when needed in OpenMP outlined regions
[PR97294]

    The following testcase FAILs, because we don't mark the child OpenMP
function
    as cfun->calls_alloca when it does call alloca.  When optimizing, during
DCE we
    reset those flags and recompute them again, but with -O0 DCE is not
performed.

    Fixed by calling notice_special_calls when moving insns to the child
function.

    cfun->calls_alloca is normally set during gimplification and most of the
    alloca calls omp-low.c does go through the gimplifier, but one spot didn't
    and built the gcall directly, so that one needs to set calls_alloca too.

    2020-10-08  Jakub Jelinek  <jakub@redhat.com>

            PR sanitizer/97294
            * tree-cfg.c (move_block_to_fn): Call notice_special_calls on
            call stmts being moved into dest_cfun.
            * omp-low.c (lower_rec_input_clauses): Set cfun->calls_alloca when
            adding __builtin_alloca_with_align call without gimplification.

            * gcc.dg/asan/pr97294.c: New test.

    (cherry picked from commit a30fcfb3b848e4b895cb47378b4a250184af3afe)

[Bug sanitizer/97294] ASAN "dynamic-stack-buffer-overflow" false positive with OpenMP reduction to std::vector

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.

[Bug sanitizer/97294] ASAN "dynamic-stack-buffer-overflow" false positive with OpenMP reduction to std::vector

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |8.5