[PATCH] D50805: [Sema] Don't warn on returning the address of a label

Richard Smith - zygoloid via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 16 12:44:46 PDT 2018


rsmith added a comment.

In https://reviews.llvm.org/D50805#1201910, @rnk wrote:

> I think the state machine use case is real, though, something like:
>
>   void *f(void *startlabel) {
>     common_work();
>     goto *startlabel;
>   state1:
>     return &&state2;
>   state2:
>     return &&state3;
>   ...
>   }
>


Per GCC's documentation, this code is wrong, and `__attribute__((noinline, noclone))` must be used. However, GCC's behavior on that case is extremely interesting: https://godbolt.org/g/xesYK1

Note that it produces a warning about returning the address of a label, just like we do. But it also says that `f` cannot be inlined because it contains a computed goto. So perhaps the GCC documentation / spec for the feature is wrong.

> Suppressing it under noinline doesn't solve the original statement expression issue

True, but it's straightforward to fix the regression for the statement expression case. I don't think that affects whether we go further than that and remove the warning for the `return` case.

> Are we really worried about users doing this?
> 
>   void *f() {
>     return &&next;
>   next:
>     ...
>   }
>   void g() {
>     void *pc = f();
>     goto *pc; // cross-functional frame re-entry! =D
>   }

A little, yes. But I think this cross-function case alone would not justify an enabled-by-default warning with false-positives on the state machine case. So I think the question is, do we think the warnings on the state machine case are false positives or not? Some options I think are reasonable:

1. We explicitly document that our extension provides an additional guarantee on top of GCC's -- that the address of a label always denotes the same label for multiple invocations of the same function -- and remove this warning (or downgrade to `-Wgcc-compat`).
2. We keep merely inheriting the specification (such as it is) for the feature from GCC's documentation, and the warning is arguably valid because any code that triggers it is (almost certainly) wrong, even though we won't currently exploit the wrongness.

Or secret option 1b: we get the GCC folks to update their documentation to explicitly state that the presence of a computed goto in a function prevents inlining and cloning in all circumstances, and remove the warning.

I've filed gcc.gnu.org/PR86983 <http://gcc.gnu.org/PR86983>; hopefully we'll get more clarification on what the semantics of this extension are supposed to be there.


Repository:
  rC Clang

https://reviews.llvm.org/D50805





More information about the cfe-commits mailing list