public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/112411] New: ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu
@ 2023-11-06 18:30 zsojka at seznam dot cz
  2023-11-07  8:37 ` [Bug target/112411] " rguenth at gcc dot gnu.org
                   ` (12 more replies)
  0 siblings, 13 replies; 14+ messages in thread
From: zsojka at seznam dot cz @ 2023-11-06 18:30 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 112411
           Summary: ICE: SIGSEGV with
                    --param=min-nondebug-insn-uid=2147483647 on
                    powerpc64le-unknown-linux-gnu
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: zsojka at seznam dot cz
  Target Milestone: ---
              Host: x86_64-pc-linux-gnu
            Target: powerpc64le-unknown-linux-gnu

Created attachment 56518
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56518&action=edit
reduced testcase

Compiler output:
$ powerpc64le-unknown-linux-gnu-gcc --param=min-nondebug-insn-uid=2147483647
testcase.c -wrapper valgrind,-q,--num-callers=100
==18938== Invalid read of size 4
==18938==    at 0xC9A477: get_attr_length_1(rtx_insn*, int (*)(rtx_insn*))
(final.cc:353)
==18938==    by 0x1489717: rs6000_insn_cost (rs6000.cc:22634)
==18938==    by 0x1489717: rs6000_insn_cost(rtx_insn*, bool) (rs6000.cc:22614)
==18938==    by 0xC6D69E: canonicalize_comparison(machine_mode, rtx_code*,
rtx_def**) (expmed.cc:6344)
==18938==    by 0xC6DD71: emit_store_flag_1(rtx_def*, rtx_code, rtx_def*,
rtx_def*, machine_mode, int, int, machine_mode) (expmed.cc:5629)
==18938==    by 0xC6E2A7: emit_store_flag(rtx_def*, rtx_code, rtx_def*,
rtx_def*, machine_mode, int, int) (expmed.cc:6037)
==18938==    by 0xC6F26A: emit_store_flag_force(rtx_def*, rtx_code, rtx_def*,
rtx_def*, machine_mode, int, int) (expmed.cc:6177)
==18938==    by 0xC89B8F: convert_mode_scalar(rtx_def*, rtx_def*, int)
(expr.cc:689)
==18938==    by 0xC7E70D: expand_expr_real_2(separate_ops*, rtx_def*,
machine_mode, expand_modifier) (expr.cc:9396)
==18938==    by 0xB4A2F0: expand_gimple_stmt_1 (cfgexpand.cc:3983)
==18938==    by 0xB4A2F0: expand_gimple_stmt(gimple*) (cfgexpand.cc:4044)
==18938==    by 0xB5067E: expand_gimple_basic_block(basic_block_def*, bool)
(cfgexpand.cc:6100)
==18938==    by 0xB5235E: (anonymous
namespace)::pass_expand::execute(function*) (cfgexpand.cc:6835)
==18938==    by 0xF7B62A: execute_one_pass(opt_pass*) (passes.cc:2641)
==18938==    by 0xF7BF0F: execute_pass_list_1(opt_pass*) (passes.cc:2750)
==18938==    by 0xF7BF48: execute_pass_list(function*, opt_pass*)
(passes.cc:2761)
==18938==    by 0xB92FF5: expand (cgraphunit.cc:1841)
==18938==    by 0xB92FF5: cgraph_node::expand() (cgraphunit.cc:1794)
==18938==    by 0xB93F09: output_in_order (cgraphunit.cc:2191)
==18938==    by 0xB93F09: symbol_table::compile() [clone .part.0]
(cgraphunit.cc:2395)
==18938==    by 0xB96EA7: compile (cgraphunit.cc:2311)
==18938==    by 0xB96EA7: symbol_table::finalize_compilation_unit()
(cgraphunit.cc:2583)
==18938==    by 0x10B3D62: compile_file() (toplev.cc:473)
==18938==    by 0x9B8903: do_compile (toplev.cc:2129)
==18938==    by 0x9B8903: toplev::main(int, char**) (toplev.cc:2285)
==18938==    by 0x9BA00A: main (main.cc:39)
==18938==  Address 0xfffffffe0000001c is not stack'd, malloc'd or (recently)
free'd
==18938== 
during RTL pass: expand
testcase.c: In function 'foo':
testcase.c:7:5: internal compiler error: Segmentation fault
    7 |   i += j;
      |     ^~

