[PATCH] Fix false positive for -Walloc-size-larger-than (PR, bootstrap/79132).

[PATCH] Fix false positive for -Walloc-size-larger-than (PR, bootstrap/79132).

[Martin Liška]

[-- Attachment #1: Type: text/plain, Size: 109 bytes --]

Hello.

Following patch fixes asan bootstrap, as mentioned in the PR.

Ready to be installed?
Thanks,
Martin

[-- Attachment #2: 0001-Fix-false-positive-for-Walloc-size-larger-than-PR-bo.patch --]
[-- Type: text/x-patch, Size: 978 bytes --]

From 6a3d1b85e124751fdb804ae86596d30ea98b54af Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 19 Jan 2017 10:25:55 +0100
Subject: [PATCH] Fix false positive for -Walloc-size-larger-than (PR
 bootstrap/79132).

gcc/ChangeLog:

2017-01-19  Martin Liska  <mliska@suse.cz>

	PR bootstrap/79132
	* tree-ssa-reassoc.c (rewrite_expr_tree_parallel): Insert assert
	that would prevent us to call alloca with -1 as argument.
---
 gcc/tree-ssa-reassoc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c
index 503edd3870d..4a796f48864 100644
--- a/gcc/tree-ssa-reassoc.c
+++ b/gcc/tree-ssa-reassoc.c
@@ -4407,6 +4407,7 @@ rewrite_expr_tree_parallel (gassign *stmt, int width,
 {
   enum tree_code opcode = gimple_assign_rhs_code (stmt);
   int op_num = ops.length ();
+  gcc_assert (op_num > 0);
   int stmt_num = op_num - 1;
   gimple **stmts = XALLOCAVEC (gimple *, stmt_num);
   int op_index = op_num - 1;
-- 
2.11.0

Re: [PATCH] Fix false positive for -Walloc-size-larger-than (PR, bootstrap/79132).

[Jeff Law]

On 01/19/2017 04:46 AM, Martin Liška wrote:
> Hello.
>
> Following patch fixes asan bootstrap, as mentioned in the PR.
>
> Ready to be installed?
> Thanks,
> Martin
>
>
> 0001-Fix-false-positive-for-Walloc-size-larger-than-PR-bo.patch
>
>
> From 6a3d1b85e124751fdb804ae86596d30ea98b54af Mon Sep 17 00:00:00 2001
> From: marxin <mliska@suse.cz>
> Date: Thu, 19 Jan 2017 10:25:55 +0100
> Subject: [PATCH] Fix false positive for -Walloc-size-larger-than (PR
>  bootstrap/79132).
>
> gcc/ChangeLog:
>
> 2017-01-19  Martin Liska  <mliska@suse.cz>
>
> 	PR bootstrap/79132
> 	* tree-ssa-reassoc.c (rewrite_expr_tree_parallel): Insert assert
> 	that would prevent us to call alloca with -1 as argument.
Maybe one day we'll be able to use the array index into STMTS to derive 
a range for stmt_num.  But we don't right now.

Otherwise I don't see any way to derive a range for stmt_num that if 
rewrite_expr_tree_parallel is not inlined into its caller.


OK for the trunk.

jeff

Fix false positive for -Walloc-size-larger-than, part II [PR79132] (was: [PATCH] Fix false positive for -Walloc-size-larger-than (PR, bootstrap/79132))

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 4689 bytes --]

Hi!

On 2017-01-23T15:10:44-0700, Jeff Law <law@redhat.com> wrote:
> On 01/19/2017 04:46 AM, Martin Liška wrote:
>> Following patch fixes asan bootstrap, as mentioned in the PR.
>>
>> Ready to be installed?

>>      * tree-ssa-reassoc.c (rewrite_expr_tree_parallel): Insert assert
>>      that would prevent us to call alloca with -1 as argument.

> Maybe one day we'll be able to use the array index into STMTS to derive
> a range for stmt_num.  But we don't right now.
>
> Otherwise I don't see any way to derive a range for stmt_num that if
> rewrite_expr_tree_parallel is not inlined into its caller.
>
>
> OK for the trunk.

Just a few years later, I've run into this one again -- with only a
slight twist; probably latent all those years...  OK to push the attached
"Fix false positive for -Walloc-size-larger-than, part II [PR79132]"?

Or, do we (incrementally?) want to formulate that "assume"-like trait yet
differently?  That is, should we gain some 'gcc_assume ([...])' in
'gcc/system.h'?

CCing Jakub as author of commit 08b51baddc53d64aa4c5e7a81ef3c4bf320293be
"c++, c: Implement C++23 P1774R8 - Portable assumptions [PR106654]":

    The following patch implements C++23 P1774R8 - Portable assumptions
    paper, by introducing support for [[assume (cond)]]; attribute for C++.
    In addition to that the patch adds [[gnu::assume (cond)]]; and
    __attribute__((assume (cond))); support to both C and C++.
    As described in C++23, the attribute argument is conditional-expression
    rather than the usual assignment-expression for attribute arguments,
    the condition is contextually converted to bool (for C truthvalue conversion
    is done on it) and is never evaluated at runtime.
    For C++ constant expression evaluation, I only check the simplest conditions
    for undefined behavior, because otherwise I'd need to undo changes to
    *ctx->global which happened during the evaluation (but I believe the spec
    allows that and we can further improve later).
    The patch uses a new internal function, .ASSUME, to hold the condition
    in the FEs.  At gimplification time, if the condition is simple/without
    side-effects, it is gimplified as if (cond) ; else __builtin_unreachable ();
    and otherwise for now dropped on the floor.  The intent is to incrementally
    outline the conditions into separate artificial functions and use
    .ASSUME further to tell the ranger and perhaps other optimization passes
    about the assumptions, as detailed in the PR.

    When implementing it, I found that assume entry hasn't been added to
    https://eel.is/c++draft/cpp.cond#6
    Jonathan said he'll file a NB comment about it, this patch assumes it
    has been added into the table as 202207L when the paper has been voted in.

    With the attributes for both C/C++, I'd say we don't need to add
    __builtin_assume with similar purpose, especially when __builtin_assume
    in LLVM is just weird.  It is strange for side-effects in function call's
    argument not to be evaluated, and LLVM in that case (annoyingly) warns
    and ignores the side-effects (but doesn't do then anything with it),
    if there are no side-effects, it will work like our
    if (!cond) __builtin_unreachable ();

(Followed by further commits re "intent is to incrementally [...]".)

The current 'gcc/doc/extend.texi', "Statement Attributes" for reference:

    @cindex @code{assume} statement attribute
    @item assume
    The @code{assume} attribute with a null statement serves as portable
    assumption.  It should have a single argument, a conditional expression,
    which is not evaluated.  If the argument would evaluate to true
    at the point where it appears, it has no effect, otherwise there
    is undefined behavior.  This is a GNU variant of the ISO C++23
    standard @code{assume} attribute, but it can be used in any version of
    both C and C++.

    @smallexample
    int
    foo (int x, int y)
    @{
      __attribute__((assume(x == 42)));
      __attribute__((assume(++y == 43)));
      return x + y;
    @}
    @end smallexample

    @code{y} is not actually incremented and the compiler can but does not
    have to optimize it to just @code{return 42 + 42;}.

(I've not actually verified that'd do the job here, but I'd be very
surprised if not.)


Grüße
 Thomas


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Attachment #2: 0001-Fix-false-positive-for-Walloc-size-larger-than-part-.patch --]
[-- Type: text/x-diff, Size: 3448 bytes --]

From 7dc7de834989d85cb1dbaf7b5a0917ba07319cfb Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Sat, 9 Sep 2023 16:49:16 +0200
Subject: [PATCH] Fix false positive for -Walloc-size-larger-than, part II
 [PR79132]
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In a GCC bootstrap, I was running into:

    In file included from [...]/gcc/system.h:729,
                     from [...]/gcc/tree-ssa-reassoc.cc:22:
    [...]/gcc/tree-ssa-reassoc.cc: In function ‘void rewrite_expr_tree_parallel(gassign*, int, bool, const vec<operand_entry*>&)’:
    [...]/gcc/../include/libiberty.h:733:36: error: argument to ‘alloca’ is too large [-Werror=alloca-larger-than=]
      733 | # define alloca(x) __builtin_alloca(x)
          |                    ~~~~~~~~~~~~~~~~^~~
    [...]/gcc/../include/libiberty.h:365:40: note: in expansion of macro ‘alloca’
      365 | #define XALLOCAVEC(T, N)        ((T *) alloca (sizeof (T) * (N)))
          |                                        ^~~~~~
    [...]/gcc/tree-ssa-reassoc.cc:5518:20: note: in expansion of macro ‘XALLOCAVEC’
     5518 |   gimple **stmts = XALLOCAVEC (gimple *, stmt_num);
          |                    ^~~~~~~~~~
    [...]/gcc/../include/libiberty.h:733:36: note: limit is 9223372036854775807 bytes, but argument is 18446744073709551608
      733 | # define alloca(x) __builtin_alloca(x)
          |                    ~~~~~~~~~~~~~~~~^~~
    [...]/gcc/../include/libiberty.h:365:40: note: in expansion of macro ‘alloca’
      365 | #define XALLOCAVEC(T, N)        ((T *) alloca (sizeof (T) * (N)))
          |                                        ^~~~~~
    [...]/gcc/tree-ssa-reassoc.cc:5518:20: note: in expansion of macro ‘XALLOCAVEC’
     5518 |   gimple **stmts = XALLOCAVEC (gimple *, stmt_num);
          |                    ^~~~~~~~~~

'18446744073709551608' means '-8', which is 'sizeof (gimple *) * (-1)'.
That's the very issue that PR79132
"False positive for -Walloc-size-larger-than= with -fsanitize=address aka. bootstrap-asan breakage"
commit ad8040243acd2a909b61b5690f7dac9ae362c945 (Subversion r244857)
"Fix false positive for -Walloc-size-larger-than (PR bootstrap/79132)"
meant to address -- the detail being that '-fsanitize=address' nowadays
doesn't seem necessary anymore to trigger this, and why I was running
into this at all, is 'configure'ing with '--enable-checking=no', which
per 'gcc/system.h' means:

    #define gcc_assert(EXPR) ((void)(0 && (EXPR)))

..., and thus the 'gcc_assert (op_num > 0);' is a no-op, eh...

	PR bootstrap/79132
	gcc/
	* tree-ssa-reassoc.cc (rewrite_expr_tree_parallel): Reformulate
	'gcc_assert' into 'gcc_unreachable'.
---
 gcc/tree-ssa-reassoc.cc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gcc/tree-ssa-reassoc.cc b/gcc/tree-ssa-reassoc.cc
index eda03bf98a6..df4ebaa8ac5 100644
--- a/gcc/tree-ssa-reassoc.cc
+++ b/gcc/tree-ssa-reassoc.cc
@@ -5513,7 +5513,8 @@ rewrite_expr_tree_parallel (gassign *stmt, int width, bool has_fma,
   enum tree_code opcode = gimple_assign_rhs_code (stmt);
   int op_num = ops.length ();
   int op_normal_num = op_num;
-  gcc_assert (op_num > 0);
+  if (op_num < 1)
+    gcc_unreachable ();
   int stmt_num = op_num - 1;
   gimple **stmts = XALLOCAVEC (gimple *, stmt_num);
   int i = 0, j = 0;
-- 
2.34.1