Re: [PATCH v2] Add no_tail_call attribute

[Alexander Monakov <amonakov at ispras dot ru>]

On Wed, 19 Jul 2017, Yuri Gribov wrote:
> So to reiterate, your logic here is that someone would wipe dlsym type
> (e.g. by casting to void *), then later cast to another type which
> lacks tailcall attribute. So proposed solution won't protect against
> situation like this.

No, it's not "my logic" per se.  As you said, we've arrived to this
situation after Glibc people raised concerns about solutions akin to

    #define dlsym(mod, sym) ({ \
      void *__r = dlsym (mod, sym); \
      __asm__ ("" : "+g" (__r)); \
      __r; })

not suppressing tailcalls in situations where dlsym is called via a pointer.
I'm saying that it's not possible to solve that entirely, because the
user will always be able to lose the attribute.  It is simply not possible
to enforce that dlsym is never tailcalled.

Compare how people are always able to workaround warn_unused_result.

> But isn't it similar to a situation when someone casts unaligned int *
> (e.g. from a packed struct) to void * and then casts back to (aligned)
> int *, expecting it to work (rather than SIGBUSing)?  If user dumps
> important semantic attribute, it's his responsibility to properly
> restore it before using the object.  In this particular case, he'd
> need to cast void * to void (*)(void) __attribute__((notailcall));

... which won't work in any compiler except bleeding-edge GCC, unlike
the asm trick that works rather broadly.  If you're saying that the
user will need to change their code, won't they prefer to change it
in a fashion that is compatible with a wider range of compilers?

> > Honestly, if the goal is to assist users of GCC (and not only Glibc users,
> > but also people compiling against Bionic/musl/BSD libcs, which probably
> > wouldn't adopt this attribute),
> 
> Could you give more details here - why would they have problems with
> notailcall approach?

Obviously I can't speak authoritatively here, but I'm thinking that Bionic and
BSD people won't be very interested in GCC extensions that Clang doesn't
support, and musl generally aims to avoid using compiler extensions unless
absolutely required (and here adding the attribute doesn't solve the problem
in all cases anyway).

> > may I suggest that a clearer course of
> > action would be to:
> >
> > 1) recognize dlsym by name and suppress tailcalls to it
> >
> >    this would solve >99% cases because calling dlsym by pointer would be rare,
> >    and has the benefit of not requiring libc header changes;
> 
> I'm not sure how this is going to save above problem with casting to void *.

It wouldn't, and that's exactly what I said - it would solve direct calls, and
indirect calls are rare and not 100% solvable in the first place.

> > and
> > 2) if we really want to offer some generic solution, can we give the users
> > a way to suppress tailcalls via a builtin? That is, introduce
> >
> >     foo = __builtin_notailcall (func (args))
> > I think this would allow libc headers to do
> >
> >     #define dlsym(sym, mod) __builtin_notailcall (dlsym (sym, mod))
> 
> Could be done but what is the advantage compared to attribute?  It
> does not seem to fix issue you posted in the beginning.

One advantage is that the builtin is more fine-grained, you can use it to
suppress at individual call sites, so you can easily express the attribute
in terms of the builtin using a macro as shown above or an inline function,
but not the other way around.

Of course it doesn't do the impossible, but the point was that if we want
to add something to the compiler, we should consider if there are more
generic/useful alternatives.

> The one and only advantage of attribute compared to Jakubs approach
> (or yours, they share the same idea of wrapping dlsym calls) is that
> it forces user to carry it around when taking address of function.

It's an inconvenience.  People won't like it when their code suddenly
stops compiling after they update libc headers.  And what's the solution
for them?  Adding GCC-8-specific attributes in their code, even if it
never had dlsym in tail position anyway?

Alexander