On 7/13/2022 1:13 AM, Richard Biener via Gcc-patches wrote:
On Tue, Jul 12, 2022 at 10:10 PM Alexander Monakov <amona...@ispras.ru> wrote:

Apologies for the prolonged silence Richard, it is a bit of an obscure topic,
and I was unsure I'd be able to handle any complications in a timely manner.
I'm ready to revisit it now, please see below.

On Mon, 17 Jan 2022, Richard Biener wrote:

On Fri, Jan 14, 2022 at 7:21 PM Alexander Monakov <amona...@ispras.ru> wrote:
A returns_twice call may have associated abnormal edges that correspond
to the "second return" from the call. If the call is duplicated, the
copies of those edges also need to be abnormal, but e.g. tracer does not
enforce that. Just prohibit the (unlikely to be useful) duplication.
The general CFG copying routines properly duplicate those edges, no?
No (in fact you say so in the next paragraph). In general I think they cannot,
abnormal edges are a special case, so it should be the responsibility of the
caller.

Tracer uses duplicate_block so it should also get copies of all successor
edges of that block.  It also only traces along normal edges.  What it might
miss is abnormal incoming edges - is that what you are referring to?
Yes (I think its entire point is to build a "trace" of duplicated blocks that
does not have incoming edges in the middle, abnormal or not).

That would be a thing we don't handle in duplicate_block on its own but
that callers are expected to do (though I don't see copy_bbs doing that
either).  I wonder if we can trigger this issue for some testcase?
Oh yes (in fact my desire to find a testcase delayed this quite a bit).
When compiling the following testcase with -O2 -ftracer:

__attribute__((returns_twice))
int rtwice_a(int), rtwice_b(int);

int f(int *x)
{
         volatile unsigned k, i = (*x);

         for (k = 1; (i = rtwice_a(i)) * k; k = 2);

         for (; (i = rtwice_b(i)) * k; k = 4);

         return k;
}

tracer manages to eliminate the ABNORMAL_DISPATCHER block completely, so
the possibility of transferring control back to rtwice_a from rtwice_b
is no longer modeled in the IR. I could spend some time "upgrading" this
to an end-to-end miscompilation, but I hope you agree this is quite broken
already.

The thing to check would be incoming abnormal edges in
can_duplicate_block_p, not (only) returns twice functions?
Unfortunately not, abnormal edges are also used for computed gotos, which are
less magic than returns_twice edges and should not block tracer I think.
I think computed gotos should use regular edges, only non-local goto should
use abnormals...

I suppose asm goto also uses abnormal edges?

Btw, I don't see how they in general are "less magic".  Sure, we have an
explicit receiver (the destination label), but we can only do edge inserts
if we have a single computed goto edge into a block (we can "move" the
label to the block created when splitting the edge).
I suspect treating them like abnormals probably came from the inability to reliably split them way back when we introduced RTL GCSE and the like.

Jeff

Reply via email to