Created attachment 52152 [details] preprocessed source file gcc-12 apparently miscompiles the libcap-2.62 function "cap_to_text()". I've seen it manifest with "ls" segfaulting in certain directories: $ ls *** buffer overflow detected ***: terminated Aborted (core dumped) #0 __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44 44 return INTERNAL_SYSCALL_ERROR_P (ret) ? INTERNAL_SYSCALL_ERRNO (ret) : 0; (gdb) bt #0 __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44 #1 0x00007f34f856932f in __pthread_kill_internal (signo=6, threadid=<optimized out>) at pthread_kill.c:78 #2 0x00007f34f8518e42 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26 #3 0x00007f34f8503457 in __GI_abort () at abort.c:79 #4 0x00007f34f855d5a8 in __libc_message (action=action@entry=do_abort, fmt=fmt@entry=0x7f34f8690291 "*** %s ***: terminated\n") at ../sysdeps/posix/libc_fatal.c:155 #5 0x00007f34f85fb042 in __GI___fortify_fail (msg=msg@entry=0x7f34f8690237 "buffer overflow detected") at fortify_fail.c:26 #6 0x00007f34f85f9b60 in __GI___chk_fail () at chk_fail.c:28 #7 0x00007f34f85f96d5 in ___sprintf_chk (s=s@entry=0x7fff9f08c6c2 ",", flag=flag@entry=1, slen=slen@entry=0, format=format@entry=0x7f34f86dc085 "%c%s%s%s") at sprintf_chk.c:37 #8 0x00007f34f86da882 in sprintf (__fmt=0x7f34f86dc085 "%c%s%s%s", __s=<optimized out>) at /usr/include/bits/stdio2.h:38 #9 cap_to_text (caps=0x5643cf9b1a38, length_p=0x0) at cap_text.c:431 #10 0x00005643cf983285 in ?? () If I replace the cap_text.o file from the gcc-12 build with one from a gcc-11.3 build, the error disappears. It also disappears when ls is run under strace. -fno-tree-vectorize does NOT help. Find attached the preprocessed cap_text.i file from libcap-2.62, as well as .S files of gcc-11.3 and 12.0 gcc version 12.0.0 20220110 (experimental) (Gentoo 12.0.0_pre9999 p2, commit 92e114d66e93d60dcef97c66cddbae38b657d768)
Created attachment 52153 [details] gcc-11.3 output
Created attachment 52154 [details] gcc-12.0 output
Looks like GCC is getting confused due to: p--; I don't get: [cap_text.c:419:3] # PT = anything _104 = p_65 + 18446744073709551615; All other places just have # PT = { D.4523 } (escaped) D.4523 being buf decl.
There is a bogus warning too: cap_text.c:431:11: warning: '%c' directive writing 1 byte into a region of size 0 [-Wformat-overflow=] In file included from /usr/include/stdio.h:894, from cap_text.c:13: In function 'sprintf', inlined from 'cap_to_text' at cap_text.c:431:11: /usr/include/bits/stdio2.h:38:10: note: '__builtin___sprintf_chk' output between 2 and 5 bytes into a destination of size 0 And the gimple looks wrong: _164 = __sprintf_chkD.1304 (p_141, 1, 0, [cap_text.c:431:11] "%c%s%s%s", _31, iftmp.56_86, iftmp.55_85, iftmp.54_84); For GCC 11 we had: _164 = __sprintf_chkD.1270 (p_141, 1, 18446744073709551615, [cap_text.c:431:11] "%c%s%s%s", _33, iftmp.56_90, iftmp.55_89, iftmp.54_88);
I will try a reduction maybe over the weekend if nobody gets to it before me.
Note it is even wrong at -O1 -fno-thread-jumps .
objsz1 produces: Computing maximum subobject size for p_61: Visiting use-def links for p_61 Visiting use-def links for p_139 Visiting use-def links for p_64 Visiting use-def links for p_29 Visiting use-def links for p_63 Visiting use-def links for p_62 Visiting use-def links for p_141 Found a dependency loop at p_61 Need to reexamine p_141 Visiting use-def links for p_144 Visiting use-def links for p_141 Reexamining p_141 p_141: maximum subobject size 0 Simplified [/usr/include/bits/stdio2.h:38:10] _161 = __builtin_object_sizeD.1280 (p_61, 1); to 18446744073709551615 Simplified [/usr/include/bits/stdio2.h:38:10] _163 = __builtin_object_sizeD.1280 (p_141, 1); to 0 Simplified [/usr/include/bits/stdio2.h:38:10] _165 = __builtin_object_sizeD.1280 (p_62, 1); to 18446744073709551615 Computing maximum subobject size for p_66: Visiting use-def links for p_66 Visiting use-def links for p_123 Visiting use-def links for p_67 Visiting use-def links for p_136 Visiting use-def links for p_126 Simplified [/usr/include/bits/stdio2.h:38:10] _168 = __builtin_object_sizeD.1280 (p_66, 1); to 18446744073709551615 Computing maximum subobject size for p_125: Visiting use-def links for p_125 Simplified [/usr/include/bits/stdio2.h:38:10] _170 = __builtin_object_sizeD.1280 (p_125, 1); to 18446744073709551615 The 0 for _163/p_141 is wrong.
(In reply to Andrew Pinski from comment #3) > Looks like GCC is getting confused due to: > p--; > > > I don't get: > [cap_text.c:419:3] # PT = anything > _104 = p_65 + 18446744073709551615; > > All other places just have # PT = { D.4523 } (escaped) > > D.4523 being buf decl. I think that might be ranger adding nonnull to a pointer where we previously have no points-to info at all (which means anything as well). The above stmt is introduced by PRE: - <bb 81> [local count: 982087419]: - goto <bb 21>; [100.00%] + <bb 94> [local count: 982087419]: + goto <bb 23>; [100.00%] + + <bb 29> [local count: 37309945]: + [cap_text.c:419:3] _104 = p_65 + 18446744073709551615; - <bb 27> [local count: 39298952]: + <bb 30> [local count: 39298952]: # PT = { D.4523 } (escaped) - # p_228 = PHI <p_65(26), [cap_text.c:403:4] p_147(20)> - [cap_text.c:419:3] # PT = { D.4523 } (escaped) - p_149 = p_228 + 18446744073709551615; + # p_228 = PHI <p_65(29), [cap_text.c:403:4] p_147(91)> + # PT = { D.4523 } (escaped) + # prephitmp_82 = PHI <_104(29), p_210(91)> which could indeed have preserved points-to info (but it would need to be tracked in the expression sets so that's not too easy).
Reduced testcase: extern __inline __attribute__ ((__gnu_inline__)) int sprintf (char *__restrict __s, const char *__restrict __fmt, ...) { return __builtin___sprintf_chk (__s, 2 - 1, __builtin_object_size (__s, 2 > 1), __fmt, __builtin_va_arg_pack ()); } void cap_to_text(int cmb) { char buf[(23 * ((2) * 32))+100]; char *p; int n, t; p = 20 + buf; for (t = 8; t--; ) { for (n = 0; n < cmb; n++) p += sprintf(p, "a,"); p--; sprintf(p, "+"); } } When p-- happens, it will always be buf+somesmallvalue.
(In reply to Andrew Pinski from comment #9) > Reduced testcase: Note this is -O1 -Wall. This is definitely a change in objsz which is calculating the size wrong. We get: <source>: In function 'cap_to_text': <source>:18:29: warning: '+' directive writing 1 byte into a region of size 0 [-Wformat-overflow=] 18 | sprintf(p, "+"); | ^
Started with r12-6030-g422f9eb7011b76c1.
Self-contained test-case: extern __inline __attribute__((__gnu_inline__)) int sprintf( char *__restrict __s, const char *__restrict __fmt, ...) { return __builtin___sprintf_chk(__s, 2 - 1, __builtin_object_size(__s, 2 > 1), __fmt, __builtin_va_arg_pack()); } int main() { char buf[16]; char *p = buf; for (int t = 0; t < 1; t++) { for (int n = 0; n < 1; n++) p += sprintf(p, "a,"); p--; sprintf(p, "+"); } __builtin_printf("buf: %s\n", buf); if (buf[0] != 'a') __builtin_abort(); return 0; }
Testcase with nicer formatting: extern inline __attribute__ ((__gnu_inline__)) int sprintf (char *restrict s, const char *restrict fmt, ...) { return __builtin___sprintf_chk (s, 1, __builtin_object_size (s, 2 > 1), fmt, __builtin_va_arg_pack ()); } void cap_to_text (int c) { char buf[1572]; char *p; int n, t; p = 20 + buf; for (t = 8; t--; ) { for (n = 0; n < c; n++) p += sprintf (p, "a,"); p--; sprintf (p, "+"); } } Indeed, early_objsz already inserts the bogus: p_16 = p_3 + 18446744073709551615; _17 = __builtin_object_size (p_16, 1); _24 = MIN_EXPR <_17, 0>; _25 = __builtin___sprintf_chk (p_16, 1, _24, "+");
Looking into it.
The master branch has been updated by Siddhesh Poyarekar <siddhesh@gcc.gnu.org>: https://gcc.gnu.org/g:026d44cbbd42653908f9faf6b80773f03e1bb1a0 commit r12-6478-g026d44cbbd42653908f9faf6b80773f03e1bb1a0 Author: Siddhesh Poyarekar <siddhesh@gotplt.org> Date: Tue Jan 11 16:07:29 2022 +0530 tree-optimization/103961: Never compute offset for -1 size Never try to compute size for offset when the object size is -1, which is either unknown maximum or uninitialized minimum irrespective of the osi->pass number. gcc/ChangeLog: PR tree-optimization/103961 * tree-object-size.c (plus_stmt_object_size): Always avoid computing offset for -1 size. gcc/testsuite/ChangeLog: PR tree-optimization/103961 * gcc.dg/pr103961.c: New test case. Co-authored-by: Jakub Jelinek <jakub@redhat.com> Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
Should be fixed with that patch. May I close this or wait for confirmation from the reporter?
(In reply to Siddhesh Poyarekar from comment #16) > Should be fixed with that patch. May I close this or wait for confirmation > from the reporter? I can no longer reproduce the original issue.
Fixed then.