On 08/18/18 12:40, Richard Sandiford wrote:
> Bernd Edlinger <bernd.edlin...@hotmail.de> writes:
>> Hi everybody,
>>
>> On 08/16/18 08:36, Bernd Edlinger wrote:
>>> Jeff Law wrote:
>>>> I wonder if the change to how we set up the initializers is ultimately
>>>> changing the section those go into and ultimately causing an overflow of
>>>> the .sdata section.
>>>
>>>
>>> Yes, that is definitely the case.
>>> Due to the -fmerge-all-constants option used
>>> named arrays with brace initializer look like string initializers
>>> and can go into the merge section if there are no embedded nul chars.
>>> But the string constants can now be huge.
>>>
>>> See my other patch about string merging:
>>> [PATCH] Handle not explicitly zero terminated strings in merge sections
>>> https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00481.html
>>>
>>>
>>> Can this section overflow?
>>>
>>
>>
>> could someone try out if this (untested) patch fixes the issue?
>>
>>
>> Thanks,
>> Bernd.
>>
>>
>> 2018-08-18 Bernd Edlinger <bernd.edlin...@hotmail.de>
>>
>> * expmed.c (simple_mem_bitfield_p): Do shift right signed.
>> * config/alpha/alpha.h (CONSTANT_ADDRESS_P): Avoid signed
>> integer overflow.
>>
>> Index: gcc/config/alpha/alpha.h
>> ===================================================================
>> --- gcc/config/alpha/alpha.h (revision 263611)
>> +++ 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/expmed.c
>> ===================================================================
>> --- gcc/expmed.c (revision 263611)
>> +++ gcc/expmed.c (working copy)
>> @@ -579,8 +579,12 @@ static bool
>> simple_mem_bitfield_p (rtx op0, poly_uint64 bitsize, poly_uint64 bitnum,
>> machine_mode mode, poly_uint64 *bytenum)
>> {
>> + poly_int64 ibit = bitnum;
>> + poly_int64 ibyte;
>> + if (!multiple_p (ibit, BITS_PER_UNIT, &ibyte))
>> + return false;
>> + *bytenum = ibyte;
>> return (MEM_P (op0)
>> - && multiple_p (bitnum, BITS_PER_UNIT, bytenum)
>> && known_eq (bitsize, GET_MODE_BITSIZE (mode))
>> && (!targetm.slow_unaligned_access (mode, MEM_ALIGN (op0))
>> || (multiple_p (bitnum, GET_MODE_ALIGNMENT (mode))
>
> Do we have a genuinely negative bit offset here? Seems like the callers
> would need to be updated if so, since the code is consistent in treating
> the offset as unsigned.
>
Aehm, yes.
The test case plural.i contains this:
static const yytype_int8 yypgoto[] =
{
-10, -10, -1
};
static const yytype_uint8 yyr1[] =
{
0, 16, 17, 18, 18, 18, 18, 18, 18, 18,
18, 18, 18, 18
};
yyn = yyr1[yyn];
yystate = yypgoto[yyn - 16] + *yyssp;
There will probably a reason why yyn can never be 0
in yyn = yyr1[yyn]; but it is not really obvious.
In plural.i.228t.optimized we have:
pretmp_400 = yypgoto[-16];
_385 = (int) pretmp_400;
goto <bb 69>; [100.00%]
which is expanded to:
(insn 927 926 928 59 (set (reg/f:DI 369)
(symbol_ref:DI ("yypgoto") [flags 0x6] <var_decl 0x7f76af1d1c60
yypgoto>)) -1
(nil))
(insn 928 927 929 59 (set (reg:DI 372)
(const_int 1 [0x1])) -1
(nil))
(insn 929 928 930 59 (set (reg:DI 371)
(ashift:DI (reg:DI 372)
(const_int 61 [0x3d]))) -1
(expr_list:REG_EQUAL (const_int 2305843009213693952 [0x2000000000000000])
(nil)))
(insn 930 929 931 59 (set (reg/f:DI 370)
(plus:DI (reg/f:DI 369)
(reg:DI 371))) -1
(nil))
(insn 931 930 932 59 (set (reg:DI 375)
(plus:DI (reg/f:DI 370)
(const_int 1 [0x1]))) -1
(nil))
(insn 932 931 933 59 (set (reg:DI 376)
(mem/u/c:DI (and:DI (plus:DI (reg/f:DI 370)
(const_int -16 [0xfffffffffffffff0]))
(const_int -8 [0xfffffffffffffff8])) [0 S8 A64])) -1
(nil))
(insn 933 932 934 59 (set (reg:DI 377)
(ashift:DI (reg:DI 376)
(minus:DI (const_int 64 [0x40])
(ashift:DI (and:DI (reg:DI 375)
(const_int 7 [0x7]))
(const_int 3 [0x3]))))) -1
(nil))
(insn 934 933 935 59 (set (reg:DI 374)
(ashiftrt:DI (reg:DI 377)
(const_int 56 [0x38]))) -1
(nil))
Certainly there is also a target bug, because
(define_insn "*movdi"
[(set (match_operand:DI 0 "nonimmediate_operand"
"=r,r,r,r,r,r,r,r, m, *f,*f, Q, r,*f")
(match_operand:DI 1 "input_operand"
"rJ,K,L,T,s,n,s,m,rJ,*fJ, Q,*f,*f, r"))]
"register_operand (operands[0], DImode)
|| reg_or_0_operand (operands[1], DImode)"
"@
mov %r1,%0
lda %0,%1($31)
ldah %0,%h1($31)
#
#
#
lda %0,%1
ldq%A1 %0,%1
stq%A0 %r1,%0
fmov %R1,%0
ldt %0,%1
stt %R1,%0
ftoit %1,%0
itoft %1,%0"
overload 7 of 14, has constraint op0 = r, op1 = s.
and assembles to lda %0,%1 so s matches symbol_ref+any_integer
I think even without the bug in the expansion, that could
happen if I write
volatile x = 0; if (x) { char *y = &yypgoto[0x20000000000000]; }
I think there should be a splitter for large offsets that gets enabled
after reload and splits the mov r,s+x into mov r,s; add r,x or something
appropriate.
So let's see, how does insn 929 get expanded?
Index: emit-rtl.c
===================================================================
--- emit-rtl.c (Revision 263644)
+++ emit-rtl.c (Arbeitskopie)
@@ -4023,6 +4023,8 @@
insn = as_a <rtx_insn *> (rtx_alloc (INSN));
INSN_UID (insn) = cur_insn_uid++;
+ if (INSN_UID (insn) == 929)
+ asm ("int3");
PATTERN (insn) = pattern;
INSN_CODE (insn) = -1;
REG_NOTES (insn) = NULL;
gdb --args ../gcc-build-alpha-linux-gnu/gcc/cc1 -fpreprocessed plural.i -quiet
-dumpbase plural.i -mlong-double-128 -mieee "-mfp-rounding-mode=d" -auxbase
plural -g -O2 -Wall -Werror -Wundef -Wwrite-strings -Wstrict-prototypes
-Wold-style-definition "-std=gnu11" -fgnu89-inline -fmerge-all-constants
-fno-stack-protector -frounding-math -fno-math-errno "-ftls-model=initial-exec"
-o plural.s
#0 make_insn_raw (pattern=pattern@entry=0x7ffff6e60c30) at
../../gcc-trunk/gcc/emit-rtl.c:4031
#1 0x00000000007fde49 in emit_insn (x=x@entry=0x7ffff6e60c30) at
../../gcc-trunk/gcc/emit-rtl.c:5108
#2 0x0000000000a92623 in expand_binop_directly (icode=CODE_FOR_ashldi3,
mode=mode@entry=E_DImode,
binoptab=binoptab@entry=ashl_optab, op0=op0@entry=0x7ffff6e60be8,
op1=op1@entry=0x7ffff6ecf860,
target=target@entry=0x7ffff6e60bd0, unsignedp=unsignedp@entry=0,
methods=methods@entry=OPTAB_WIDEN, last=last@entry=0x7ffff6e618c0)
at ../../gcc-trunk/gcc/optabs.c:1114
#3 0x0000000000a8fe62 in expand_binop (mode=mode@entry=E_DImode,
binoptab=<optimized out>,
binoptab@entry=ashl_optab, op0=op0@entry=0x7ffff6e60be8, op1=<optimized
out>,
target=target@entry=0x7ffff6e60bd0, unsignedp=unsignedp@entry=0,
methods=methods@entry=OPTAB_WIDEN)
at ../../gcc-trunk/gcc/optabs.c:1186
#4 0x0000000000f1421b in alpha_emit_set_const_1
(target=target@entry=0x7ffff6e60bd0,
mode=mode@entry=E_DImode, c=c@entry=2305843009213693952, n=n@entry=2,
no_output=no_output@entry=false) at
../../gcc-trunk/gcc/config/alpha/alpha.c:1925
#5 0x0000000000f147a0 in alpha_emit_set_const (target=0x7ffff6e60bd0,
mode=E_DImode,
c=2305843009213693952, n=3, no_output=<optimized out>)
at ../../gcc-trunk/gcc/config/alpha/alpha.c:2048
#6 0x0000000000f1545a in alpha_split_const_mov (mode=mode@entry=E_DImode,
operands=operands@entry=0x7fffffffc7b0) at
../../gcc-trunk/gcc/config/alpha/alpha.c:2214
#7 0x0000000000f15536 in alpha_expand_mov (mode=mode@entry=E_DImode,
operands=operands@entry=0x7fffffffc7b0) at
../../gcc-trunk/gcc/config/alpha/alpha.c:2263
#8 0x00000000010f0f67 in gen_movdi (operand0=0x7ffff6e60bd0,
operand1=0x7ffff6da8580)
at ../../gcc-trunk/gcc/config/alpha/alpha.md:4062
#9 0x0000000000833893 in operator() (a1=0x7ffff6da8580, a0=0x7ffff6e60bd0,
this=<optimized out>)
at ../../gcc-trunk/gcc/recog.h:301
#10 emit_move_insn_1 (x=x@entry=0x7ffff6e60bd0, y=y@entry=0x7ffff6da8580)
at ../../gcc-trunk/gcc/expr.c:3694
#11 0x0000000000833b86 in emit_move_insn (x=x@entry=0x7ffff6e60bd0,
y=y@entry=0x7ffff6da8580)
at ../../gcc-trunk/gcc/expr.c:3790
#12 0x0000000000810aa0 in copy_to_mode_reg (mode=mode@entry=E_DImode,
x=0x7ffff6da8580)
at ../../gcc-trunk/gcc/explow.c:632
#13 0x0000000000a8b69a in maybe_legitimize_operand (op=0x7fffffffca40,
opno=<optimized out>,
icode=CODE_FOR_adddi3) at ../../gcc-trunk/gcc/optabs.c:7168
#14 maybe_legitimize_operands (icode=icode@entry=CODE_FOR_adddi3,
opno=opno@entry=0,
nops=nops@entry=3, ops=ops@entry=0x7fffffffca10) at
../../gcc-trunk/gcc/optabs.c:7292
#15 0x0000000000a8bcde in maybe_gen_insn (icode=icode@entry=CODE_FOR_adddi3,
nops=nops@entry=3,
ops=ops@entry=0x7fffffffca10) at ../../gcc-trunk/gcc/optabs.c:7311
#16 0x0000000000a925ff in expand_binop_directly (icode=CODE_FOR_adddi3,
mode=mode@entry=E_DImode,
binoptab=binoptab@entry=add_optab, op0=op0@entry=0x7ffff6e60b40,
op1=op1@entry=0x7ffff6da8580,
target=target@entry=0x0, unsignedp=unsignedp@entry=1,
methods=methods@entry=OPTAB_LIB_WIDEN,
last=last@entry=0x7ffff6e61880) at ../../gcc-trunk/gcc/optabs.c:1098
#17 0x0000000000a8fe62 in expand_binop (mode=mode@entry=E_DImode,
binoptab=<optimized out>,
op0=op0@entry=0x7ffff6e60b40, op1=<optimized out>, target=target@entry=0x0,
unsignedp=unsignedp@entry=1, methods=OPTAB_LIB_WIDEN) at
../../gcc-trunk/gcc/optabs.c:1186
#18 0x0000000000a923e3 in expand_simple_binop (mode=mode@entry=E_DImode,
code=code@entry=PLUS,
op0=op0@entry=0x7ffff6e60b40, op1=<optimized out>, target=target@entry=0x0,
unsignedp=unsignedp@entry=1, methods=<optimized out>,
methods@entry=OPTAB_LIB_WIDEN)
at ../../gcc-trunk/gcc/optabs.c:917
#19 0x0000000000f12d6f in alpha_legitimize_address_1 (x=0x7ffff6e60b40,
x@entry=0x7ffff6e60ba0,
scratch=scratch@entry=0x0, mode=<optimized out>) at
../../gcc-trunk/gcc/config/alpha/alpha.c:1159
#20 0x0000000000f133cb in alpha_legitimize_address (x=0x7ffff6e60ba0,
oldx=<optimized out>,
mode=<optimized out>) at ../../gcc-trunk/gcc/config/alpha/alpha.c:1177
#21 0x0000000000810922 in memory_address_addr_space (mode=E_QImode,
x=0x7ffff6e60ba0,
as=<optimized out>) at ../../gcc-trunk/gcc/explow.c:450
#22 0x00000000007f689d in change_address_1 (memref=memref@entry=0x7ffff6e60b88,
mode=mode@entry=E_QImode, addr=addr@entry=0x7ffff6e60ba0,
validate=validate@entry=1,
inplace=inplace@entry=false) at ../../gcc-trunk/gcc/emit-rtl.c:2288
#23 0x00000000007fab5c in adjust_address_1 (memref=memref@entry=0x7ffff6e60b88,
mode=mode@entry=E_QImode, offset=..., validate=validate@entry=1,
adjust_address=adjust_address@entry=1,
adjust_object=adjust_object@entry=1, size=...,
size@entry=...) at ../../gcc-trunk/gcc/emit-rtl.c:2420
#24 0x000000000081942a in extract_bit_field_1
(str_rtx=str_rtx@entry=0x7ffff6e60b88,
bitsize=bitsize@entry=..., bitnum=..., bitnum@entry=...,
unsignedp=unsignedp@entry=0,
target=target@entry=0x0, mode=mode@entry=E_QImode, tmode=<optimized out>,
tmode@entry=E_QImode,
reverse=reverse@entry=false, fallback_p=fallback_p@entry=true,
alt_rtl=alt_rtl@entry=0x0)
at ../../gcc-trunk/gcc/expmed.c:1805
#25 0x000000000081a063 in extract_bit_field (str_rtx=0x7ffff6e60b88,
bitsize=..., bitnum=...,
unsignedp=0, target=target@entry=0x0, mode=mode@entry=E_QImode,
tmode=tmode@entry=E_QImode,
reverse=false, alt_rtl=alt_rtl@entry=0x0) at
../../gcc-trunk/gcc/expmed.c:2097
#26 0x000000000082f828 in expand_expr_real_1 (exp=<optimized out>,
target=<optimized out>,
tmode=<optimized out>, modifier=EXPAND_NORMAL, alt_rtl=0x0,
inner_reference_p=<optimized out>)
at ../../gcc-trunk/gcc/expr.c:10801
#27 0x000000000083047a in expand_expr_real_1 (exp=0x7ffff6df8f30,
target=<optimized out>,
tmode=E_SImode, modifier=EXPAND_NORMAL, alt_rtl=0x0,
inner_reference_p=<optimized out>)
at ../../gcc-trunk/gcc/expr.c:9861
#28 0x000000000083d9de in expand_expr (modifier=EXPAND_NORMAL, mode=E_SImode,
target=0x0,
exp=0x7ffff6df8f30) at ../../gcc-trunk/gcc/expr.h:279
#29 expand_expr_real_2 (ops=ops@entry=0x7fffffffd8a0, target=<optimized out>,
tmode=E_SImode,
modifier=modifier@entry=EXPAND_NORMAL) at ../../gcc-trunk/gcc/expr.c:8455
#30 0x000000000070ba16 in expand_gimple_stmt_1 (stmt=stmt@entry=0x7ffff6e0eb40)
at ../../gcc-trunk/gcc/cfgexpand.c:3674
#31 0x000000000070d4ad in expand_gimple_stmt (stmt=stmt@entry=0x7ffff6e0eb40)
at ../../gcc-trunk/gcc/cfgexpand.c:3734
#32 0x000000000070dbbe in expand_gimple_basic_block (bb=0x7ffff6deb478,
disable_tail_calls=disable_tail_calls@entry=false) at
../../gcc-trunk/gcc/cfgexpand.c:5770
#33 0x0000000000713597 in (anonymous namespace)::pass_expand::execute
(this=<optimized out>,
fun=0x7ffff70ab4d0) at ../../gcc-trunk/gcc/cfgexpand.c:6373
#34 0x0000000000ab4e25 in execute_one_pass (pass=pass@entry=0x1b28700)
at ../../gcc-trunk/gcc/passes.c:2446
#35 0x0000000000ab5738 in execute_pass_list_1 (pass=0x1b28700) at
../../gcc-trunk/gcc/passes.c:2535
#36 0x0000000000ab5795 in execute_pass_list (fn=0x7ffff70ab4d0, pass=<optimized
out>)
at ../../gcc-trunk/gcc/passes.c:2546
#37 0x000000000074efe8 in cgraph_node::expand (this=0x7ffff6cfb840)
at ../../gcc-trunk/gcc/cgraphunit.c:2116
#38 0x00000000007508a1 in expand_all_functions () at
../../gcc-trunk/gcc/cgraphunit.c:2254
#39 symbol_table::compile (this=this@entry=0x7ffff6ecc000) at
../../gcc-trunk/gcc/cgraphunit.c:2605
#40 0x0000000000752c75 in symbol_table::finalize_compilation_unit
(this=0x7ffff6ecc000)
at ../../gcc-trunk/gcc/cgraphunit.c:2698
#41 0x0000000000b97d6d in compile_file () at ../../gcc-trunk/gcc/toplev.c:480
#42 0x000000000059cb5e in do_compile () at ../../gcc-trunk/gcc/toplev.c:2165
#43 toplev::main (this=this@entry=0x7fffffffdce0, argc=argc@entry=28,
argv=argv@entry=0x7fffffffdde8)
at ../../gcc-trunk/gcc/toplev.c:2300
#44 0x000000000059ecb7 in main (argc=28, argv=0x7fffffffdde8) at
../../gcc-trunk/gcc/main.c:39
(gdb) frame 26
#26 0x000000000082f828 in expand_expr_real_1 (exp=<optimized out>,
target=<optimized out>,
tmode=<optimized out>, modifier=EXPAND_NORMAL, alt_rtl=0x0,
inner_reference_p=<optimized out>)
at ../../gcc-trunk/gcc/expr.c:10801
10801 ext_mode, ext_mode, reversep,
alt_rtl);
(gdb) list
10796 reversep = TYPE_REVERSE_STORAGE_ORDER (type);
10797
10798 op0 = extract_bit_field (op0, bitsize, bitpos, unsignedp,
10799 (modifier == EXPAND_STACK_PARM
10800 ? NULL_RTX : target),
10801 ext_mode, ext_mode, reversep,
alt_rtl);
10802
10803 /* If the result has a record type and the mode of OP0 is an
10804 integral mode then, if BITSIZE is narrower than this mode
10805 and this is for big-endian data, we must put the field
(gdb) p bitpos
$1 = {<poly_int_pod<1u, long>> = {coeffs = {-128}}, <No data fields>}
(gdb) frame 25
#25 0x000000000081a063 in extract_bit_field (str_rtx=0x7ffff6e60b88,
bitsize=..., bitnum=...,
unsignedp=0, target=target@entry=0x0, mode=mode@entry=E_QImode,
tmode=tmode@entry=E_QImode,
reverse=false, alt_rtl=alt_rtl@entry=0x0) at
../../gcc-trunk/gcc/expmed.c:2097
2097 target, mode, tmode, reverse, true,
alt_rtl);
(gdb) list
2092 return extract_bit_field_1 (str_rtx, ibitsize, ibitnum, unsignedp,
2093 target, mode, tmode, reverse, true,
alt_rtl);
2094 }
2095
2096 return extract_bit_field_1 (str_rtx, bitsize, bitnum, unsignedp,
2097 target, mode, tmode, reverse, true,
alt_rtl);
2098 }
2099 ^L
2100 /* Use shifts and boolean operations to extract a field of BITSIZE bits
2101 from bit BITNUM of OP0. If OP0_MODE is defined, it is the mode of
OP0,
(gdb) p/x bitnum
$3 = {<poly_int_pod<1u, unsigned long>> = {coeffs = {0xffffffffffffff80}}, <No
data fields>}
(gdb) frame 24
#24 0x000000000081942a in extract_bit_field_1
(str_rtx=str_rtx@entry=0x7ffff6e60b88,
bitsize=bitsize@entry=..., bitnum=..., bitnum@entry=...,
unsignedp=unsignedp@entry=0,
target=target@entry=0x0, mode=mode@entry=E_QImode, tmode=<optimized out>,
tmode@entry=E_QImode,
reverse=reverse@entry=false, fallback_p=fallback_p@entry=true,
alt_rtl=alt_rtl@entry=0x0)
at ../../gcc-trunk/gcc/expmed.c:1805
1805 op0 = adjust_bitfield_address (op0, mode1, bytenum);
(gdb) list
1800 /* Extraction of a full MODE1 value can be done with a load as long as
1801 the field is on a byte boundary and is sufficiently aligned. */
1802 poly_uint64 bytenum;
1803 if (simple_mem_bitfield_p (op0, bitsize, bitnum, mode1, &bytenum))
1804 {
1805 op0 = adjust_bitfield_address (op0, mode1, bytenum);
1806 if (reverse)
1807 op0 = flip_storage_order (mode1, op0);
1808 return convert_extracted_bit_field (op0, mode, tmode, unsignedp);
1809 }
(gdb) p/x bytenum
$5 = {<poly_int_pod<1u, unsigned long>> = {coeffs = {0x1ffffffffffffff0}}, <No
data fields>}
The byte offset is completely wrong now, due to the bitnum was initially a
negative integer
and got converted to unsigned. At the moment when that is converted to byte
offsets it is
done wrong. I think it is too much to change everything to signed arithmetics,
but at least when converting a bit pos to a byte pos, it should be done in
signed arithmetics.
So yes, I think a negative offset can happen. And it must be handled correctly.
Thanks
Bernd.
