From 6825143135e5a6b9be7041d3319d32c03c2ed701 Mon Sep 17 00:00:00 2001
From: Alberto Bertogli <albertito@blitiri.com.ar>
Date: Sun, 26 Aug 2012 21:56:52 +0100
Subject: [PATCH 07/18] preload/posix: Use RTLD_NEXT when available

This patch makes the posix preloader use RTLD_NEXT for finding the original
symbols of the functions we're wrapping, instead of needing
dlopen(LIBC_SONAME). It's a GNU extension, but very very useful.

The change also requires some reorganization, including a special case for
failing the function during its own initialization, which can happen if
dlopen()/dlsym() invoke the functions we are wrapping.

That works as long as those functions can cope with the failures, which is the
case in all tested platforms so far (and we're no worse than we were before
anyway).

Much of the initialization and libc handling is simplified, so the patch
removes more than twice the lines it adds.

The original motivation for this patch, however, is to fix the removal of the
malloc hooks in the latest glibc versions. With it, malloc() and friends no
longer need special treatment. Thanks a lot to Georg Profos for reporting this
issue, along with code samples for reproducing it.

Signed-off-by: Alberto Bertogli <albertito@blitiri.com.ar>
---
 preload/posix/Makefile                 |    4 +
 preload/posix/codegen.c                |   39 +++++------
 preload/posix/codegen.h                |   29 +++++---
 preload/posix/function_list.in         |    2 -
 preload/posix/generate                 |    4 +-
 preload/posix/modules/libc.mm.custom.c |  113 --------------------------------
 preload/posix/modules/libc.mm.mod      |   21 ++++++
 preload/posix/modules/posix.custom.c   |   24 +++++--
 8 files changed, 83 insertions(+), 153 deletions(-)
 delete mode 100644 preload/posix/modules/libc.mm.custom.c
 create mode 100644 preload/posix/modules/libc.mm.mod

diff --git a/preload/posix/Makefile b/preload/posix/Makefile
index b2afa58..b269781 100644
--- a/preload/posix/Makefile
+++ b/preload/posix/Makefile
@@ -60,6 +60,10 @@ $(OBJS): build-flags
 .c.o:
 	$(NICE_CC) $(ALL_CFLAGS) -c $< -o $@
 
+# We define _GNU_SOURCE to get RTLD_NEXT if available; on non-GNU
+# platforms it should be harmless.
+codegen.o: codegen.c
+	$(NICE_CC) $(ALL_CFLAGS) -D_GNU_SOURCE -c $< -o $@
 
 # some platforms do not have libdl, we only use it if available
 build-needlibdl:
diff --git a/preload/posix/codegen.c b/preload/posix/codegen.c
index 177fdcd..691d6f0 100644
--- a/preload/posix/codegen.c
+++ b/preload/posix/codegen.c
@@ -6,9 +6,6 @@
 #include "codegen.h"
 #include "build-env.h"
 
-/* Dynamically load libc */
-void *_fiu_libc;
-
 /* Recursion counter, per-thread */
 int __thread _fiu_called = 0;
 
@@ -18,30 +15,32 @@ int __thread _fiu_called = 0;
 #warning "Building without using constructor priorities"
 #endif
 
-void constructor_attr(200) _fiu_init(void)
+/* Get a symbol from libc.
+ * This function is a wrapper around dlsym(libc, ...), that we use to abstract
+ * away how we get the libc wrapper, because on some platforms there are
+ * better shortcuts. */
+void *libc_symbol(const char *symbol)
 {
-	static int initialized = 0;
-
-	/* When built without constructor priorities, we could be called more
-	 * than once during the initialization phase: one because we're marked
-	 * as a constructor, and another when one of the other constructors
-	 * sees that it doesn't have _fiu_libc set. */
-
-	printd("_fiu_init() start (%d)\n", initialized);
-	if (initialized)
-		goto exit;
+#ifdef RTLD_NEXT
+	return dlsym(RTLD_NEXT, symbol);
+#else
+	/* We don't want to get this over and over again, so we set it once
+	 * and reuse it afterwards. */
+	static void *_fiu_libc = NULL;
 
-	_fiu_libc = dlopen(LIBC_SONAME, RTLD_NOW);
 	if (_fiu_libc == NULL) {
-		fprintf(stderr, "Error loading libc: %s\n", dlerror());
-		exit(1);
+		_fiu_libc = dlopen(LIBC_SONAME, RTLD_NOW);
+		if (_fiu_libc == NULL) {
+			fprintf(stderr, "Error loading libc: %s\n", dlerror());
+			exit(1);
+		}
 	}
-	initialized = 1;
 
-exit:
-	printd("_fiu_init() done\n");
+	return dlsym(_fiu_libc, symbol);
+#endif
 }
 
