Hi!
This fixes a wrong code issue in expand_expr_real_1 which happens because
a negative bitpos is actually able to reach extract_bit_field which
does all computations with poly_uint64, thus the offset 0x1ffffffffffffff0.
To avoid that I propose to use Jakub's r204444 patch from the expand_assignment
also in the expand_expr_real_1.
This is a rather unlikely thing to happen, as there are lots of checks that are
of course all target dependent between the get_inner_reference and the
actual extract_bit_field call, and all other code paths may or may not have a
problem
with negative bit offsets. Most don't have a problem, but the bitpos needs to
be
folded into offset before it is used, therefore it is necessary to handle the
negative
bitpos very far away from the extract_bit_field call. Doing that later is IMHO
not
possible.
The fix in CONSTANT_ADDRESS_P is actually unrelated, and I only spotted it
because
this macro is used in alpha_legitimize_address which is of course what one looks
at first if something like that happens.
I think even with this bogus offset it should not have caused a linker error,
so there
is probably a second problem in the *movdi code pattern of the alpha.md,
because it
should be split into instructions that don't cause a link error.
Once the target is fixed to split the impossible assembler instruction, the
test case
will probably no longer be able to detect the pattern in the assembly.
Therefore the test case looks both at the assembler output and the expand rtl
dump
to spot the bogus offset. I only check the first 12 digits of the bogus
constant,
because it is actually dependent on the target configuration:
I built first a cross-compiler without binutils, and it did used a slightly
different
offset of 0x2000000000000000, (configured with: --target=alpha-linux-gnu
--enable-languages=c
--disable-libgcc --disable-libssp --disable-libquadmath --disable-libgomp
--disable-libatomic)
when the binutils are installed at where --prefix points, the offset is
0x1ffffffffffffff0.
Regarding the alpha target, I could not do more than build a cross compiler and
run
make check-gcc-c RUNTESTFLAGS="alpha.exp=pr86984.c".
Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?
Thanks
Bernd.
2018-08-19 Bernd Edlinger <bernd.edlin...@hotmail.de>
PR target/86984
* expr.c (expand_assignment): Assert that bitpos is positive.
(store_field): Likewise
(expand_expr_real_1): Make sure that bitpos is positive.
* config/alpha/alpha.h (CONSTANT_ADDRESS_P): Avoid signed
integer overflow.
testsuite:
2018-08-19 Bernd Edlinger <bernd.edlin...@hotmail.de>
PR target/86984
* gcc.target/alpha/pr86984.c: New test.
Index: gcc/config/alpha/alpha.h
===================================================================
--- gcc/config/alpha/alpha.h (revision 263644)
+++ gcc/config/alpha/alpha.h (working copy)
@@ -678,7 +678,7 @@ enum reg_class {
#define CONSTANT_ADDRESS_P(X) \
(CONST_INT_P (X) \
- && (unsigned HOST_WIDE_INT) (INTVAL (X) + 0x8000) < 0x10000)
+ && (UINTVAL (X) + 0x8000) < 0x10000)
/* The macros REG_OK_FOR..._P assume that the arg is a REG rtx
and check its validity for a certain class.
Index: gcc/expr.c
===================================================================
--- gcc/expr.c (revision 263644)
+++ gcc/expr.c (working copy)
@@ -5270,6 +5270,7 @@ expand_assignment (tree to, tree from, bool nontem
MEM_VOLATILE_P (to_rtx) = 1;
}
+ gcc_assert (known_ge (bitpos, 0));
if (optimize_bitfield_assignment_op (bitsize, bitpos,
bitregion_start, bitregion_end,
mode1, to_rtx, to, from,
@@ -7046,6 +7047,7 @@ store_field (rtx target, poly_int64 bitsize, poly_
}
/* Store the value in the bitfield. */
+ gcc_assert (known_ge (bitpos, 0));
store_bit_field (target, bitsize, bitpos,
bitregion_start, bitregion_end,
mode, temp, reverse);
@@ -10545,6 +10547,14 @@ expand_expr_real_1 (tree exp, rtx target, machine_
mode2
= CONSTANT_P (op0) ? TYPE_MODE (TREE_TYPE (tem)) : GET_MODE (op0);
+ /* Make sure bitpos is not negative, it can wreak havoc later. */
+ if (maybe_lt (bitpos, 0))
+ {
+ gcc_assert (offset == NULL_TREE);
+ offset = size_int (bits_to_bytes_round_down (bitpos));
+ bitpos = num_trailing_bits (bitpos);
+ }
+
/* If we have either an offset, a BLKmode result, or a reference
outside the underlying object, we must force it to memory.
Such a case can occur in Ada if we have unchecked conversion
@@ -10795,6 +10805,7 @@ expand_expr_real_1 (tree exp, rtx target, machine_
&& GET_MODE_CLASS (ext_mode) == MODE_INT)
reversep = TYPE_REVERSE_STORAGE_ORDER (type);
+ gcc_assert (known_ge (bitpos, 0));
op0 = extract_bit_field (op0, bitsize, bitpos, unsignedp,
(modifier == EXPAND_STACK_PARM
? NULL_RTX : target),
Index: gcc/testsuite/gcc.target/alpha/pr86984.c
===================================================================
--- gcc/testsuite/gcc.target/alpha/pr86984.c (revision 0)
+++ gcc/testsuite/gcc.target/alpha/pr86984.c (working copy)
@@ -0,0 +1,96 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wall -Wwrite-strings -Werror -fmerge-all-constants -fno-stack-protector -mieee -fdump-rtl-expand" } */
+
+struct expression {
+ unsigned long int num;
+};
+union YYSTYPE {
+ unsigned long int num;
+ struct expression *exp;
+};
+
+typedef union YYSTYPE YYSTYPE;
+
+struct expression * new_exp_0 (int);
+
+union yyalloc {
+ short yyss_alloc;
+};
+
+static const signed char yypact[] = {
+ -9, -9, -10, -10, -9, 8, 36, -10, 13, -10, -9, -9, -9, -9, -9, -9, -9, -10, 26, 41, 45, 18, -2, 14, -10, -9, 36 };
+static const unsigned char yydefact[] = {
+ 0, 0, 12, 11, 0, 0, 2, 10, 0, 1, 0, 0, 0, 0, 0, 0, 0, 13, 0, 4, 5, 6, 7, 8, 9, 0, 3 };
+
+static const signed char yypgoto[3] = "\366\366\377";
+static const signed char yydefgoto[3] = "\377\005\006";
+
+static const unsigned char yytable[] = {
+ 7, 1, 2, 8, 3, 4, 15, 16, 9, 18, 19, 20, 21, 22, 23, 24, 10, 11, 12, 13, 14, 15, 16, 16, 26, 14, 15, 16, 17, 10, 11, 12, 13, 14, 15, 16, 0, 0, 25, 10, 11, 12, 13, 14, 15, 16, 12, 13, 14, 15, 16, 13, 14, 15, 16 };
+
+static const signed char yycheck[] = {
+ 1, 10, 11, 4, 13, 14, 8, 9, 0, 10, 11, 12, 13, 14, 15, 16, 3, 4, 5, 6, 7, 8, 9, 9, 25, 7, 8, 9, 15, 3, 4, 5, 6, 7, 8, 9, -1, -1, 12, 3, 4, 5, 6, 7, 8, 9, 5, 6, 7, 8, 9, 6, 7, 8, 9 };
+
+static const unsigned char yyr1[] = {
+ 0, 16, 17, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18, 18 };
+
+static const unsigned char yyr2[] = {
+ 0, 2, 1, 5, 3, 3, 3, 3, 3, 3, 2, 1, 1, 3 };
+
+int __gettextparse (void)
+{
+ int yystate = 0;
+ short yyssa[200];
+ short *yyss = yyss;
+ short *yyssp = yyssa;
+ YYSTYPE yyvsa[200];
+ YYSTYPE *yyvsp = yyvsa;
+ enum { yystacksize = 200 };
+ int yylen = 0;
+ goto yysetstate;
+ yynewstate: yyssp++;
+ yysetstate: *yyssp = yystate;
+
+ if (yyss + yystacksize - 1 <= yyssp)
+ {
+ long unsigned int yysize = yyssp - yyss + 1;
+ {
+ short *yyss1 = yyss;
+ union yyalloc *yyptr = (union yyalloc *) __builtin_malloc ((yystacksize * (sizeof (short) + sizeof (YYSTYPE)) + (sizeof (union yyalloc) - 1)));
+ if (!yyptr) return 0;
+ __builtin_memcpy (&yyptr->yyss_alloc, yyss, yysize * sizeof *(yyss));
+ yyss = &yyptr->yyss_alloc;
+ if (yyss1 != yyssa) __builtin_free (yyss1);
+ }
+ if (yyss + yystacksize - 1 <= yyssp)
+ return 0;
+ }
+
+ int yyn = yypact[yystate];
+ if (yyn == -10)
+ goto yydefault;
+
+ yyn = yytable[yyn];
+ if (yyn <= 0)
+ goto yyreduce;
+
+ yydefault: yyn = yydefact[yystate];
+ yyreduce: yylen = yyr2[yyn];
+
+ YYSTYPE yyval;
+ if (yyn == 12 && (yyval.exp = new_exp_0 (0)) != 0)
+ (yyval.exp)->num = (yyvsp[0].num);
+
+ (yyvsp -= yylen, yyssp -= yylen);
+ yyn = yyr1[yyn];
+ yystate = yypgoto[yyn - 16] + *yyssp;
+ if (0 <= yystate && yystate <= 54 && yycheck[yystate] == *yyssp)
+ yystate = yytable[yystate];
+ else
+ yystate = yydefgoto[yyn - 16];
+
+ goto yynewstate;
+}
+
+/* { dg-final { scan-rtl-dump-not "const_int 230584300921" "expand" } } */
+/* { dg-final { scan-assembler-not "yypgoto\\+230584300921" } } */