$ powerpc64le-unknown-linux-gnu-gcc -v
Using built-in specs.
COLLECT_GCC=/repo/gcc-trunk/binary-latest-powerpc64le/bin/powerpc64le-unknown-linux-gnu-gcc
COLLECT_LTO_WRAPPER=/repo/gcc-trunk/binary-trunk-r14-5141-20231106022717-g1a55724f787-checking-yes-rtl-df-extra-powerpc64le/bin/../libexec/gcc/powerpc64le-unknown-linux-gnu/14.0.0/lto-wrapper
Target: powerpc64le-unknown-linux-gnu
Configured with: /repo/gcc-trunk//configure --enable-languages=c,c++
--enable-valgrind-annotations --disable-nls --enable-checking=yes,rtl,df,extra
--with-cloog --with-ppl --with-isl
--with-sysroot=/usr/powerpc64le-unknown-linux-gnu --build=x86_64-pc-linux-gnu
--host=x86_64-pc-linux-gnu --target=powerpc64le-unknown-linux-gnu
--with-ld=/usr/bin/powerpc64le-unknown-linux-gnu-ld
--with-as=/usr/bin/powerpc64le-unknown-linux-gnu-as --disable-libstdcxx-pch
--prefix=/repo/gcc-trunk//binary-trunk-r14-5141-20231106022717-g1a55724f787-checking-yes-rtl-df-extra-powerpc64le
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 14.0.0 20231106 (experimental) (GCC)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Bug target/112411] ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu
  2023-11-06 18:30 [Bug target/112411] New: ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu zsojka at seznam dot cz
@ 2023-11-07  8:37 ` rguenth at gcc dot gnu.org
  2023-11-07  8:55 ` linkw at gcc dot gnu.org
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: rguenth at gcc dot gnu.org @ 2023-11-07  8:37 UTC (permalink / raw)
  To: gcc-bugs

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

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

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
I think setting -param=min-nondebug-insn-uid=2147483647 will cause the debug
insn uid to eventually overflow which is an unwanted condition that maybe isn't
checked for.

