[PATCH] Fix register corruption bug in ree
Thu Sep 11 20:10:00 GMT 2014
On 09/08/14 11:21, Wilco Dijkstra wrote:
>> Thanks! Jakub noticed a potential problem in this area a while back,
>> but I never came up with any code to trigger and have kept that issue on
>> my todo list ever since.
>> Rather than ensuring the inserted copy write a single register, it seems
>> to me we're better off ensuring that the number of hard registers hasn't
>> changed. Obviously that's not much of an issue for things like aarch64,
>> x86_64, but for the embedded 32 bit parts it's likely much more important.
>> So I think the test is something like
>> x = get_extended_src_reg (SET_SRC (PATTERN (cand->insn)))
>> if (HARD_REGNO_NREGS (REGNO (new_reg), cand->mode)
>> != HARD_REGNO_NREGS (REGNO (new_reg), GET_MODE (SET_DEST (*orig_set))
>> return false;
> I'm not 100% sure the rest of the code will correctly deal with multiple
> registers in all possible combinations. I think adding more asserts would help
> avoid similar silent codegeneration failures. For example, should widening
> from 1 to 2 registers:
> (set (reg:TI x2) (zero_extend:TI (reg:DI x2)))
> be regarded as a "copy_needed"?
No in this case.
> And what about TI x2 = zero_extend DI x3?
yes in this case.
> Both cases have overlaps, but copy_needed is set differently...
Right. copy_needed isn't a property of whether or not we have an
overlap. Any time the source and destination refer to different
registers (regardless of overlap), then need_copy should be set.
> I also noticed there are more places where get_extended_src_reg is not used
> (eg. in the code that emits the final copy).
It's certainly possible I missed additional cases to use that helper
when it was added. Thanks for taking care of it.
Feel free to pass along asserts you think are appropriate.
> Anyway here is the modified check:
Thanks. Just needs an updated ChangeLog and it's good for the trunk.
More information about the Gcc-patches