This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Fix up wrong-code caused by store_expr (PR middle-end/90025)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Richard Biener <rguenther at suse dot de>, Jeff Law <law at redhat dot com>, Eric Botcazou <ebotcazou at adacore dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Wed, 10 Apr 2019 09:20:10 +0200
- Subject: [PATCH] Fix up wrong-code caused by store_expr (PR middle-end/90025)
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
Hi!
In r268957 aka PR66152 fix I've slightly changed the store_expr
code which does store_by_pieces for just a part of an array and
clear_storage for the rest and since then the following testcase
is miscompiled on s390x-linux. I believe one could construct other testcase
that would be miscompiled before as well though.
The problem is that store_by_pieces with RETURN_END returns a MEM object
with QImode and thus also MEM_SIZE of 1, we then adjust_address it to
BLKmode which doesn't change MEM_SIZE and nothing at least in the generic
clear_storage code tweaks that MEM_SIZE later on (and while perhaps some
targets do, e.g. s390 does not). Because in this case the size is
really known and constant, the following patch makes sure we do
what adjust_address does, but in addition make sure to set MEM_SIZE
properly. When MEM_SIZE is 1 and in the testcase there is a store after
it to the first few bytes of the remainder, RTL DSE happily removes that
memory clear insn emitted for clear_storage, because if it was really size
1, it would be indeed redundant, but there are 20 bytes that are not
redundant after it.
Bootstrapped/regtested on {x86_64,i686,s390x,powerpc64le}-linux, ok for
trunk?
2019-04-10 Jakub Jelinek <jakub@redhat.com>
PR middle-end/90025
* expr.c (store_expr): Set properly size on the MEM passed to
clear_storage.
* gcc.c-torture/execute/pr90025.c: New test.
--- gcc/expr.c.jj 2019-02-21 00:01:59.418057433 +0100
+++ gcc/expr.c 2019-04-09 12:49:20.704457867 +0200
@@ -5658,7 +5658,8 @@ store_expr (tree exp, rtx target, int ca
dest_mem = store_by_pieces (target, str_copy_len, string_cst_read_str,
(void *) str, MEM_ALIGN (target), false,
RETURN_END);
- clear_storage (adjust_address (dest_mem, BLKmode, 0),
+ clear_storage (adjust_address_1 (dest_mem, BLKmode, 0, 1, 1, 0,
+ exp_len - str_copy_len),
GEN_INT (exp_len - str_copy_len), BLOCK_OP_NORMAL);
return NULL_RTX;
}
--- gcc/testsuite/gcc.c-torture/execute/pr90025.c.jj 2019-04-09 12:59:35.876449686 +0200
+++ gcc/testsuite/gcc.c-torture/execute/pr90025.c 2019-04-09 12:59:20.156705430 +0200
@@ -0,0 +1,28 @@
+/* PR middle-end/90025 */
+
+__attribute__((noipa)) void
+bar (char *p)
+{
+ int i;
+ for (i = 0; i < 6; i++)
+ if (p[i] != "foobar"[i])
+ __builtin_abort ();
+ for (; i < 32; i++)
+ if (p[i] != '\0')
+ __builtin_abort ();
+}
+
+__attribute__((noipa)) void
+foo (unsigned int x)
+{
+ char s[32] = { 'f', 'o', 'o', 'b', 'a', 'r', 0 };
+ ((unsigned int *) s)[2] = __builtin_bswap32 (x);
+ bar (s);
+}
+
+int
+main ()
+{
+ foo (0);
+ return 0;
+}
Jakub