Alex, do you remember why this param exists?  Can we simply remove it?  Should
we apply a sane range to the allowed values?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Bug target/112411] ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu
  2023-11-06 18:30 [Bug target/112411] New: ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu zsojka at seznam dot cz
  2023-11-07  8:37 ` [Bug target/112411] " rguenth at gcc dot gnu.org
@ 2023-11-07  8:55 ` linkw at gcc dot gnu.org
  2023-11-07 11:02 ` rsandifo at gcc dot gnu.org
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: linkw at gcc dot gnu.org @ 2023-11-07  8:55 UTC (permalink / raw)
  To: gcc-bugs

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

Kewen Lin <linkw at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2023-11-07
                 CC|                            |linkw at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org,
                   |                            |rsandifo at gcc dot gnu.org

--- Comment #2 from Kewen Lin <linkw at gcc dot gnu.org> ---
Confirmed, can be reproduced with native powerpc64 compiler.

It fails at the code:

 352│   if (insn_lengths_max_uid > INSN_UID (insn))
 353├───> return insn_lengths[INSN_UID (insn)];

p INSN_UID (insn)
$1 = (int &) @0x7ffff5830384: -2147483641

p insn_lengths_max_uid
$2 = 0

p insn_lengths
$3 = (int *) 0x0

The given --param=min-nondebug-insn-uid=2147483647 causes the assigned insn uid
becomes negative. At that time, insn_lengths isn't ready yet, it can workaround
with further checking insn_lengths first.

I noticed that those uid variables are of signed type, such as:

  x_cur_insn_uid @ emit_status
  x_cur_debug_insn_uid @ emit_status
  insn_uid @ rtx_def

shouldn't they be with unsigned type instead? or is it by design?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Bug target/112411] ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu
  2023-11-06 18:30 [Bug target/112411] New: ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu zsojka at seznam dot cz
  2023-11-07  8:37 ` [Bug target/112411] " rguenth at gcc dot gnu.org
  2023-11-07  8:55 ` linkw at gcc dot gnu.org
@ 2023-11-07 11:02 ` rsandifo at gcc dot gnu.org
  2023-11-30  2:35 ` aoliva at gcc dot gnu.org
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: rsandifo at gcc dot gnu.org @ 2023-11-07 11:02 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #3 from Richard Sandiford <rsandifo at gcc dot gnu.org> ---
This parameter is firmly into “developer only” territory, so I'm not sure we
should try to enforce a range.  IMO we should just recommend that the parameter
isn't included in fuzz testing.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Bug target/112411] ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu
  2023-11-06 18:30 [Bug target/112411] New: ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu zsojka at seznam dot cz
                   ` (2 preceding siblings ...)
  2023-11-07 11:02 ` rsandifo at gcc dot gnu.org
@ 2023-11-30  2:35 ` aoliva at gcc dot gnu.org
  2023-11-30  8:32 ` zsojka at seznam dot cz
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: aoliva at gcc dot gnu.org @ 2023-11-30  2:35 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #4 from Alexandre Oliva <aoliva at gcc dot gnu.org> ---
yeah, its intended use case is for debugging -fcompare-debug fails, reducing
superfluous differences between rtl dumps.

In theory GCC may overflow insn uids regardless of this param, and if guarding
against that is important, there should be code to check for it when
incrementing insn uids, but I suppose we can live without that.

Such limit values are definitely not useful for fuzz testing, but I'd be
curious whether values that do not cause overflows would cause any codegen
differences.  I think not, and it really shouldn't, but this thought makes me
hesitant to recommend against using this option for fuzz testing.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Bug target/112411] ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu
  2023-11-06 18:30 [Bug target/112411] New: ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu zsojka at seznam dot cz
                   ` (3 preceding siblings ...)
  2023-11-30  2:35 ` aoliva at gcc dot gnu.org
@ 2023-11-30  8:32 ` zsojka at seznam dot cz
  2023-11-30  8:39 ` rguenther at suse dot de
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: zsojka at seznam dot cz @ 2023-11-30  8:32 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #5 from Zdenek Sojka <zsojka at seznam dot cz> ---
Thank you for the evaluation.

I am not reporting ICEs (assertion / checking failures) due to overflows caused
by extreme --param values:
- this param is typically causing ICEs due to insn UID overflow
- params affecting inlining or unrolling often cause ICEs due to
function::last_clique wraparound (it's 16bit uint)
- generally, I am trying to consider if a real-world scenario could trigger the
ICE

This is different, since there is no assertion or checking failure, but a
segmentation fault; that is uncommon.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Bug target/112411] ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu
  2023-11-06 18:30 [Bug target/112411] New: ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu zsojka at seznam dot cz
                   ` (4 preceding siblings ...)
  2023-11-30  8:32 ` zsojka at seznam dot cz
@ 2023-11-30  8:39 ` rguenther at suse dot de
  2023-12-06 17:47 ` jakub at gcc dot gnu.org
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: rguenther at suse dot de @ 2023-11-30  8:39 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #6 from rguenther at suse dot de <rguenther at suse dot de> ---
On Thu, 30 Nov 2023, zsojka at seznam dot cz wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112411
> 
> --- Comment #5 from Zdenek Sojka <zsojka at seznam dot cz> ---
> Thank you for the evaluation.
>
> - params affecting inlining or unrolling often cause ICEs due to
> function::last_clique wraparound (it's 16bit uint)

Ah, do you have a testcase for this?  I see we have an assert
in IL verification but not really any handling of the overflow
itself (there is a way to gracefully handle it though!).  A testcase
would really be useful (in a new bug).

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Bug target/112411] ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu
  2023-11-06 18:30 [Bug target/112411] New: ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu zsojka at seznam dot cz
                   ` (5 preceding siblings ...)
  2023-11-30  8:39 ` rguenther at suse dot de
@ 2023-12-06 17:47 ` jakub at gcc dot gnu.org
  2023-12-06 18:19 ` jakub at gcc dot gnu.org
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-12-06 17:47 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I'm using this param all the time, but typically with low values (like 1000 or
10000 depending on how large a problematic function is).
I guess enforcing a range to be [1, INT_MAX / 2] or so wouldn't hurt anything
and the likelyhood of overflow in that case is low (and likelyhood of needing
more than INT_MAX / 2 debug insns in one function is also low).

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Bug target/112411] ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu
  2023-11-06 18:30 [Bug target/112411] New: ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu zsojka at seznam dot cz
                   ` (6 preceding siblings ...)
  2023-12-06 17:47 ` jakub at gcc dot gnu.org
@ 2023-12-06 18:19 ` jakub at gcc dot gnu.org
  2023-12-06 18:51 ` jakub at gcc dot gnu.org
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-12-06 18:19 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Note, on
void
foo (void)
{
}
./cc1 -quiet --param min-nondebug-insn-uid=0x2aaaa000 pr112411.c
still works but is fairly slow (various passes index arrays by INSN_UID and if
the uids are huge, it takes a lot of memory and compile time to clear them).
But
./cc1 -quiet --param min-nondebug-insn-uid=0x2aaaaaaa a.c

cc1: out of memory allocating 18446744065119617112 bytes after a total of
1626112 bytes
Seems lra does some vec handling by hand and uses an int variable for it:
lra.cc (check_and_expand_insn_recog_data):
  lra_insn_recog_data_len = index * 3 / 2 + 1;
Shall we change lra_insn_recog_data_len to size_t (and tweak comparisons to
avoid signed vs. unsigned compares)?
Or at least adjust the above such that the computation is say done in unsigned
HOST_WIDE_INT and use INT_MAX for lra_insn_recog_data_len on overflow rather
than negative value (including UB at compile time)?

With the following patch it compiles even with
./cc1 -quiet --param min-nondebug-insn-uid=0x40000000 a.c
but my 32GB RAM + 24GB swap was almost full with that.

--- gcc/params.opt.jj   2023-11-02 07:49:18.010852541 +0100
+++ gcc/params.opt      2023-12-06 18:55:57.045420935 +0100
@@ -779,7 +779,7 @@ Common Joined UInteger Var(param_min_loo
 The minimum threshold for probability of semi-invariant condition statement to
trigger loop split.

 -param=min-nondebug-insn-uid=
-Common Joined UInteger Var(param_min_nondebug_insn_uid) Param
+Common Joined UInteger Var(param_min_nondebug_insn_uid) Param IntegerRange(0,
1073741824)
 The minimum UID to be used for a nondebug insn.

 -param=min-size-for-stack-sharing=
--- gcc/lra.cc.jj       2023-12-05 13:17:29.642260866 +0100
+++ gcc/lra.cc  2023-12-06 19:14:52.117499420 +0100
@@ -764,11 +764,13 @@ static void
 check_and_expand_insn_recog_data (int index)
 {
   int i, old;
+  HOST_WIDE_INT len;

   if (lra_insn_recog_data_len > index)
     return;
   old = lra_insn_recog_data_len;
-  lra_insn_recog_data_len = index * 3 / 2 + 1;
+  len = index * HOST_WIDE_INT_C (3) / 2 + 1;
+  lra_insn_recog_data_len = MIN (len, INT_MAX);
   lra_insn_recog_data = XRESIZEVEC (lra_insn_recog_data_t,
                                    lra_insn_recog_data,
                                    lra_insn_recog_data_len);

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Bug target/112411] ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu
  2023-11-06 18:30 [Bug target/112411] New: ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu zsojka at seznam dot cz
                   ` (7 preceding siblings ...)
  2023-12-06 18:19 ` jakub at gcc dot gnu.org
@ 2023-12-06 18:51 ` jakub at gcc dot gnu.org
  2023-12-07  7:56 ` [Bug middle-end/112411] " rguenth at gcc dot gnu.org
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-12-06 18:51 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
If we wanted to support just INSN_UIDs up to 0x55555554, it could be even just
  lra_insn_recog_data_len = index * 3U / 2 + 1;

Looking around, vec.cc has similar limitation, it uses unsigned rather than int
alloc,
and
...
    /* Grow slower when large.  */
    alloc = (alloc * 3 / 2);

  /* If this is still too small, set it to the right size. */
  if (alloc < desired)
    alloc = desired;
So, my understanding is for vectors up to 0x55555555 elements it will do the
expected thing, for larger it will simply reallocate to the desired exact value
(i.e. as if it was an exact growth rather than the normal exponential one).
Perhaps what we could do in both places use
    alloc = (alloc * (size_t) 3 / 2);
plus the if (alloc < desired) alloc = desired;, such that at least on 64-bit
hosts we'd use an exponential growth even after 0x55555555 a little bit, but on
32-bit hosts we'd keep what we do.  On the lra side then
  lra_insn_recog_data_len = index * (size_t) 3 / 2;
  if (lra_insn_recog_data_len <= index)
    lra_insn_recog_data_len = index + 1;

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Bug middle-end/112411] ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu
  2023-11-06 18:30 [Bug target/112411] New: ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu zsojka at seznam dot cz
                   ` (8 preceding siblings ...)
  2023-12-06 18:51 ` jakub at gcc dot gnu.org
@ 2023-12-07  7:56 ` rguenth at gcc dot gnu.org
  2023-12-08  7:33 ` cvs-commit at gcc dot gnu.org
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: rguenth at gcc dot gnu.org @ 2023-12-07  7:56 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
vec<> doesn't really support "large" vectors:

  unsigned m_alloc : 31;
  unsigned m_using_auto_storage : 1;
  unsigned m_num;

so it's basically using 'int'.  I guess we should make that
alloc = (alloc * 3 / 2) use size_t though or simply do (alloc / 2) * 3
since we know alloc >= 16 here so the rounding error isn't important.

Still ICEing on overflow here would be desirable.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Bug middle-end/112411] ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu
  2023-11-06 18:30 [Bug target/112411] New: ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu zsojka at seznam dot cz
                   ` (9 preceding siblings ...)
  2023-12-07  7:56 ` [Bug middle-end/112411] " rguenth at gcc dot gnu.org
@ 2023-12-08  7:33 ` cvs-commit at gcc dot gnu.org
  2023-12-08  8:01 ` cvs-commit at gcc dot gnu.org
  2023-12-08  8:01 ` jakub at gcc dot gnu.org
  12 siblings, 0 replies; 14+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-12-08  7:33 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #11 from GCC 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:39a1ab9c33b6067b1cc843408886e7ba709fbb62

