[PATCH v2] generate EH info for volatile asm statements (PR93981)

[J.W. Jagersma]

On 2020-03-07 20:20, Segher Boessenkool wrote:
> Hi!
> 
> On Sat, Mar 07, 2020 at 06:12:45PM +0100, J.W. Jagersma wrote:
>> The following patch extends the generation of exception handling
>> information to cover volatile asms too.  This was already mostly
>> implemented, and only minor changes are required in order to make it
>> work.
> 
> This should wait for stage 1, IMO.  Looks pretty good to me, thanks!

Hi, thanks for your response.
What does stage 1 refer to?  I'm sorry, this is my first gcc patch and
I'm still learning how this all works.

> Some comments:
> 
>> +When non-call exceptions (@option{-fnon-call-exceptions}) are enabled, a
>> +@code{volatile asm} statement is also allowed to throw exceptions.  If it
>> +does, then the compiler assumes that its output operands have not been written
>> +yet.
> 
> That reads as if the compiler assumes the outputs retain their original
> value, but that isn't true (I hope!)  The compiler assumes the output
> are clobbered, but it doesn't assume they are assigned any definite
> value?

Register outputs are assumed to be clobbered, yes.  For memory outputs
this is not the case, if the asm writes it before throwing then the
memory operand retains this value.  It should be the user's
responsibility to ensure that an asm has no side-effects if it throws.

I didn't see any way to make this work either, since create_tmp_var
will not make a temporary that is TREE_ADDRESSABLE.  It's not practical
anyway since you could pass a giant struct as operand and have to copy
the entire thing.  Or the operand could refer to memory-mapped IO space
in which case write operations can have side-effects.

I do agree that more specific wording can be used here.  Maybe
something like this?

+When non-call exceptions (@option{-fnon-call-exceptions}) are enabled, a
+@code{volatile asm} statement is also allowed to throw exceptions.  If it
+does, then all register output operands are assumed to be clobbered.

>> +  try
>> +    {
>> +      asm ("ud2");
>> +    }
> 
> Yeah that won't work on other than x86.  Maybe you want to use something
> like gc.dg/nop.h?  You could use what __builtin_trap generates, but you
> want asm of course.

The test case is in g++.target/i386/ for this reason.  The platform-
specific part is throwing from a signal handler, that needs to be
supported by the runtime environment.  It works on Linux but not on
mingw-w64 or msys, for example.  I am not sure if it's better to set
xfail for all platforms except *-linux-*, or if each platform where it
is expected to fail should be listed individually (and frankly, I'm not
sure how either of this is done in DejaGNU syntax).

>> +++ b/gcc/tree-eh.c
>> @@ -2077,6 +2077,8 @@ lower_eh_constructs_2 (struct leh_state *state, gimple_stmt_iterator *gsi)
>> +    record_throwing_stmt:
>>         /* Look for things that can throw exceptions, and record them.  */
>>         if (state->cur_region && stmt_could_throw_p (cfun, stmt))
> 
> The indent of the label is incorrect (it should be flush left, but some
> emacs users like one space, instead; same indent as the case labels
> isn't correct).

Will edit that, thanks.