[Bug other/105729] New: False positive UBsan "reference binding to null pointer of type" when evaluating array indexing which throws exception

[Bug other/105729] New: False positive UBsan "reference binding to null pointer of type" when evaluating array indexing which throws exception

[egor_suvorov at mail dot ru]

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

            Bug ID: 105729
           Summary: False positive UBsan "reference binding to null
                    pointer of type" when evaluating array indexing which
                    throws exception
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: other
          Assignee: unassigned at gcc dot gnu.org
          Reporter: egor_suvorov at mail dot ru
  Target Milestone: ---

Consider the following code:

int range_check(int x) {
    throw 0;
}
struct Bar {};
struct Foo {
    Bar *data = nullptr;
    const Bar &get(int x) const {
        return data[range_check(x)];
    }
};
int main() {
    Foo b;
    try {
        b.get(-1);
    } catch (...) {
    }
}

In my understanding, it contains no UB: although binding a reference to
`data[range_check(x)]` would result in UB due to a null pointer reference (or
an invalid pointer altogether), it is never evaluated as `range_check` just
throws an exception. Hence, this program should not trigger any UBsan
(undefined sanitizer) warnings.

I was able to reproduce it on Ubuntu 20.04 with both "g++ (Ubuntu
9.4.0-1ubuntu1~20.04.1) 9.4.0" and "gcc version 10.3.0 (Ubuntu
10.3.0-1ubuntu1~20.04)":

1. Compile the code from above with `g++ -fsanitize=undefined a.cpp`
2. Run `./a.out`
3. Expected output: empty. Real output:

a.cpp:8:23: runtime error: reference binding to null pointer of type 'const
struct Bar'

This also reproduces on the trunk version of GCC at the Compiler Explorer
(https://godbolt.org/z/q5edxMbaE), but I was unable to find Clang version with
such behavior.

Additional information:

* If you set a `catch throw` in GDB in the program above, you get stopped
inside `__cxa_throw` after the `reference binding to null pointer` message is
emitted by the sanitizer. So it looks like the check is performed before
actually running the `range_check` function, which is a good idea unless
exceptions are involved.
* Moving the field to a local/global variable hides the issue
* Removing const-qualification from the method hides the issue

[Bug other/105729] False positive UBsan "reference binding to null pointer of type" when evaluating array indexing which throws exception

[marxin at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-05-26
             Status|UNCONFIRMED                 |NEW
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |marxin at gcc dot gnu.org,
                   |                            |mpolacek at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
@Jakub, Marek: Can you please take a look?

[Bug other/105729] False positive UBsan "reference binding to null pointer of type" when evaluating array indexing which throws exception

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I think the problem is that fold_unary optimizes
conversion of (const struct Bar *) ((const struct Foo *) this)->data +
(sizetype) range_check (x)
to const struct Bar &
type into conversion of the lhs of the POINTER_PLUS_EXPR
to const struct Bar & type followed by POINTER_PLUS_EXPR.

[Bug sanitizer/105729] False positive UBsan "reference binding to null pointer of type" when evaluating array indexing which throws exception

[jakub at gcc dot gnu.org]

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

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

[Bug sanitizer/105729] False positive UBsan "reference binding to null pointer of type" when evaluating array indexing which throws exception

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 53039
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53039&action=edit
gcc13-pr105729.patch

Untested fix.

[Bug sanitizer/105729] False positive UBsan "reference binding to null pointer of type" when evaluating array indexing which throws exception

[cvs-commit at gcc dot gnu.org]

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

--- 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:e2f014fcefcd2ad56b31995329820bbd99072eae

commit r13-795-ge2f014fcefcd2ad56b31995329820bbd99072eae
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri May 27 11:40:42 2022 +0200

    fold-const: Fix up -fsanitize=null in C++ [PR105729]

    The following testcase triggers a false positive UBSan binding a reference
    to null diagnostics.
    In the FE we instrument conversions from pointer to reference type
    to diagnose at runtime if the operand of such a conversion is 0.
    The problem is that a GENERIC folding folds
    ((const struct Bar *) ((const struct Foo *) this)->data) + (sizetype)
range_check (x)
    conversion to const struct Bar & by converting to that the first
    operand of the POINTER_PLUS_EXPR.  But that changes when the
-fsanitize=null
    binding to reference runtime check occurs.  Without the optimization,
    it is invoked on the result of the POINTER_PLUS_EXPR, and as range_check
    call throws, that means it never triggers in the testcase.
    With the optimization, it checks whether this->data is NULL and it is.

    The following patch avoids that optimization during GENERIC folding when
    -fsanitize=null is enabled and it is a cast from non-REFERENCE_TYPE to
    REFERENCE_TYPE.

    2022-05-27  Jakub Jelinek  <jakub@redhat.com>

            PR sanitizer/105729
            * fold-const.cc (fold_unary_loc): Don't optimize (X &) ((Y *) z +
w)
            to (X &) z + w if -fsanitize=null during GENERIC folding.

            * g++.dg/ubsan/pr105729.C: New test.

[Bug sanitizer/105729] False positive UBsan "reference binding to null pointer of type" when evaluating array indexing which throws exception

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:2f3ccb79ca859332915cb29a1c965a0f7be2408c

commit r12-8434-g2f3ccb79ca859332915cb29a1c965a0f7be2408c
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri May 27 11:40:42 2022 +0200

    fold-const: Fix up -fsanitize=null in C++ [PR105729]

    The following testcase triggers a false positive UBSan binding a reference
    to null diagnostics.
    In the FE we instrument conversions from pointer to reference type
    to diagnose at runtime if the operand of such a conversion is 0.
    The problem is that a GENERIC folding folds
    ((const struct Bar *) ((const struct Foo *) this)->data) + (sizetype)
range_check (x)
    conversion to const struct Bar & by converting to that the first
    operand of the POINTER_PLUS_EXPR.  But that changes when the
-fsanitize=null
    binding to reference runtime check occurs.  Without the optimization,
    it is invoked on the result of the POINTER_PLUS_EXPR, and as range_check
    call throws, that means it never triggers in the testcase.
    With the optimization, it checks whether this->data is NULL and it is.

    The following patch avoids that optimization during GENERIC folding when
    -fsanitize=null is enabled and it is a cast from non-REFERENCE_TYPE to
    REFERENCE_TYPE.

    2022-05-27  Jakub Jelinek  <jakub@redhat.com>

            PR sanitizer/105729
            * fold-const.cc (fold_unary_loc): Don't optimize (X &) ((Y *) z +
w)
            to (X &) z + w if -fsanitize=null during GENERIC folding.

            * g++.dg/ubsan/pr105729.C: New test.

    (cherry picked from commit e2f014fcefcd2ad56b31995329820bbd99072eae)