commit r14-6302-g39a1ab9c33b6067b1cc843408886e7ba709fbb62
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Dec 8 08:29:44 2023 +0100

    Add IntegerRange for -param=min-nondebug-insn-uid= and fix vector growing
in LRA and vec [PR112411]

    As documented, --param min-nondebug-insn-uid= is very useful in debugging
    -fcompare-debug issues in RTL dumps, without it it is really hard to
    find differences.  With it, DEBUG_INSNs generally use low INSN_UIDs
    (1+) and non-DEBUG_INSNs use INSN_UIDs from the parameter up.
    For good results, the parameter should be larger than the number of
    DEBUG_INSNs in all or at least problematic functions, so I typically
    use --param min-nondebug-insn-uid=10000 or --param
    min-nondebug-insn-uid=1000.

    The PR is about using --param min-nondebug-insn-uid=2147483647 or
    similar behavior can be achieved with that minus some epsilon,
    INSN_UIDs for the non-debug insns then wrap around and as they are signed,
    all kinds of things break.  Obviously, that can happen even without that
    option, but functions containing more than 2147483647 insns usually don't
    compile much earlier due to getting out of memory.
    As it is a debugging option, I'd prefer not to impose any drastically small
    limits on it because if a function has a lot of DEBUG_INSNs, it is useful
    to start still above them, otherwise the allocation of uids will DTRT
    even for DEBUG_INSNs but there will be then differences in non-DEBUG_INSN
    allocations.

    So, the following patch uses 0x40000000 limit, half the maximum amount for
    DEBUG_INSNs and half for non-DEBUG_INSNs, it will still result in very
    unlikely overflows in real world.

    Note, using large min-nondebug-insn-uid is very expensive for compile time
    memory and compile time, because DF as well as various RTL passes use
    arrays indexed by INSN_UIDs, e.g. LRA with sizeof (void *) elements,
    ditto df (df->insns).

    Now, in LRA I've ran into ICEs already with
    --param min-nondebug-insn-uid=0x2aaaaaaa
    on 64-bit host.  It uses a custom vector management and wants to grow
    allocation 1.5x when growing, but all this computation is done in int,
    so already 0x2aaaaaab * 3 / 2 + 1 overflows to negative value.  And
    unlike vec.cc growing which also uses unsigned int type for the above
    (and the + 1 is not there), it also doesn't make sure if there is an
    overflow that it allocates at least as much as needed, vec.cc
    does
      if ...
      else
        /* Grow slower when large.  */
        alloc = (alloc * 3 / 2);

      /* If this is still too small, set it to the right size. */
      if (alloc < desired)
        alloc = desired;
    so even if there is overflow during the * 1.5 computation, but
    desired is still representable in the range of the alloced counter
    (31-bits in both vec.h and LRA), it doesn't grow exponentially but
    at least works for the current value.

    The patch now uses there
      lra_insn_recog_data_len = index * 3U / 2;
      if (lra_insn_recog_data_len <= index)
        lra_insn_recog_data_len = index + 1;
    basically do what vec.cc does.  I thought we could do better for
    both vec.cc and LRA on 64-bit hosts even without growing the allocated
    counters, but now that I look at it again, perhaps we can't.
    The above overflows already with original alloc or lra_insn_recog_data_len
    0x55555556, where 0x5555555 * 3U / 2 is still 0x7fffffff
    and so representable in the 32-bit, but 0x55555556 * 3U / 2 is
    1.  I thought that we could use alloc * (size_t) 3 / 2 so that on 64-bit
    hosts it wouldn't overflow that quickly, but 0x55555556 * (size_t) 3 / 2
    there is 0x80000001 which is still ok in unsigned, but given that vec.h
    then stores the counter into unsigned m_alloc:31; bit-field, it is too
