Re: [PATCH v2] Add no_tail_call attribute

[Yuri Gribov <tetra2005@gmail.com>]

On Wed, Jul 19, 2017 at 4:30 PM, Alexander Monakov <amonakov@ispras.ru> wrote:
> On Wed, 19 Jul 2017, Jeff Law wrote:
>> > Glibc people were worried that attribute would be lost when taking a
>> > pointer to function
>> > (https://sourceware.org/ml/libc-alpha/2017-01/msg00482.html). I think
>> > their reasoning was that return address is a shadow argument for
>> > dlsym-like functions so this would cause a (most likely inadvertent)
>> > ABI error.
>> Fair enough, but does the right thing happen if the function's
>> definition is decorated?  Can you add a test for that in the testsuite?
>
> How would passing pointers to dlsym via a 'void *' work after this patch?

Hi Alex,

Interesting, do people really do this sort of stuff?

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.

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));

> 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?

> 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 *.

> 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.

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.

> I'm sorry for bringing up objections late, but I really ask for alternatives
> to be considered, and I can take a stab at implementing either of the above
> if the general course is agreed upon.

It's totally fine to bring up objections as long as we clearly list
advantages and disadvantages of alternatives.

-Y