[Bug ipa/111960] [14 Regression] ICE: during GIMPLE pass: rebuild_frequencies: SIGSEGV (Invalid read of size 4) with -fdump-tree-rebuild_frequencies-all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111960 --- Comment #13 from Jan Hubicka --- > Should be fixed now. Thanks! I was testing with stage3 compiler, so that is the reason. Indeed dropping the buffer is a good idea.
[Bug ipa/111960] [14 Regression] ICE: during GIMPLE pass: rebuild_frequencies: SIGSEGV (Invalid read of size 4) with -fdump-tree-rebuild_frequencies-all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111960 Jakub Jelinek changed: What|Removed |Added Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #12 from Jakub Jelinek --- Should be fixed now.
[Bug ipa/111960] [14 Regression] ICE: during GIMPLE pass: rebuild_frequencies: SIGSEGV (Invalid read of size 4) with -fdump-tree-rebuild_frequencies-all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111960 --- Comment #11 from GCC Commits --- The master branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:a0782531b8270f0fdb3f3e09b4ce544d5d1eef14 commit r14-9133-ga0782531b8270f0fdb3f3e09b4ce544d5d1eef14 Author: Jakub Jelinek Date: Thu Feb 22 13:07:25 2024 +0100 profile-count: Don't dump through a temporary buffer [PR111960] The profile_count::dump (char *, struct function * = NULL) const; method has a single caller, the profile_count::dump (FILE *f, struct function *fun) const; method and for that going through a temporary buffer is just slower and opens doors for buffer overflows, which is exactly why this P1 was filed. The buffer size is 64 bytes, the previous maximum "%" PRId64 " (%s)" would print up to 61 bytes in there (19 bytes for arbitrary uint64_t:61 bitfield printed as signed, "estimated locally, globally 0 adjusted" i.e. 38 bytes longest %s and 4 other characters). Now, after the r14-2389 changes, it can be 19 + 38 plus 11 other characters + %.4f, which is worst case 309 chars before decimal point, decimal point and 4 digits after it, so total 382 bytes. So, either we could bump the buffer[64] to buffer[400], or the following patch just drops the indirection through buffer and prints it directly to stream. After all, having APIs which fill in some buffer without passing down the size of the buffer is just asking for buffer overflows over time. 2024-02-22 Jakub Jelinek PR ipa/111960 * profile-count.h (profile_count::dump): Remove overload with char * first argument. * profile-count.cc (profile_count::dump): Change overload with char * first argument which uses sprintf into the overfload with FILE * first argument and use fprintf instead. Remove overload which wrapped it.
[Bug ipa/111960] [14 Regression] ICE: during GIMPLE pass: rebuild_frequencies: SIGSEGV (Invalid read of size 4) with -fdump-tree-rebuild_frequencies-all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111960
--- Comment #10 from Jakub Jelinek ---
So, I believe the really problematic change was r14-2389-g3cce8d98f270f48f
which introduced at least in theory the buffer overflow, before that the
maximum string length no matter what m_val was was 62 chars.
Now, I wonder what is the reason to have methods dump it into a buffer and then
dump the buffer to FILE *, when the former method is only used in the latter
method and nowhere else.
2024-02-21 Jakub Jelinek
PR ipa/111960
* profile-count.h (profile_count::dump): Remove overload with
char * first argument.
* profile-count.cc (profile_count::dump): Change overload with char *
first argument which uses sprintf into the overfload with FILE *
first argument and use fprintf instead. Remove overload which wrapped
it.
--- gcc/profile-count.h.jj 2024-01-03 11:51:30.309748150 +0100
+++ gcc/profile-count.h 2024-02-21 21:04:22.338905728 +0100
@@ -1299,9 +1299,6 @@ public:
/* Output THIS to F. */
void dump (FILE *f, struct function *fun = NULL) const;
- /* Output THIS to BUFFER. */
- void dump (char *buffer, struct function *fun = NULL) const;
-
/* Print THIS to stderr. */
void debug () const;
--- gcc/profile-count.cc.jj 2024-01-03 11:51:40.782602796 +0100
+++ gcc/profile-count.cc2024-02-21 21:05:28.521994913 +0100
@@ -84,34 +84,24 @@ const char *profile_quality_display_name
"precise"
};
-/* Dump THIS to BUFFER. */
+/* Dump THIS to F. */
void
-profile_count::dump (char *buffer, struct function *fun) const
+profile_count::dump (FILE *f, struct function *fun) const
{
if (!initialized_p ())
-sprintf (buffer, "uninitialized");
+fprintf (f, "uninitialized");
else if (fun && initialized_p ()
&& fun->cfg
&& ENTRY_BLOCK_PTR_FOR_FN (fun)->count.initialized_p ())
-sprintf (buffer, "%" PRId64 " (%s, freq %.4f)", m_val,
+fprintf (f, "%" PRId64 " (%s, freq %.4f)", m_val,
profile_quality_display_names[m_quality],
to_sreal_scale (ENTRY_BLOCK_PTR_FOR_FN (fun)->count).to_double
());
else
-sprintf (buffer, "%" PRId64 " (%s)", m_val,
+fprintf (f, "%" PRId64 " (%s)", m_val,
profile_quality_display_names[m_quality]);
}
-/* Dump THIS to F. */
-
-void
-profile_count::dump (FILE *f, struct function *fun) const
-{
- char buffer[64];
- dump (buffer, fun);
- fputs (buffer, f);
-}
-
/* Dump THIS to stderr. */
void
patch certainly fixes the buffer overflow... Or of course just enlarge the
buffer.
But, I don't really see anything that would bound sreal values to be within
some small double range, the range of m_exp is range of int, so in theory the
ldexp can always overflow to infinity or result in close to maximum finite
representable doubles.
[Bug ipa/111960] [14 Regression] ICE: during GIMPLE pass: rebuild_frequencies: SIGSEGV (Invalid read of size 4) with -fdump-tree-rebuild_frequencies-all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111960
--- Comment #9 from Jakub Jelinek ---
Ok, so what I see is a buffer overflow during
97 sprintf (buffer, "%" PRId64 " (%s, freq %.4f)", m_val,
98 profile_quality_display_names[m_quality],
99 to_sreal_scale (ENTRY_BLOCK_PTR_FOR_FN
(fun)->count).to_double ());
in
#0 profile_count::dump (this=0x7fffe9f37a18, buffer=0x7fffd7a0
"\300\327\377\377\377\177", fun=0x7fffea3020b8) at
../../gcc/profile-count.cc:97
#1 0x00c92609 in profile_count::dump (this=0x7fffe9f37a18,
f=0x3ab2930, fun=0x7fffea3020b8) at ../../gcc/profile-count.cc:111
#2 0x0065fefc in dump_bb_info (outf=0x3ab2930, bb=, indent=2, flags=266692985, do_header=true,
do_footer=false) at ../../gcc/cfg.cc:812
#3 0x0068f3eb in dump_bb (outf=0x3ab2930, bb=, indent=2, flags=266692985) at ../../gcc/cfghooks.cc:302
#4 0x00e31800 in dump_function_to_file (fndecl=, file=0x3ab2930, flags=266692985) at
../../gcc/tree-cfg.cc:8458
#5 0x00c4e038 in execute_function_dump (fn=0x7fffea3020b8,
data=0x3a156d0) at ../../gcc/passes.cc:1797
#6 0x00c4dbd9 in do_per_function (callback=0xc4dfd0
, data=0x3a156d0) at
../../gcc/passes.cc:1687
#7 0x00c503e1 in execute_one_pass (pass=) at ../../gcc/passes.cc:2722
buffer points to
107 void
108 profile_count::dump (FILE *f, struct function *fun) const
109 {
110 char buffer[64];
variable and the values I see are:
(gdb) p m_val
$18 = 2305843009213693950
(gdb) p profile_quality_display_names[m_quality]
$19 = 0x2ba5722 "estimated locally"
(gdb) p to_sreal_scale (((basic_block)0x7fffea2df600)->count, (bool *)
0).to_double ()
$20 = 1.4411518807585587e+17
So, that is 19 chars for m_val, 17 chars for the name and printing
144115188075855872. double value, 23 chars, plus 11 chars on top of that.
That is 70 chars, overflow by 6 bytes.
Whether the above comes from some badly initialized values or what, no idea.
That said, generally %PRId64 can print at most 20 chars, %s 38 chars and %.4f
can print
at most 309 digits before the decimal dot, then 1 char for the decimal dot and
4 digits after the dot.
That is already 69 chars without the double at all, plus 314 for the double.
Are you sure you want to print %.4f and not say %.4g so that there is a
reasonable upper bound for the size of the double printing? Or use %.4f or
%.4e conditionally depending on if the double is larger than some limit.
[Bug ipa/111960] [14 Regression] ICE: during GIMPLE pass: rebuild_frequencies: SIGSEGV (Invalid read of size 4) with -fdump-tree-rebuild_frequencies-all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111960 --- Comment #8 from Jakub Jelinek --- Ah, not that easily, only in -O0 built compilers, including stage1 of bootstrapped compilers, stage2/3 don't.
[Bug ipa/111960] [14 Regression] ICE: during GIMPLE pass: rebuild_frequencies: SIGSEGV (Invalid read of size 4) with -fdump-tree-rebuild_frequencies-all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111960 --- Comment #7 from Andrew Pinski --- (In reply to Jakub Jelinek from comment #6) > Strange, reproduces for me just fine > /volume/tor/opt/notnfs/gcc-bisect/obj/gcc/cc1.r14-9114 -quiet -O > --param=max-inline-recursive-depth=100 > -fdump-tree-rebuild_frequencies-all pr111960.c > during GIMPLE pass: rebuild_frequencies > dump file: pr111960.c.111t.rebuild_frequencies1 > Segmentation fault (core dumped) Now I am guessing some stack corruption going on due to both the location of the crash and not everyone able to reproduce it.
[Bug ipa/111960] [14 Regression] ICE: during GIMPLE pass: rebuild_frequencies: SIGSEGV (Invalid read of size 4) with -fdump-tree-rebuild_frequencies-all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111960 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #6 from Jakub Jelinek --- Strange, reproduces for me just fine /volume/tor/opt/notnfs/gcc-bisect/obj/gcc/cc1.r14-9114 -quiet -O --param=max-inline-recursive-depth=100 -fdump-tree-rebuild_frequencies-all pr111960.c during GIMPLE pass: rebuild_frequencies dump file: pr111960.c.111t.rebuild_frequencies1 Segmentation fault (core dumped)
[Bug ipa/111960] [14 Regression] ICE: during GIMPLE pass: rebuild_frequencies: SIGSEGV (Invalid read of size 4) with -fdump-tree-rebuild_frequencies-all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111960
--- Comment #5 from Jan Hubicka ---
hmm. cfg.cc:815 for me is:
fputs (", maybe hot", outf);
which seems quite safe.
The problem does not seem to reproduce for me:
jh@ryzen3:~/gcc/build/gcc> ./xgcc -B ./ tt.c -O
--param=max-inline-recursive-depth=100 -fdump-tree-rebuild_frequencies-all
-wrapper valgrind
==25618== Memcheck, a memory error detector
==25618== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==25618== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
==25618== Command: ./cc1 -quiet -iprefix
/home/jh/gcc/build/gcc/../lib64/gcc/x86_64-pc-linux-gnu/14.0.1/ -isystem
./include -isystem ./include-fixed tt.c -quiet -dumpdir a- -dumpbase tt.c
-dumpbase-ext .c -mtune=generic -march=x86-64 -O
-fdump-tree-rebuild_frequencies-all --param=max-inline-recursive-depth=100
-o /tmp/ccpkfjdK.s
==25618==
==25618==
==25618== HEAP SUMMARY:
==25618== in use at exit: 1,818,714 bytes in 1,175 blocks
==25618== total heap usage: 39,645 allocs, 38,470 frees, 12,699,874 bytes
allocated
==25618==
==25618== LEAK SUMMARY:
==25618==definitely lost: 0 bytes in 0 blocks
==25618==indirectly lost: 0 bytes in 0 blocks
==25618== possibly lost: 8,032 bytes in 1 blocks
==25618==still reachable: 1,810,682 bytes in 1,174 blocks
==25618== suppressed: 0 bytes in 0 blocks
==25618== Rerun with --leak-check=full to see details of leaked memory
==25618==
==25618== For lists of detected and suppressed errors, rerun with: -s
==25618== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
==25627== Memcheck, a memory error detector
==25627== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==25627== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
==25627== Command: ./as --64 -o /tmp/ccp5TNme.o /tmp/ccpkfjdK.s
==25627==
==25637== Memcheck, a memory error detector
==25637== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==25637== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
==25637== Command: ./collect2 -plugin ./liblto_plugin.so
-plugin-opt=./lto-wrapper -plugin-opt=-fresolution=/tmp/cclWZD7F.res
-plugin-opt=-pass-through=-lgcc -plugin-opt=-pass-through=-lgcc_s
-plugin-opt=-pass-through=-lc -plugin-opt=-pass-through=-lgcc
-plugin-opt=-pass-through=-lgcc_s --eh-frame-hdr -m elf_x86_64 -dynamic-linker
/lib64/ld-linux-x86-64.so.2 /lib/../lib64/crt1.o /lib/../lib64/crti.o
./crtbegin.o -L. -L/lib/../lib64 -L/usr/lib/../lib64 /tmp/ccp5TNme.o -lgcc
--push-state --as-needed -lgcc_s --pop-state -lc -lgcc --push-state --as-needed
-lgcc_s --pop-state ./crtend.o /lib/../lib64/crtn.o
==25637==
/usr/lib64/gcc/x86_64-suse-linux/13/../../../../x86_64-suse-linux/bin/ld:
/lib/../lib64/crt1.o: in function `_start':
/home/abuild/rpmbuild/BUILD/glibc-2.38/csu/../sysdeps/x86_64/start.S:103:(.text+0x2b):
undefined reference to `main'
collect2: error: ld returned 1 exit status
==25637==
==25637== HEAP SUMMARY:
==25637== in use at exit: 89,760 bytes in 39 blocks
==25637== total heap usage: 175 allocs, 136 frees, 106,565 bytes allocated
==25637==
==25637== LEAK SUMMARY:
==25637==definitely lost: 0 bytes in 0 blocks
==25637==indirectly lost: 0 bytes in 0 blocks
==25637== possibly lost: 0 bytes in 0 blocks
==25637==still reachable: 89,760 bytes in 39 blocks
==25637== of which reachable via heuristic:
==25637== newarray : 1,544 bytes in 1 blocks
==25637== suppressed: 0 bytes in 0 blocks
==25637== Rerun with --leak-check=full to see details of leaked memory
==25637==
==25637== For lists of detected and suppressed errors, rerun with: -s
==25637== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
[Bug ipa/111960] [14 Regression] ICE: during GIMPLE pass: rebuild_frequencies: SIGSEGV (Invalid read of size 4) with -fdump-tree-rebuild_frequencies-all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111960 Richard Biener changed: What|Removed |Added Priority|P3 |P1
[Bug ipa/111960] [14 Regression] ICE: during GIMPLE pass: rebuild_frequencies: SIGSEGV (Invalid read of size 4) with -fdump-tree-rebuild_frequencies-all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111960 --- Comment #4 from Zdenek Sojka --- (In reply to Sam James from comment #3) > I was confused at first until I realised rebuild_frequencies-all got turned > into a proper pass in r14-2524-gaa6741ef2e0c31, so no bisection needed imo. Even if -fdump-tree-all-all is used instead, the above is the first failing commit.
[Bug ipa/111960] [14 Regression] ICE: during GIMPLE pass: rebuild_frequencies: SIGSEGV (Invalid read of size 4) with -fdump-tree-rebuild_frequencies-all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111960 --- Comment #3 from Sam James --- I was confused at first until I realised rebuild_frequencies-all got turned into a proper pass in r14-2524-gaa6741ef2e0c31, so no bisection needed imo.
[Bug ipa/111960] [14 Regression] ICE: during GIMPLE pass: rebuild_frequencies: SIGSEGV (Invalid read of size 4) with -fdump-tree-rebuild_frequencies-all
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111960
Andrew Pinski changed:
What|Removed |Added
Component|tree-optimization |ipa
CC||hubicka at gcc dot gnu.org,
||marxin at gcc dot gnu.org
--- Comment #2 from Andrew Pinski ---
Reduced (C) testcase:
```
int _token;
static inline
void CommaExpr (void)
{
for (; _token; CommaExpr ())
;
}
void Compile (void)
{
CommaExpr ();
}
```
`-O --param=max-inline-recursive-depth=100
-fdump-tree-rebuild_frequencies-all`