much.

    With the lra.cc change, one can actually compile simple function
    with -O0 on 64-bit host with --param min-nondebug-insn-uid=0x40000000
    (i.e. the new limit), but already needed quite a big part of my 32GB
    RAM + 24GB swap.
    The patch adds a dg-skip-if for that case though, because such option
    is way too much for 32-bit hosts even at -O0 and empty function,
    and with -O3 on a longer function it is too much for average 64-bit host
    as well.  Without the dg-skip-if I got on 64-bit host:
    cc1: out of memory allocating 571230784744 bytes after a total of 2772992
bytes
    and
    cc1: out of memory allocating 1388 bytes after a total of 2002944 bytes
    on 32-bit host.  A test requiring more than 532GB of RAM on 64-bit hosts
    is just too much for our testsuite.

    2023-12-08  Jakub Jelinek  <jakub@redhat.com>

            PR middle-end/112411
            * params.opt (-param=min-nondebug-insn-uid=): Add
            IntegerRange(0, 1073741824).
            * lra.cc (check_and_expand_insn_recog_data): Use 3U rather than 3
            in * 3 / 2 computation and if the result is smaller or equal to
            index, use index + 1.

            * gcc.dg/params/blocksort-part.c: Add dg-skip-if for
            --param min-nondebug-insn-uid=1073741824.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Bug middle-end/112411] ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu
  2023-11-06 18:30 [Bug target/112411] New: ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu zsojka at seznam dot cz
                   ` (10 preceding siblings ...)
  2023-12-08  7:33 ` cvs-commit at gcc dot gnu.org
