On Mon, May 23, 2016 at 4:29 PM, Bin.Cheng <amker.ch...@gmail.com> wrote:
> On Mon, May 23, 2016 at 3:16 PM, Richard Biener
> <richard.guent...@gmail.com> wrote:
>> On Mon, May 23, 2016 at 3:23 PM, Bin Cheng <bin.ch...@arm.com> wrote:
>>> Hi,
>>> When working on PR69710, I ran into this latent bug in which alignment 
>>> information is wrongly updated for pointer variables.  It results in memory 
>>> exceptions on x86_64 after patch for PR69710.  Scenario is that 
>>> copy_ref_info tries to update base's alignment in TARGET_MEM_REF[base + 
>>> index << step].  But case with NULL TMR_STEP (which implies the step is 1) 
>>> is not handled here.  This patch fixes the bug by simply checking NULL 
>>> TMR_STEP.  The conditions about TMR_STEP could be relaxed if TMR_INDEX is 
>>> an induction variable which has aligned initial value and step.  But that 
>>> needs non-trivial code refactoring since copy_ref_info is uses by different 
>>> parts of compiler.
>>>
>>> Bootstrap and test on x86_64.  Is it OK?
>>
>> I think it is ok but having !TMR_STEP when TMR_INDEX is non-NULL is
>> IMHO bad and we should enforce
>> that this doesn't happen in TMR verification in
>> tree-cfg.c:verify_types_in_gimple_reference.  I also notice
>> that if TMR_INDEX is NULL then TMR_STEP being NULL shouldn't block
> Yeah, patch updated slightly to handle this case.

Ok.

Thanks,
Richard.

> Thanks,
> bin
>> alignment transfer (in that case
>> TMR_INDEX2 is likely non-NULL though).
>>
>> Thanks,
>> Richard.
>>
>>> Thanks,
>>> bin
>>>
>>> 2016-05-20 Bin Cheng  <bin.ch...@arm.com>
>>>
>>>         * tree-ssa-address.c (copy_ref_info): Check null TMR_STEP.

Reply via email to