I want to later create an alternative patch to this that uses a linker script (e.g., libc/aliases.ld) to copy symbols - and then we can decide which approach is nicer (or less ugly).
-- Nadav Har'El [email protected] On Tue, Aug 4, 2020 at 1:40 AM Nadav Har'El <[email protected]> wrote: > For a long time, adding an alias to a function required us using the > "weak_alias" macro in the same file as the original function, which > caused us to modify some Musl files we didn't want to modify. > > In this patch I add a new mechanism for creating an alias for functions > in *other* source files. Unfortunately, as this requires cooperation > from the linker, we (ab)used the linker's "ifunc" (STT_GNU_IFUNC) > support - already used by OSv - to copy the function's address at > load time. This is a waste, but a really tiny waste. I'm putting > this patch up as an RFC to see if someone can come up with a better > solution - hopefully to get the static linker (which links OSv) > to copy symbols. E.g., maybe doing this as a linker script would > have been nicer. > > In any case, this patch demonstrates how being able to create > aliases in a separate file (here libc/aliases.c) allows us to > drop a change from musl (here res_init.c). > > Signed-off-by: Nadav Har'El <[email protected]> > --- > Makefile | 3 ++- > libc/aliases.c | 31 +++++++++++++++++++++++++++++++ > libc/network/res_init.c | 7 ------- > 3 files changed, 33 insertions(+), 8 deletions(-) > create mode 100644 libc/aliases.c > delete mode 100644 libc/network/res_init.c > > diff --git a/Makefile b/Makefile > index cd490a76..9ec206b3 100644 > --- a/Makefile > +++ b/Makefile > @@ -944,6 +944,7 @@ libc += internal/floatscan.o > libc += internal/intscan.o > libc += internal/libc.o > libc += internal/shgetc.o > +libc += aliases.o > > musl += ctype/__ctype_get_mb_cur_max.o > musl += ctype/__ctype_tolower_loc.o > @@ -1375,7 +1376,7 @@ musl += network/getservbyport.o > libc += network/getifaddrs.o > libc += network/if_nameindex.o > musl += network/if_freenameindex.o > -libc += network/res_init.o > +musl += network/res_init.o > > musl += prng/rand.o > musl += prng/rand_r.o > diff --git a/libc/aliases.c b/libc/aliases.c > new file mode 100644 > index 00000000..2d16cd10 > --- /dev/null > +++ b/libc/aliases.c > @@ -0,0 +1,31 @@ > +/* > + * Copyright (C) 2020 ScyllaDB > + * > + * This work is open source software, licensed under the terms of the > + * BSD license as described in the LICENSE file in the top-level > directory. > + */ > + > +// Creating function aliases using the "alias(...)" attribute or our > +// weak_alias() macro (which uses alias(...)) requires the alias to be > +// defined in the same source file as the original function. The reason is > +// that this mechanism doesn't use any special linker trickery - it simply > +// generates the same address for both functions. > +// In the past this requirement forced us to modify Musl files just to > add an > +// alias,or to use inefficient workarounds like wrapper functions. In > this file > +// we use a different trick based on the linker: ifunc - functions run > during > +// boot when relocating these symbols. This has a runtime penalty at load > time, > +// unfortunately, but it's tiny. > +// It would be nice if in the future we can find a different aliasing > +// technique which can copy the address during the kernel's static > linking, > +// not at runtime. x86 has "copy relocation", for example, but it is not > +// exposed to C code and is x86-only. The benefit of the ifunc technique > is > +// that it is standard - and we actually already use it in OSv (for > choosing > +// between different implementations of the string functions). > + > +#define QUOTE(x) #x > +#define ALIAS(oldname, newname) \ > + extern void oldname(void); \ > + static void* resolve_##newname(void) { return oldname; } \ > + void newname(void) __attribute__((ifunc(QUOTE(resolve_##newname)))); > + > +ALIAS(res_init, __res_init) > diff --git a/libc/network/res_init.c b/libc/network/res_init.c > deleted file mode 100644 > index 66f3f95a..00000000 > --- a/libc/network/res_init.c > +++ /dev/null > @@ -1,7 +0,0 @@ > -#include "libc.h" > - > -int res_init() > -{ > - return 0; > -} > -weak_alias(res_init, __res_init); > -- > 2.26.2 > > -- You received this message because you are subscribed to the Google Groups "OSv Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/CANEVyjsMS6uBpa801Wvyj4dVsp%3Dqh8y_tWDuu_6VLxB1AxLe3w%40mail.gmail.com.
