Created attachment 40812 [details] rh1422456.ii.bz2 The following testcase aborts starting with r235622 when compiled with: -march=armv7-a -mtune=cortex-a8 -mfloat-abi=hard -mfpu=vfpv3-d16 -mabi=aapcs-linux -O2 -std=c++11
So, I've patched the r235622 cross-compiler to: --- tree-ssa-alias.c.xx 2017-02-21 19:17:36.000000000 +0100 +++ tree-ssa-alias.c 2017-02-22 11:37:12.714172891 +0100 @@ -359,26 +359,31 @@ ptrs_compare_unequal (tree ptr1, tree pt } if (obj1 && obj2) - /* Other code handles this correctly, no need to duplicate it here. */; + /* Other code handles this correctly, no need to duplicate it here. */ + return false; else if (obj1 && TREE_CODE (ptr2) == SSA_NAME) { struct ptr_info_def *pi = SSA_NAME_PTR_INFO (ptr2); if (!pi) return false; - return !pt_solution_includes (&pi->pt, obj1); + if (!pt_solution_includes (&pi->pt, obj1)) + return false; } else if (TREE_CODE (ptr1) == SSA_NAME && obj2) { struct ptr_info_def *pi = SSA_NAME_PTR_INFO (ptr1); if (!pi) return false; - return !pt_solution_includes (&pi->pt, obj2); + if (!pt_solution_includes (&pi->pt, obj2)) + return false; } + else + return false; - /* ??? We'd like to handle ptr1 != NULL and ptr1 != ptr2 - but those require pt.null to be conservatively correct. */ - - return false; +gcc_assert (TREE_CODE (ptr2) == SSA_NAME); +int x = atoi (getenv ("PTRS_COMPARE_UNEQUAL")); +debug_generic_stmt (ptr2); +return SSA_NAME_VERSION (ptr2) == x; } /* Returns whether reference REF to BASE may refer to global memory. */ and for the SSA_NAMEs printed with PTRS_COMPARE_UNEQUAL=10000 tested assembly for PTRS_COMPARE_UNEQUAL set to all those versions. This gave me (10000 is randomly chosen SSA_NAME that doesn't appear in the list, so effectively all ptrs_compare_unequal return false): ===rh1422456.10000=== ===rh1422456.122=== *** Error in `/tmp/rh1422456.122': free(): invalid size: 0xbede60b8 *** ======= Backtrace: ========= /lib/libc.so.6(+0x6b7e0)[0xb6bf47e0] /lib/libc.so.6(+0x757f4)[0xb6bfe7f4] /lib/libc.so.6(cfree+0x5c)[0xb6c02778] /tmp/rh1422456.122[0x11bb4] ======= Memory map: ======== 00010000-00014000 r-xp 00000000 00:24 53702 /tmp/rh1422456.122 00023000-00024000 rw-p 00003000 00:24 53702 /tmp/rh1422456.122 01c1c000-01c41000 rw-p 00000000 00:00 0 [heap] b6b87000-b6b89000 rw-p 00000000 00:00 0 b6b89000-b6cd4000 r-xp 00000000 fd:00 2888289 /usr/lib/libc-2.24.so b6cd4000-b6ce3000 ---p 0014b000 fd:00 2888289 /usr/lib/libc-2.24.so b6ce3000-b6ce5000 r--p 0014a000 fd:00 2888289 /usr/lib/libc-2.24.so b6ce5000-b6ce6000 rw-p 0014c000 fd:00 2888289 /usr/lib/libc-2.24.so b6ce6000-b6ce9000 rw-p 00000000 00:00 0 b6ce9000-b6d05000 r-xp 00000000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d05000-b6d14000 ---p 0001c000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d14000-b6d15000 r--p 0001b000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d15000-b6d16000 rw-p 0001c000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d16000-b6d87000 r-xp 00000000 fd:00 2888295 /usr/lib/libm-2.24.so b6d87000-b6d96000 ---p 00071000 fd:00 2888295 /usr/lib/libm-2.24.so b6d96000-b6d97000 r--p 00070000 fd:00 2888295 /usr/lib/libm-2.24.so b6d97000-b6d98000 rw-p 00071000 fd:00 2888295 /usr/lib/libm-2.24.so b6d98000-b6ed1000 r-xp 00000000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6ed1000-b6ee1000 ---p 00139000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6ee1000-b6ee6000 r--p 00139000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6ee6000-b6ee8000 rw-p 0013e000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6ee8000-b6eea000 rw-p 00000000 00:00 0 b6eea000-b6f0d000 r-xp 00000000 fd:00 2888281 /usr/lib/ld-2.24.so b6f19000-b6f1c000 rw-p 00000000 00:00 0 b6f1c000-b6f1d000 r--p 00022000 fd:00 2888281 /usr/lib/ld-2.24.so b6f1d000-b6f1e000 rw-p 00023000 fd:00 2888281 /usr/lib/ld-2.24.so bed00000-bede7000 rw-p 00000000 00:00 0 [stack] bee71000-bee72000 r-xp 00000000 00:00 0 [sigpage] ffff0000-ffff1000 r-xp 00000000 00:00 0 [vectors] Aborted (core dumped) ===rh1422456.129=== *** Error in `/tmp/rh1422456.129': free(): invalid size: 0xbe8100d0 *** ======= Backtrace: ========= /lib/libc.so.6(+0x6b7e0)[0xb6c7e7e0] /lib/libc.so.6(+0x757f4)[0xb6c887f4] /lib/libc.so.6(cfree+0x5c)[0xb6c8c778] /tmp/rh1422456.129[0x11c08] ======= Memory map: ======== 00010000-00014000 r-xp 00000000 00:24 42913 /tmp/rh1422456.129 00023000-00024000 rw-p 00003000 00:24 42913 /tmp/rh1422456.129 0084e000-00873000 rw-p 00000000 00:00 0 [heap] b6c11000-b6c13000 rw-p 00000000 00:00 0 b6c13000-b6d5e000 r-xp 00000000 fd:00 2888289 /usr/lib/libc-2.24.so b6d5e000-b6d6d000 ---p 0014b000 fd:00 2888289 /usr/lib/libc-2.24.so b6d6d000-b6d6f000 r--p 0014a000 fd:00 2888289 /usr/lib/libc-2.24.so b6d6f000-b6d70000 rw-p 0014c000 fd:00 2888289 /usr/lib/libc-2.24.so b6d70000-b6d73000 rw-p 00000000 00:00 0 b6d73000-b6d8f000 r-xp 00000000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d8f000-b6d9e000 ---p 0001c000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d9e000-b6d9f000 r--p 0001b000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d9f000-b6da0000 rw-p 0001c000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6da0000-b6e11000 r-xp 00000000 fd:00 2888295 /usr/lib/libm-2.24.so b6e11000-b6e20000 ---p 00071000 fd:00 2888295 /usr/lib/libm-2.24.so b6e20000-b6e21000 r--p 00070000 fd:00 2888295 /usr/lib/libm-2.24.so b6e21000-b6e22000 rw-p 00071000 fd:00 2888295 /usr/lib/libm-2.24.so b6e22000-b6f5b000 r-xp 00000000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f5b000-b6f6b000 ---p 00139000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f6b000-b6f70000 r--p 00139000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f70000-b6f72000 rw-p 0013e000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f72000-b6f74000 rw-p 00000000 00:00 0 b6f74000-b6f97000 r-xp 00000000 fd:00 2888281 /usr/lib/ld-2.24.so b6fa3000-b6fa6000 rw-p 00000000 00:00 0 b6fa6000-b6fa7000 r--p 00022000 fd:00 2888281 /usr/lib/ld-2.24.so b6fa7000-b6fa8000 rw-p 00023000 fd:00 2888281 /usr/lib/ld-2.24.so be7f0000-be811000 rw-p 00000000 00:00 0 [stack] bea88000-bea89000 r-xp 00000000 00:00 0 [sigpage] ffff0000-ffff1000 r-xp 00000000 00:00 0 [vectors] Aborted (core dumped) ===rh1422456.12=== ===rh1422456.136=== *** Error in `/tmp/rh1422456.136': free(): invalid size: 0xbec940e8 *** ======= Backtrace: ========= /lib/libc.so.6(+0x6b7e0)[0xb6c227e0] /lib/libc.so.6(+0x757f4)[0xb6c2c7f4] /lib/libc.so.6(cfree+0x5c)[0xb6c30778] /tmp/rh1422456.136[0x11c5c] ======= Memory map: ======== 00010000-00014000 r-xp 00000000 00:24 53731 /tmp/rh1422456.136 00023000-00024000 rw-p 00003000 00:24 53731 /tmp/rh1422456.136 01971000-01996000 rw-p 00000000 00:00 0 [heap] b6bb5000-b6bb7000 rw-p 00000000 00:00 0 b6bb7000-b6d02000 r-xp 00000000 fd:00 2888289 /usr/lib/libc-2.24.so b6d02000-b6d11000 ---p 0014b000 fd:00 2888289 /usr/lib/libc-2.24.so b6d11000-b6d13000 r--p 0014a000 fd:00 2888289 /usr/lib/libc-2.24.so b6d13000-b6d14000 rw-p 0014c000 fd:00 2888289 /usr/lib/libc-2.24.so b6d14000-b6d17000 rw-p 00000000 00:00 0 b6d17000-b6d33000 r-xp 00000000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d33000-b6d42000 ---p 0001c000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d42000-b6d43000 r--p 0001b000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d43000-b6d44000 rw-p 0001c000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d44000-b6db5000 r-xp 00000000 fd:00 2888295 /usr/lib/libm-2.24.so b6db5000-b6dc4000 ---p 00071000 fd:00 2888295 /usr/lib/libm-2.24.so b6dc4000-b6dc5000 r--p 00070000 fd:00 2888295 /usr/lib/libm-2.24.so b6dc5000-b6dc6000 rw-p 00071000 fd:00 2888295 /usr/lib/libm-2.24.so b6dc6000-b6eff000 r-xp 00000000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6eff000-b6f0f000 ---p 00139000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f0f000-b6f14000 r--p 00139000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f14000-b6f16000 rw-p 0013e000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f16000-b6f18000 rw-p 00000000 00:00 0 b6f18000-b6f3b000 r-xp 00000000 fd:00 2888281 /usr/lib/ld-2.24.so b6f47000-b6f4a000 rw-p 00000000 00:00 0 b6f4a000-b6f4b000 r--p 00022000 fd:00 2888281 /usr/lib/ld-2.24.so b6f4b000-b6f4c000 rw-p 00023000 fd:00 2888281 /usr/lib/ld-2.24.so bec00000-bec95000 rw-p 00000000 00:00 0 [stack] befc2000-befc3000 r-xp 00000000 00:00 0 [sigpage] ffff0000-ffff1000 r-xp 00000000 00:00 0 [vectors] Aborted (core dumped) ===rh1422456.138=== ===rh1422456.13=== ===rh1422456.140=== ===rh1422456.142=== ===rh1422456.144=== ===rh1422456.14=== ===rh1422456.15=== ===rh1422456.22=== *** Error in `/tmp/rh1422456.22': munmap_chunk(): invalid pointer: 0xbeff70e8 *** ======= Backtrace: ========= /lib/libc.so.6(+0x6b7e0)[0xb6c4d7e0] /lib/libc.so.6(cfree+0x288)[0xb6c5b9a4] /tmp/rh1422456.22[0x11b64] ======= Memory map: ======== 00010000-00014000 r-xp 00000000 00:24 53758 /tmp/rh1422456.22 00023000-00024000 rw-p 00003000 00:24 53758 /tmp/rh1422456.22 00d78000-00d9d000 rw-p 00000000 00:00 0 [heap] b6be0000-b6be2000 rw-p 00000000 00:00 0 b6be2000-b6d2d000 r-xp 00000000 fd:00 2888289 /usr/lib/libc-2.24.so b6d2d000-b6d3c000 ---p 0014b000 fd:00 2888289 /usr/lib/libc-2.24.so b6d3c000-b6d3e000 r--p 0014a000 fd:00 2888289 /usr/lib/libc-2.24.so b6d3e000-b6d3f000 rw-p 0014c000 fd:00 2888289 /usr/lib/libc-2.24.so b6d3f000-b6d42000 rw-p 00000000 00:00 0 b6d42000-b6d5e000 r-xp 00000000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d5e000-b6d6d000 ---p 0001c000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d6d000-b6d6e000 r--p 0001b000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d6e000-b6d6f000 rw-p 0001c000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d6f000-b6de0000 r-xp 00000000 fd:00 2888295 /usr/lib/libm-2.24.so b6de0000-b6def000 ---p 00071000 fd:00 2888295 /usr/lib/libm-2.24.so b6def000-b6df0000 r--p 00070000 fd:00 2888295 /usr/lib/libm-2.24.so b6df0000-b6df1000 rw-p 00071000 fd:00 2888295 /usr/lib/libm-2.24.so b6df1000-b6f2a000 r-xp 00000000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f2a000-b6f3a000 ---p 00139000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f3a000-b6f3f000 r--p 00139000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f3f000-b6f41000 rw-p 0013e000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f41000-b6f43000 rw-p 00000000 00:00 0 b6f43000-b6f66000 r-xp 00000000 fd:00 2888281 /usr/lib/ld-2.24.so b6f72000-b6f75000 rw-p 00000000 00:00 0 b6f75000-b6f76000 r--p 00022000 fd:00 2888281 /usr/lib/ld-2.24.so b6f76000-b6f77000 rw-p 00023000 fd:00 2888281 /usr/lib/ld-2.24.so befd7000-beff8000 rw-p 00000000 00:00 0 [stack] beffa000-beffb000 r-xp 00000000 00:00 0 [sigpage] ffff0000-ffff1000 r-xp 00000000 00:00 0 [vectors] Aborted (core dumped) ===rh1422456.38=== *** Error in `/tmp/rh1422456.38': munmap_chunk(): invalid pointer: 0xbec230e8 *** ======= Backtrace: ========= /lib/libc.so.6(+0x6b7e0)[0xb6be17e0] /lib/libc.so.6(cfree+0x288)[0xb6bef9a4] /tmp/rh1422456.38[0x11b64] ======= Memory map: ======== 00010000-00014000 r-xp 00000000 00:24 56388 /tmp/rh1422456.38 00023000-00024000 rw-p 00003000 00:24 56388 /tmp/rh1422456.38 0148c000-014b1000 rw-p 00000000 00:00 0 [heap] b6b74000-b6b76000 rw-p 00000000 00:00 0 b6b76000-b6cc1000 r-xp 00000000 fd:00 2888289 /usr/lib/libc-2.24.so b6cc1000-b6cd0000 ---p 0014b000 fd:00 2888289 /usr/lib/libc-2.24.so b6cd0000-b6cd2000 r--p 0014a000 fd:00 2888289 /usr/lib/libc-2.24.so b6cd2000-b6cd3000 rw-p 0014c000 fd:00 2888289 /usr/lib/libc-2.24.so b6cd3000-b6cd6000 rw-p 00000000 00:00 0 b6cd6000-b6cf2000 r-xp 00000000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6cf2000-b6d01000 ---p 0001c000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d01000-b6d02000 r--p 0001b000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d02000-b6d03000 rw-p 0001c000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d03000-b6d74000 r-xp 00000000 fd:00 2888295 /usr/lib/libm-2.24.so b6d74000-b6d83000 ---p 00071000 fd:00 2888295 /usr/lib/libm-2.24.so b6d83000-b6d84000 r--p 00070000 fd:00 2888295 /usr/lib/libm-2.24.so b6d84000-b6d85000 rw-p 00071000 fd:00 2888295 /usr/lib/libm-2.24.so b6d85000-b6ebe000 r-xp 00000000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6ebe000-b6ece000 ---p 00139000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6ece000-b6ed3000 r--p 00139000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6ed3000-b6ed5000 rw-p 0013e000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6ed5000-b6ed7000 rw-p 00000000 00:00 0 b6ed7000-b6efa000 r-xp 00000000 fd:00 2888281 /usr/lib/ld-2.24.so b6f06000-b6f09000 rw-p 00000000 00:00 0 b6f09000-b6f0a000 r--p 00022000 fd:00 2888281 /usr/lib/ld-2.24.so b6f0a000-b6f0b000 rw-p 00023000 fd:00 2888281 /usr/lib/ld-2.24.so bec03000-bec24000 rw-p 00000000 00:00 0 [stack] beee5000-beee6000 r-xp 00000000 00:00 0 [sigpage] ffff0000-ffff1000 r-xp 00000000 00:00 0 [vectors] Aborted (core dumped) ===rh1422456.45=== ===rh1422456.47=== ===rh1422456.49=== ===rh1422456.51=== ===rh1422456.71=== *** Error in `/tmp/rh1422456.71': free(): invalid size: 0xbebad0b8 *** ======= Backtrace: ========= /lib/libc.so.6(+0x6b7e0)[0xb6c457e0] /lib/libc.so.6(+0x757f4)[0xb6c4f7f4] /lib/libc.so.6(cfree+0x5c)[0xb6c53778] /tmp/rh1422456.71[0x11bb4] ======= Memory map: ======== 00010000-00014000 r-xp 00000000 00:24 56482 /tmp/rh1422456.71 00023000-00024000 rw-p 00003000 00:24 56482 /tmp/rh1422456.71 014a2000-014c7000 rw-p 00000000 00:00 0 [heap] b6bd8000-b6bda000 rw-p 00000000 00:00 0 b6bda000-b6d25000 r-xp 00000000 fd:00 2888289 /usr/lib/libc-2.24.so b6d25000-b6d34000 ---p 0014b000 fd:00 2888289 /usr/lib/libc-2.24.so b6d34000-b6d36000 r--p 0014a000 fd:00 2888289 /usr/lib/libc-2.24.so b6d36000-b6d37000 rw-p 0014c000 fd:00 2888289 /usr/lib/libc-2.24.so b6d37000-b6d3a000 rw-p 00000000 00:00 0 b6d3a000-b6d56000 r-xp 00000000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d56000-b6d65000 ---p 0001c000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d65000-b6d66000 r--p 0001b000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d66000-b6d67000 rw-p 0001c000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d67000-b6dd8000 r-xp 00000000 fd:00 2888295 /usr/lib/libm-2.24.so b6dd8000-b6de7000 ---p 00071000 fd:00 2888295 /usr/lib/libm-2.24.so b6de7000-b6de8000 r--p 00070000 fd:00 2888295 /usr/lib/libm-2.24.so b6de8000-b6de9000 rw-p 00071000 fd:00 2888295 /usr/lib/libm-2.24.so b6de9000-b6f22000 r-xp 00000000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f22000-b6f32000 ---p 00139000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f32000-b6f37000 r--p 00139000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f37000-b6f39000 rw-p 0013e000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f39000-b6f3b000 rw-p 00000000 00:00 0 b6f3b000-b6f5e000 r-xp 00000000 fd:00 2888281 /usr/lib/ld-2.24.so b6f6a000-b6f6d000 rw-p 00000000 00:00 0 b6f6d000-b6f6e000 r--p 00022000 fd:00 2888281 /usr/lib/ld-2.24.so b6f6e000-b6f6f000 rw-p 00023000 fd:00 2888281 /usr/lib/ld-2.24.so beb00000-bebae000 rw-p 00000000 00:00 0 [stack] bef27000-bef28000 r-xp 00000000 00:00 0 [sigpage] ffff0000-ffff1000 r-xp 00000000 00:00 0 [vectors] Aborted (core dumped) ===rh1422456.77=== *** Error in `/tmp/rh1422456.77': free(): invalid size: 0xbe93c0d0 *** ======= Backtrace: ========= /lib/libc.so.6(+0x6b7e0)[0xb6c7c7e0] /lib/libc.so.6(+0x757f4)[0xb6c867f4] /lib/libc.so.6(cfree+0x5c)[0xb6c8a778] /tmp/rh1422456.77[0x11c08] ======= Memory map: ======== 00010000-00014000 r-xp 00000000 00:24 50128 /tmp/rh1422456.77 00023000-00024000 rw-p 00003000 00:24 50128 /tmp/rh1422456.77 01830000-01855000 rw-p 00000000 00:00 0 [heap] b6c0f000-b6c11000 rw-p 00000000 00:00 0 b6c11000-b6d5c000 r-xp 00000000 fd:00 2888289 /usr/lib/libc-2.24.so b6d5c000-b6d6b000 ---p 0014b000 fd:00 2888289 /usr/lib/libc-2.24.so b6d6b000-b6d6d000 r--p 0014a000 fd:00 2888289 /usr/lib/libc-2.24.so b6d6d000-b6d6e000 rw-p 0014c000 fd:00 2888289 /usr/lib/libc-2.24.so b6d6e000-b6d71000 rw-p 00000000 00:00 0 b6d71000-b6d8d000 r-xp 00000000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d8d000-b6d9c000 ---p 0001c000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d9c000-b6d9d000 r--p 0001b000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d9d000-b6d9e000 rw-p 0001c000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d9e000-b6e0f000 r-xp 00000000 fd:00 2888295 /usr/lib/libm-2.24.so b6e0f000-b6e1e000 ---p 00071000 fd:00 2888295 /usr/lib/libm-2.24.so b6e1e000-b6e1f000 r--p 00070000 fd:00 2888295 /usr/lib/libm-2.24.so b6e1f000-b6e20000 rw-p 00071000 fd:00 2888295 /usr/lib/libm-2.24.so b6e20000-b6f59000 r-xp 00000000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f59000-b6f69000 ---p 00139000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f69000-b6f6e000 r--p 00139000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f6e000-b6f70000 rw-p 0013e000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f70000-b6f72000 rw-p 00000000 00:00 0 b6f72000-b6f95000 r-xp 00000000 fd:00 2888281 /usr/lib/ld-2.24.so b6fa1000-b6fa4000 rw-p 00000000 00:00 0 b6fa4000-b6fa5000 r--p 00022000 fd:00 2888281 /usr/lib/ld-2.24.so b6fa5000-b6fa6000 rw-p 00023000 fd:00 2888281 /usr/lib/ld-2.24.so be900000-be93d000 rw-p 00000000 00:00 0 [stack] bed70000-bed71000 r-xp 00000000 00:00 0 [sigpage] ffff0000-ffff1000 r-xp 00000000 00:00 0 [vectors] Aborted (core dumped) ===rh1422456.83=== *** Error in `/tmp/rh1422456.83': free(): invalid size: 0xbec950e8 *** ======= Backtrace: ========= /lib/libc.so.6(+0x6b7e0)[0xb6c3f7e0] /lib/libc.so.6(+0x757f4)[0xb6c497f4] /lib/libc.so.6(cfree+0x5c)[0xb6c4d778] /tmp/rh1422456.83[0x11c5c] ======= Memory map: ======== 00010000-00014000 r-xp 00000000 00:24 50146 /tmp/rh1422456.83 00023000-00024000 rw-p 00003000 00:24 50146 /tmp/rh1422456.83 01457000-0147c000 rw-p 00000000 00:00 0 [heap] b6bd2000-b6bd4000 rw-p 00000000 00:00 0 b6bd4000-b6d1f000 r-xp 00000000 fd:00 2888289 /usr/lib/libc-2.24.so b6d1f000-b6d2e000 ---p 0014b000 fd:00 2888289 /usr/lib/libc-2.24.so b6d2e000-b6d30000 r--p 0014a000 fd:00 2888289 /usr/lib/libc-2.24.so b6d30000-b6d31000 rw-p 0014c000 fd:00 2888289 /usr/lib/libc-2.24.so b6d31000-b6d34000 rw-p 00000000 00:00 0 b6d34000-b6d50000 r-xp 00000000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d50000-b6d5f000 ---p 0001c000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d5f000-b6d60000 r--p 0001b000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d60000-b6d61000 rw-p 0001c000 fd:00 2906708 /usr/lib/libgcc_s-7-20170219.so.1 b6d61000-b6dd2000 r-xp 00000000 fd:00 2888295 /usr/lib/libm-2.24.so b6dd2000-b6de1000 ---p 00071000 fd:00 2888295 /usr/lib/libm-2.24.so b6de1000-b6de2000 r--p 00070000 fd:00 2888295 /usr/lib/libm-2.24.so b6de2000-b6de3000 rw-p 00071000 fd:00 2888295 /usr/lib/libm-2.24.so b6de3000-b6f1c000 r-xp 00000000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f1c000-b6f2c000 ---p 00139000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f2c000-b6f31000 r--p 00139000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f31000-b6f33000 rw-p 0013e000 fd:00 2888698 /usr/lib/libstdc++.so.6.0.23 b6f33000-b6f35000 rw-p 00000000 00:00 0 b6f35000-b6f58000 r-xp 00000000 fd:00 2888281 /usr/lib/ld-2.24.so b6f64000-b6f67000 rw-p 00000000 00:00 0 b6f67000-b6f68000 r--p 00022000 fd:00 2888281 /usr/lib/ld-2.24.so b6f68000-b6f69000 rw-p 00023000 fd:00 2888281 /usr/lib/ld-2.24.so bec00000-bec96000 rw-p 00000000 00:00 0 [stack] bee5f000-bee60000 r-xp 00000000 00:00 0 [sigpage] ffff0000-ffff1000 r-xp 00000000 00:00 0 [vectors] Aborted (core dumped) ===rh1422456.84=== ===rh1422456.85=== ===rh1422456.86=== ===rh1422456.87=== This means that for 8 different SSA_NAMEs it is enough to return true a single time.
E.g. on 122 valgrind says: ==21905== Invalid free() / delete / delete[] / realloc() ==21905== at 0x484B224: operator delete(void*) (vg_replace_malloc.c:576) ==21905== by 0x11BB3: path_expression_grammar<__gnu_cxx::__normal_iterator<char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >::path_expression_grammar() (in /tmp/rh1422456.122) ==21905== Address 0xbd8fd048 is on thread 1's stack Looking at tree dumps from PTRS_COMPARE_UNEQUAL=10000 (works) and PTRS_COMPARE_UNEQUAL=122 (calls free on invalid arg), I see the first change in *.fre1 in path_expression_grammar<Iterator>::path_expression_grammar() function, MEM[(struct &)this_10(D) + 56] ={v} {CLOBBER}; MEM[(struct function_base *)this_10(D) + 56B].vtable = 0B; _122 = MEM[(char * *)&D.552047]; - if (&D.552047.D.20672._M_local_buf != _122) - goto <bb 9>; - else - goto <bb 10>; - - <bb 9>: operator delete (_122); - - <bb 10>: MEM[(struct &)&D.552047] ={v} {CLOBBER}; D.552047 ={v} {CLOBBER}; D.552048 ={v} {CLOBBER}; _29 = &this_10(D)->attr; and @@ -12334,15 +12330,7 @@ path_expression_grammar<Iterator>::path_ <L4>: _140 = MEM[(char * *)&D.552047]; - if (&D.552047.D.20672._M_local_buf != _140) - goto <bb 27>; - else - goto <bb 28>; - - <bb 27>: operator delete (_140); - - <bb 28>: MEM[(struct &)&D.552047] ={v} {CLOBBER}; D.552047 ={v} {CLOBBER}; resx 7 (all other changes are just in bb numbers, as some bbs have been removed in the latter case). No idea what to figure out from this though, it just removes some operator delete calls (free), but that shouldn't cause free being called on invalid, worst case just memory leak.
(In reply to Jakub Jelinek from comment #2) > E.g. on 122 valgrind says: > ==21905== Invalid free() / delete / delete[] / realloc() > ==21905== at 0x484B224: operator delete(void*) (vg_replace_malloc.c:576) > ==21905== by 0x11BB3: > path_expression_grammar<__gnu_cxx::__normal_iterator<char const*, > std::__cxx11::basic_string<char, std::char_traits<char>, > std::allocator<char> > > >::path_expression_grammar() (in /tmp/rh1422456.122) > ==21905== Address 0xbd8fd048 is on thread 1's stack > > Looking at tree dumps from PTRS_COMPARE_UNEQUAL=10000 (works) and > PTRS_COMPARE_UNEQUAL=122 (calls free on invalid arg), I see the first change > in *.fre1 in path_expression_grammar<Iterator>::path_expression_grammar() > function, > MEM[(struct &)this_10(D) + 56] ={v} {CLOBBER}; > MEM[(struct function_base *)this_10(D) + 56B].vtable = 0B; > _122 = MEM[(char * *)&D.552047]; > - if (&D.552047.D.20672._M_local_buf != _122) > - goto <bb 9>; > - else > - goto <bb 10>; > - > - <bb 9>: > operator delete (_122); > - > - <bb 10>: > MEM[(struct &)&D.552047] ={v} {CLOBBER}; > D.552047 ={v} {CLOBBER}; > D.552048 ={v} {CLOBBER}; > _29 = &this_10(D)->attr; > and > @@ -12334,15 +12330,7 @@ path_expression_grammar<Iterator>::path_ > > <L4>: > _140 = MEM[(char * *)&D.552047]; > - if (&D.552047.D.20672._M_local_buf != _140) > - goto <bb 27>; > - else > - goto <bb 28>; > - > - <bb 27>: > operator delete (_140); > - > - <bb 28>: > MEM[(struct &)&D.552047] ={v} {CLOBBER}; > D.552047 ={v} {CLOBBER}; > resx 7 > > (all other changes are just in bb numbers, as some bbs have been removed in > the latter case). > No idea what to figure out from this though, it just removes some operator > delete calls (free), but that shouldn't cause free being called on invalid, > worst case just memory leak. It more looks like it will now call delete on _140 where it might have not (if equal to &..._M_local_buf). This smells like a PTA issue and the question is why D.552047 is thought to not point to D.552047.D.20672._M_local_buf. I'm trying to reproduce the above with a cross.
I don't see any unconditional delete() in .fre1 or .fre3 (r245696 - did you do your analysis with r235622? There were PTA fixes afterwards).
Repeated on current trunk r245696, this time with: --- gcc/tree-ssa-alias.c.jj 2017-01-10 08:12:48.000000000 +0100 +++ gcc/tree-ssa-alias.c 2017-02-24 12:15:17.869368974 +0100 @@ -387,7 +387,13 @@ ptrs_compare_unequal (tree ptr1, tree pt || ! decl_binds_to_current_def_p (obj1)) return false; } - return !pt_solution_includes (&pi->pt, obj1); + if (!pt_solution_includes (&pi->pt, obj1)) +{ +gcc_assert (TREE_CODE (ptr2) == SSA_NAME); +int x = atoi (getenv ("PTRS_COMPARE_UNEQUAL")); +debug_generic_stmt (ptr2); +return SSA_NAME_VERSION (ptr2) == x; +} } /* ??? We'd like to handle ptr1 != NULL and ptr1 != ptr2 hack. Unpatched trunk fails, with -fno-tree-pre succeeds and with this hack and PTRS_COMPARE_UNEQUAL=111111 succeeds as well. Let me try the various ssa name versions next.
for i in /tmp/rh1422456-_*.s; do j=`basename $i .s`; echo ===$j===; g++ -o /tmp/$j{,.s}; /tmp/$j; done ===rh1422456-_150=== ===rh1422456-_155=== ===rh1422456-_192=== ===rh1422456-_212=== ===rh1422456-_233=== ===rh1422456-_262=== ===rh1422456-_286=== rh1422456-_286: /tmp/rh1422456.C:50: int main(int, char**): Assertion `r && itr == end' failed. Aborted (core dumped) ===rh1422456-_299=== ===rh1422456-_335=== ===rh1422456-_375=== rh1422456-_375: /tmp/rh1422456.C:50: int main(int, char**): Assertion `r && itr == end' failed. Aborted (core dumped) ===rh1422456-_416=== ===rh1422456-_424=== ===rh1422456-_460=== ===rh1422456-_568=== ===rh1422456-_647=== ===rh1422456-_9=== rh1422456-_9: /tmp/rh1422456.C:50: int main(int, char**): Assertion `r && itr == end' failed. Aborted (core dumped) So it is just 3 SSA_NAMEs that result in abort this time (and no invalid free most likely). _9 is actually this_9(D), let me try _286 now how it affects the tree dumps. Of course it might be some RTL bug only triggered through the changes.
Comparing PTRS_COMPARE_UNEQUAL={287,286} fre3 (287 doesn't occur in the list, so it is never optimize using ptrs_compare_unequal, 286 is only optimize for _286), the differences are: _286 = &MEM[(struct function *)this_9(D) + 100B].D.542596; - if (&D.616011.D.542596 == _286) - goto <bb 116>; [46.53%] - else - goto <bb 79>; [53.47%] - - <bb 79> [50.53%]: ... and - <bb 98> [45.34%]: - if (&tmp == _286) - goto <bb 111>; [26.74%] - else - goto <bb 99>; [73.26%] - - <bb 99> [33.22%]: plus lots of renumbering of basic blocks; the number of skipped bbs is big though.
On Fri, 24 Feb 2017, jakub at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > > --- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> --- > Comparing PTRS_COMPARE_UNEQUAL={287,286} fre3 (287 doesn't occur in the list, > so it is never optimize using ptrs_compare_unequal, 286 is only optimize for > _286), > the differences are: > _286 = &MEM[(struct function *)this_9(D) + 100B].D.542596; > - if (&D.616011.D.542596 == _286) > - goto <bb 116>; [46.53%] > - else > - goto <bb 79>; [53.47%] > - > - <bb 79> [50.53%]: > ... > and > - <bb 98> [45.34%]: > - if (&tmp == _286) > - goto <bb 111>; [26.74%] > - else > - goto <bb 99>; [73.26%] > - > - <bb 99> [33.22%]: > plus lots of renumbering of basic blocks; the number of skipped bbs is big > though. Assuming D.616011 and tmp are automatic vars that's obviously correct. _286 is just 'this' offsetted.
As there are two different comparisons, I've in the debugger tried to manually optimize just one of them and not the other one (return 0 in gdb), and it seems that it is the: &tmp == _286 comparison that matters (when optimized into false, testcase aborts), the other comparison doesn't matter. tmp is indeed an automatic variable and _286 is this parameter + some constant offset, so indeed fre3 is right to optimize that away.
-fno-schedule-insns fixes it even with PTRS_COMPARE_UNEQUAL=286 (while -fno-schedule-insns2 still aborts). Let's consider this a target bug for now (especially when it doesn't fail on any other arch).
I'll have a look but -fno-schedule-insns "fixing things" has been known to uncover alias analysis bugs from the GIMPLE level due to the scheduler using the alias information to reorder memory ops
Hi, I don't know if it helps, but on the assembler level there are only two instructions that need to be moved to make the test case pass: diff -u rh1422456-orig.s rh1422456.s --- rh1422456-orig.s 2017-02-26 22:46:30.068919262 +0100 +++ rh1422456.s 2017-02-26 22:46:56.445920712 +0100 @@ -2046,10 +2046,10 @@ ldr ip, [r4, #100] add lr, sp, #52 movw r3, #:lower16:_ZZN5boost9function4IbRN9__gnu_cxx17__normal_iteratorIPKcNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEEERKSB_RNS_6spirit7contextINS_6fusion4consIRSA_NSH_4nil_EEENSH_6vectorIJEEEEERKNSF_2qi10char_classINSF_3tag9char_codeINSS_5spaceENSF_13char_encoding13standard_wideEEEEEE9assign_toINSQ_6detail13parser_binderINSQ_4plusINSQ_10differenceINSR_INST_INSS_5char_ESW_EEEENSQ_12literal_charINSV_8standardELb1ELb0EEEEEEEN4mpl_5bool_ILb1EEEEEEEvT_E13stored_vtable + strb r6, [sp, #293] ldm r5, {r0, r1, r2} cmp ip, #0 movt r3, #:upper16:_ZZN5boost9function4IbRN9__gnu_cxx17__normal_iteratorIPKcNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEEERKSB_RNS_6spirit7contextINS_6fusion4consIRSA_NSH_4nil_EEENSH_6vectorIJEEEEERKNSF_2qi10char_classINSF_3tag9char_codeINSS_5spaceENSF_13char_encoding13standard_wideEEEEEE9assign_toINSQ_6detail13parser_binderINSQ_4plusINSQ_10differenceINSR_INST_INSS_5char_ESW_EEEENSQ_12literal_charINSV_8standardELb1ELb0EEEEEEEN4mpl_5bool_ILb1EEEEEEEvT_E13stored_vtable - strb r6, [sp, #293] orr r3, r3, #1 str r7, [sp, #288] str r3, [sp, #48] @@ -2062,10 +2062,10 @@ ldr ip, [r4, #144] add lr, sp, #68 movw r3, #:lower16:_ZZN5boost9function4IbRN9__gnu_cxx17__normal_iteratorIPKcNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEEERKSB_RNS_6spirit7contextINS_6fusion4consIRSA_NSH_4nil_EEENSH_6vectorIJEEEEERKNSF_11unused_typeEE9assign_toINSF_2qi6detail13parser_binderINSV_16lexeme_directiveINSV_4plusINSV_10differenceINSV_10char_classINSF_3tag9char_codeINS12_5char_ENSF_13char_encoding13standard_wideEEEEENSV_12literal_charINS15_8standardELb1ELb0EEEEEEEEEN4mpl_5bool_ILb1EEEEEEEvT_E13stored_vtable + strb r6, [sp, #293] ldm r5, {r0, r1, r2} cmp ip, #0 movt r3, #:upper16:_ZZN5boost9function4IbRN9__gnu_cxx17__normal_iteratorIPKcNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEEERKSB_RNS_6spirit7contextINS_6fusion4consIRSA_NSH_4nil_EEENSH_6vectorIJEEEEERKNSF_11unused_typeEE9assign_toINSF_2qi6detail13parser_binderINSV_16lexeme_directiveINSV_4plusINSV_10differenceINSV_10char_classINSF_3tag9char_codeINS12_5char_ENSF_13char_encoding13standard_wideEEEEENSV_12literal_charINS15_8standardELb1ELb0EEEEEEEEEN4mpl_5bool_ILb1EEEEEEEvT_E13stored_vtable - strb r6, [sp, #293] orr r3, r3, #1 str r7, [sp, #288] str r3, [sp, #64] => r5 = sp+292, so strb and ldm overlap. In the reload pass I can find the sp+293 mem rtx: (insn 593 1693 598 67 (parallel [ (set (reg:SI 0 r0) (mem/c:SI (reg/f:SI 5 r5 [680]) [19 MEM[(struct function4D.541978 *)&D.615141].D.542200.functorD.466600+0 S4 A32])) (set (reg:SI 1 r1) (mem/c:SI (plus:SI (reg/f:SI 5 r5 [680]) (const_int 4 [0x4])) [19 MEM[(struct function4D.541978 *)&D.615141].D.542200.functorD.466600+4 S4 A32])) (set (reg:SI 2 r2) (mem/c:SI (plus:SI (reg/f:SI 5 r5 [680]) (const_int 8 [0x8])) [19 MEM[(struct function4D.541978 *)&D.615141].D.542200.functorD.466600+8 S4 A32])) ]) "rh1422456.cc":151826 364 {*ldm3_} (nil)) (insn 598 593 586 67 (set (reg:CC 100 cc) (compare:CC (reg/f:SI 12 ip [orig:181 _251 ] [181]) (const_int 0 [0]))) "rh1422456.cc":151823 196 {*arm_cmpsi_insn} (nil)) (insn 586 598 591 67 (set (mem/c:QI (plus:SI (reg/f:SI 13 sp) (const_int 293 [0x125])) [101 MEM[(struct parser_binderD.572000 *)&D.615141 + 5B]+0 S1 A8]) (reg:QI 6 r6 [493])) 192 {*arm_movqi_insn} (nil)) ... and: (insn 878 1691 883 99 (parallel [ (set (reg:SI 0 r0) (mem/c:SI (reg/f:SI 5 r5 [680]) [19 MEM[(struct function4D.546780 *)&D.615194].D.547000.functorD.466600+0 S4 A32])) (set (reg:SI 1 r1) (mem/c:SI (plus:SI (reg/f:SI 5 r5 [680]) (const_int 4 [0x4])) [19 MEM[(struct function4D.546780 *)&D.615194].D.547000.functorD.466600+4 S4 A32])) (set (reg:SI 2 r2) (mem/c:SI (plus:SI (reg/f:SI 5 r5 [680]) (const_int 8 [0x8])) [19 MEM[(struct function4D.546780 *)&D.615194].D.547000.functorD.466600+8 S4 A32])) ]) "rh1422456.cc":151826 364 {*ldm3_} (nil)) (insn 883 878 871 99 (set (reg:CC 100 cc) (compare:CC (reg/f:SI 12 ip [orig:180 _249 ] [180]) (const_int 0 [0]))) "rh1422456.cc":151823 196 {*arm_cmpsi_insn} (nil)) (insn 871 883 876 99 (set (mem/c:QI (plus:SI (reg/f:SI 13 sp) (const_int 293 [0x125])) [105 MEM[(struct parser_binderD.573224 *)&D.615194 + 5B]+0 S1 A8]) (reg:QI 6 r6 [564])) 192 {*arm_movqi_insn} (nil))
(In reply to Bernd Edlinger from comment #12) > Hi, > > I don't know if it helps, but on the assembler level there are only two > instructions that need to be moved to make the test case pass: That of course helps a lot, that is what I was trying to narrow through the hacks. Before sched1 there is: (insn 598 595 1707 67 (set (mem/c:QI (plus:SI (reg/f:SI 102 sfp) (const_int -19 [0xffffffffffffffed])) [98 MEM[(struct parser_binder *)&D.616009 + 5B]+0 S1 A8]) (subreg:QI (reg:SI 505) 0)) 192 {*arm_movqi_insn} (expr_list:REG_DEAD (reg:SI 505) (nil))) ... (insn 604 603 605 67 (set (reg/f:SI 692) (plus:SI (reg/f:SI 102 sfp) (const_int -20 [0xffffffffffffffec]))) "/usr/include/boost/function/function_template.hpp":998 4 {*arm_addsi3} (nil)) (insn 605 604 606 67 (parallel [ (set (reg:SI 0 r0) (mem/c:SI (reg/f:SI 692) [19 MEM[(struct function4 *)&D.616009].D.542442.functor+0 S4 A32])) (set (reg:SI 1 r1) (mem/c:SI (plus:SI (reg/f:SI 692) (const_int 4 [0x4])) [19 MEM[(struct function4 *)&D.616009].D.542442.functor+4 S4 A32])) (set (reg:SI 2 r2) (mem/c:SI (plus:SI (reg/f:SI 692) (const_int 8 [0x8])) [19 MEM[(struct function4 *)&D.616009].D.542442.functor+8 S4 A32])) ]) "/usr/include/boost/function/function_template.hpp":998 364 {*ldm3_} (nil)) (current trunk, the above command line options, no patches), so ignoring strict aliasing, there is a must alias in between the middle mem in insn 605 and the mem insn 598 stores to. Now if I do: break sched_analyze_2 if x->code == MEM && insn->u2.insn_uid == 605 run continue then x is: (mem/c:SI (plus:SI (reg/f:SI 692) (const_int 4 [0x4])) [19 MEM[(struct function4 *)&D.616009].D.542442.functor+4 S4 A32]) and deps->pending_write_mems is: (expr_list:REG_DEP_TRUE (mem/f/c:SI (plus:SI (reg/f:SI 102 sfp) (const_int -264 [0xfffffffffffffef8])) [18 tmp.D.542442.vtable+0 S4 A64]) (expr_list:REG_DEP_TRUE (mem/c:QI (plus:SI (reg/f:SI 102 sfp) (const_int -19 [0xffffffffffffffed])) [98 MEM[(struct parser_binder *)&D.616009 + 5B]+0 S1 A8]) (nil))) This then calls true_dependence with mem: (mem/c:QI (plus:SI (reg/f:SI 102 sfp) (const_int -19 [0xffffffffffffffed])) [98 MEM[(struct parser_binder *)&D.616009 + 5B]+0 S1 A8]) x: (mem/c:SI (plus:SI (reg/f:SI 692) (const_int 4 [0x4])) [19 MEM[(struct function4 *)&D.616009].D.542442.functor+4 S4 A32]) and we return 0 because of: 2931 if (mems_in_disjoint_alias_sets_p (x, mem)) 2932 return 0; In *.optimized dump I believe this is: MEM[(struct parser_binder *)&D.616009 + 5B] = 93; value_283 = (size_t) &stored_vtable.base; value_284 = value_283 | 1; value.75_285 = (struct vtable_base *) value_284; MEM[(struct function4 *)&D.616009].D.542442.vtable = value.75_285; MEM[(struct &)&tmp] ={v} {CLOBBER}; tmp.D.542442.vtable = value.75_285; tmp.D.542442.functor = MEM[(struct function4 *)&D.616009].D.542442.functor; insn 598 is that MEM[(struct parser_binder *)&D.616009 + 5B] = 93; and insn 605 (ldm3_) is the load for the structure assignment tmp.D.542442.functor = MEM[(struct function4 *)&D.616009].D.542442.functor; Richard, could you please have a look from the alias oracle POV?
Seems to be void move_assign(function10& f) { if (&f == this) return; { try { if (!f.empty()) { this->vtable = f.vtable; if (this->has_trivial_copy_and_destroy()) this->functor = f.functor; ^^^ for the aggregate copy but lineno info looks confused for the aliasing store. It points at operator=(Functor f) { self_type(f).swap(*this); return *this; } the boost code for function looks quite convoluted with its vtable handling... It looks like ESRA creates that piecewise store, but it's hard to track down exactly where it comes from. Example transform looks like + char f$p$subject$subject$right$ch; struct parser_binder f; bool D.611696; struct parser_binder f; @@ -15343,7 +11698,8 @@ <bb 2> [100.00%]: <&0x7f50d29d3d70> f = f; - <&0x7f50d29d3dc0> f = f; + <&0x7f50d2a16050> f$p$subject$subject$right$ch_16 = MEM[(struct parser_binder *)&f + 1B]; + <&0x7f50d2a160a0> MEM[(struct parser_binder *)&f + 1B] = f$p$subject$subject$right$ch_16; <&0x7f50d2a14240> _13 = boost::detail::function::has_empty_target (&f); there's no offset 5 one at this point so inlining eventually comes up with <bb 2> [100.00%]: <&0x7f50d2a16b40> f = f; <&0x7f50d2a314b0> f$1_39 = MEM[(struct parser_binder *)&f + 1B]; <&0x7f50d2a16b90> f$1_11 = f$1_39; <&0x7f50d2a16be0> MEM[(struct &)&D.573835] ={v} {CLOBBER}; <&0x7f50d2a16c30> MEM[(struct function_base *)&D.573835].vtable = 0B; <&0x7f50d2a16c80> MEM[(struct parser_binder *)&f + 1B] = f$1_11; <&0x7f50d2a28bd0> _12 = boost::detail::function::has_empty_target (&f); <&0x7f50d2a16cd0> if (_12 != 0) goto <bb 4>; [46.00%] else goto <bb 3>; [54.00%] <bb 3> [54.00%]: <&0x7f50d2a16d20> MEM[(struct parser_binder *)&D.573835 + 5B] = f$1_11; note that .original shows size_t value = (size_t) &stored_vtable.base; <<cleanup_point size_t value = (size_t) &stored_vtable.base;>>; <<cleanup_point <<< Unknown tree: expr_stmt (void) (value = value | 1) >>>>>; <<cleanup_point <<< Unknown tree: expr_stmt (void) (((struct function4 *) this)->D.542200.vtable = (struct vtable_base *) value) >>>>>; and thus questionable casts. The question ultimatively boils down to validity of the testcase.
w/o SRA the cases look like <bb 97> [48.23%]: <&0x7f3d13ec30f0> f = f; <&0x7f3d13ec3140> MEM[(struct parser_binder *)&D.614964 + 4B] = f; <&0x7f3d13ec3190> value_405 = (size_t) &stored_vtable.base; <&0x7f3d1f70cdc0> value_406 = value_405 | 1; <&0x7f3d13ec31e0> value.90_407 = (struct vtable_base *) value_406; <&0x7f3d13ec3230> MEM[(struct function4 *)&D.614964].D.547000.vtable = value.90_407; <&0x7f3d13ebd9b0> MEM[(struct &)&tmp] ={v} {CLOBBER}; <&0x7f3d13ec3370> tmp.D.547000.vtable = value.90_407; <&0x7f3d13ec3460> tmp.D.547000.functor = MEM[(struct function4 *)&D.614964].D.547000.functor; <&0x7f3d13ec35f0> MEM[(struct function4 *)&D.614964].D.547000.vtable = 0B; <&0x7f3d13f06d70> _135 = MEM[(struct vtable_base * *)this_10(D) + 144B]; <&0x7f3d13f06dc0> if (_135 != 0B) which look similar from TBAA perspective (parser_binder vs. function4). Might not miscompile because of pure luck of course.
With -Wsystem-headers there are various warnings, e.g. /usr/include/boost/function/function_template.hpp:572:11: warning: placement new constructing an object of type ‘boost::spirit::qi::detail::parser_binder<boost::spirit::qi::plus<boost::spirit::qi::difference<boost::spirit::qi::char_class<boost::spirit::tag::char_code<boost::spirit::tag::char_, boost::spirit::char_encoding::standard_wide> >, boost::spirit::qi::literal_char<boost::spirit::char_encoding::standard, true, false> > >, mpl_::bool_<true> >’ and size ‘2’ in a region of type ‘char’ and size ‘1’ [-Wplacement-new=] new (reinterpret_cast<void*>(&functor.data)) FunctionObj(f); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/boost/function/function_base.hpp:308:13: warning: placement new constructing an object of type ‘boost::detail::function::functor_manager_common<boost::spirit::qi::detail::parser_binder<boost::spirit::qi::plus<boost::spirit::qi::difference<boost::spirit::qi::char_class<boost::spirit::tag::char_code<boost::spirit::tag::char_, boost::spirit::char_encoding::standard_wide> >, boost::spirit::qi::literal_char<boost::spirit::char_encoding::standard, true, false> > >, mpl_::bool_<true> > >::functor_type {aka boost::spirit::qi::detail::parser_binder<boost::spirit::qi::plus<boost::spirit::qi::difference<boost::spirit::qi::char_class<boost::spirit::tag::char_code<boost::spirit::tag::char_, boost::spirit::char_encoding::standard_wide> >, boost::spirit::qi::literal_char<boost::spirit::char_encoding::standard, true, false> > >, mpl_::bool_<true> >}’ and size ‘2’ in a region of type ‘char’ and size ‘1’ [-Wplacement-new=] new (reinterpret_cast<void*>(&out_buffer.data)) functor_type(*in_functor); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ etc.
But if I change: - mutable char data; + mutable char data[sizeof (void *) + 2 * sizeof (bool)]; so that the union field occupies the size of some other union members, the warning is gone, but I still see: add r5, sp, #292 ... ldm r5, {r0, r1, r2} ... strb r6, [sp, #293]
(In reply to Richard Biener from comment #14) > Seems to be > > void move_assign(function10& f) > { > if (&f == this) > return; > > { try { > if (!f.empty()) { > this->vtable = f.vtable; > if (this->has_trivial_copy_and_destroy()) > this->functor = f.functor; > ^^^ Indeed. > for the aggregate copy but lineno info looks confused for the aliasing store. > It points at > > operator=(Functor f) > { > self_type(f).swap(*this); > return *this; > } The inline location of the aliasing store is: In function ‘bool boost::detail::function::basic_vtable4<R, T0, T1, T2, T3>::assign_to(FunctionObj, boost::detail::function::function_buffer&, boost::detail::function::function_obj_tag) const [with FunctionObj = boost::spirit::qi::detail::parser_binder<boost::spirit::qi::plus<boost::spirit::qi::difference<boost::spirit::qi::char_class<boost::spirit::tag::char_code<boost::spirit::tag::char_, boost::spirit::char_encoding::standard_wide> >, boost::spirit::qi::literal_char<boost::spirit::char_encoding::standard, true, false> > >, mpl_::bool_<true> >; R = bool; T0 = __gnu_cxx::__normal_iterator<const char*, std::__cxx11::basic_string<char> >&; T1 = const __gnu_cxx::__normal_iterator<const char*, std::__cxx11::basic_string<char> >&; T2 = boost::spirit::context<boost::fusion::cons<std::__cxx11::basic_string<char>&, boost::fusion::nil_>, boost::fusion::vector<> >&; T3 = const boost::spirit::qi::char_class<boost::spirit::tag::char_code<boost::spirit::tag::space, boost::spirit::char_encoding::standard_wide> >&]’, inlined from ‘void boost::function4<R, T1, T2, T3, T4>::assign_to(Functor) [with Functor = boost::spirit::qi::detail::parser_binder<boost::spirit::qi::plus<boost::spirit::qi::difference<boost::spirit::qi::char_class<boost::spirit::tag::char_code<boost::spirit::tag::char_, boost::spirit::char_encoding::standard_wide> >, boost::spirit::qi::literal_char<boost::spirit::char_encoding::standard, true, false> > >, mpl_::bool_<true> >; R = bool; T0 = __gnu_cxx::__normal_iterator<const char*, std::__cxx11::basic_string<char> >&; T1 = const __gnu_cxx::__normal_iterator<const char*, std::__cxx11::basic_string<char> >&; T2 = boost::spirit::context<boost::fusion::cons<std::__cxx11::basic_string<char>&, boost::fusion::nil_>, boost::fusion::vector<> >&; T3 = const boost::spirit::qi::char_class<boost::spirit::tag::char_code<boost::spirit::tag::space, boost::spirit::char_encoding::standard_wide> >&]’ at /usr/include/boost/function/function_template.hpp:498:45, inlined from ‘boost::function4<R, T1, T2, T3, T4>::function4(Functor, typename boost::enable_if_c<(! boost::is_integral<Functor>::value), int>::type) [with Functor = boost::spirit::qi::detail::parser_binder<boost::spirit::qi::plus<boost::spirit::qi::difference<boost::spirit::qi::char_class<boost::spirit::tag::char_code<boost::spirit::tag::char_, boost::spirit::char_encoding::standard_wide> >, boost::spirit::qi::literal_char<boost::spirit::char_encoding::standard, true, false> > >, mpl_::bool_<true> >; R = bool; T0 = __gnu_cxx::__normal_iterator<const char*, std::__cxx11::basic_string<char> >&; T1 = const __gnu_cxx::__normal_iterator<const char*, std::__cxx11::basic_string<char> >&; T2 = boost::spirit::context<boost::fusion::cons<std::__cxx11::basic_string<char>&, boost::fusion::nil_>, boost::fusion::vector<> >&; T3 = const boost::spirit::qi::char_class<boost::spirit::tag::char_code<boost::spirit::tag::space, boost::spirit::char_encoding::standard_wide> >&]’ at /usr/include/boost/function/function_template.hpp:727:7, inlined from ‘boost::function<R(T0, T1, T2, T3)>::function(Functor, typename boost::enable_if_c<(! boost::is_integral<Functor>::value), int>::type) [with Functor = boost::spirit::qi::detail::parser_binder<boost::spirit::qi::plus<boost::spirit::qi::difference<boost::spirit::qi::char_class<boost::spirit::tag::char_code<boost::spirit::tag::char_, boost::spirit::char_encoding::standard_wide> >, boost::spirit::qi::literal_char<boost::spirit::char_encoding::standard, true, false> > >, mpl_::bool_<true> >; R = bool; T0 = __gnu_cxx::__normal_iterator<const char*, std::__cxx11::basic_string<char> >&; T1 = const __gnu_cxx::__normal_iterator<const char*, std::__cxx11::basic_string<char> >&; T2 = boost::spirit::context<boost::fusion::cons<std::__cxx11::basic_string<char>&, boost::fusion::nil_>, boost::fusion::vector<> >&; T3 = const boost::spirit::qi::char_class<boost::spirit::tag::char_code<boost::spirit::tag::space, boost::spirit::char_encoding::standard_wide> >&]’ at /usr/include/boost/function/function_template.hpp:1073:16, inlined from ‘boost::spirit::qi::rule<Iterator, T1, T2, T3, T4>& boost::spirit::qi::operator%=(boost::spirit::qi::rule<Iterator, T1, T2, T3, T4>&, Expr&&) [with Expr = const boost::proto::exprns_::expr<boost::proto::tagns_::tag::unary_plus, boost::proto::argsns_::list1<const boost::proto::exprns_::expr<boost::proto::tagns_::tag::minus, boost::proto::argsns_::list2<boost::spirit::terminal<boost::spirit::tag::char_code<boost::spirit::tag::char_, boost::spirit::char_encoding::standard_wide> >&, boost::proto::exprns_::expr<boost::proto::tagns_::tag::terminal, boost::proto::argsns_::term<const char&>, 0> >, 2>&>, 1>; Iterator = __gnu_cxx::__normal_iterator<const char*, std::__cxx11::basic_string<char> >; T1 = std::__cxx11::basic_string<char>(); T2 = boost::proto::exprns_::expr<boost::proto::tagns_::tag::terminal, boost::proto::argsns_::term<boost::spirit::tag::char_code<boost::spirit::tag::space, boost::spirit::char_encoding::standard_wide> >, 0>; T3 = boost::spirit::unused_type; T4 = boost::spirit::unused_type]’ at /usr/include/boost/function/function_template.hpp:1126:5, inlined from ‘path_expression_grammar<Iterator>::path_expression_grammar() [with Iterator = __gnu_cxx::__normal_iterator<const char*, std::__cxx11::basic_string<char> >]’ at /tmp/rh1422456.C:36:5: That is: template<typename F> bool assign_to(F f, function_buffer& functor) const { typedef typename get_function_tag<F>::type tag; return assign_to(f, functor, tag()); } and template<typename Functor> function4(Functor f ,typename boost::enable_if_c< !(is_integral<Functor>::value), int>::type = 0 ) : function_base() { this->assign_to(f); } and template<typename Functor> function(Functor f ,typename boost::enable_if_c< !(is_integral<Functor>::value), int>::type = 0 ) : base_type(f) { } and operator=(Functor f) { self_type(f).swap(*this); return *this; } and attr %= +(char_ - ']'); but there is no location for the innermost inlined frame :(.
The MEM[(struct parser_binder *)&D.614964 + 4B] = f;'s ultimate origin is: if (D.589607 != 0B) { iftmp.77 = try { MEM[(struct parser_binder *)D.589607] = f; ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This stmt } catch { operator delete (D.589607, NON_LVALUE_EXPR <D.589606>); }, (struct parser_binder *) D.589607;; } else { iftmp.77 = (struct parser_binder *) D.589607; } from: template<typename FunctionObj> void assign_functor(FunctionObj f, function_buffer& functor, mpl::true_) const { new (reinterpret_cast<void*>(&functor.data)) FunctionObj(f); } which looks like a placement new into that mutable char data; field of boost::detail::function::function_buffer, and then the whole union is copied including the placement new created data in there. No idea if that is valid C++.
I believe this all boils down to: inline void* operator new(__SIZE_TYPE__, void *p) { return p; } struct A { A (float x) : f (x) {} float f; }; struct B { int x; union U { int a; char b[sizeof (float)]; } u; int y; }; __attribute__((noinline, noclone)) void bar (B &x, B &y) { if (x.x != 0 || x.y != 3 || y.x != 0 || y.y != 3) __builtin_abort (); float f; __builtin_memcpy (&f, x.u.b, sizeof (float)); if (f != 3.5f) __builtin_abort (); __builtin_memcpy (&f, y.u.b, sizeof (float)); if (f != 3.5f) __builtin_abort (); } __attribute__((noinline, noclone)) B * baz (B &x) { return &x; } __attribute__((noinline, noclone)) void foo (float x) { B b { 0, {}, 3 }, c; B *p = baz (b); new (b.u.b) A (x); c = *p; bar (*p, c); } int main () { foo (3.5f); } which fails also on x86_64-linux at -O2. And that testcase regressed with r223126. Now whether this is valid C++, no idea, placement new is messy.
On February 27, 2017 5:46:44 PM GMT+01:00, "jakub at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org> wrote: >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > >Jakub Jelinek <jakub at gcc dot gnu.org> changed: > > What |Removed |Added >---------------------------------------------------------------------------- > CC| |redi at gcc dot gnu.org > >--- Comment #20 from Jakub Jelinek <jakub at gcc dot gnu.org> --- >I believe this all boils down to: >inline void* operator new(__SIZE_TYPE__, void *p) { return p; } >struct A { A (float x) : f (x) {} float f; }; >struct B >{ > int x; > union U > { > int a; > char b[sizeof (float)]; > } u; > int y; >}; > >__attribute__((noinline, noclone)) void >bar (B &x, B &y) >{ > if (x.x != 0 || x.y != 3 || y.x != 0 || y.y != 3) > __builtin_abort (); > float f; > __builtin_memcpy (&f, x.u.b, sizeof (float)); > if (f != 3.5f) > __builtin_abort (); > __builtin_memcpy (&f, y.u.b, sizeof (float)); > if (f != 3.5f) > __builtin_abort (); >} > >__attribute__((noinline, noclone)) >B * >baz (B &x) >{ > return &x; >} > >__attribute__((noinline, noclone)) void >foo (float x) >{ > B b { 0, {}, 3 }, c; > B *p = baz (b); > new (b.u.b) A (x); > c = *p; > bar (*p, c); >} > >int >main () >{ > foo (3.5f); >} > >which fails also on x86_64-linux at -O2. And that testcase regressed >with >r223126. Now whether this is valid C++, no idea, placement new is >messy. We have one or two duplicates that were resolved invalid. We can't reasonably support this in the ME anyway and the lvalue used to access this is not one of those listed compatible with A.
(In reply to Jakub Jelinek from comment #20) > which fails also on x86_64-linux at -O2. And that testcase regressed with > r223126. Now whether this is valid C++, no idea, placement new is messy. This test case can't be valid, suppose the A has a copy constructor that that is also not called when B is moved around.
(In reply to Bernd Edlinger from comment #22) > (In reply to Jakub Jelinek from comment #20) > > which fails also on x86_64-linux at -O2. And that testcase regressed with > > r223126. Now whether this is valid C++, no idea, placement new is messy. > > This test case can't be valid, suppose the A has a copy constructor > that that is also not called when B is moved around. The canonical fix is to put the type you placement new into the union storage into the union as regular member rather than having a char[] member in the union.
(In reply to Richard Biener from comment #23) > (In reply to Bernd Edlinger from comment #22) > > (In reply to Jakub Jelinek from comment #20) > > > which fails also on x86_64-linux at -O2. And that testcase regressed with > > > r223126. Now whether this is valid C++, no idea, placement new is messy. > > > > This test case can't be valid, suppose the A has a copy constructor > > that that is also not called when B is moved around. > > The canonical fix is to put the type you placement new into the union storage > into the union as regular member rather than having a char[] member in the > union. Yes. Of course you cannot put a non-POD type in a union, but maybe a pointer to A, that is probably what boost should do in their functor class.
On Tue, 28 Feb 2017, bernd.edlinger at hotmail dot de wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > > --- Comment #24 from Bernd Edlinger <bernd.edlinger at hotmail dot de> --- > (In reply to Richard Biener from comment #23) > > (In reply to Bernd Edlinger from comment #22) > > > (In reply to Jakub Jelinek from comment #20) > > > > which fails also on x86_64-linux at -O2. And that testcase regressed with > > > > r223126. Now whether this is valid C++, no idea, placement new is messy. > > > > > > This test case can't be valid, suppose the A has a copy constructor > > > that that is also not called when B is moved around. > > > > The canonical fix is to put the type you placement new into the union storage > > into the union as regular member rather than having a char[] member in the > > union. > > Yes. Of course you cannot put a non-POD type in a union, > but maybe a pointer to A, that is probably what boost should > do in their functor class. You can put a non-POD into a union but then you need to provide copy/move/etc. constructors as they are otherwise default deleted.
What about PODs or say int? Let's say I have a class with union in it and in that union say float and char array, then use placement new an int with some value into the char array in the union. What will then happen if I default copy the class (default assignment operator or copy constructor)? The following doesn't fail, but I think true_dependence still returns 0 for (mem/c:SI (plus:DI (reg/f:DI 7 sp) (const_int 4 [0x4])) [4 MEM[(int *)&b + 4B]+0 S4 A32]) and (mem:DI (reg/v/f:DI 0 ax [orig:87 p ] [87]) [1 MEM[(const struct B &)p_4]+0 S8 A32]) where the former is from the placement new store of int and the load is from the structure assignment. inline void* operator new(__SIZE_TYPE__, void *p) { return p; } struct B { float x; union U { float a; char b[sizeof (int)]; } u; float y; }; __attribute__((noinline, noclone)) void bar (B &x, B &y) { if (x.x != 0 || x.y != 3 || y.x != 0 || y.y != 3) __builtin_abort (); int f; __builtin_memcpy (&f, x.u.b, sizeof (int)); if (f != 81) __builtin_abort (); __builtin_memcpy (&f, y.u.b, sizeof (int)); if (f != 81) __builtin_abort (); } __attribute__((noinline, noclone)) B * baz (B &x) { return &x; } __attribute__((noinline, noclone)) void foo (int x) { B b { 0.0f, {}, 3.0f }, c; B *p = baz (b); new (b.u.b) int (x); c = *p; bar (*p, c); } int main () { foo (81); }
On Tue, 28 Feb 2017, jakub at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > > --- Comment #26 from Jakub Jelinek <jakub at gcc dot gnu.org> --- > What about PODs or say int? Let's say I have a class with union in it and in > that union say float and char array, then use placement new an int with some > value into the char array in the union. What will then happen if I default > copy the class (default assignment operator or copy constructor)? The > following doesn't fail, but I think true_dependence still returns 0 for > (mem/c:SI (plus:DI (reg/f:DI 7 sp) > (const_int 4 [0x4])) [4 MEM[(int *)&b + 4B]+0 S4 A32]) > and > (mem:DI (reg/v/f:DI 0 ax [orig:87 p ] [87]) [1 MEM[(const struct B &)p_4]+0 S8 > A32]) > where the former is from the placement new store of int and the load is from > the structure assignment. > > inline void* operator new(__SIZE_TYPE__, void *p) { return p; } > struct B > { > float x; > union U > { > float a; > char b[sizeof (int)]; > } u; > float y; > }; > > __attribute__((noinline, noclone)) void > bar (B &x, B &y) > { > if (x.x != 0 || x.y != 3 || y.x != 0 || y.y != 3) > __builtin_abort (); > int f; > __builtin_memcpy (&f, x.u.b, sizeof (int)); > if (f != 81) > __builtin_abort (); > __builtin_memcpy (&f, y.u.b, sizeof (int)); > if (f != 81) > __builtin_abort (); > } > > __attribute__((noinline, noclone)) > B * > baz (B &x) > { > return &x; > } > > __attribute__((noinline, noclone)) void > foo (int x) > { > B b { 0.0f, {}, 3.0f }, c; > B *p = baz (b); > new (b.u.b) int (x); > c = *p; I don't think *p is of any type allowed to access the object of type int.
To quote the only eventually relevant part(s) of 3.10/10: - an aggregate or union type that includes one of the aforementioned types among its elements or non-static data members (...) ... - a char or unsigned char type note the order. For union U { char c[4]; float f; } the stored value of type 'int' is not accessed through any of those when accessing it as U.
Jonathan said he'll file upstream boost bugreport about this. So closing.
The code in comment 20 still fails even with attribute((may_alias)), so I'm not sure how the boost code can be fixed: inline void* operator new(__SIZE_TYPE__, void *p) { return p; } struct A { A (float x) : f (x) {} float f; }; static_assert(sizeof(A) == sizeof(float), ""); struct B { int x; union __attribute__((__may_alias__)) U { int a; char b[sizeof (float)]; } u; int y; }; __attribute__((noinline, noclone)) void bar (B &x, B &y) { if (x.x != 0) __builtin_abort (); if (x.y != 3) __builtin_abort (); if (y.x != 0) __builtin_abort (); if (y.y != 3) __builtin_abort (); float f; __builtin_memcpy (&f, x.u.b, sizeof (float)); if (f != 3.5f) __builtin_abort (); __builtin_memcpy (&f, y.u.b, sizeof (float)); if (f != 3.5f) __builtin_abort (); } __attribute__((noinline, noclone)) B * baz (B &x) { return &x; } __attribute__((noinline, noclone)) void foo (float x) { B b { 0, {}, 3 }, c; B *p = baz (b); new (b.u.b) A (x); c = *p; bar (*p, c); } int main () { foo (3.5f); }
Thus reopening.
On Tue, 7 Mar 2017, redi at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > > Jonathan Wakely <redi at gcc dot gnu.org> changed: > > What |Removed |Added > ---------------------------------------------------------------------------- > Resolution|INVALID |FIXED > > --- Comment #30 from Jonathan Wakely <redi at gcc dot gnu.org> --- > The code in comment 20 still fails even with attribute((may_alias)), so I'm not > sure how the boost code can be fixed: > > inline void* operator new(__SIZE_TYPE__, void *p) { return p; } > struct A { A (float x) : f (x) {} float f; }; > static_assert(sizeof(A) == sizeof(float), ""); > struct B > { > int x; > union __attribute__((__may_alias__)) U > { > int a; > char b[sizeof (float)]; > } u; > int y; > }; > > __attribute__((noinline, noclone)) void > bar (B &x, B &y) > { > if (x.x != 0) > __builtin_abort (); > if (x.y != 3) > __builtin_abort (); > if (y.x != 0) > __builtin_abort (); > if (y.y != 3) > __builtin_abort (); > float f; > __builtin_memcpy (&f, x.u.b, sizeof (float)); > if (f != 3.5f) > __builtin_abort (); > __builtin_memcpy (&f, y.u.b, sizeof (float)); > if (f != 3.5f) > __builtin_abort (); > } > > __attribute__((noinline, noclone)) > B * > baz (B &x) > { > return &x; > } > > __attribute__((noinline, noclone)) void > foo (float x) > { > B b { 0, {}, 3 }, c; > B *p = baz (b); > new (b.u.b) A (x); The issue is that while operator new takes a ref-all pointer here it just returns void * which is then passed as this to A::A and thus construction happens via a non-ref-all pointer. That is, C++ abstraction comes in the way of may_alias (basically casting that away again). So you need to place may-alias at a point to make the following stmt safe: > c = *p; which means placing it on B, not only on the union (p is a B). Thus do struct B { int x; union U { int a; char b[sizeof (float)]; } u; int y; } __attribute__((may_alias));
From IRC discussion today about this: <jakub> so, for PR79671 would you be ok with say new may_alias_field attribute allowed on FIELD_DECLs only that forces has_zero_child on the containing type? <jakub> or may_alias_fields type attribute that would force has_zero_child on that type <jakub> I have no idea what else boost/libstdc++ could use for the mess they want/need to do <richi> didn't I provide a solution for them? <richi> IMHO the bug is still INVALID <richi> not sure if adding has_zero_child will have any effect - the same issue exists for B with char[] array member you placement new into <jakub> the problem with that solution is that you need to move it to the outermost type, say if somebody puts that weirdo boost class into some other class, then assignments will not work again <richi> iff the FE decides that copying PODs is semantically equivalent to memcpy (alias-wise) then it has to emit it that way (but as said in comment #28 there is no evidence the standard suggests that) <richi> yes, because C++ doesnt' work that way <richi> it's very simple... <richi> and I bet adding ->has_zero_child = 1 to B doesn't actually help in general <jakub> my understanding was that jason disagrees with that <richi> I see no comment in the bug from jason <jakub> let's discuss it later today when jason/jwakely are both around <richi> the C++ FE can simply use alias-set zero for POD assignments then <richi> thus change c = *p; to MEM[&c, alias-set-zero] = MEM[p, alias-set-zero]; <richi> the important thing to notice is that it is 'c = *p' that is undefined
C++14 12.8/16 says "The implicitly-defined copy/move constructor for a union X copies the object representation (3.9) of X." 3.9/4 says "The object representation of an object of type T is the sequence of N unsigned char objects... For trivially copyable types, the value representation is a set of bits in the object representation that determines a value,..." this suggests that the copying should work but the C++ FE may not simply elide the copy construction by emitting c = *p; because that does _not_ implement memcpy semantics for the union member. Note the above may not apply at all here if B is POD and thus the assignment is an assignment of PODs (I don't know all of the standard).
(In reply to Richard Biener from comment #34) > C++14 12.8/16 says > > "The implicitly-defined copy/move constructor for a union X copies the > object representation (3.9) of X." > > 3.9/4 says > > "The object representation of an object of type T is the sequence of N > unsigned char objects... For trivially copyable types, the value > representation is a set of bits in the object representation that determines > a value,..." > > this suggests that the copying should work but the C++ FE may not simply > elide the copy construction by emitting > > c = *p; > > because that does _not_ implement memcpy semantics for the union member. > > Note the above may not apply at all here if B is POD and thus the assignment > is an assignment of PODs (I don't know all of the standard). It would still conflict with the wording of 3.10/10 (that is, if 3.10/10 is fulfilled the middle-end handles the copying above correct).
(In reply to Richard Biener from comment #34) > C++14 12.8/16 says > > "The implicitly-defined copy/move constructor for a union X copies the > object representation (3.9) of X." > > 3.9/4 says > > "The object representation of an object of type T is the sequence of N > unsigned char objects... For trivially copyable types, the value > representation is a set of bits in the object representation that determines > a value,..." > > this suggests that the copying should work but the C++ FE may not simply > elide the copy construction by emitting > > c = *p; > > because that does _not_ implement memcpy semantics for the union member. Nothing in the standard makes memcpy magic; 3.9/3 says, For any trivially copyable type T, if two pointers to T point to distinct T objects obj1 and obj2, where neither obj1 nor obj2 is a base-class subobject, if the underlying bytes (4.4) making up obj1 are copied into obj2, obj2 shall subsequently hold the same value as obj1. It just talks about copying the underlying bytes, not necessarily using memcpy to do so. And a copy of a B is a memberwise copy, which implies copying the U member, which as you quote means copying the object representation, which is the underlying bytes.
On Fri, 24 Mar 2017, jason at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > > --- Comment #36 from Jason Merrill <jason at gcc dot gnu.org> --- > (In reply to Richard Biener from comment #34) > > C++14 12.8/16 says > > > > "The implicitly-defined copy/move constructor for a union X copies the > > object representation (3.9) of X." > > > > 3.9/4 says > > > > "The object representation of an object of type T is the sequence of N > > unsigned char objects... For trivially copyable types, the value > > representation is a set of bits in the object representation that determines > > a value,..." > > > > this suggests that the copying should work but the C++ FE may not simply > > elide the copy construction by emitting > > > > c = *p; > > > > because that does _not_ implement memcpy semantics for the union member. > > Nothing in the standard makes memcpy magic; 3.9/3 says, > > For any trivially copyable type T, if two pointers to T point to distinct T > objects obj1 and obj2, where neither obj1 nor obj2 is a base-class subobject, > if the underlying bytes (4.4) making up obj1 are copied into obj2, obj2 shall > subsequently hold the same value as obj1. > > It just talks about copying the underlying bytes, not necessarily using memcpy > to do so. And a copy of a B is a memberwise copy, which implies copying the U > member, which as you quote means copying the object representation, which is > the underlying bytes. Yes. But then the act of copying, the c = *p; stmt needs to still follow 3.10/10, *p is not a valid glvalue to access the object live in p->u.b which is of type A.
Simplified testcase for discussion (is not "miscompiled"): struct S { union { int i; } u; }; int main() { S s; new (&s.u.i) float (2.0); S q = s; if (*reinterpret_cast<float *>(&q.u.i) != 2.0) abort (); } so you say q = s is valid because the object representation of the union is copied. I say after storing 2.0 to s.u.i the access 's' in q = s is invalid as you are accessing the stored value (a float) via a glvalue of inappropriate type.
one thing I do not understand is this: if I do this in Jakub's test case: new (b.u.b) A (x); //c = *p; __builtin_memcpy(&c, p, sizeof(B)); bar (*p, c); the mis-compilation goes away, if I add this to Jakub's test case: struct B { int x; union U { int a; char b[sizeof (float)]; } u; int y; B() {} B(int xx, U uu, int yy):x(xx), u(uu), y(yy) {} B(B &o) { __asm("":::"memory"); __builtin_memcpy(this, &o, sizeof(B)); __asm("":::"memory"); } }; The mis-compilation gets not fixed. Why?
Yikes, I need an assignement operator: B(const B &o) { __builtin_memcpy(this, &o, sizeof(B)); } B& operator = (const B &o) { __builtin_memcpy(this, &o, sizeof(B)); return *this; } now it seems to work...
(In reply to Richard Biener from comment #38) > Simplified testcase for discussion (is not "miscompiled"): > > struct S { > union { int i; } u; > }; > > int main() > { > S s; > new (&s.u.i) float (2.0); > S q = s; > if (*reinterpret_cast<float *>(&q.u.i) != 2.0) > abort (); > } > > so you say q = s is valid because the object representation of the union > is copied. I say after storing 2.0 to s.u.i the access 's' in q = s is > invalid as you are accessing the stored value (a float) via a glvalue of > inappropriate type. I say what's wrong with this testcase is that storing a float in an int field is undefined.
(In reply to Richard Biener from comment #14) > Seems to be > > void move_assign(function10& f) > { > if (&f == this) > return; > > { try { > if (!f.empty()) { > this->vtable = f.vtable; > if (this->has_trivial_copy_and_destroy()) > this->functor = f.functor; > ^^^ > > for the aggregate copy but lineno info looks confused for the aliasing store. Changing this aggregate copy to: __builtin_memcpy(&this->functor, &f.functor, sizeof(f.functor)); fixes the crash on armv7hl. No amount of may_alias attributes on the definition of the union detail::function::function_buffer type helped.
Would something like this also work? union function_buffer { [...] mutable char data; function_buffer() {} function_buffer(const function_buffer &other) { __builtin_memcpy(this, &other, sizeof(*this)); } function_buffer& operator = (const function_buffer &other) { __builtin_memcpy(this, &other, sizeof(*this)); return *this; } };
Created attachment 41048 [details] trial patch Does this fix the issue? I don't have an ARM setup handy for testing.
(In reply to rguenther@suse.de from comment #32) > So you need to place may-alias at a point to make the following > stmt safe: > > > c = *p; > > which means placing it on B, not only on the union (p is a B). > > Thus do > > struct B > { > int x; > union U > { > int a; > char b[sizeof (float)]; > } u; > int y; > } __attribute__((may_alias)); In the real code there is no B, there is just the union, and it is assigned directly, so it's more like: union function_buffer_members { void* p; void(*fp)(); }; union function_buffer { function_buffer_members members; char data[sizeof(function_buffer_members)]; }; struct function_base { mutable function_buffer functor; }; struct function : function_base { void func(const function& f) { this->functor = f.functor; } }; So it should only be necessary to put __attribute__((may_alias)) on union function_buffer, right? That doesn't fix the problem though.
(In reply to Jonathan Wakely from comment #45) > (In reply to rguenther@suse.de from comment #32) > > So you need to place may-alias at a point to make the following > > stmt safe: > > > > > c = *p; > > > > which means placing it on B, not only on the union (p is a B). > > > > Thus do > > > > struct B > > { > > int x; > > union U > > { > > int a; > > char b[sizeof (float)]; > > } u; > > int y; > > } __attribute__((may_alias)); > > In the real code there is no B, there is just the union, and it is assigned > directly, so it's more like: > > union function_buffer_members { > void* p; > void(*fp)(); > }; > > union function_buffer { > function_buffer_members members; > char data[sizeof(function_buffer_members)]; > }; > > struct function_base { > mutable function_buffer functor; > }; > > struct function : function_base { > void func(const function& f) { > this->functor = f.functor; > } > }; > > So it should only be necessary to put __attribute__((may_alias)) on union > function_buffer, right? That doesn't fix the problem though. Yes, it seems, the __attribute__((may_alias)) does not propagate from structure members to enclosing structure: If B has the may_alias, but it is a member of C then the test case fails again: inline void* operator new(__SIZE_TYPE__, void *p) { return p; } struct A { A (float x) : f (x) {} float f; }; struct B { int x; union U { int a; char b[sizeof (float)]; } u; int y; } __attribute__((may_alias)); struct C { struct B b; }; __attribute__((noinline, noclone)) void bar (B &x, B &y) { if (x.x != 0 || x.y != 3 || y.x != 0 || y.y != 3) __builtin_abort (); float f; __builtin_memcpy (&f, x.u.b, sizeof (float)); if (f != 3.5f) __builtin_abort (); __builtin_memcpy (&f, y.u.b, sizeof (float)); if (f != 3.5f) __builtin_abort (); } __attribute__((noinline, noclone)) C * baz (C &x) { return &x; } __attribute__((noinline, noclone)) void foo (float x) { C b { 0, {}, 3 }, c; C *p = baz (b); new (b.b.u.b) A (x); c.b = p->b; bar (p->b, c.b); } int main () { foo (3.5f); }
(In reply to Bernd Edlinger from comment #46) > Yes, it seems, the __attribute__((may_alias)) does not propagate from > structure members to enclosing structure: What enclosing structure? You're apparently agreeing with me, but saying something different. In the real code, and the example above, _there_ _is_ _no_ _enclosing_ _structure_. I understand that it doesn't propagate to enclosing structures, that's fine. But if a type X has the attribute then it doesn't need to propagate anywhere, it should affect X. But it doesn't fix the original bug that started all this.
(In reply to Jonathan Wakely from comment #47) > (In reply to Bernd Edlinger from comment #46) > > Yes, it seems, the __attribute__((may_alias)) does not propagate from > > structure members to enclosing structure: > > What enclosing structure? You're apparently agreeing with me, but saying > something different. In the real code, and the example above, _there_ _is_ > _no_ _enclosing_ _structure_. I understand that it doesn't propagate to > enclosing structures, that's fine. But if a type X has the attribute then it > doesn't need to propagate anywhere, it should affect X. But it doesn't fix > the original bug that started all this. I think I agree with you, that is surprising that the may_alias does not do what we need. I think the enclosing structure is: class function_base { public: function_base() : vtable(0) { } [...] public: detail::function::vtable_base* get_vtable() const { return reinterpret_cast<detail::function::vtable_base*>( reinterpret_cast<std::size_t>(vtable) & ~static_cast<std::size_t>(0x01)); } bool has_trivial_copy_and_destroy() const { return reinterpret_cast<std::size_t>(vtable) & 0x01; } detail::function::vtable_base* vtable; mutable detail::function::function_buffer functor; }; well, in fact even further down the class hierarchy. the offending statement is: this->functor = f.functor; I think, if a functor may in general alias to anything, it is possible that the outer object does not alias. Does that make sense?
While I think that adding explicit copy/assignment constructors as in comment #43 should work and looks like the most portable solution for boost, I am unsure if may_alias shouldn't really work different. I thought of another use of may_alias that also applies to C: Isn't the plan to add may_alias to the struct sockaddr_storage ? And wouldn't you then expect to add a sockaddr_storage to a structure like: struct A { struct sockaddr_storage s; }; does'nt that mean that A a, b; a.s = b.s; will also ignore the may_alias ?
(In reply to Jason Merrill from comment #44) > Created attachment 41048 [details] > trial patch > > Does this fix the issue? I don't have an ARM setup handy for testing. I should. Note that what changed with GCC 7 is only that unions with char members no longer behave as alias-set zero but 12.8/16 talks about all unions, not just unions with char members. Now I read comment#14 as that _only_ char[] members (of structs or unions) may ever "contain" different dynamic types. Any pointer to a part of the standard that singles out char[] that way? Note your proposed patch will weaken TBAA for all structs/unions with a char (array) member for the task of aggregate copying. Might not be too bad as open-coding memberwise copy would expose alias-set zero writes as well. The change that exposed char[] union handling differences is r223126 | rguenth | 2015-05-13 12:53:42 +0200 (Wed, 13 May 2015) | 10 lines 2015-05-13 Richard Biener <rguenther@suse.de> PR middle-end/66110 * alias.c (alias_sets_conflict_p): Do not treat has_zero_child specially. which is to avoid having aggregates with char members aliasing with everything, instead they only alias with alias subsets (including explicit zero). So if you take the original reduced testcase in this PR and exchange the char[] array inside the union with a int[] array then the "bug" should trigger with older GCC as well. That is, GCC never implemented 12.8/16.
Doesn't 3.10/10 explicitly say that it is undefined to use a union to to move an object representation that is not a member of the union? "If a program attempts to access the stored value of an object through a glvalue of other than one of the following types the behavior is undefined:52 — the dynamic type of the object, — a cv-qualified version of the dynamic type of the object, — a type similar (as defined in 4.4) to the dynamic type of the object, — a type that is the signed or unsigned type corresponding to the dynamic type of the object, — a type that is the signed or unsigned type corresponding to a cv-qualified version of the dynamic type of the object, — an aggregate or union type that includes one of the aforementioned types among its elements or non- static data members (including, recursively, an element or non-static data member of a subaggregate or contained union), — a type that is a (possibly cv-qualified) base class type of the dynamic type of the object, — a char or unsigned char type."
On Mon, 27 Mar 2017, bernd.edlinger at hotmail dot de wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > > --- Comment #51 from Bernd Edlinger <bernd.edlinger at hotmail dot de> --- > Doesn't 3.10/10 explicitly say that it is undefined to use a union to > to move an object representation that is not a member of the union? That was my reading... but 3.10/10 talks about "attempts to access the stored value of an object" and Jason says that this doesn't apply to d = *p but the result of the decomposition to memberwise copy plus union special handling (where it wouldn't apply at all) So the question (DR?) is whether 3.10/10 applies to the access as written in the source or to it after the decomposition happened. > "If a program attempts to access the stored value of an object through a > glvalue of other than one of the > following types the behavior is undefined:52 > — the dynamic type of the object, > — a cv-qualified version of the dynamic type of the object, > — a type similar (as defined in 4.4) to the dynamic type of the object, > — a type that is the signed or unsigned type corresponding to the dynamic type > of the object, > — a type that is the signed or unsigned type corresponding to a cv-qualified > version of the dynamic type > of the object, > — an aggregate or union type that includes one of the aforementioned types > among its elements or non- > static data members (including, recursively, an element or non-static data > member of a subaggregate > or contained union), > — a type that is a (possibly cv-qualified) base class type of the dynamic type > of the object, > — a char or unsigned char type."
(In reply to rguenther@suse.de from comment #52) > On Mon, 27 Mar 2017, bernd.edlinger at hotmail dot de wrote: > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > > > > --- Comment #51 from Bernd Edlinger <bernd.edlinger at hotmail dot de> --- > > Doesn't 3.10/10 explicitly say that it is undefined to use a union to > > to move an object representation that is not a member of the union? > > That was my reading... but 3.10/10 talks about "attempts to access > the stored value of an object" and Jason says that this doesn't apply > to d = *p but the result of the decomposition to memberwise copy > plus union special handling (where it wouldn't apply at all) > The boost code was: "this->functor = f.functor;" thus directly accessing the union, so there was no decomposition to memberwise copy, right?
On Mon, 27 Mar 2017, bernd.edlinger at hotmail dot de wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > > --- Comment #53 from Bernd Edlinger <bernd.edlinger at hotmail dot de> --- > (In reply to rguenther@suse.de from comment #52) > > On Mon, 27 Mar 2017, bernd.edlinger at hotmail dot de wrote: > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > > > > > > --- Comment #51 from Bernd Edlinger <bernd.edlinger at hotmail dot de> --- > > > Doesn't 3.10/10 explicitly say that it is undefined to use a union to > > > to move an object representation that is not a member of the union? > > > > That was my reading... but 3.10/10 talks about "attempts to access > > the stored value of an object" and Jason says that this doesn't apply > > to d = *p but the result of the decomposition to memberwise copy > > plus union special handling (where it wouldn't apply at all) > > > > The boost code was: "this->functor = f.functor;" > thus directly accessing the union, so there was no decomposition > to memberwise copy, right? Right. But still the other clause says the storage representation is transfered and so you could read into that that no "access" happens and thus 3.10/10 doesn't apply.
(In reply to Bernd Edlinger from comment #53) > The boost code was: "this->functor = f.functor;" And I still don't understand why may_alias doesn't help on the type of this->functor.
I think may_alias attribute only affects pointers/references (pointer/reference to may_alias type is considered to reference any type), but in this->functor = f.functor the only pointer is this and *this likely isn't may_alias, and the bug is about f.functor access anyway.
OK, so this appears to work (assuming may_alias on function_buffer): function_buffer& aliasing_ref = f.functor; this->functor = aliasing_ref; It changes the GIMPLE from: f1_2(D)->D.2297.functor = MEM[(const struct function *)f2_3(D)].D.2297.functor; to: f1_2(D)->D.2297.functor = MEM[(const union function_buffer & {ref-all})f2_3(D)]; Can I rely on that continuing to work? Is it likely to perform better than just doing memcpy? __builtin_memcpy(&this->functor, &f.functor, sizeof(f.functor));
But this seemingly equivalent code doesn't work: this->functor = static_cast<function_buffer&>(f.functor);
On Mon, 27 Mar 2017, redi at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > > --- Comment #58 from Jonathan Wakely <redi at gcc dot gnu.org> --- > But this seemingly equivalent code doesn't work: > > this->functor = static_cast<function_buffer&>(f.functor); Is function_buffer may_alias? Then it should work unless the FE messes up via folding. struct C { int i; }__attribute__((may_alias)) ; C a, b; void foo() { a = static_cast <C&> (b); } in .original (correct): <<cleanup_point <<< Unknown tree: expr_stmt (void) (a = *(const struct C & {ref-all}) &b) >>>>>; in .gimple (wrong-code): void foo() () { a = b; } caused by gimple_fold_indirect_ref_rhs doing tree gimple_fold_indirect_ref (tree t) { tree ptype = TREE_TYPE (t), type = TREE_TYPE (ptype); tree sub = t; tree subtype; STRIP_NOPS (sub); subtype = TREE_TYPE (sub); if (!POINTER_TYPE_P (subtype)) return NULL_TREE; if (TREE_CODE (sub) == ADDR_EXPR) { tree op = TREE_OPERAND (sub, 0); tree optype = TREE_TYPE (op); /* *&p => p */ if (useless_type_conversion_p (type, optype)) return op; some of the transforms properly preserve ptype semantics but most don't
-> PR80222
(In reply to rguenther@suse.de from comment #59) > On Mon, 27 Mar 2017, redi at gcc dot gnu.org wrote: > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > > > > --- Comment #58 from Jonathan Wakely <redi at gcc dot gnu.org> --- > > But this seemingly equivalent code doesn't work: > > > > this->functor = static_cast<function_buffer&>(f.functor); > > Is function_buffer may_alias? Yes. Here's what I tested: union function_buffer_members { void* p; void(*fp)(); }; union function_buffer { function_buffer_members members; char data[sizeof(function_buffer_members)]; } __attribute__((may_alias)); struct function_base { mutable function_buffer functor; }; struct function : function_base { void func(const function& f) { const function_buffer& aliasing_ref = f.functor; this->functor = aliasing_ref; } }; void blah(function& f1, function& f2) { f1.func(f2); } The -fdump-tree-optimized dump is: ;; Function void blah(function&, function&) (_Z4blahR8functionS0_, funcdef_no=1, decl_uid=2315, cgraph_uid=1, symbol_order=1) void blah(function&, function&) (struct function & f1, struct function & f2) { <bb 2> [100.00%]: # DEBUG this => f1_2(D) # DEBUG f => f2_3(D) # DEBUG D#1 => &MEM[(const struct function *)f2_3(D)].D.2297.functor # DEBUG aliasing_ref => D#1 f1_2(D)->D.2297.functor = MEM[(const union function_buffer & {ref-all})f2_3(D)]; # DEBUG this => NULL # DEBUG f => NULL return; } This has {ref-all}. If I use static_cast: union function_buffer_members { void* p; void(*fp)(); }; union function_buffer { function_buffer_members members; char data[sizeof(function_buffer_members)]; } __attribute__((may_alias)); struct function_base { mutable function_buffer functor; }; struct function : function_base { void func(const function& f) { this->functor = static_cast<const function_buffer&>(f.functor); } }; void blah(function& f1, function& f2) { f1.func(f2); } Then the dump doesn't have {ref-all}: ;; Function void blah(function&, function&) (_Z4blahR8functionS0_, funcdef_no=1, decl_uid=2314, cgraph_uid=1, symbol_order=1) void blah(function&, function&) (struct function & f1, struct function & f2) { <bb 2> [100.00%]: # DEBUG this => f1_2(D) # DEBUG f => f2_3(D) f1_2(D)->D.2297.functor = MEM[(const struct function *)f2_3(D)].D.2297.functor; # DEBUG this => NULL # DEBUG f => NULL return; }
(In reply to Jason Merrill from comment #44) > Created attachment 41048 [details] > trial patch > > Does this fix the issue? I don't have an ARM setup handy for testing. Yes. I just tried, it and the crash is gone. However if 3.10/10 does not apply here, then it would be good to remove the special handling of the char type, and just look for a union and/or maybe a may_alias attribute.
(In reply to rguenther@suse.de from comment #54) > > > > --- Comment #51 from Bernd Edlinger <bernd.edlinger at hotmail dot de> --- > > > > Doesn't 3.10/10 explicitly say that it is undefined to use a union to > > > > to move an object representation that is not a member of the union? > But still the other clause says the storage representation is transfered > and so you could read into that that no "access" happens and thus > 3.10/10 doesn't apply. Right, union copy copies the bytes of the object representation, i.e. memcpy. (In reply to Richard Biener from comment #50) > Note that what changed with GCC 7 is only that unions with char members > no longer behave as alias-set zero but 12.8/16 talks about all unions, > not just unions with char members. > > Now I read comment#14 as that _only_ char[] members (of structs or unions) > may ever "contain" different dynamic types. Any pointer to a part of > the standard that singles out char[] that way? char is special that way because it can be used to access the stored value of an object of any type (3.10/10.8). Now that C++17 introduces std::byte for this sort of thing, I hope to transition away from the permissive aliasing behavior of char.
I would find it extremely surprising if in a = b; the RHS doesn't constitute an access to the value of object 'b' (even depending on the type of b). Are you really saying this Jason? (just trying to make extra sure) (e.g. 5.17/2 is saying about the assignment operator, before any differentiation between class and non-class types: "In simple assignment (=), the value of the expression replaces that of the object referred to by the left operand." How could it talk about the value of the expression if the RHS doesn't constitute an access to the value of that expression? While /4 specifies that the actual assignment is carried out by the copy/move assignment operator and hence via object representation for unions when implicit (12.8/29), we cannot simply ignore the above sentence, can we?)
(In reply to Michael Matz from comment #64) > I would find it extremely surprising if in > > a = b; > > the RHS doesn't constitute an access to the value of object 'b' (even > depending on the type of b). Are you really saying this Jason? (just trying > to make extra > sure) It accesses b, but it doesn't access the object stored in b's char[N] member via placement new.
(In reply to Michael Matz from comment #64) > I would find it extremely surprising if in > > a = b; > > the RHS doesn't constitute an access to the value of object 'b' (even > depending on the type of b). Are you really saying this Jason? (just trying > to make extra sure) Well, there's a kind of access involved in forming a reference to each of the members of b. I'm having trouble finding text pertaining to this; the closest I'm coming up with is in 12.7 [class.cdtor]: To form a pointer to (or access the value of) a direct non-static member of an object obj, the construction of obj shall have started and its destruction shall not have completed, otherwise the computation of the pointer value (or accessing the member value) results in undefined behavior. > (e.g. 5.17/2 is saying about the assignment operator, before any > differentiation between class and non-class types: > "In simple assignment (=), the value of the expression replaces that of the > object referred to by the left operand." > How could it talk about the value of the expression if the RHS doesn't > constitute an access to the value of that expression? While /4 specifies that > the actual assignment is carried out by the copy/move assignment operator > and hence via object representation for unions when implicit (12.8/29), we > cannot simply ignore the above sentence, can we?) The operator semantics described in clause 5 [expr] apply to the built-in operators, not any overloaded operators. Assignment of classes is always done by way of an assignment operator function, even if it happens to be trivial and therefore open-coded as a block copy, so the above doesn't apply to classes.
(In reply to Jason Merrill from comment #66) > The operator semantics described in clause 5 [expr] apply to the built-in > operators, not any overloaded operators. Assignment of classes is always > done by way of an assignment operator function, even if it happens to be > trivial and therefore open-coded as a block copy, so the above doesn't apply > to classes. I think that's at least slippery. If none of 5.18 would apply to class types at all there would be no need for 5.18/3 (if non-class type) 5.18/4 (if class-type) 5.18/5 (class objects are special), or 5.18/9.2 (braced-inits for class objects). So if /2 would have been conditional on "non-class type" as well, like /3, it wouldn't matter, but so ... (This is of course only a side-track, I used clause 5 merely because like you I have difficulties to find a real definition of what constitutes an access to the value of an object :) )
(In reply to Jonathan Wakely from comment #65) > It accesses b, but it doesn't access the object stored in b's char[N] member > via placement new. Okay, let's go with this. So the copying of the union is then defined (as a memcpy equivalent). Then there's still the question if the following sequences are valid: // assume T1 and T2 are some types and new is trivial placement new union U {T1 a; char b[sizeof T2];} x,y; new (x.b) T2(); // 1 y = x; // 2 T2 t; memcpy(&t, y.b, sizeof T2); // 3 t; // 4 y.a; // 5 We have said that (2) is valid, obviously (3) in isolation is valid as well, but it influences the validity of (4). (5) is invalid as it's not the active member of the union y (which is instead b). (4) is valid if y.b contained a T2, which is only the case if (2) transferred the dynamic type from x.b _and_ (1) was valid to start with and dynamically typed x.b to be of type T2. So, it all boils down to if (1) is valid and types x.b to T2, even though it has a different declared type. For C we say it's not, because char[] is asymmetric: you can access all types via a char*, but you can't change the dynamic type of a declared char array to contain arbitrary other things (well, the ME memory model does cater for this and makes it valid, even though it's invalid in C). I guess you're arguing that (1) is valid in C++ and that then due to 3.9/3 and 12.8/29 also (2) and (4) are. I guess it can be defined to be so, but I wonder what the type of 'x' is after (1)? It can't be T2, because clearly x.b is valid even if T2 doesn't contain a member 'b'. So it must stay a union, but in order to transfer the type T2 in (2) it must also contain T2, so is it the type 'union {T1 a; char b[sizeof T2]; T2 <unnamed>;}' then (conceptually)? Messy :)
On March 27, 2017 8:11:10 PM GMT+02:00, "matz at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org> wrote: >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > >--- Comment #68 from Michael Matz <matz at gcc dot gnu.org> --- >(In reply to Jonathan Wakely from comment #65) >> It accesses b, but it doesn't access the object stored in b's char[N] >member >> via placement new. > >Okay, let's go with this. So the copying of the union is then defined >(as a memcpy equivalent). Then there's still the question if the >following >sequences are valid: > >// assume T1 and T2 are some types and new is trivial placement new >union U {T1 a; char b[sizeof T2];} x,y; >new (x.b) T2(); // 1 >y = x; // 2 >T2 t; >memcpy(&t, y.b, sizeof T2); // 3 >t; // 4 >y.a; // 5 > >We have said that (2) is valid, obviously (3) in isolation is valid as >well, >but it influences the validity of (4). (5) is invalid as it's not the >active member of the union y (which is instead b). > >(4) is valid if y.b contained a T2, which is only the case if (2) >transferred >the dynamic type from x.b _and_ (1) was valid to start with and >dynamically >typed x.b to be of type T2. > >So, it all boils down to if (1) is valid and types x.b to T2, even >though it >has a different declared type. For C we say it's not, because char[] >is >asymmetric: you can access all types via a char*, but you can't change >the >dynamic type of a declared char array to contain arbitrary other things >(well, >the ME memory model does cater for this and makes it valid, even though >it's >invalid in C). > >I guess you're arguing that (1) is valid in C++ and that then due to >3.9/3 >and 12.8/29 also (2) and (4) are. I guess it can be defined to be so, >but >I wonder what the type of 'x' is after (1)? It can't be T2, because >clearly >x.b is valid even if T2 doesn't contain a member 'b'. So it must stay >a union, >but in order to transfer the type T2 in (2) it must also contain T2, so >is it >the type 'union {T1 a; char b[sizeof T2]; T2 <unnamed>;}' then >(conceptually)? >Messy :) As I noted elsewhere union members in C++ seem to be pure convenience and a union contains implicit members of all types (well, somehow factor in alignment). Of course Jason argues char[] is special and introduces this but I can't find text anywhere to support that or require char[] and not, say int[]. Richard.
(In reply to rguenther@suse.de from comment #69) > As I noted elsewhere union members in C++ seem to be pure convenience and a > union contains implicit members of all types (well, somehow factor in > alignment). Well, the whole introduction of "object representation" is clearly a very bad idea if you want to avoid the above, but let's assume that the above is really not intended. So let's wait for answers to the questions and not see darkness wherever we go :)
(In reply to Michael Matz from comment #68) > (In reply to Jonathan Wakely from comment #65) > > It accesses b, but it doesn't access the object stored in b's char[N] member > > via placement new. > > Okay, let's go with this. So the copying of the union is then defined > (as a memcpy equivalent). Then there's still the question if the following > sequences are valid: > > // assume T1 and T2 are some types and new is trivial placement new > union U {T1 a; char b[sizeof T2];} x,y; > new (x.b) T2(); // 1 > y = x; // 2 > T2 t; > memcpy(&t, y.b, sizeof T2); // 3 > t; // 4 > y.a; // 5 > > We have said that (2) is valid, obviously (3) in isolation is valid as well, N.B. (3) is not valid if T2 isn't trivially copyable. But for the cases we care about, it is (the code takes a completely different path otherwise). > but it influences the validity of (4). (5) is invalid as it's not the > active member of the union y (which is instead b). > > (4) is valid if y.b contained a T2, which is only the case if (2) transferred > the dynamic type from x.b _and_ (1) was valid to start with and dynamically > typed x.b to be of type T2. > > So, it all boils down to if (1) is valid and types x.b to T2, even though it > has a different declared type. I don't think it changes the type of x.b, but it does begin the lifetime of an object of type T2 at that location. As I see it, the question is whether copying the object representation of that object to another location means we have another object at the second location. We all seem to agree that copying the object representation using memcpy *does* do that. We don't agree whether copying the object representation via an implicit union copy does that. I don't see a distinction between copying the object representation via memcpy or the union copy.
(In reply to rguenther@suse.de from comment #69) > As I noted elsewhere union members in C++ seem to be pure convenience and a > union contains implicit members of all types (well, somehow factor in > alignment). Of course Jason argues char[] is special and introduces this > but I can't find text anywhere to support that or require char[] and not, > say int[]. This is clarified somewhat in C++17, by http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0137r1.html Note that this wording makes only unsigned char[] special, not signed or plain char[].
On Mon, 27 Mar 2017, jason at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > > --- Comment #72 from Jason Merrill <jason at gcc dot gnu.org> --- > (In reply to rguenther@suse.de from comment #69) > > As I noted elsewhere union members in C++ seem to be pure convenience and a > > union contains implicit members of all types (well, somehow factor in > > alignment). Of course Jason argues char[] is special and introduces this > > but I can't find text anywhere to support that or require char[] and not, > > say int[]. > > This is clarified somewhat in C++17, by > > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0137r1.html > > Note that this wording makes only unsigned char[] special, not signed or plain > char[]. Ah, I remember seeing this. So this introduces object size as a way to allow some TBAA to happen. Specifically it forbids creating a series of float sub-objects inside a char[sizeof(float)*4] sub-object due to "there is no smaller array object that satisfies these constraints"(?). One would need to use sth like struct { unsigned char storage[sizeof(float)]; } st[4]; to work around that "limitation". Not if it's really possible to take advantage of that size restriction in alias analysis though... If we'd want to implement sth like this in the middle-end I'd suggest to force frontends to mark these storage areas in some way. In the C++ case all unsigned char arrays -- does the "array of N unsigned char" wording allow for struct C {} to be created within a unsigned char (no array!) member or has it to be unsigned char[1]?). I suggest sth like /* For an ARRAY_TYPE, indicates that when the dynamic type of the storage it provides differs from the declard type it is still valid to access it via the declared type or the type of a containing object. */ #define ARRAY_TYPE_IS_STORAGE (NODE) \ (ARRAY_TYPE_CHECK (NODE)->type_common.array_type_is_storage)
(In reply to rguenther@suse.de from comment #73) > > Ah, I remember seeing this. So this introduces object size as a way > to allow some TBAA to happen. Specifically it forbids creating > a series of float sub-objects inside a char[sizeof(float)*4] sub-object > due to "there is no smaller array object that satisfies these > constraints"(?). One would need to use sth like > > struct { unsigned char storage[sizeof(float)]; } st[4]; > I think that is meant differently, but I am not sure if I understand this kind of english good enough..., the sample uses: template<typename ...T> struct AlignedUnion { alignas(T...) unsigned char data[max(sizeof(T)...)]; }; so that data array is max of sizeof(int) and sizeof(char) ... the more they write the less clear it becomes :) > to work around that "limitation". Not if it's really possible to > take advantage of that size restriction in alias analysis though... > > If we'd want to implement sth like this in the middle-end I'd suggest > to force frontends to mark these storage areas in some way. > In the C++ case all unsigned char arrays -- does the "array of N unsigned > char" > wording allow for struct C {} to be created within a unsigned char > (no array!) member or has it to be unsigned char[1]?). > > I suggest sth like > > /* For an ARRAY_TYPE, indicates that when the dynamic type of the storage > it provides differs from the declard type it is still valid to access > it via the declared type or the type of a containing object. */ > #define ARRAY_TYPE_IS_STORAGE (NODE) \ > (ARRAY_TYPE_CHECK (NODE)->type_common.array_type_is_storage)
On Tue, 28 Mar 2017, bernd.edlinger at hotmail dot de wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > > --- Comment #74 from Bernd Edlinger <bernd.edlinger at hotmail dot de> --- > (In reply to rguenther@suse.de from comment #73) > > > > Ah, I remember seeing this. So this introduces object size as a way > > to allow some TBAA to happen. Specifically it forbids creating > > a series of float sub-objects inside a char[sizeof(float)*4] sub-object > > due to "there is no smaller array object that satisfies these > > constraints"(?). One would need to use sth like > > > > struct { unsigned char storage[sizeof(float)]; } st[4]; > > > > I think that is meant differently, but I am not sure if I > understand this kind of english good enough..., the sample uses: > > template<typename ...T> > struct AlignedUnion { > alignas(T...) unsigned char data[max(sizeof(T)...)]; > }; > > so that data array is max of sizeof(int) and sizeof(char) > > ... the more they write the less clear it becomes :) I was thinking about how to arrange for _multiple_ objects to be constructed inside a memory area, the interesting size clause seems to require workarounds like above. Otherwise the "array element" choice doesn't make much sense I guess.
FYI this is also on the same topic: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2011/n3296.html#US27 Question: "Related to core issue 1027, consider: int f() { union U { double d; } u1, u2; (int&)u1.d = 1; u2 = u1; return (int&)u2.d; } Does this involve undefined behavior? 3.8/4 seems to say that it's OK to clobber u1 with an int object. Then union assignment copies the object representation, possibly creating an int object in u2 and making the return statement well-defined. If this is well-defined, compilers are significantly limited in the assumptions they can make about type aliasing. On the other hand, the variant where U has an array of unsigned char member must be well-defined in order to support std::aligned_storage." Proposed Resolution: "Clarify that this testcase is undefined, but that adding an array of unsigned char to union U would make it well-defined--if a storage location is allocated with a particular type, it should be undefined to create an object in that storage if it would be undefined to access the stored value of the object through the allocated type." Disposition: "REJECTED Resolving this question was not deemed essential for this revision of the Standard, but core issues 1116 remains open for possible consideration in a future revision." Does this mean that the boost code is still invalid because there is no array of unsigned char ? Or does it not count because it was REJECTED ?
It was rejected in 2010, but see http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1116 for the later discussion, and: "[Adopted at the June, 2016 meeting as document P0137R1.]" Which Jason already pointed to in Comment 72.
Ah, adopted is the opposite of rejected, fine. Does this mean that the boost code is still incorrect, because it is not using an array of unsigned char ?
That's a new change for C++17, and I don't think today's GCC will treat it any differently if it uses unsigned char[] instead of char[].
but they use just: "mutable char data;"
(In reply to Bernd Edlinger from comment #80) > but they use just: "mutable char data;" No, actually it doesn't, see comment 45. The original testcase that this reprot came from is https://bugzilla.redhat.com/show_bug.cgi?id=1422456 which uses Boost 1.63 and that has an array of char. Jakub used the wrong version of Boost to create the code attached here.
Btw, for the attached patch it would fail to handle the case of passing the aggregate by value and the inliner introducing the aggregate copy: inline void* operator new(__SIZE_TYPE__, void* __p) { return __p; } struct X { union { int i; char s[4]; } u; }; static float foo (X y) { return reinterpret_cast <float &>(y.u.s); } int main () { X x; new (&x.u.s) float (1.0); if (foo (x) != 1.0) __builtin_abort (); } generates at -O2 after inlining: int main() () { void * D.2385; float D.2383; struct X y; struct X x; float _7; bool retval.0_8; <bb 2> [100.00%]: MEM[(float *)&x] = 1.0e+0; y = x; _7 = MEM[(float &)&y]; ...
(In reply to Jonathan Wakely from comment #81) > Boost 1.63 has an array of char. Seems they should change that to unsigned char (or std::byte).
I think the patch should also handle arrays of unions like: union BB { unsigned char x[10]; }; struct B { BB x[16]; }; struct A { B b; }; A a1, a2; void test() { a1.b = a2.b; } compiles to: ; Function void test() (_Z4testv, funcdef_no=0, decl_uid=2384, cgraph_uid=0, symbol_order=2) void test() () { <bb 2> [0.00%]: a1.b = a2.b; return; } while in this example struct B { unsigned char x[16]; }; struct A { B b; }; A a1, a2; void test() { a1.b = a2.b; } compiles to: ;; Function void test() (_Z4testv, funcdef_no=0, decl_uid=2348, cgraph_uid=0, symbol_order=2) void test() () { <bb 2> [0.00%]: MEM[(void *)&a1] = MEM[(void *)&a2]; return; } which is probably unnecessary, because all valid examples used a union with an array of unsigned char. Right?
(In reply to Bernd Edlinger from comment #84) > all valid examples used a union with an array of unsigned char. Right? There doesn't need to be a union, just an array of unsigned char.
Created attachment 41101 [details] another trial patch This is another approach for a patch that reflects what I understood so far what the C++17 spec wants from us. I think it should resolve the issue Richard raised.
On April 1, 2017 4:40:19 PM GMT+02:00, "bernd.edlinger at hotmail dot de" <gcc-bugzilla@gcc.gnu.org> wrote: >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > >--- Comment #86 from Bernd Edlinger <bernd.edlinger at hotmail dot de> >--- >Created attachment 41101 [details] > --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=41101&action=edit >another trial patch > >This is another approach for a patch that reflects what I understood >so far what the C++17 spec wants from us. >I think it should resolve the issue Richard raised. That doesn't properly work with LTO
(In reply to rguenther@suse.de from comment #87) > On April 1, 2017 4:40:19 PM GMT+02:00, "bernd.edlinger at hotmail dot de" > <gcc-bugzilla@gcc.gnu.org> wrote: > >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > > > >--- Comment #86 from Bernd Edlinger <bernd.edlinger at hotmail dot de> > >--- > >Created attachment 41101 [details] > > --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=41101&action=edit > >another trial patch > > > >This is another approach for a patch that reflects what I understood > >so far what the C++17 spec wants from us. > >I think it should resolve the issue Richard raised. > > That doesn't properly work with LTO oops... I think this does not work either, at cp/decl.c (start_enum): /* std::byte aliases anything. */ if (enumtype != error_mark_node && TYPE_CONTEXT (enumtype) == std_node && !strcmp ("byte", TYPE_NAME_STRING (enumtype))) TYPE_ALIAS_SET (enumtype) = 0; I looked at -flto -fdump-rtl-final and see the alias set is 1: #include <cstddef> //namespace std { enum class byte: unsigned char {}; }; union t { int x; std::byte k[1]; }; t t1; t t2; void test() { t1.k[0] = t2.k[0]; } int main() { test(); } g++ -std=c++17 -flto -fdump-rtl-final t.cc /tmp/cc*.final: (insn 5 2 6 2 (set (reg:QI 0 ax [orig:87 _1 ] [87]) (mem/j/c:QI (symbol_ref:DI ("t2") [flags 0x2] <var_decl 0x7f2a492dfab0 t2>) [1 t2.k+0 S1 A32])) "t.cc":11 84 {*movqi_internal} (nil)) (insn 6 5 13 2 (set (mem/j/c:QI (symbol_ref:DI ("t1") [flags 0x2] <var_decl 0x7f2a492dfb40 t1>) [1 t1.k+0 S1 A32]) (reg:QI 0 ax [orig:87 _1 ] [87])) "t.cc":11 84 {*movqi_internal} (nil)) [1 = alias set 1 but should be 0
Created attachment 41103 [details] updated patch should work in principle with LTO, but there is still another bug somewhere which makes __attribute__((may_alias)) and the std::byte handling fail in the end. consider this test example: cat t.cc #include <cstddef> //namespace std { enum class byte: unsigned char {}; }; struct t { int x; //std::byte kk[1]; unsigned char k; } __attribute__((may_alias)); t t1; t t2; t *t3; t *t4; void __attribute__((noinline, noclone)) test() { t1 = t2; *t4 = *t3; } int main() { test(); } g++ -std=c++17 -O2 -fdump-rtl-final t.cc t.cc.309r.final: (insn:TI 6 2 7 2 (set (reg:DI 0 ax [orig:90 MEM[(const struct t & {ref-all})&t2] ] [90]) (mem/c:DI (symbol_ref:DI ("t2") [flags 0x2] <var_decl 0x7f973b7302d0 t2>) [0 MEM[(const struct t & {ref-all})&t2]+0 S8 A64])) "t.cc":17 81 {*movdi_internal} (expr_list:REG_EQUIV (mem/c:DI (symbol_ref:DI ("t2") [flags 0x2] <var_decl 0x7f973b7302d0 t2>) [0 MEM[(const struct t & {ref-all})&t2]+0 S8 A64]) (nil))) (insn:TI 7 6 14 2 (set (mem/c:DI (symbol_ref:DI ("t1") [flags 0x2] <var_decl 0x7f973b730240 t1>) [2 t1+0 S8 A64]) (reg:DI 0 ax [orig:90 MEM[(const struct t & {ref-all})&t2] ] [90])) "t.cc":17 81 {*movdi_internal} (expr_list:REG_DEAD (reg:DI 0 ax [orig:90 MEM[(const struct t & {ref-all})&t2] ] [90]) (nil))) (insn 14 7 10 2 (set (reg/f:DI 0 ax [orig:87 t3.1_1 ] [87]) (mem/f/c:DI (symbol_ref:DI ("t3") [flags 0x2] <var_decl 0x7f973b730360 t3>) [1 t3+0 S8 A64])) "t.cc":18 81 {*movdi_internal} (expr_list:REG_EQUIV (mem/f/c:DI (symbol_ref:DI ("t3") [flags 0x2] <var_decl 0x7f973b730360 t3>) [1 t3+0 S8 A64]) (nil))) (insn:TI 10 14 15 2 (set (reg:DI 1 dx [orig:91 MEM[(const struct t & {ref-all})t3.1_1] ] [91]) (mem:DI (reg/f:DI 0 ax [orig:87 t3.1_1 ] [87]) [0 MEM[(const struct t & {ref-all})t3.1_1]+0 S8 A32])) "t.cc":18 81 {*movdi_internal} (expr_list:REG_DEAD (reg/f:DI 0 ax [orig:87 t3.1_1 ] [87]) (nil))) (insn 15 10 11 2 (set (reg/f:DI 0 ax [orig:88 t4.2_2 ] [88]) (mem/f/c:DI (symbol_ref:DI ("t4") [flags 0x2] <var_decl 0x7f973b7303f0 t4>) [1 t4+0 S8 A64])) "t.cc":18 81 {*movdi_internal} (expr_list:REG_EQUIV (mem/f/c:DI (symbol_ref:DI ("t4") [flags 0x2] <var_decl 0x7f973b7303f0 t4>) [1 t4+0 S8 A64]) (nil))) (insn:TI 11 15 23 2 (set (mem:DI (reg/f:DI 0 ax [orig:88 t4.2_2 ] [88]) [0 *t4.2_2+0 S8 A32]) (reg:DI 1 dx [orig:91 MEM[(const struct t & {ref-all})t3.1_1] ] [91])) "t.cc":18 81 {*movdi_internal} (expr_list:REG_DEAD (reg:DI 1 dx [orig:91 MEM[(const struct t & {ref-all})t3.1_1] ] [91]) (expr_list:REG_DEAD (reg/f:DI 0 ax [orig:88 t4.2_2 ] [88]) (nil)))) I think the access to t1 uses alias set 2, and that is wrong. The other accesses are ok.
On April 2, 2017 2:09:07 AM GMT+02:00, "bernd.edlinger at hotmail dot de" <gcc-bugzilla@gcc.gnu.org> wrote: >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > >Bernd Edlinger <bernd.edlinger at hotmail dot de> changed: > > What |Removed |Added >---------------------------------------------------------------------------- > Attachment #41101 [details]|0 |1 > is obsolete| | > >--- Comment #89 from Bernd Edlinger <bernd.edlinger at hotmail dot de> >--- >Created attachment 41103 [details] > --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=41103&action=edit >updated patch > >should work in principle with LTO, >but there is still another bug >somewhere which makes __attribute__((may_alias)) >and the std::byte handling fail in the end. > >consider this test example: > >cat t.cc >#include <cstddef> >//namespace std { enum class byte: unsigned char {}; }; >struct t { > int x; > //std::byte kk[1]; > unsigned char k; >} __attribute__((may_alias)); > >t t1; >t t2; >t *t3; >t *t4; > >void __attribute__((noinline, noclone)) >test() >{ > t1 = t2; Can't spot the issue in the RTL you quote but may_alias does not apply to this assignment. > *t4 = *t3; >} > >int main() >{ > test(); >} > >g++ -std=c++17 -O2 -fdump-rtl-final t.cc > >t.cc.309r.final: >(insn:TI 6 2 7 2 (set (reg:DI 0 ax [orig:90 MEM[(const struct t & >{ref-all})&t2] ] [90]) > (mem/c:DI (symbol_ref:DI ("t2") [flags 0x2] <var_decl 0x7f973b7302d0 >t2>) [0 MEM[(const struct t & {ref-all})&t2]+0 S8 A64])) "t.cc":17 81 >{*movdi_internal} > (expr_list:REG_EQUIV (mem/c:DI (symbol_ref:DI ("t2") [flags 0x2] ><var_decl 0x7f973b7302d0 t2>) [0 MEM[(const struct t & {ref-all})&t2]+0 >S8 >A64]) > (nil))) >(insn:TI 7 6 14 2 (set (mem/c:DI (symbol_ref:DI ("t1") [flags 0x2] ><var_decl >0x7f973b730240 t1>) [2 t1+0 S8 A64]) > (reg:DI 0 ax [orig:90 MEM[(const struct t & {ref-all})&t2] ] [90])) >"t.cc":17 81 {*movdi_internal} > (expr_list:REG_DEAD (reg:DI 0 ax [orig:90 MEM[(const struct t & >{ref-all})&t2] ] [90]) > (nil))) >(insn 14 7 10 2 (set (reg/f:DI 0 ax [orig:87 t3.1_1 ] [87]) >(mem/f/c:DI (symbol_ref:DI ("t3") [flags 0x2] <var_decl 0x7f973b730360 >t3>) [1 t3+0 S8 A64])) "t.cc":18 81 {*movdi_internal} > (expr_list:REG_EQUIV (mem/f/c:DI (symbol_ref:DI ("t3") [flags 0x2] ><var_decl 0x7f973b730360 t3>) [1 t3+0 S8 A64]) > (nil))) >(insn:TI 10 14 15 2 (set (reg:DI 1 dx [orig:91 MEM[(const struct t & >{ref-all})t3.1_1] ] [91]) >(mem:DI (reg/f:DI 0 ax [orig:87 t3.1_1 ] [87]) [0 MEM[(const struct t & >{ref-all})t3.1_1]+0 S8 A32])) "t.cc":18 81 {*movdi_internal} > (expr_list:REG_DEAD (reg/f:DI 0 ax [orig:87 t3.1_1 ] [87]) > (nil))) >(insn 15 10 11 2 (set (reg/f:DI 0 ax [orig:88 t4.2_2 ] [88]) >(mem/f/c:DI (symbol_ref:DI ("t4") [flags 0x2] <var_decl 0x7f973b7303f0 >t4>) [1 t4+0 S8 A64])) "t.cc":18 81 {*movdi_internal} > (expr_list:REG_EQUIV (mem/f/c:DI (symbol_ref:DI ("t4") [flags 0x2] ><var_decl 0x7f973b7303f0 t4>) [1 t4+0 S8 A64]) > (nil))) >(insn:TI 11 15 23 2 (set (mem:DI (reg/f:DI 0 ax [orig:88 t4.2_2 ] [88]) >[0 >*t4.2_2+0 S8 A32]) > (reg:DI 1 dx [orig:91 MEM[(const struct t & {ref-all})t3.1_1] ] [91])) >"t.cc":18 81 {*movdi_internal} > (expr_list:REG_DEAD (reg:DI 1 dx [orig:91 MEM[(const struct t & >{ref-all})t3.1_1] ] [91]) > (expr_list:REG_DEAD (reg/f:DI 0 ax [orig:88 t4.2_2 ] [88]) > (nil)))) > > > >I think the access to t1 uses alias set 2, and that is wrong. >The other accesses are ok.
I mean here is [2 t1+0 S8 A64] and 2 is the alias set info in the MEM_REF, I have expected that to be 0. (insn:TI 7 6 14 2 (set (mem/c:DI (symbol_ref:DI ("t1") [flags 0x2] <var_decl 0x7f973b730240 t1>) [2 t1+0 S8 A64]) (reg:DI 0 ax [orig:90 MEM[(const struct t & {ref-all})&t2] ] [90])) "t.cc":17 81 {*movdi_internal} (expr_list:REG_DEAD (reg:DI 0 ax [orig:90 MEM[(const struct t & {ref-all})&t2] ] [90]) (nil)))
It is an interesting fact that g++ -flto -fdump-rtl-final t.cc has correct rtl: (insn 7 6 8 2 (set (mem/c:DI (symbol_ref:DI ("t1") [flags 0x2] <var_decl 0x7f583248fab0 t1>) [0 t1+0 S8 A64]) (reg:DI 0 ax [90])) "t.cc":17 81 {*movdi_internal} (nil))
I tried a bit more with lto and discovered that -flto -O2 removes all the code from test() although it is marked noinline, noclone To work around that, I added an asm statement initializing all these mem-refs, now the test case looks like that: cat t.cc #include <cstddef> //namespace std { enum class byte: unsigned char {}; }; struct t { int x; //std::byte kk[1]; unsigned char k; } __attribute__((may_alias)); t t1; t t2; t *t3; t *t4; void __attribute__((noinline, noclone)) test() { t1 = t2; *t4 = *t3; } int main() { asm("":"=m"(t1), "=m"(t2), "=m"(t3), "=m"(t4)); test(); } This prevents -flto -O2 from removing the mem-refs. But I discovered that the asm output arguments don't use the alias-set zero: g++ -flto -O2 -fdump-rtl-final t.cc has this rtl from the asm stmt: (insn:TI 5 2 6 2 (parallel [ (set (mem/c:DI (symbol_ref:DI ("t1") [flags 0x2] <var_decl 0x7f0d3d106ab0 t1>) [2 t1+0 S8 A64]) (asm_operands:DI ("") ("=m") 0 [] [] [] t.cc:23)) (set (mem/c:DI (symbol_ref:DI ("t2") [flags 0x2] <var_decl 0x7f0d3d106b40 t2>) [2 t2+0 S8 A64]) (asm_operands:DI ("") ("=m") 1 [] [] [] t.cc:23)) (set (mem/f/c:DI (symbol_ref:DI ("t3") [flags 0x2] <var_decl 0x7f0d3d106bd0 t3>) [1 t3+0 S8 A64]) (asm_operands:DI ("") ("=m") 2 [] [] [] t.cc:23)) (set (mem/f/c:DI (symbol_ref:DI ("t4") [flags 0x2] <var_decl 0x7f0d3d106c60 t4>) [1 t4+0 S8 A64]) (asm_operands:DI ("") ("=m") 3 [] [] [] t.cc:23)) (clobber (reg:CCFP 18 fpsr)) (clobber (reg:CC 17 flags)) ]) "t.cc":23 -1 (expr_list:REG_UNUSED (reg:CCFP 18 fpsr) (expr_list:REG_UNUSED (reg:CC 17 flags) (nil)))) Again alias set 2 for t1, and t2. This is identical also for non-lto build.
I always wondered why get_alias_set does not use the may_alias attribute like this: Index: alias.c =================================================================== --- alias.c (revision 246605) +++ alias.c (working copy) @@ -928,6 +928,9 @@ get_alias_set (tree t) return 0; } + if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (t))) + return 0; + /* See if the language has special handling for this type. */ set = lang_hooks.get_alias_set (t); if (set != -1) this would fix the remaining fall-out.
On April 2, 2017 10:54:28 AM GMT+02:00, "bernd.edlinger at hotmail dot de" <gcc-bugzilla@gcc.gnu.org> wrote: >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > >--- Comment #94 from Bernd Edlinger <bernd.edlinger at hotmail dot de> >--- >I always wondered why get_alias_set does not use >the may_alias attribute like this: > > >Index: alias.c >=================================================================== >--- alias.c (revision 246605) >+++ alias.c (working copy) >@@ -928,6 +928,9 @@ get_alias_set (tree t) > return 0; > } > >+ if (lookup_attribute ("may_alias", TYPE_ATTRIBUTES (t))) >+ return 0; >+ > /* See if the language has special handling for this type. */ > set = lang_hooks.get_alias_set (t); > if (set != -1) > >this would fix the remaining fall-out. Because that is not how it was designed or documented to work :)
As Richard mentioned on IRC, e.g. the x86intrin.h __m[125]* types have may_alias attributes and such a change would slow down code that puts those into structures/unions.
I thought that at that point t is the type of to the outer reference. thus in this example: cat t1.c #include <x86intrin.h> union xx { __m64 m; long long l; }; union xx t; __m64 test(long long x) { t.l=x; return t.m; } gcc -S t1.c -fdump-rtl-final I see this in t1.c.309r.final (insn 7 6 8 2 (set (mem/j/c:DI (symbol_ref:DI ("t") [flags 0x2] <var_decl 0x7fe3bff8a090 t>) [1 t.l+0 S8 A64]) (reg:DI 0 ax [89])) "t1.c":13 81 {*movdi_internal} (nil)) (insn 8 7 11 2 (set (reg:V2SI 0 ax [orig:87 _4 ] [87]) (mem/j/c:V2SI (symbol_ref:DI ("t") [flags 0x2] <var_decl 0x7fe3bff8a090 t>) [1 t.m+0 S8 A64])) "t1.c":14 1100 {*movv2si_internal} (nil))
I realized that my latest patch triggers a pre-existing bug in the libstdc++ testsuite => PR80287 So the following patch would be needed on top of it: Index: gcc/cp/class.c =================================================================== --- gcc/cp/class.c (revision 246605) +++ gcc/cp/class.c (working copy) @@ -2060,12 +2060,14 @@ static void fixup_may_alias (tree klass) { - tree t; + tree t, v; for (t = TYPE_POINTER_TO (klass); t; t = TYPE_NEXT_PTR_TO (t)) - TYPE_REF_CAN_ALIAS_ALL (t) = true; + for (v = TYPE_MAIN_VARIANT (t); v; v = TYPE_NEXT_VARIANT (v)) + TYPE_REF_CAN_ALIAS_ALL (v) = true; for (t = TYPE_REFERENCE_TO (klass); t; t = TYPE_NEXT_REF_TO (t)) - TYPE_REF_CAN_ALIAS_ALL (t) = true; + for (v = TYPE_MAIN_VARIANT (t); v; v = TYPE_NEXT_VARIANT (v)) + TYPE_REF_CAN_ALIAS_ALL (v) = true; } /* Early variant fixups: we apply attributes at the beginning of the class
(In reply to rguenther@suse.de from comment #95) > > > >this would fix the remaining fall-out. > > Because that is not how it was designed or documented to work :) So yes, it seems I misunderstood what may_alias should do. It is defined on the type, however it has only meaning on the pointers to that type, but not on the instances. But I wanted to have an attribute to express in the tree that instances and pointers to that type may alias anything. How about adding a new type attribute, that does what I meant, like always_alias for instance?
On Tue, 4 Apr 2017, bernd.edlinger at hotmail dot de wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > > --- Comment #99 from Bernd Edlinger <bernd.edlinger at hotmail dot de> --- > (In reply to rguenther@suse.de from comment #95) > > > > > >this would fix the remaining fall-out. > > > > Because that is not how it was designed or documented to work :) > > So yes, it seems I misunderstood what may_alias should do. > > It is defined on the type, however it has only meaning on the > pointers to that type, but not on the instances. > > But I wanted to have an attribute to express in the tree > that instances and pointers to that type may alias anything. > > How about adding a new type attribute, that does what I meant, > like always_alias for instance? That's what I proposed the C++ FE to do with a new tree type flag. That could be exposed to users with an attribute as well, of course.
(In reply to rguenther@suse.de from comment #100) > On Tue, 4 Apr 2017, bernd.edlinger at hotmail dot de wrote: > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79671 > > > > --- Comment #99 from Bernd Edlinger <bernd.edlinger at hotmail dot de> --- > > (In reply to rguenther@suse.de from comment #95) > > > > > > > >this would fix the remaining fall-out. > > > > > > Because that is not how it was designed or documented to work :) > > > > So yes, it seems I misunderstood what may_alias should do. > > > > It is defined on the type, however it has only meaning on the > > pointers to that type, but not on the instances. > > > > But I wanted to have an attribute to express in the tree > > that instances and pointers to that type may alias anything. > > > > How about adding a new type attribute, that does what I meant, > > like always_alias for instance? > > That's what I proposed the C++ FE to do with a new tree type flag. That > could be exposed to users with an attribute as well, of course. good. I have all the bits together, I just have to rename the flag.
Fixed.
Author: rguenth Date: Wed Apr 12 07:35:49 2017 New Revision: 246866 URL: https://gcc.gnu.org/viewcvs?rev=246866&root=gcc&view=rev Log: 2017-04-12 Richard Biener <rguenther@suse.de> Bernd Edlinger <bernd.edlinger@hotmail.de> PR middle-end/79671 * alias.c (component_uses_parent_alias_set_from): Handle TYPE_TYPELESS_STORAGE. (get_alias_set): Likewise. * tree-core.h (tree_type_common): Add typeless_storage flag. * tree.h (TYPE_TYPELESS_STORAGE): New macro. * stor-layout.c (place_union_field): Set TYPE_TYPELESS_STORAGE for types containing members with TYPE_TYPELESS_STORAGE. (place_field): Likewise. (layout_type): Likewise for ARRAY_TYPE. * lto-streamer-out.c (hash_tree): Hash TYPE_TYPELESS_STORAGE. * tree-streamer-in.c (unpack_ts_type_common_value_fields): Stream TYPE_TYPELESS_STORAGE. * tree-streamer-out.c (pack_ts_type_common_value_fields): Likewise. lto/ * lto.c (compare_tree_sccs_1): Compare TYPE_TYPELESS_STORAGE. cp/ * tree.c (build_cplus_array_type): Set TYPE_TYPELESS_STORAGE for arrays of character or std::byte type. * g++.dg/torture/pr79671.C: New testcase. * g++.dg/lto/pr79671_0.C: Likewise. * g++.dg/lto/pr79671_1.c: Likewise. Added: trunk/gcc/testsuite/g++.dg/lto/pr79671_0.C trunk/gcc/testsuite/g++.dg/lto/pr79671_1.c trunk/gcc/testsuite/g++.dg/torture/pr79671.C Modified: trunk/gcc/ChangeLog trunk/gcc/alias.c trunk/gcc/cp/ChangeLog trunk/gcc/cp/tree.c trunk/gcc/lto-streamer-out.c trunk/gcc/lto/ChangeLog trunk/gcc/lto/lto.c trunk/gcc/stor-layout.c trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-core.h trunk/gcc/tree-streamer-in.c trunk/gcc/tree-streamer-out.c trunk/gcc/tree.h