This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, PR 57748] Check for out of bounds access
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Bernd Edlinger <bernd dot edlinger at hotmail dot de>
- Cc: Martin Jambor <mjambor at suse dot cz>, GCC Patches <gcc-patches at gcc dot gnu dot org>, Eric Botcazou <ebotcazou at adacore dot com>
- Date: Wed, 23 Oct 2013 17:36:41 +0200
- Subject: Re: [PATCH, PR 57748] Check for out of bounds access
- Authentication-results: sourceware.org; auth=none
- References: <DUB122-W396D54115D70C9FF68A8D2E43C0 at phx dot gbl> <CAFiYyc13uxba37WySOxXf14UVJW15_DM7jggNGbR4H2cK96Eiw at mail dot gmail dot com> <DUB122-W323D4AAAEA1AFA890BF243E43C0 at phx dot gbl> <CAFiYyc13wC0jmZ4xELAiL5L5Sbh0yauZWViP2hTdR+H4s0iPSw at mail dot gmail dot com> <DUB122-W4D571A91831593BA22438E43C0 at phx dot gbl> <CAFiYyc1ECAdBJTkcziSL_f9SQyYUPY9k19QWqkN+RfLO=i2zaQ at mail dot gmail dot com> <20130910193228 dot GE6732 at virgil dot suse> <DUB122-W22DAEEE94B5ED3308F6967E4390 at phx dot gbl> <CAFiYyc3AHggjJbEmZvC_fh67o+LVpTBRdwyUv8T6EhQFE2vNWQ at mail dot gmail dot com> <DUB122-W281E996A4885309956418EE4250 at phx dot gbl> <20130916230945 dot GK6732 at virgil dot suse> <DUB122-W209385567C3551814F6331E4020 at phx dot gbl>
On Tue, Oct 22, 2013 at 3:50 PM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
> Hi,
>
> On Tue, 17 Sep 2013 01:09:45, Martin Jambor wrote:
>>> @@ -4773,6 +4738,8 @@ expand_assignment (tree to, tree from, b
>>> if (MEM_P (to_rtx)
>>> && GET_MODE (to_rtx) == BLKmode
>>> && GET_MODE (XEXP (to_rtx, 0)) != VOIDmode
>>> + && bitregion_start == 0
>>> + && bitregion_end == 0
>>> && bitsize> 0
>>> && (bitpos % bitsize) == 0
>>> && (bitsize % GET_MODE_ALIGNMENT (mode1)) == 0
>>>
>> ...
>>
>> I'm not sure to what extent the hunk adding tests for bitregion_start
>> and bitregion_end being zero is connected to this issue. I do not see
>> any of the testcases exercising that path. If it is indeed another
>> problem, I think it should be submitted (and potentially committed) as
>> a separate patch, preferably with a testcase.
>>
>
> Meanwhile I am able to give an example where that code is executed
> with bitpos = 64, bitsize=32, bitregion_start = 32, bitregion_end = 95.
>
> Afterwards bitpos=0, bitsize=32, which is completely outside
> bitregion_start=32, bitregion_end=95.
>
> However this can only be seen in the debugger, as the store_field
> goes thru a code path that does not look at bitregion_start/end.
>
> Well that is at least extremely ugly, and I would not be sure, that
> I cannot come up with a sample that crashes or creates wrong code.
>
> Currently I think that maybe the best way to fix that would be this:
>
> --- gcc/expr.c 2013-10-21 08:27:09.546035668 +0200
> +++ gcc/expr.c 2013-10-22 15:19:56.749476525 +0200
> @@ -4762,6 +4762,9 @@ expand_assignment (tree to, tree from, b
> && MEM_ALIGN (to_rtx) == GET_MODE_ALIGNMENT (mode1))
> {
> to_rtx = adjust_address (to_rtx, mode1, bitpos / BITS_PER_UNIT);
> + bitregion_start = 0;
> + if (bitregion_end>= (unsigned HOST_WIDE_INT) bitpos)
> + bitregion_end -= bitpos;
> bitpos = 0;
> }
>
> Any suggestions?
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. OTOH this code is _very_ old...
what happens if you remove the whole case?
Richard.
>
>
> Regards
> Bernd.