Re: [PATCH] Fix PR c/14411

[Zack Weinberg <zack at codesourcery dot com>]

Alexey Neyman <alex.neyman@auriga.ru> writes:

>> I think I'm with Joseph on this - it should be "returns_twice" because
>> then it makes sense to apply the attribute to other functions with the
>> same property, that don't necessarily have setjmp's other behavior;
>> vfork() is an example.
>
> But why do we have __attribute__((malloc)) instead of
> __attribute__((returns_nonaliased_pointer)) then?

Calling it "attribute malloc" is somewhat better motivated - all
functions to which attribute malloc might reasonably be applied, have
a broad category of properties in common with malloc itself.  We
happen only to use the one property that the pointer it returns cannot
alias anything at that point, but we could use others (for instance, I
personally think it would be reasonable to optimize out a call to
malloc if its return value was discarded - others may dispute this).

By contrast, the only thing vfork and setjmp have in common is that
they return twice in the same process, and in all other regards they
are dramatically different.

>> This isn't a good description of the attribute.  I don't know why this
>> is different from noreturn, so I can't suggest a better description.
>
> Well, the only place it is used is the setting of the calls_longjmp
> bitfield in the 'struct function'. That field, however, is only
> referenced by the gcc/config/i386/sysv3.h, and this header file
> appears unused (is not listed for any configuration in config.gcc).
> Perhaps, all these should be removed? If so, should I address this
> issue with this patch?

Ooh, good catch.  i386/sysv3.h is indeed dead, and not only is it the
only place that uses current_function_calls_longjmp, it's the only
non-pointless definition of NON_SAVING_SETJMP, which gets referenced
all over the RTL optimizers.

I would tell you to submit a separate patch getting rid of all this,
except that it's faster for me to deal with it myself.  What you
should do is wait for my patch to go in (which will happen in a couple
hours) and then resync yours.  Remove attribute longjmp from your
patch; instead add a note to the documentation for attribute setjmp
(/returns_twice) saying that the matching longjmp-alike, if any,
should be marked with attribute noreturn.

>> The patch otherwise looks good.  Do you have a copyright assignment on
>> file with the FSF?
>
> In progress.

Bother.  We can't take this without a copyright assignment.  You're
going to have to wait until your paperwork clears.  In any case, this
patch is a new feature and 4.0 is in feature freeze, so you would have
to wait for 4.1 to be opened.

zw