[RFC PATCH] Add new mechanism for function aliases

[Nadav Har'El]

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 <n...@scylladb.com>
---
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

[Nadav Har'El]

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

n...@scylladb.com

[Nadav Har'El]

On Tue, Aug 4, 2020 at 1:41 AM Nadav Har'El <n...@scylladb.com> wrote:

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

Done. Just sent a v2 with a linker script - and I like it better. It's simpler and doesn't waste a microsecond at boot time :-)