On 8/28/18 3:37 AM, Richard Biener wrote: > On Mon, Aug 27, 2018 at 10:31 PM Jeff Law <l...@redhat.com> wrote: >> >> >> We recently changes tree-ssa-dse.c to not trim stores outside the bounds >> of the referenced object. This is generally a good thing. >> >> However, there are cases where the object doesn't have a usable size. >> We see this during kernel builds, at least on the microblaze target. >> >> We've got... >> >> _1 = p_47(D)->stack; >> childregs_48 = &MEM[(void *)_1 + 8040B]; >> [ ... ] >> memset (childregs_48, 0, 152); >> [ ... ] >> MEM[(struct pt_regs *)_1 + 8040B].pt_mode = 1; >> >> >> We want to trim the memset call as the assignment to pt_mode is the last >> word written by the memset call, thus making the store of that word via >> memset dead. >> >> Our ref->base is: >> >> (gdb) p debug_tree ($66) >> <mem_ref 0x7fffe8b946e0 >> type <void_type 0x7fffe9e210a8 void VOID >> align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type >> 0x7fffe9e210a8 >> pointer_to_this <pointer_type 0x7fffe9e21150>> >> >> arg:0 <ssa_name 0x7fffe8f8a2d0 >> type <pointer_type 0x7fffe9e21150 type <void_type 0x7fffe9e210a8 >> void> >> public unsigned SI >> size <integer_cst 0x7fffe9e0d678 constant 32> >> unit-size <integer_cst 0x7fffe9e0d690 constant 4> >> align:32 warn_if_not_align:0 symtab:0 alias-set 8 >> canonical-type 0x7fffe9e21150 context <translation_unit_decl >> 0x7fffe8f72438 /tmp/process.i> >> pointer_to_this <pointer_type 0x7fffe9e28bd0> >> reference_to_this <reference_type 0x7fffe9e282a0>> >> visited >> def_stmt _1 = p_47(D)->stack; >> version:1 >> ptr-info 0x7fffe8f65fa8> >> arg:1 <integer_cst 0x7fffe8f65f60 type <pointer_type 0x7fffe9e21150> >> constant 8040>> >> >> >> Note the void type. Those do not have a suitable TYPE_SIZE_UNIT, thus >> causing an ICE when we try to reference it within compute_trims: >> /* But don't trim away out of bounds accesses, as this defeats >> proper warnings. */ >> if (*trim_tail >> && compare_tree_int (TYPE_SIZE_UNIT (TREE_TYPE (ref->base)), >> last_orig) <= 0) >> >> >> The fix is obvious enough. Don't do the check when the underlying type >> has no usable TYPE_SIZE_UNIT. >> >> I pondered moving the tests into the code that determines whether or not >> we do byte tracking DSE, but decided the current location was fine. >> >> Bootstrapped and regression tested on x86. Also verified the original >> testfile will build with a microblaze cross compiler. >> >> Installing on the trunk momentarily. > > Hmm, you seem to get ref->base from an address but you know types > on addresses do not have any meaning, so ... how does It doesn't affect correctness. Essentially when we have information which indicates there might be an out of bounds write, we leave the statement as-is.
So if the type specified a smaller size than reality, then we potentially might suppress an optimization. If the type specified a large size than reality, then nothing changes. > > /* But don't trim away out of bounds accesses, as this defeats > proper warnings. > > We could have a type with no TYPE_SIZE_UNIT or we could have a VLA > where TYPE_SIZE_UNIT is not a constant. */ > if (*trim_tail > && TYPE_SIZE_UNIT (TREE_TYPE (ref->base)) > && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (ref->base))) == INTEGER_CST > && compare_tree_int (TYPE_SIZE_UNIT (TREE_TYPE (ref->base)), > last_orig) <= 0) > *trim_tail = 0; > > make any sense in the above case where ref->base is even based on a > pointer? I'd say the > above should be at least > > if (*trim_tail > && DECL_P (ref->base) > && .... > > ? Not sure if we ever have decls with incomplete types so eventually > the new check > isn't needed. We could have something casted to a void type. Can't remember where it happened, but it certainly came up. jeff
