libspl: hide global data objects

Currently libspl is a static archive that is linked into multiple shared
objects, which then re-export its symbols. We intend to fix this soon.

For the moment though, most programs shipped with OpenZFS depend on two
or more of these shared objects, and see the same symbols twice. For
functions this is not a problem, as they do not have any mutable state
and so the linker can simply select the first one and use that for all.

For global data objects however, each shared object will have direct
(non-relocatable) references to its own instance of the symbol, such
that changes on one will not necessarily be seen by the other. While
this shouldn't be a problem in practice as these reexported interfaces
are not supposed to be used, they are technically undefined behaviour in
C (C17 6.9.2) and are reported by ASAN as a violation of C++'s "One
Definition Rule".

To fix this, we hide these globals inside their compilation units, and
add access functions and macros as appropriate to preserve the existing
API (though not ABI).

Sponsored-by: https://despairlabs.com/sponsor/
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rob Norris <robn@despairlabs.com>
Closes #17861

Author: robn

cmd/zstream/zstream_redup.c

@@ -191,9 +191,9 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose)
 #ifdef _ILP32
 	uint64_t max_rde_size = SMALLEST_POSSIBLE_MAX_RDT_MB << 20;
 #else
-	uint64_t physmem = sysconf(_SC_PHYS_PAGES) * sysconf(_SC_PAGESIZE);
+	uint64_t physbytes = sysconf(_SC_PHYS_PAGES) * sysconf(_SC_PAGESIZE);
 	uint64_t max_rde_size =
