This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] PR69195, Reload confused by invalid reg equivs
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Jeff Law <law at redhat dot com>
- Cc: Jakub Jelinek <jakub at redhat dot com>, Alan Modra <amodra at gmail dot com>, Andreas Schwab <schwab at linux-m68k dot org>, Bernd Schmidt <bschmidt at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 14 Mar 2016 10:56:06 +0100
- Subject: Re: [PATCH] PR69195, Reload confused by invalid reg equivs
- Authentication-results: sourceware.org; auth=none
- References: <20160304145430 dot GD9617 at bubble dot grove dot modra dot org> <56D9C86A dot 8050302 at redhat dot com> <20160307144656 dot GF9617 at bubble dot grove dot modra dot org> <20160310091808 dot GO9617 at bubble dot grove dot modra dot org> <877fh8ycip dot fsf at linux-m68k dot org> <20160311221624 dot GB16812 at bubble dot grove dot modra dot org> <20160311231350 dot GC16812 at bubble dot grove dot modra dot org> <20160312075821 dot GK3017 at tucnak dot redhat dot com> <56E3D2BF dot 90508 at redhat dot com> <20160312090725 dot GD16812 at bubble dot grove dot modra dot org> <20160312092940 dot GL3017 at tucnak dot redhat dot com> <2EBC7A3F-3136-4284-9155-D5C0A7875320 at gmail dot com> <56E44CBD dot 6010806 at redhat dot com>
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
>
- References:
- [RFC] PR69195, Reload confused by invalid reg equivs
- Re: [RFC] PR69195, Reload confused by invalid reg equivs
- Re: [RFC] PR69195, Reload confused by invalid reg equivs
- [PATCH] PR69195, Reload confused by invalid reg equivs
- Re: [PATCH] PR69195, Reload confused by invalid reg equivs
- Re: [PATCH] PR69195, Reload confused by invalid reg equivs
- Re: [PATCH] PR69195, Reload confused by invalid reg equivs
- Re: [PATCH] PR69195, Reload confused by invalid reg equivs
- Re: [PATCH] PR69195, Reload confused by invalid reg equivs
- Re: [PATCH] PR69195, Reload confused by invalid reg equivs
- Re: [PATCH] PR69195, Reload confused by invalid reg equivs
- Re: [PATCH] PR69195, Reload confused by invalid reg equivs
- Re: [PATCH] PR69195, Reload confused by invalid reg equivs