@ 2023-12-08  8:01 ` cvs-commit at gcc dot gnu.org
  2023-12-08  8:01 ` jakub at gcc dot gnu.org
  12 siblings, 0 replies; 14+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-12-08  8:01 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #12 from GCC 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:8f60f5499e10d19218cada082e0909516ebf0e74

commit r14-6305-g8f60f5499e10d19218cada082e0909516ebf0e74
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Dec 8 08:56:33 2023 +0100

    haifa-sched: Avoid overflows in extend_h_i_d [PR112411]

    On Thu, Dec 07, 2023 at 09:36:23AM +0100, Jakub Jelinek wrote:
    > Without the dg-skip-if I got on 64-bit host with
    > -O3 --param min-nondebug-insn-uid=0x40000000:
    > cc1: out of memory allocating 571230784744 bytes after a total of 2772992
bytes

    I've looked at this and the problem is in haifa-sched.cc:
    9047        h_i_d.safe_grow_cleared (3 * get_max_uid () / 2, true);
    get_max_uid () is 0x4000024d with the --param
min-nondebug-insn-uid=0x40000000
    and so 3 * get_max_uid () / 2 actually overflows to -536870028 but as vec.h
    then treats the value as unsigned, it attempts to allocate
    0xe0000374U * 152UL bytes, i.e. those 532GB.  If the above is fixed to do
    3U * get_max_uid () / 2 instead, it will get slightly better and will only
    need 0x60000373U * 152UL bytes, i.e. 228GB.

    Perhaps more could be helped by making the vector indirect (contain
pointers
    to haifa_insn_data_def rather than the structures themselves) and pool
