public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "cvs-commit at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug sanitizer/108894] -fsanitize=bounds missing bounds provided by __builtin_dynamic_object_size()
Date: Tue, 28 Feb 2023 10:39:35 +0000	[thread overview]
Message-ID: <bug-108894-4-nkivwXrmQJ@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-108894-4@http.gcc.gnu.org/bugzilla/>

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

--- Comment #14 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:c7728805a7107444683290cd629d13f089130a0d

commit r13-6375-gc7728805a7107444683290cd629d13f089130a0d
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Feb 28 11:38:46 2023 +0100

    ubsan: Honor -fstrict-flex-arrays= in -fsanitize=bounds [PR108894]

    While this isn't really a regression, the -fstrict-flex-arrays*
    option is new in GCC 13 and so I think we should make -fsanitize=bounds
    play with it well from the beginning.

    The current behavior is that -fsanitize=bounds considers all trailing
    arrays as flexible member-like arrays and both -fsanitize=bounds and
    -fsanitize=bounds-strict because of a bug don't even instrument
    [0] arrays at all, not as trailing nor when followed by other members.

    I think -fstrict-flex-arrays* options can be considered as language
    mode changing options, by default flexible member-like arrays are
    handled like flexible arrays, but that option can change the set of
    the arrays which are treated like that.  So, -fsanitize=bounds should
    change with that on what is considered acceptable and what isn't.
    While -fsanitize=bounds-strict should reject them all always to
    continue previous behavior.

    The following patch implements that.  To support [0] array instrumentation,
    I had to change the meaning of the bounds argument to .UBSAN_BOUNDS,
    previously it was the TYPE_MAX_VALUE of the domain unless ignore_off_by_one
    (used for taking address of the array element rather than accessing it;
    in that case 1 is added to the bound argument) and the later lowered checks
    were if (index > bound) report_failure ().
    The problem with that is that for [0] arrays where (at least for C++)
    the max value is all ones, for accesses that condition will be never true;
    for addresses of elements it was working (in C++) correctly before.
    This patch changes it to add 1 + ignore_off_by_one, so -1 becomes 0 or
    1 for &array_ref and changing the lowering to be if (index >= bound)
    report_failure ().  Furthermore, as C represents the [0] arrays with
    NULL TYPE_MAX_VALUE, I treated those like the C++ ones.

    2023-02-28  Jakub Jelinek  <jakub@redhat.com>

            PR sanitizer/108894
    gcc/
            * ubsan.cc (ubsan_expand_bounds_ifn): Emit index >= bound
            comparison rather than index > bound.
            * gimple-fold.cc (gimple_fold_call): Use tree_int_cst_lt
            rather than tree_int_cst_le for IFN_UBSAN_BOUND comparison.
            * doc/invoke.texi (-fsanitize=bounds): Document that whether
            flexible array member-like arrays are instrumented or not depends
            on -fstrict-flex-arrays* options of strict_flex_array attributes.
            (-fsanitize=bounds-strict): Document that flexible array members
            are not instrumented.
    gcc/c-family/
            * c-common.h (c_strict_flex_array_level_of): Declare.
            * c-common.cc (c_strict_flex_array_level_of): New function,
            moved and renamed from c-decl.cc's strict_flex_array_level_of.
            * c-ubsan.cc (ubsan_instrument_bounds): Fix comment typo.  For
            C check c_strict_flex_array_level_of whether a trailing array
            should be treated as flexible member like.  Handle C [0] arrays.
            Add 1 + index_off_by_one rather than index_off_by_one to bounds
            and use tree_int_cst_lt rather than tree_int_cst_le for idx vs.
            bounds comparison.
    gcc/c/
            * c-decl.cc (strict_flex_array_level_of): Move to c-common.cc
            and rename to c_strict_flex_array_level_of.
            (is_flexible_array_member_p): Adjust caller.
    gcc/testsuite/
            * gcc.dg/ubsan/bounds-4.c: New test.
            * gcc.dg/ubsan/bounds-4a.c: New test.
            * gcc.dg/ubsan/bounds-4b.c: New test.
            * gcc.dg/ubsan/bounds-4c.c: New test.
            * gcc.dg/ubsan/bounds-4d.c: New test.
            * g++.dg/ubsan/bounds-1.C: New test.

  parent reply	other threads:[~2023-02-28 10:39 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-22 20:57 [Bug sanitizer/108894] New: " kees at outflux dot net
2023-02-22 21:03 ` [Bug sanitizer/108894] " kees at outflux dot net
2023-02-22 21:05 ` kees at outflux dot net
2023-02-22 21:06 ` pinskia at gcc dot gnu.org
2023-02-22 21:16 ` mpolacek at gcc dot gnu.org
2023-02-22 21:37 ` jakub at gcc dot gnu.org
2023-02-23  8:41 ` rguenth at gcc dot gnu.org
2023-02-23  8:57 ` jakub at gcc dot gnu.org
2023-02-23 14:24 ` mpolacek at gcc dot gnu.org
2023-02-23 19:40 ` qinzhao at gcc dot gnu.org
2023-02-23 19:43 ` jakub at gcc dot gnu.org
2023-02-23 21:10 ` qinzhao at gcc dot gnu.org
2023-02-23 21:13 ` jakub at gcc dot gnu.org
2023-02-23 21:21 ` qinzhao at gcc dot gnu.org
2023-02-27 16:52 ` jakub at gcc dot gnu.org
2023-02-27 20:18 ` qinzhao at gcc dot gnu.org
2023-02-28 10:39 ` cvs-commit at gcc dot gnu.org [this message]
2023-03-01  9:51 ` cvs-commit at gcc dot gnu.org

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bug-108894-4-nkivwXrmQJ@http.gcc.gnu.org/bugzilla/ \
    --to=gcc-bugzilla@gcc.gnu.org \
    --cc=gcc-bugs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).