[Bug sanitizer/105729] False positive UBsan "reference binding to null pointer of type" when evaluating array indexing which throws exception

[cvs-commit at gcc dot gnu.org]

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

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

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

commit r11-10079-ge396f501f8a33439dee3ee5eef5b1d3a928852d9
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri May 27 11:40:42 2022 +0200

    fold-const: Fix up -fsanitize=null in C++ [PR105729]

    The following testcase triggers a false positive UBSan binding a reference
    to null diagnostics.
    In the FE we instrument conversions from pointer to reference type
    to diagnose at runtime if the operand of such a conversion is 0.
    The problem is that a GENERIC folding folds
    ((const struct Bar *) ((const struct Foo *) this)->data) + (sizetype)
range_check (x)
    conversion to const struct Bar & by converting to that the first
    operand of the POINTER_PLUS_EXPR.  But that changes when the
-fsanitize=null
    binding to reference runtime check occurs.  Without the optimization,
    it is invoked on the result of the POINTER_PLUS_EXPR, and as range_check
    call throws, that means it never triggers in the testcase.
    With the optimization, it checks whether this->data is NULL and it is.

    The following patch avoids that optimization during GENERIC folding when
    -fsanitize=null is enabled and it is a cast from non-REFERENCE_TYPE to
    REFERENCE_TYPE.

    2022-05-27  Jakub Jelinek  <jakub@redhat.com>

            PR sanitizer/105729
            * fold-const.c (fold_unary_loc): Don't optimize (X &) ((Y *) z + w)
            to (X &) z + w if -fsanitize=null during GENERIC folding.

            * g++.dg/ubsan/pr105729.C: New test.

    (cherry picked from commit e2f014fcefcd2ad56b31995329820bbd99072eae)

[Bug sanitizer/105729] False positive UBsan "reference binding to null pointer of type" when evaluating array indexing which throws exception

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 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:76562138659d6a3e6f11e467dd1ffd6d19f3df75

commit r10-10855-g76562138659d6a3e6f11e467dd1ffd6d19f3df75
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri May 27 11:40:42 2022 +0200

    fold-const: Fix up -fsanitize=null in C++ [PR105729]

    The following testcase triggers a false positive UBSan binding a reference
    to null diagnostics.
    In the FE we instrument conversions from pointer to reference type
    to diagnose at runtime if the operand of such a conversion is 0.
    The problem is that a GENERIC folding folds
    ((const struct Bar *) ((const struct Foo *) this)->data) + (sizetype)
range_check (x)
    conversion to const struct Bar & by converting to that the first
    operand of the POINTER_PLUS_EXPR.  But that changes when the
-fsanitize=null
    binding to reference runtime check occurs.  Without the optimization,
    it is invoked on the result of the POINTER_PLUS_EXPR, and as range_check
    call throws, that means it never triggers in the testcase.
    With the optimization, it checks whether this->data is NULL and it is.

    The following patch avoids that optimization during GENERIC folding when
    -fsanitize=null is enabled and it is a cast from non-REFERENCE_TYPE to
    REFERENCE_TYPE.

    2022-05-27  Jakub Jelinek  <jakub@redhat.com>

            PR sanitizer/105729
            * fold-const.c (fold_unary_loc): Don't optimize (X &) ((Y *) z + w)
            to (X &) z + w if -fsanitize=null during GENERIC folding.

            * g++.dg/ubsan/pr105729.C: New test.

    (cherry picked from commit e2f014fcefcd2ad56b31995329820bbd99072eae)

[Bug sanitizer/105729] False positive UBsan "reference binding to null pointer of type" when evaluating array indexing which throws exception

[jakub at gcc dot gnu.org]

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

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

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

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