[PATCH] Fix register corruption bug in ree

Jeff Law law@redhat.com
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 mailing list