For the following function void mach_parse_compressed(unsigned char* ptr, unsigned long int* val) { if (ptr[0] < 0xC0U) { *val = ptr[0] + ptr[1]; return; } *val = ((unsigned long int)(ptr[0]) << 24) | ((unsigned long int)(ptr[1]) << 16) | ((unsigned long int)(ptr[2]) << 8) | ptr[3]; } starting with GCC 5.1, with -O2 -fPIC, the following is generated on x86_64: mach_parse_compressed(unsigned char*, unsigned long*): movl (%rdi), %eax <--- this load is not safe before branching bswap %eax movl %eax, %edx movzbl (%rdi), %eax cmpb $-65, %al jbe .L5 movl %edx, %eax movq %rax, (%rsi) ret .L5: movzbl 1(%rdi), %edx addl %edx, %eax cltq movq %rax, (%rsi) ret "movl (%rdi), %eax" loads all of ptr[0]..ptr[3] even though before we compare ptr[0] with 0xC0U, we don't know whether we can assume that ptr[1]..ptr[3] does not point past the end of an object. GCC 5.1/5.2/5.3/5.4/6.1/6.2 are all affected. Versions 4.7, 4.8, 4.9 are not. I have checked this using Ubuntu GCC build and http://gcc.godbolt.org/. If necessary, I can build GCC from pristine .tar.gz and re-test.
Created attachment 39663 [details] Test program containing the function for Valgrind/ASan/memory protection errors A test program for Linux to show the various ASan/Valgrind/SIGSEGV error this bug produces.
Confirmed. The bswap pass is putting the word load and __builtin_bswap after the first load instead right after the last load.
Regressed with r211778 aka PR61517.
Confirmed.
Got a patch, testing it now.
(In reply to Thomas Preud'homme from comment #5) > Got a patch, testing it now. Bootstrapped and testsuite came back clean. Trying to turn the code in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77673#c0 into a testcase. Expect a patch soon. Best regards.
(In reply to Thomas Preud'homme from comment #6) > (In reply to Thomas Preud'homme from comment #5) > > Got a patch, testing it now. > > Bootstrapped and testsuite came back clean. Trying to turn the code in > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77673#c0 into a testcase. > Expect a patch soon. > > Best regards. Patch is open for review at https://gcc.gnu.org/ml/gcc-patches/2016-11/msg02337.html
Author: thopre01 Date: Fri Nov 25 10:03:38 2016 New Revision: 242869 URL: https://gcc.gnu.org/viewcvs?rev=242869&root=gcc&view=rev Log: Fix PR77673: bswap loads passed end of object 2016-11-25 Thomas Preud'homme <thomas.preudhomme@arm.com> gcc/ PR tree-optimization/77673 * tree-ssa-math-opts.c (struct symbolic_number): Add new src field. (init_symbolic_number): Initialize src field from src parameter. (perform_symbolic_merge): Select most dominated statement as the source statement. Set src field of resulting n structure from the input src with the lowest address. (find_bswap_or_nop): Rename source_stmt into ins_stmt. (bswap_replace): Rename src_stmt into ins_stmt. Initially get source of load from src field rather than insertion statement. Cancel optimization if statement analyzed is not dominated by the insertion statement. (pass_optimize_bswap::execute): Rename src_stmt to ins_stmt. Compute dominance information. gcc/testsuite/ PR tree-optimization/77673 * gcc.dg/pr77673.c: New test. Added: trunk/gcc/testsuite/gcc.dg/pr77673.c Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-ssa-math-opts.c
Author: thopre01 Date: Wed Dec 14 09:58:23 2016 New Revision: 243635 URL: https://gcc.gnu.org/viewcvs?rev=243635&root=gcc&view=rev Log: Fix PR77673: bswap loads passed end of object 2016-12-14 Thomas Preud'homme <thomas.preudhomme@arm.com> Backport from mainline 2016-11-25 Thomas Preud'homme <thomas.preudhomme@arm.com> gcc/ PR tree-optimization/77673 * tree-ssa-math-opts.c (struct symbolic_number): Add new src field. (init_symbolic_number): Initialize src field from src parameter. (perform_symbolic_merge): Select most dominated statement as the source statement. Set src field of resulting n structure from the input src with the lowest address. (find_bswap_or_nop): Rename source_stmt into ins_stmt. (bswap_replace): Rename src_stmt into ins_stmt. Initially get source of load from src field rather than insertion statement. Cancel optimization if statement analyzed is not dominated by the insertion statement. (pass_optimize_bswap::execute): Rename src_stmt to ins_stmt. Compute dominance information. gcc/testsuite/ PR tree-optimization/77673 * gcc.dg/pr77673.c: New test. Added: branches/gcc-6-branch/gcc/testsuite/gcc.dg/pr77673.c Modified: branches/gcc-6-branch/gcc/ChangeLog branches/gcc-6-branch/gcc/testsuite/ChangeLog branches/gcc-6-branch/gcc/tree-ssa-math-opts.c
Author: thopre01 Date: Wed Dec 14 10:07:01 2016 New Revision: 243637 URL: https://gcc.gnu.org/viewcvs?rev=243637&root=gcc&view=rev Log: Fix PR77673: bswap loads passed end of object 2016-12-14 Thomas Preud'homme <thomas.preudhomme@arm.com> Backport from mainline 2016-11-25 Thomas Preud'homme <thomas.preudhomme@arm.com> gcc/ PR tree-optimization/77673 * tree-ssa-math-opts.c (struct symbolic_number): Add new src field. (init_symbolic_number): Initialize src field from src parameter. (perform_symbolic_merge): Select most dominated statement as the source statement. Set src field of resulting n structure from the input src with the lowest address. (find_bswap_or_nop): Rename source_stmt into ins_stmt. (bswap_replace): Rename src_stmt into ins_stmt. Initially get source of load from src field rather than insertion statement. Cancel optimization if statement analyzed is not dominated by the insertion statement. (pass_optimize_bswap::execute): Rename src_stmt to ins_stmt. Compute dominance information. gcc/testsuite/ PR tree-optimization/77673 * gcc.dg/pr77673.c: New test. Added: branches/gcc-5-branch/gcc/testsuite/gcc.dg/pr77673.c Modified: branches/gcc-5-branch/gcc/ChangeLog branches/gcc-5-branch/gcc/testsuite/ChangeLog branches/gcc-5-branch/gcc/tree-ssa-math-opts.c
Fixed in all affected branches.