+
 /* this runs after all function-specific constructors */
 static void constructor_attr(250) _fiu_init_final(void)
 {
diff --git a/preload/posix/codegen.h b/preload/posix/codegen.h
index 80b3344..efee7bc 100644
--- a/preload/posix/codegen.h
+++ b/preload/posix/codegen.h
@@ -2,17 +2,15 @@
 #ifndef _FIU_CODEGEN
 #define _FIU_CODEGEN
 
-#include <dlfcn.h>		/* dlsym() */
 #include <fiu.h>		/* fiu_* */
 #include <stdlib.h>		/* NULL, random() */
 
-/* Pointer to the dynamically loaded library */
-extern void *_fiu_libc;
-void _fiu_init(void);
-
 /* Recursion counter, per-thread */
 extern int __thread _fiu_called;
 
+/* Get a symbol from libc */
+void *libc_symbol(const char *symbol);
+
 /* Some compilers support constructor priorities. Since we don't rely on them,
  * but use them for clarity purposes, use a macro so libfiu builds on systems
  * where they're not supported.
@@ -76,18 +74,20 @@ extern int __thread _fiu_called;
  */
 
 /* Generates the common top of the wrapped function */
-#define mkwrap_top(RTYPE, NAME, PARAMS, PARAMSN, PARAMST)	\
+#define mkwrap_top(RTYPE, NAME, PARAMS, PARAMSN, PARAMST, ON_ERR) \
 	static RTYPE (*_fiu_orig_##NAME) PARAMS = NULL;		\
 								\
+	static int _fiu_in_init_##NAME = 0;			\
+								\
 	static void constructor_attr(201) _fiu_init_##NAME(void) \
 	{							\
 		rec_inc();					\
-								\
-		if (_fiu_libc == NULL)				\
-			_fiu_init();				\
+		_fiu_in_init_##NAME++;				\
 								\
 		_fiu_orig_##NAME = (RTYPE (*) PARAMST)		\
-				dlsym(_fiu_libc, #NAME);	\
+				libc_symbol(#NAME);		\
+								\
+		_fiu_in_init_##NAME--;				\
 		rec_dec();					\
 	}							\
 								\
@@ -97,6 +97,15 @@ extern int __thread _fiu_called;
 		int fstatus;					\
 								\
 		if (_fiu_called) {				\
+			if (_fiu_orig_##NAME == NULL) {		\
+				if (_fiu_in_init_##NAME) {	\
+					printd("fail on init\n"); \
+					return ON_ERR;		\
+				} else {			\
+					printd("get orig\n");	\
+					_fiu_init_##NAME();	\
+				}				\
+			}					\
 			printd("orig\n");			\
 			return (*_fiu_orig_##NAME) PARAMSN;	\
 		}						\
diff --git a/preload/posix/function_list.in b/preload/posix/function_list.in
index d82bf59..94e71d5 100644
--- a/preload/posix/function_list.in
+++ b/preload/posix/function_list.in
@@ -8,8 +8,6 @@ It is not set in stone, although it shouldn't change frequently.
 Manually written
 ----------------
 
-malloc				libc/mm/malloc
-realloc				libc/mm/realloc
 open				posix/io/oc/open
 
 
diff --git a/preload/posix/generate b/preload/posix/generate
index e7aac43..9c93ef6 100755
--- a/preload/posix/generate
+++ b/preload/posix/generate
@@ -100,9 +100,9 @@ class Function:
 		paramst = ', '.join([i[0] for i in self.params_info])
 		paramsn = ', '.join([i[1] for i in self.params_info])
 
-		f.write('mkwrap_top(%s, %s, (%s), (%s), (%s))\n' % \
+		f.write('mkwrap_top(%s, %s, (%s), (%s), (%s), (%s))\n' % \
 				(self.ret_type, self.name, self.params,
-					paramsn, paramst) )
+					paramsn, paramst, self.on_error) )
 
 		if self.reduce:
 			f.write('mkwrap_body_reduce("%s/reduce", %s)\n' % \
diff --git a/preload/posix/modules/libc.mm.custom.c b/preload/posix/modules/libc.mm.custom.c
deleted file mode 100644
index e94dec3..0000000
--- a/preload/posix/modules/libc.mm.custom.c
+++ /dev/null
@@ -1,113 +0,0 @@
-
-/*
- * Custom-made wrappers for malloc() and realloc().
- *
- * They can't be made generic because, at least on glibc, they're used before
- * constructors are called.
- *
- * We use __malloc_hook, the glibc-specific interface, so this is glibc-only.
- */
-
-/* We need to include features.h, which in turns defines __GLIBC__. However,
- * features.h is glibc-specific and does not exist on other platforms, so we
- * can't include it directly or the file won't build.
- * Instead, we have to resort to this ugly trick of including limits.h, which
- * is standard and we know glibc's implementation includes features.h.
- */
-#include <limits.h>
-
-#ifndef __GLIBC__
-  #warning "Not using glibc, so no malloc() wrappers will be available"
-#else
-
-#include "codegen.h"
-
-#include <malloc.h>
-
-
-/* Original glibc's hooks, saved in our init function */
-static void *(*old_malloc_hook)(size_t, const void *);
-static void *(*old_realloc_hook)(void *ptr, size_t size, const void *caller);
-
-/* Our own hooks, that will replace glibc's */
-static void *fiu_malloc_hook(size_t size, const void *caller);
-static void *fiu_realloc_hook(void *ptr, size_t size, const void *caller);
-
-
-/* We are not going to use __malloc_initialize_hook because it would be called
- * before our library initialization functions, which include fiu_init().
- * Instead, we will use a constructor just like the other wrappers, but it
- * will run after them just to make things tidier (NOT because it is
- * necessary). */
-
-static void constructor_attr(202) fiu_init_malloc(void)
-{
-	/* Save original hooks, used in ours to prevent unwanted recursion */
-	old_malloc_hook = __malloc_hook;
-	old_realloc_hook = __realloc_hook;
-
-	__malloc_hook = fiu_malloc_hook;
-	__realloc_hook = fiu_realloc_hook;
-}
-
-
-static void *fiu_malloc_hook(size_t size, const void *caller)
-{
-	void *r;
-	int fstatus;
-
-	/* fiu_fail() may call anything */
-	rec_inc();
-
-	/* See __malloc_hook(3) for details */
-	__malloc_hook = old_malloc_hook;
-
-	fstatus = fiu_fail("libc/mm/malloc");
-	if (fstatus != 0) {
-		r = NULL;
-		goto exit;
-	}
-
-	printd("calling orig\n");
-	r = malloc(size);
-
-exit:
-	old_malloc_hook = __malloc_hook;
-	__malloc_hook = fiu_malloc_hook;
-
-	rec_dec();
-	return r;
-}
-
-
-static void *fiu_realloc_hook(void *ptr, size_t size, const void *caller)
-{
-	void *r;
-	int fstatus;
-
-	/* fiu_fail() may call anything */
-	rec_inc();
-
-	/* See __malloc_hook(3) for details */
-	__realloc_hook = old_realloc_hook;
-
-	fstatus = fiu_fail("libc/mm/realloc");
-	if (fstatus != 0) {
-		r = NULL;
-		goto exit;
-	}
-
-	printd("calling orig\n");
-	r = realloc(ptr, size);
-
-exit:
-	old_realloc_hook = __realloc_hook;
-	__realloc_hook = fiu_realloc_hook;
-
-	rec_dec();
-	return r;
-}
-
-
-#endif // defined __GLIBC__
-
diff --git a/preload/posix/modules/libc.mm.mod b/preload/posix/modules/libc.mm.mod
new file mode 100644
index 0000000..20f9f04
--- /dev/null
+++ b/preload/posix/modules/libc.mm.mod
@@ -0,0 +1,21 @@
+
+include: <errno.h>
+include: <stdlib.h>
+
+fiu name base: libc/mm/
+
+
+void *malloc(size_t size);
+	on error: NULL
+	valid errnos: ENOMEM
+
+void *calloc(size_t nmemb, size_t size);
+	on error: NULL
+	valid errnos: ENOMEM
+
+void *realloc(void *ptr, size_t size);
+	on error: NULL
+	valid errnos: ENOMEM
+
+# Note we don't wrap free() as it does not return anything.
+
diff --git a/preload/posix/modules/posix.custom.c b/preload/posix/modules/posix.custom.c
index 1c709cd..959c58b 100644
--- a/preload/posix/modules/posix.custom.c
+++ b/preload/posix/modules/posix.custom.c
@@ -18,15 +18,17 @@
  * of arguments */
 static int (*_fiu_orig_open) (const char *pathname, int flags, ...) = NULL;
 
+static int _fiu_in_init_open = 0;
+
 static void constructor_attr(201) _fiu_init_open(void)
 {
 	rec_inc();
-
-	if (_fiu_libc == NULL)
-		_fiu_init();
+	_fiu_in_init_open++;
 
 	_fiu_orig_open = (int (*) (const char *, int, ...))
-			 dlsym(_fiu_libc, "open");
+		libc_symbol("open");
+
+	_fiu_in_init_open--;
 	rec_dec();
 }
 
@@ -56,13 +58,23 @@ int open(const char *pathname, int flags, ...)
 		mode = 0;
 	}
 
+	/* Differences from the generated code end here */
+
 	if (_fiu_called) {
+		if (_fiu_orig_open == NULL) {
+			if (_fiu_in_init_open) {
+				printd("fail on init\n");
+				return -1;
+			} else {
+				printd("get orig\n");
+				_fiu_init_open();
+			}
+		}
+
 		printd("orig\n");
 		return (*_fiu_orig_open) (pathname, flags, mode);
 	}
 
-	/* Differences from the generated code end here */
-
 	printd("fiu\n");
 
 	/* fiu_fail() may call anything */
-- 
1.6.2.2.646.gb214

