[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 --- Comment #24 from Andrew Pinski --- (In reply to Jiri Slaby from comment #22) > (In reply to Andrew Pinski from comment #19) > > *** Bug 106939 has been marked as a duplicate of this bug. *** > > Provided that many duplicates (even nested -- see that bug too) -- everyone > expects this to work. Would it be possible for gcc to at least dump a > warning? Also see PR 116910 where I recommended we document the proper way of doing this too.
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 Sam James changed: What|Removed |Added CC||sjames at gcc dot gnu.org See Also||https://gcc.gnu.org/bugzill ||a/show_bug.cgi?id=115498 --- Comment #23 from Sam James --- See PR115498 and the See Also'd bugs there.
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 --- Comment #22 from Jiri Slaby --- (In reply to Andrew Pinski from comment #19) > *** Bug 106939 has been marked as a duplicate of this bug. *** Provided that many duplicates (even nested -- see that bug too) -- everyone expects this to work. Would it be possible for gcc to at least dump a warning?
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 Andrew Pinski changed: What|Removed |Added CC||cooper.qu at linux dot alibaba.com --- Comment #21 from Andrew Pinski --- *** Bug 101979 has been marked as a duplicate of this bug. ***
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 Andrew Pinski changed: What|Removed |Added CC||stian.skjelstad at gmail dot com --- Comment #20 from Andrew Pinski --- *** Bug 107010 has been marked as a duplicate of this bug. ***
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 Andrew Pinski changed: What|Removed |Added CC||neoxic at icloud dot com --- Comment #19 from Andrew Pinski --- *** Bug 106939 has been marked as a duplicate of this bug. ***
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 --- Comment #18 from Richard Biener --- Author: rguenth Date: Wed Nov 2 08:29:48 2016 New Revision: 241776 URL: https://gcc.gnu.org/viewcvs?rev=241776&root=gcc&view=rev Log: 2016-11-02 Richard Biener PR tree-optimization/78035 PR tree-optimization/77964 * gimple-pretty-print.c (pp_points_to_solution): Print vars_contains_interposable. * tree-ssa-alias.c: Include varasm.h. (ptrs_compare_unequal): Check vars_contains_interposable and decl_binds_to_current_def_p. (dump_points_to_solution): Dump vars_contains_interposable. * tree-ssa-alias.h (struct pt_solution): Add vars_contains_interposable flag. * tree-ssa-structalias.c: Include varasm.h. (set_uids_in_ptset): Record whether vars contains a not decl_binds_to_current_def_p variable in vars_contains_interposable. (ipa_escaped_pt): Update initializer. * gcc.target/i386/pr78035.c: New testcase. Added: trunk/gcc/testsuite/gcc.target/i386/pr78035.c Modified: trunk/gcc/ChangeLog trunk/gcc/gimple-pretty-print.c trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-ssa-alias.c trunk/gcc/tree-ssa-alias.h trunk/gcc/tree-ssa-structalias.c
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 --- Comment #17 from Jakub Jelinek --- (In reply to Jiri Slaby from comment #16) > (In reply to Jakub Jelinek from comment #15) > > lots of them that rely on pointer arithmetics being defined only within the > > same object. > > Sure, but the two pointers (taken implicitly of the arrays) are within the > same object. So I do not see, why it wouldn't work? I.e. where exactly this > breaks the C specs? No. In C extern struct builtin_fw __start_builtin_fw[]; extern struct builtin_fw __end_builtin_fw[]; declares two external arrays, thus they are two independent objects. It is like if you have: int a[10]; int b[10]; in your program, although they might be allocated adjacent, such that int *p = &a[10]; int *q = &b[0]; memcmp (&p, &q, sizeof (p)) == 0; &b[0] - &a[0] is still UB. What you do with __start_*/__end_* symbols is nothing you can define in C, you need linker support or asm for that, and to use it without UB you also need to use an optimization barrier that has been suggested.
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 --- Comment #16 from Jiri Slaby --- (In reply to Jakub Jelinek from comment #15) > lots of them that rely on pointer arithmetics being defined only within the > same object. Sure, but the two pointers (taken implicitly of the arrays) are within the same object. So I do not see, why it wouldn't work? I.e. where exactly this breaks the C specs?
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 --- Comment #15 from Jakub Jelinek --- (In reply to Jiri Slaby from comment #14) > (In reply to Andrew Pinski from comment #10) > > (In reply to Markus Trippelsdorf from comment #9) > > > Is subtracting undefined, too? > > Yes. Comparing two unrelated arrays or subtracting them is undefined. > > But they are not unrelated arrays. So what from the C standard actually > makes (and allows) gcc think they are unrelated? C doesn't have any notion of "related" declarations. > And given gcc 7 is to be released yet, can we have a switch to disable this > optimization? This is nothing new in GCC 7, you've most likely just been extremely lucky in the past that it happened to work as you expected. Other projects had to change similar UB code years ago. It isn't just a single optimization, but lots of them that rely on pointer arithmetics being defined only within the same object.
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 --- Comment #14 from Jiri Slaby --- (In reply to Andrew Pinski from comment #10) > (In reply to Markus Trippelsdorf from comment #9) > > Is subtracting undefined, too? > Yes. Comparing two unrelated arrays or subtracting them is undefined. But they are not unrelated arrays. So what from the C standard actually makes (and allows) gcc think they are unrelated? And given gcc 7 is to be released yet, can we have a switch to disable this optimization?
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 Markus Trippelsdorf changed: What|Removed |Added URL||http://marc.info/?t=1466867 ||2242&r=1&w=2 --- Comment #13 from Markus Trippelsdorf --- See: http://marc.info/?t=14668672242&r=1&w=2 for kernel side discussion.
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 Andrew Pinski changed: What|Removed |Added CC||jirislaby at gmail dot com --- Comment #12 from Andrew Pinski --- *** Bug 77981 has been marked as a duplicate of this bug. ***
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 --- Comment #11 from Marc Glisse --- (In reply to Andrew Pinski from comment #5) > Looks like a kernel issue. An asm ("", "+r"(x)); is needed in the source for > __start_builtin_fw and __end_builtin_fw Shouldn't we recommend "+g" instead of "+r"? (In reply to Markus Trippelsdorf from comment #9) > Is subtracting undefined, too? > > - for (b_fw = __start_builtin_fw; b_fw != __end_builtin_fw; b_fw++) { > + for (b_fw = __start_builtin_fw; __end_builtin_fw - b_fw; b_fw++) { See bug 77813.
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 --- Comment #10 from Andrew Pinski --- (In reply to Markus Trippelsdorf from comment #9) > Is subtracting undefined, too? Yes. Comparing two unrelated arrays or subtracting them is undefined.
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 Markus Trippelsdorf changed: What|Removed |Added Status|WAITING |RESOLVED Resolution|--- |INVALID --- Comment #9 from Markus Trippelsdorf --- Is subtracting undefined, too? - for (b_fw = __start_builtin_fw; b_fw != __end_builtin_fw; b_fw++) { + for (b_fw = __start_builtin_fw; __end_builtin_fw - b_fw; b_fw++) {
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 --- Comment #8 from Andrew Pinski --- (In reply to Jakub Jelinek from comment #7) > You don't need it for both. > struct builtin_fw *b_fw = __start_builtin_fw; > asm ("" : "+r" (b_fw)); > for (; b_fw != __end_builtin_fw; b_fw++) { > should be enough. And indeed, without that it is undefined behavior. You might want to do end only then instead of the start. Because in theory GCC could say it is always false as &__end_builtin_fw[-1] is undefined behavior too so GCC say if the initial value was not __end_builtin_fw[0] go into an infinite loop.
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 --- Comment #7 from Jakub Jelinek --- You don't need it for both. struct builtin_fw *b_fw = __start_builtin_fw; asm ("" : "+r" (b_fw)); for (; b_fw != __end_builtin_fw; b_fw++) { should be enough. And indeed, without that it is undefined behavior.
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 --- Comment #6 from Andrew Pinski --- That is change: extern struct builtin_fw __start_builtin_fw[]; extern struct builtin_fw __end_builtin_fw[]; static bool fw_get_builtin_firmware(struct firmware *fw, const char *name, void *buf, size_t size) { struct builtin_fw *b_fw; for (b_fw = __start_builtin_fw; b_fw != __end_builtin_fw; b_fw++) { --- CUT -- to (there might already be a macro in linux which does the asm like below already): extern struct builtin_fw __start_builtin_fw[]; extern struct builtin_fw __end_builtin_fw[]; static bool fw_get_builtin_firmware(struct firmware *fw, const char *name, void *buf, size_t size) { struct builtin_fw *b_fw; struct builtin_fw *b_fw_start = __start_builtin_fw, b_fw_end = __end_builtin_fw; asm("":"+r"(b_fw_start)); asm("":"+r"(b_fw_end)); for (b_fw = b_fw_start; b_fw != b_fw_end; b_fw++) {
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 Andrew Pinski changed: What|Removed |Added Status|NEW |WAITING --- Comment #5 from Andrew Pinski --- Looks like a kernel issue. An asm ("", "+r"(x)); is needed in the source for __start_builtin_fw and __end_builtin_fw
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 Markus Trippelsdorf changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2016-10-13 Ever confirmed|0 |1 --- Comment #4 from Markus Trippelsdorf --- Started with r235622: commit 73447cc5d17178b0a756be48133e55fdc7574c13 Author: rguenth Date: Fri Apr 29 08:36:49 2016 + 2016-04-29 Richard Biener PR tree-optimization/13962 PR tree-optimization/65686 * tree-ssa-alias.h (ptrs_compare_unequal): Declare. * tree-ssa-alias.c (ptrs_compare_unequal): New function using PTA to compare pointers. * match.pd: Add pattern for pointer equality compare simplification using ptrs_compare_unequal good: callkmem_cache_alloc testq %rax, %rax movq%rax, %r12 je .L28 movq$__start_builtin_fw, %r13 cmpq$__end_builtin_fw, %r13 jne .L86 jmp .L30 .L32: addq$24, %r13 cmpq$__end_builtin_fw, %r13 je .L30 .L86: movq0(%r13), %rsi movq%rbx, %rdi callstrcmp -- bad: callkmem_cache_alloc testq %rax, %rax movq%rax, %r15 movq(%rsp), %r8 je .L6 movq$__start_builtin_fw, %rbx jmp .L7 .L20: addq$24, %rbx .L7: movq(%rbx), %rsi movq%rbp, %rdi callstrcmp
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 --- Comment #3 from Markus Trippelsdorf --- (In reply to Jakub Jelinek from comment #2) > Can you bisect? Suspect changes might be the shrink wrapping changes from > last night - r241063 and r241059-r241061. It is much older issue. Even gcc from 20160928 fails. And bisecting is annoying, because unfortunately the failure only happens on bare metal (booting under qemu is fine). So I have to reboot all the time. I will try to bisect by looking at the assembly output.
[Bug middle-end/77964] [7 Regression] Linux kernel firmware loader miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77964 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #2 from Jakub Jelinek --- Can you bisect? Suspect changes might be the shrink wrapping changes from last night - r241063 and r241059-r241061.