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] PR69195, Reload confused by invalid reg equivs


On Sat, Mar 12, 2016 at 6:07 PM, Jeff Law <law@redhat.com> wrote:
> On 03/12/2016 04:10 AM, Richard Biener wrote:
>>
>> On March 12, 2016 10:29:40 AM GMT+01:00, Jakub Jelinek <jakub@redhat.com>
>> wrote:
>>>
>>> On Sat, Mar 12, 2016 at 07:37:25PM +1030, Alan Modra wrote:
>>>>>
>>>>> I believe Alan's point is DSE deleted the assignment to x which
>>>
>>> can't be
>>>>>
>>>>> right as long as we've left in goto *&x.
>>>>>
>>>>> The goto *&x should be a use of x and thus should have kept the
>>>
>>> assignment
>>>>>
>>>>> live.
>>>>
>>>>
>>>> Right, I wasn't trying to say that ira.c:indirect_jump_optimize is
>>>> OK.  It needs the patch I posted or perhaps even better a test of
>>>> DF_REF_INSN_INFO rather than !DF_REF_IS_ARTIFICIAL (simply because
>>>
>>> the
>>>>
>>>> flag test is reading another field, and we need to read
>>>> DF_REF_INSN_INFO anyway).
>>>
>>>
>>> Ok, that was my point.  BTW, DSE isn't the only one that deletes x = 0;
>>> cddce deletes it too.  -fno-tree-dse -fno-tree-dce preserves it till
>>> expansion.
>>
>>
>> GIMPLE_GOTO doesn't have VOPs and I don't think that we'd want VUSEs on
>> all gotos. But just having them on indirect gotos would be inconsistent.
>>
>> I believe the code is undefined anyway and out of scope of a reasonable
>> QOI.
>
> Undefined?  Most likely.  But we still have to do something sensible. As
> Jakub noted, a user could create the problematic code just as easily as
> DCE/DSE, so IRA probably needs to be tolerant of this situation.
>
> So it seems like you're suggesting we leave DCE/DSE alone (declaring this
> usage undefined) and fix IRA to be tolerant, right?

Tolerant as in not crash?  Yes.

Note that DCE/DSE would be happy if the stores were global memory.  After
my recent fix even addresses based on functions and labels work here.

What does not work is if you jump to automatic storage the compiler knows
how to compute liveness of as gotos are not considered here.  I can't think
of a way to make the IL handle this without severely pessimizing regular
DCE/DSE or making it very "hacky" in only giving VUSEs to gotos that
possibly may reach "local" memory.

That said, I can of course try and will once I see a testcase we break that
matters in real life from a correctness perspective - like I did for that ARM
kernel patching bug.

>> Using alloca to create/jump to code on the stack should work (we might
>> transform that into a decl though).
>
> Given that executable stacks are a huge security hole, I'd be willing to go
> out on a limb and declare that undefined as well.  It's not as clear cut,
> but that's the argument I'd make.

Well, I thought about somebody trying to build trampolines in a way exposed
to GCC.

> And yes, I realize that goes in opposition to what GCC has allowed for 20+
> years.  I still think it'd be the right thing to do.

Did we allow this?  Not by design but rather by accident I suppose.

Richard.

> jeff
>


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