-	    MAX((physmem * MAX_RDT_PHYSMEM_PERCENT) / 100,
+	    MAX((physbytes * MAX_RDT_PHYSMEM_PERCENT) / 100,
 	    SMALLEST_POSSIBLE_MAX_RDT_MB << 20);
 #endif
 

cmd/ztest.c

@@ -140,9 +140,9 @@
 #include <sys/zfs_impl.h>
 #include <sys/backtrace.h>
 #include <libzpool.h>
+#include <libspl.h>
 
 static int ztest_fd_data = -1;
-static int ztest_fd_rand = -1;
 
 typedef struct ztest_shared_hdr {
 	uint64_t	zh_hdr_size;
@@ -903,13 +903,10 @@ ztest_random(uint64_t range)
 {
 	uint64_t r;
 
-	ASSERT3S(ztest_fd_rand, >=, 0);
-
 	if (range == 0)
 		return (0);
 
-	if (read(ztest_fd_rand, &r, sizeof (r)) != sizeof (r))
-		fatal(B_TRUE, "short read from /dev/urandom");
+	random_get_pseudo_bytes((uint8_t *)&r, sizeof (r));
 
 	return (r % range);
 }
@@ -8146,10 +8143,8 @@ ztest_raidz_expand_run(ztest_shared_t *zs, spa_t *spa)
 	/* Setup a 1 MiB buffer of random data */
 	uint64_t bufsize = 1024 * 1024;
 	void *buffer = umem_alloc(bufsize, UMEM_NOFAIL);
+	random_get_pseudo_bytes((uint8_t *)&buffer, bufsize);
 
-	if (read(ztest_fd_rand, buffer, bufsize) != bufsize) {
-		fatal(B_TRUE, "short read from /dev/urandom");
-	}
 	/*
 	 * Put some data in the pool and then attach a vdev to initiate
 	 * reflow.
@@ -8955,13 +8950,7 @@ main(int argc, char **argv)
 		exit(EXIT_FAILURE);
 	}
 
-	/*
-	 * Force random_get_bytes() to use /dev/urandom in order to prevent
-	 * ztest from needlessly depleting the system entropy pool.
-	 */
-	random_path = "/dev/urandom";
-	ztest_fd_rand = open(random_path, O_RDONLY | O_CLOEXEC);
-	ASSERT3S(ztest_fd_rand, >=, 0);
+	libspl_init();
 
 	if (!fd_data_str) {
 		process_options(argc, argv);

lib/libspl/include/sys/misc.h

@@ -31,9 +31,6 @@
 
 #include <sys/utsname.h>
 
-extern const char *random_path;
-extern const char *urandom_path;
-
 /*
  * Hostname information
  */

lib/libspl/include/sys/systm.h

@@ -29,6 +29,8 @@
 #ifndef _LIBSPL_SYS_SYSTM_H
 #define	_LIBSPL_SYS_SYSTM_H
 
-extern uint64_t physmem;
+uint64_t libspl_physmem(void);
+
+#define	physmem	libspl_physmem()
 
 #endif

lib/libspl/include/sys/taskq.h

@@ -86,8 +86,11 @@ typedef struct taskq {
 
 #define	TASKQID_INVALID		((taskqid_t)0)
 
-extern taskq_t *system_taskq;
-extern taskq_t *system_delay_taskq;
+extern taskq_t *_system_taskq(void);
+extern taskq_t *_system_delay_taskq(void);
+
+#define	system_taskq		_system_taskq()
+#define	system_delay_taskq	_system_delay_taskq()
 
 extern taskq_t	*taskq_create(const char *, int, pri_t, int, int, uint_t);
 extern taskq_t	*taskq_create_synced(const char *, int, pri_t, int, int, uint_t,

lib/libspl/include/sys/thread.h

@@ -58,11 +58,9 @@ typedef pthread_t	kthread_t;
 #define	current_is_reclaim_thread()	(0)
 
 /* in libzpool, p0 exists only to have its address taken */
-typedef struct proc {
-	uintptr_t	this_is_never_used_dont_dereference_it;
-} proc_t;
+typedef void (proc_t)(void);
+extern void p0(void);
 
-extern struct proc p0;
 #define	curproc		(&p0)
 
 #define	PS_NONE		-1

lib/libspl/libspl.c

@@ -31,11 +31,18 @@
 #include <assert.h>
 #include <unistd.h>
 #include <sys/misc.h>
+#include <sys/systm.h>
 #include <sys/utsname.h>
 #include "libspl_impl.h"
 
-uint64_t physmem;
-struct utsname hw_utsname;
+static uint64_t hw_physmem = 0;
+static struct utsname hw_utsname = {};
+
+uint64_t
+libspl_physmem(void)
+{
+	return (hw_physmem);
+}
 
 utsname_t *
 utsname(void)
@@ -46,7 +53,7 @@ utsname(void)
 void
 libspl_init(void)
 {
-	physmem = sysconf(_SC_PHYS_PAGES);
+	hw_physmem = sysconf(_SC_PHYS_PAGES);
 
 	VERIFY0(uname(&hw_utsname));
 

lib/libspl/random.c

@@ -32,15 +32,22 @@
 #include <sys/random.h>
 #include "libspl_impl.h"
 
-const char *random_path = "/dev/random";
-const char *urandom_path = "/dev/urandom";
+#define	RANDOM_PATH	"/dev/random"
+#define	URANDOM_PATH	"/dev/urandom"
+
 static int random_fd = -1, urandom_fd = -1;
 
 void
 random_init(void)
 {
-	VERIFY((random_fd = open(random_path, O_RDONLY | O_CLOEXEC)) != -1);
-	VERIFY((urandom_fd = open(urandom_path, O_RDONLY | O_CLOEXEC)) != -1);
+	/* Handle multiple calls. */
+	if (random_fd != -1) {
+		ASSERT3U(urandom_fd, !=, -1);
+		return;
+	}
+
+	VERIFY((random_fd = open(RANDOM_PATH, O_RDONLY | O_CLOEXEC)) != -1);
+	VERIFY((urandom_fd = open(URANDOM_PATH, O_RDONLY | O_CLOEXEC)) != -1);
 }
 
 void

lib/libspl/taskq.c

@@ -36,9 +36,20 @@
 #include <sys/taskq.h>
 #include <sys/kmem.h>
 
-int taskq_now;
-taskq_t *system_taskq;
-taskq_t *system_delay_taskq;
+static taskq_t *__system_taskq = NULL;
+static taskq_t *__system_delay_taskq = NULL;
+
+taskq_t
+*_system_taskq(void)
+{
+	return (__system_taskq);
+}
+
+taskq_t
+*_system_delay_taskq(void)
+{
+	return (__system_delay_taskq);
+}
 
 static pthread_key_t taskq_tsd;
 
@@ -111,11 +122,6 @@ taskq_dispatch(taskq_t *tq, task_func_t func, void *arg, uint_t tqflags)
 {
 	taskq_ent_t *t;
 
-	if (taskq_now) {
-		func(arg);
-		return (1);
-	}
-
 	mutex_enter(&tq->tq_lock);
 	ASSERT(tq->tq_flags & TASKQ_ACTIVE);
 	if ((t = task_alloc(tq, tqflags)) == NULL) {
@@ -378,9 +384,6 @@ taskq_member(taskq_t *tq, kthread_t *t)
 {
 	int i;
 
-	if (taskq_now)
-		return (1);
-
 	for (i = 0; i < tq->tq_nthreads; i++)
 		if (tq->tq_threadlist[i] == t)
 			return (1);
@@ -405,18 +408,18 @@ void
 system_taskq_init(void)
 {
 	VERIFY0(pthread_key_create(&taskq_tsd, NULL));
-	system_taskq = taskq_create("system_taskq", 64, maxclsyspri, 4, 512,
+	__system_taskq = taskq_create("system_taskq", 64, maxclsyspri, 4, 512,
 	    TASKQ_DYNAMIC | TASKQ_PREPOPULATE);
-	system_delay_taskq = taskq_create("delay_taskq", 4, maxclsyspri, 4,
+	__system_delay_taskq = taskq_create("delay_taskq", 4, maxclsyspri, 4,
 	    512, TASKQ_DYNAMIC | TASKQ_PREPOPULATE);
 }
 
 void
 system_taskq_fini(void)
 {
-	taskq_destroy(system_taskq);
-	system_taskq = NULL; /* defensive */
-	taskq_destroy(system_delay_taskq);
-	system_delay_taskq = NULL;
+	taskq_destroy(__system_taskq);
+	__system_taskq = NULL; /* defensive */
+	taskq_destroy(__system_delay_taskq);
+	__system_delay_taskq = NULL;
 	VERIFY0(pthread_key_delete(taskq_tsd));
 }

lib/libspl/thread.c

@@ -32,7 +32,7 @@
 #include <sys/thread.h>
 
 /* this only exists to have its address taken */
-struct proc p0;
+void p0(void) {}
 
 /*
  * =========================================================================