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, Part 2


Hi Eric,


On Fri, 27 Sep 2013 10:36:57, Eric Botcazou wrote:
>
>> Sure, but the modifier is not meant to force something into memory,
>> especially when it is already in an register. Remember, we are only
>> talking of structures here, and we only want to access one member.
>>
>> It is more the other way round:
>> It says: "You do not have to load the value in a register, if it is already
>> in memory I'm happy"
>
> EXPAND_MEMORY means we are interested in a memory result, even if
> the memory is constant and we could have propagated a constant value. */
>
> We definitely want to propagate constant values here, look at the code below.
> And it already lists explicit cases where we really need to splill to memory.
>
> --
> Eric Botcazou


I'd like to continue this discussion by proposing this updated patch.

I have now added a new value "EXPAND_REFERENCE" to the expand_modifier
enumeration. It is almost like EXPAND_MEMORY but it does not interfere with
constant values.

I have done the same modification to VIEW_CONVERT_EXPR too, because
this is a possible inner reference, itself. It is however inherently hard to
test around this code.

To understand this patch it is good to know what type of object the
return value "tem" of get_inner_reference can be.

From the program logic at get_inner_reference it is clear that the
return value may *not* be BIT_FIELD_REF, COMPONENT_REF, ARRAY_REF,
ARRAY_RANGE_REF, REALPART_EXPR, IMAGPART_EXPR. The result may
be VIEW_CONVERT_EXPR only on a STRICT_ALIGNMENT target. This is probably
further restricted because exp is gimplified.

Usually the result will be a MEM_REF or a SSA_NAME of the memory where
the structure is to be found.

When you look at where EXPAND_MEMORY is handled you see it is special-cased
in TARGET_MEM_REF, MEM_REF, ARRAY_REF, COMPONENT_REF, BIT_FIELD_REF,
ARRAY_RANGE_REF.

At TARGET_MEM_REF, MEM_REF, VIEW_CONVERT_EXPR, it should be the
same if EXPAND_MEMORY, EXPAND_WRITE or EXPAND_REFERENCE is given:
If it is an unaligned memory, we just return the unaligned reference.

This was missing for VIEW_CONVERT_EXPR, and unfortunately I have no test case,
because it is only a problem for STRICT_ALIGNMENT targets, and even there it will
certainly be really hard to find test cases that exercise this code.

In ARRAY_REF, COMPONENT_REF, BIT_FIELD_REF, ARRAY_RANGE_REF
we do not have to touch the handling of the outer modifier. However we pass
EXPAND_REFERENCE to the inner object, which should not be a recursive
use of any ARRAY_REF, COMPONENT_REF, BIT_FIELD_REF, ARRAY_RANGE_REF.

TARGET_MEM_REF, MEM_REF and VIEW_CONVERT_EXPR know how to handle
EXPAND_REFERENCE, anything else handles it like EXPAND_NORMAL.


OK, what do you think of it now?

Thanks
Bernd. 		 	   		  

Attachment: changelog-pr57748-2.txt
Description: Text document

Attachment: patch-pr57748-2.diff
Description: Binary data


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