allocate
    those, but the more important question is how sparse are uids in normal
    compilations without those large --param min-nondebug-insn-uid= parameters.
    Because if they aren't enough, such a change would increase compile time
memory
    just to help the unusual case.

    2023-12-08  Jakub Jelinek  <jakub@redhat.com>

            PR middle-end/112411
            * haifa-sched.cc (extend_h_i_d): Use 3U instead of 3 in
            3 * get_max_uid () / 2 calculation.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Bug middle-end/112411] ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu
  2023-11-06 18:30 [Bug target/112411] New: ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu zsojka at seznam dot cz
                   ` (11 preceding siblings ...)
  2023-12-08  8:01 ` cvs-commit at gcc dot gnu.org
@ 2023-12-08  8:01 ` jakub at gcc dot gnu.org
  12 siblings, 0 replies; 14+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-12-08  8:01 UTC (permalink / raw)
  To: gcc-bugs

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

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

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

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Now fixed.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2023-12-08  8:01 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-06 18:30 [Bug target/112411] New: ICE: SIGSEGV with --param=min-nondebug-insn-uid=2147483647 on powerpc64le-unknown-linux-gnu zsojka at seznam dot cz
2023-11-07  8:37 ` [Bug target/112411] " rguenth at gcc dot gnu.org
2023-11-07  8:55 ` linkw at gcc dot gnu.org
2023-11-07 11:02 ` rsandifo at gcc dot gnu.org
2023-11-30  2:35 ` aoliva at gcc dot gnu.org
2023-11-30  8:32 ` zsojka at seznam dot cz
2023-11-30  8:39 ` rguenther at suse dot de
2023-12-06 17:47 ` jakub at gcc dot gnu.org
2023-12-06 18:19 ` jakub at gcc dot gnu.org
2023-12-06 18:51 ` jakub at gcc dot gnu.org
2023-12-07  7:56 ` [Bug middle-end/112411] " rguenth at gcc dot gnu.org
2023-12-08  7:33 ` cvs-commit at gcc dot gnu.org
2023-12-08  8:01 ` cvs-commit at gcc dot gnu.org
2023-12-08  8:01 ` jakub at gcc dot gnu.org

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).