This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [PATCH, PR 57748] Check for out of bounds access


Hi,

On Thu, 24 Oct 2013 11:56:52, Richard Biener wrote:
>
> On Thu, Oct 24, 2013 at 10:37 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>>> if bitregion_start/end are used after the adjust_address call then they have
>>> to be adjusted (or bitpos left in place). In fact why we apply byte-parts
>>> of bitpos here only if offset != 0 is weird.
>>
>> Presumably to be able to do arithmetic on symbols when the offset is variable,
>> which can save one addition in the final code.
>
> Doesn't that also apply to arithmetic on symbols when the offset is NULL?
> But yes, the codegen example posted shows this kind of difference
> (though it doesn't seem to save anything for that case). I'd have expected
> a more explicit guarding of this case, like with MEM_P (to_rtx)
> && SYMBOL_REF_P (XEXP (to_rtx, 0)) though, eventually even looking
> whether the resulting address is legitimate. But ... doesn't forwprop
> fix this up later anyway?
>
> Either way, bitregion_start/end is clearly wrong after we hit this case.
> I'm always fast approving removal of "odd" code (less-code-is-better
> doctrine...), but in this case maybe the code can be improved instead.
> The comment before it is also odd and likely only applies to later
> added restrictions. Just to quote it again:
>
> if (offset != 0)
> {
> ...
> /* A constant address in TO_RTX can have VOIDmode, we must not try
> to call force_reg for that case. Avoid that case. */
> if (MEM_P (to_rtx)
> && GET_MODE (to_rtx) == BLKmode
> && GET_MODE (XEXP (to_rtx, 0)) != VOIDmode
> && bitsize> 0
> && (bitpos % bitsize) == 0
> && (bitsize % GET_MODE_ALIGNMENT (mode1)) == 0
> && MEM_ALIGN (to_rtx) == GET_MODE_ALIGNMENT (mode1))
> {
> to_rtx = adjust_address (to_rtx, mode1, bitpos / BITS_PER_UNIT);
> bitpos = 0;
> }
>
> to_rtx = offset_address (to_rtx, offset_rtx,
> highest_pow2_factor_for_target (to,
> offset));
> }
>

In the initial commit from 1998 where this was introduced that
block contained an explicit force_reg call, but not that comment.

Bernd.
>
> Thanks,
> Richard.
>
>> --
>> Eric Botcazou 		 	   		  

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]