[DTrace-devel] [PATCH 2/7] usdt: remove relocations rather than changing their type to R_*_NONE

Kris Van Hees kris.van.hees at oracle.com
Tue Oct 31 15:25:27 UTC 2023


The original DTrace code used to depend on STV_ELIMINATE and XXX
to have the linker clean up USDT probe related relocations.  This
does not work on Linux, so DTrace on Linux changed the type of those
relocations to be R_*_NONE since those relocations get ignored.

Binutils no longer allows that for PIC objects when a locally referenced
symbol is involved.

DTrace will now remove those relocations since they are no longer needed
anyway.

A test is included to ensure that PIC objects can be linked successfully
when USDT probes are used.

Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
Reviewed-by: Nick Alcock <nick.alcock at oracle.com>
---
 libdtrace/dt_link.c           | 153 +++++++++++++++++++++-------------
 test/unittest/usdt/tst.pie.r  |   2 +
 test/unittest/usdt/tst.pie.sh |  79 ++++++++++++++++++
 3 files changed, 175 insertions(+), 59 deletions(-)
 create mode 100644 test/unittest/usdt/tst.pie.r
 create mode 100755 test/unittest/usdt/tst.pie.sh

diff --git a/libdtrace/dt_link.c b/libdtrace/dt_link.c
index b2148a8b..59532370 100644
--- a/libdtrace/dt_link.c
+++ b/libdtrace/dt_link.c
@@ -75,11 +75,12 @@ static const char DTRACE_SHSTRTAB64[] = "\0"
 static const char DOFSTR[] = "__SUNW_dof";
 static const char DOFLAZYSTR[] = "___SUNW_dof";
 
-typedef struct dt_link_pair {
-	struct dt_link_pair *dlp_next;	/* next pair in linked list */
+typedef struct dt_link_set {
+	struct dt_link_set *dlp_next;	/* next set in linked list */
 	void *dlp_str;			/* buffer for string table */
 	void *dlp_sym;			/* buffer for symbol table */
-} dt_link_pair_t;
+	void *dlp_rel;			/* buffer for relocations */
+} dt_link_set_t;
 
 typedef struct dof_elf32 {
 	uint32_t de_nrel;		/* relocation count */
@@ -992,11 +993,11 @@ dt_modtext(dtrace_hdl_t *dtp, char *p, GElf_Rela *rela, uint32_t *off)
 /*PRINTFLIKE5*/
 _dt_printflike_(5,6)
 static int
-dt_link_error(dtrace_hdl_t *dtp, Elf *elf, int fd, dt_link_pair_t *bufs,
+dt_link_error(dtrace_hdl_t *dtp, Elf *elf, int fd, dt_link_set_t *bufs,
     const char *format, ...)
 {
 	va_list ap;
-	dt_link_pair_t *pair;
+	dt_link_set_t *set;
 
 	va_start(ap, format);
 	dt_set_errmsg(dtp, NULL, NULL, NULL, 0, format, ap);
@@ -1008,11 +1009,12 @@ dt_link_error(dtrace_hdl_t *dtp, Elf *elf, int fd, dt_link_pair_t *bufs,
 	if (fd >= 0)
 		close(fd);
 
-	while ((pair = bufs) != NULL) {
-		bufs = pair->dlp_next;
-		dt_free(dtp, pair->dlp_str);
-		dt_free(dtp, pair->dlp_sym);
-		dt_free(dtp, pair);
+	while ((set = bufs) != NULL) {
+		bufs = set->dlp_next;
+		dt_free(dtp, set->dlp_str);
+		dt_free(dtp, set->dlp_sym);
+		dt_free(dtp, set->dlp_rel);
+		dt_free(dtp, set);
 	}
 
 	return dt_set_errno(dtp, EDT_COMPILER);
@@ -1032,6 +1034,7 @@ process_obj(dtrace_hdl_t *dtp, const char *obj, int *eprobesp)
 	Elf_Data *data_rel, *data_sym, *data_str, *data_tgt;
 	GElf_Shdr shdr_rel, shdr_sym, shdr_str, shdr_tgt;
 	GElf_Sym rsym, fsym, dsym;
+	GElf_Rel rel;
 	GElf_Rela rela;
 	char *s, *p, *r;
 	char pname[DTRACE_PROVNAMELEN];
@@ -1040,7 +1043,7 @@ process_obj(dtrace_hdl_t *dtp, const char *obj, int *eprobesp)
 	uint32_t off, eclass, emachine1, emachine2;
 	size_t symsize, nsym, isym, istr, len;
 	key_t objkey;
-	dt_link_pair_t *pair, *bufs = NULL;
+	dt_link_set_t *set, *bufs = NULL;
 	dt_strtab_t *strtab;
 	int flags = dtp->dt_link_no_mmap ? ELF_C_RDWR : ELF_C_RDWR_MMAP;
 
@@ -1111,6 +1114,8 @@ process_obj(dtrace_hdl_t *dtp, const char *obj, int *eprobesp)
 
 	scn_rel = NULL;
 	while ((scn_rel = elf_nextscn(elf, scn_rel)) != NULL) {
+		int		nrel, j;
+
 		if (gelf_getshdr(scn_rel, &shdr_rel) == NULL)
 			goto err;
 
@@ -1177,19 +1182,23 @@ process_obj(dtrace_hdl_t *dtp, const char *obj, int *eprobesp)
 		 * We take a first pass through all the relocations to
 		 * populate our string table and count the number of extra
 		 * symbols we'll require.
+		 *
+		 * We count the number of matching relocations so that we can
+		 * remove them from the relocation table once we are done with
+		 * them.
 		 */
 		strtab = dt_strtab_create(BUFSIZ);
 		nsym = 0;
 		isym = data_sym->d_size / symsize;
 		istr = data_str->d_size;
+		nrel = 0;
+		set = NULL;
 
 		for (i = 0; i < shdr_rel.sh_size / shdr_rel.sh_entsize; i++) {
-
 			if (shdr_rel.sh_type == SHT_RELA) {
 				if (gelf_getrela(data_rel, i, &rela) == NULL)
 					continue;
 			} else {
-				GElf_Rel rel;
 				if (gelf_getrel(data_rel, i, &rel) == NULL)
 					continue;
 				rela.r_offset = rel.r_offset;
@@ -1208,6 +1217,8 @@ process_obj(dtrace_hdl_t *dtp, const char *obj, int *eprobesp)
 			if (strncmp(s, dt_prefix, sizeof(dt_prefix) - 1) != 0)
 				continue;
 
+			nrel++;
+
 			if (dt_elf_symtab_lookup(data_sym, isym, rela.r_offset,
 			    shdr_rel.sh_info, &fsym) != 0) {
 				dt_strtab_destroy(strtab);
@@ -1270,37 +1281,36 @@ process_obj(dtrace_hdl_t *dtp, const char *obj, int *eprobesp)
 
 			dt_strtab_destroy(strtab);
 
-			if ((pair = dt_alloc(dtp, sizeof(*pair))) == NULL)
+			if ((set = dt_zalloc(dtp, sizeof(*set))) == NULL)
 				goto err;
 
-			if ((pair->dlp_str = dt_alloc(dtp, data_str->d_size +
-			    len)) == NULL) {
-				dt_free(dtp, pair);
+			set->dlp_str = dt_alloc(dtp, data_str->d_size + len);
+			if (set->dlp_str == NULL) {
+				dt_free(dtp, set);
 				goto err;
 			}
 
-			if ((pair->dlp_sym = dt_alloc(dtp, data_sym->d_size +
-			    nsym * symsize)) == NULL) {
-				dt_free(dtp, pair->dlp_str);
-				dt_free(dtp, pair);
+			set->dlp_sym = dt_alloc(dtp, data_sym->d_size +
+						     nsym * symsize);
+			if (set->dlp_sym == NULL) {
+				dt_free(dtp, set->dlp_str);
+				dt_free(dtp, set);
 				goto err;
 			}
 
-			pair->dlp_next = bufs;
-			bufs = pair;
+			set->dlp_next = bufs;
+			bufs = set;
 
-			memcpy(pair->dlp_str, data_str->d_buf,
-			    data_str->d_size);
-			data_str->d_buf = pair->dlp_str;
+			memcpy(set->dlp_str, data_str->d_buf, data_str->d_size);
+			data_str->d_buf = set->dlp_str;
 			data_str->d_size += len;
 			elf_flagdata(data_str, ELF_C_SET, ELF_F_DIRTY);
 
 			shdr_str.sh_size += len;
 			gelf_update_shdr(scn_str, &shdr_str);
 
-			memcpy(pair->dlp_sym, data_sym->d_buf,
-			    data_sym->d_size);
-			data_sym->d_buf = pair->dlp_sym;
+			memcpy(set->dlp_sym, data_sym->d_buf, data_sym->d_size);
+			data_sym->d_buf = set->dlp_sym;
 			data_sym->d_size += nsym * symsize;
 			elf_flagdata(data_sym, ELF_C_SET, ELF_F_DIRTY);
 
@@ -1316,13 +1326,12 @@ process_obj(dtrace_hdl_t *dtp, const char *obj, int *eprobesp)
 		 * Now that the tables have been allocated, perform the
 		 * modifications described above.
 		 */
-		for (i = 0; i < shdr_rel.sh_size / shdr_rel.sh_entsize; i++) {
-
+		for (i = j = 0; i < shdr_rel.sh_size / shdr_rel.sh_entsize;
+		     i++) {
 			if (shdr_rel.sh_type == SHT_RELA) {
 				if (gelf_getrela(data_rel, i, &rela) == NULL)
 					continue;
 			} else {
-				GElf_Rel rel;
 				if (gelf_getrel(data_rel, i, &rel) == NULL)
 					continue;
 				rela.r_offset = rel.r_offset;
@@ -1338,8 +1347,25 @@ process_obj(dtrace_hdl_t *dtp, const char *obj, int *eprobesp)
 
 			s = (char *)data_str->d_buf + rsym.st_name;
 
-			if (strncmp(s, dt_prefix, sizeof(dt_prefix) - 1) != 0)
+			if (strncmp(s, dt_prefix, sizeof(dt_prefix) - 1) != 0) {
+				/*
+				 * If there was an earlier relocation that was
+				 * to be removed, we need to copy this one to
+				 * an earlier slot.
+				 */
+				if (j < i) {
+					mod = 1;
+					elf_flagdata(data_rel, ELF_C_SET, ELF_F_DIRTY);
+
+					if (shdr_rel.sh_type == SHT_RELA)
+						gelf_update_rela(data_rel, j, &rela);
+					else
+						gelf_update_rel(data_rel, j, &rel);
+				}
+
+				j++;
 				continue;
+			}
 
 			s += sizeof(dt_prefix) - 1;
 
@@ -1441,32 +1467,40 @@ process_obj(dtrace_hdl_t *dtp, const char *obj, int *eprobesp)
 			 */
 			if (rsym.st_shndx != SHN_SUNW_IGNORE) {
 				mod = 1;
-				elf_flagdata(data_tgt, ELF_C_SET, ELF_F_DIRTY);
+				elf_flagdata(data_sym, ELF_C_SET, ELF_F_DIRTY);
 				rsym.st_shndx = SHN_SUNW_IGNORE;
 				gelf_update_sym(data_sym, ndx, &rsym);
 			}
+		}
 
-			/*
-			 * This relocation is no longer needed.  The linker on
-			 * Linux doesn't know about SHN_SUNW_IGNORE, so we mark
-			 * the relocation with type NONE.  Failing to do so
-			 * causes the linker to try to fill in an address on
-			 * top of the NOPs we so carefully planted.
-			 */
-			if (rela.r_info != GELF_R_INFO(ndx,0)) {
-				mod = 1;
-				elf_flagdata(data_tgt, ELF_C_SET, ELF_F_DIRTY);
-				if (shdr_rel.sh_type == SHT_RELA) {
-					rela.r_info = GELF_R_INFO(ndx, 0);
-					gelf_update_rela(data_rel, i, &rela);
-				} else {
-					GElf_Rel rel;
-
-					rel.r_offset = rela.r_offset;
-					rel.r_info = GELF_R_INFO(ndx, 0);
-					gelf_update_rel(data_rel, i, &rel);
-				}
+		/*
+		 * If some relocations were removed, shrink the relocation
+		 * table.
+		 */
+		if (nrel > 0) {
+			size_t	sz = data_rel->d_size -
+				     nrel * shdr_rel.sh_entsize;
+
+			/* If we do not have a link set yet, allocate one. */
+			if (set == NULL) {
+				set = dt_zalloc(dtp, sizeof(*set));
+				if (set == NULL)
+					goto err;
 			}
+
+			set->dlp_rel = dt_alloc(dtp, sz);
+			if (set->dlp_rel == NULL)
+				goto err;
+
+			mod = 1;
+			elf_flagdata(data_rel, ELF_C_SET, ELF_F_DIRTY);
+
+			memcpy(set->dlp_rel, data_rel->d_buf, sz);
+			data_rel->d_buf = set->dlp_rel;
+			data_rel->d_size = sz;
+
+			shdr_rel.sh_size = sz;
+			gelf_update_shdr(scn_rel, &shdr_rel);
 		}
 	}
 
@@ -1476,11 +1510,12 @@ process_obj(dtrace_hdl_t *dtp, const char *obj, int *eprobesp)
 	elf_end(elf);
 	close(fd);
 
-	while ((pair = bufs) != NULL) {
-		bufs = pair->dlp_next;
-		dt_free(dtp, pair->dlp_str);
-		dt_free(dtp, pair->dlp_sym);
-		dt_free(dtp, pair);
+	while ((set = bufs) != NULL) {
+		bufs = set->dlp_next;
+		dt_free(dtp, set->dlp_str);
+		dt_free(dtp, set->dlp_sym);
+		dt_free(dtp, set->dlp_rel);
+		dt_free(dtp, set);
 	}
 
 	return 0;
diff --git a/test/unittest/usdt/tst.pie.r b/test/unittest/usdt/tst.pie.r
new file mode 100644
index 00000000..0cd43db7
--- /dev/null
+++ b/test/unittest/usdt/tst.pie.r
@@ -0,0 +1,2 @@
+test:func:go
+
diff --git a/test/unittest/usdt/tst.pie.sh b/test/unittest/usdt/tst.pie.sh
new file mode 100755
index 00000000..854cd85b
--- /dev/null
+++ b/test/unittest/usdt/tst.pie.sh
@@ -0,0 +1,79 @@
+#!/bin/bash
+#
+# Oracle Linux DTrace.
+# Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+# Licensed under the Universal Permissive License v 1.0 as shown at
+# http://oss.oracle.com/licenses/upl.
+#
+if [ $# != 1 ]; then
+	echo expected one argument: '<'dtrace-path'>'
+	exit 2
+fi
+
+dtrace=$1
+CC=/usr/bin/gcc
+CFLAGS="-fno-inline -pie"
+
+DIRNAME="$tmpdir/usdt-pie.$$.$RANDOM"
+mkdir -p $DIRNAME
+cd $DIRNAME
+
+cat > prov.d <<EOF
+provider test_prov {
+	probe go();
+};
+EOF
+
+$dtrace -h -s prov.d
+if [ $? -ne 0 ]; then
+	echo "failed to generate header file" >& 2
+	exit 1
+fi
+
+cat > test.c <<EOF
+#include <sys/types.h>
+#include "prov.h"
+
+static void
+func(void)
+{
+	TEST_PROV_GO();
+}
+
+int
+main(int argc, char **argv)
+{
+	func();
+}
+EOF
+
+${CC} ${CFLAGS} -c test.c
+if [ $? -ne 0 ]; then
+	echo "failed to compile test.c" >& 2
+	exit 1
+fi
+$dtrace -G -s prov.d test.o
+if [ $? -ne 0 ]; then
+	echo "failed to create DOF" >& 2
+	exit 1
+fi
+${CC} ${CFLAGS} -o test test.o prov.o
+if [ $? -ne 0 ]; then
+	echo "failed to link final executable" >& 2
+	exit 1
+fi
+
+script()
+{
+	$dtrace -c ./test -qs /dev/stdin <<EOF
+	test_prov\$target:::
+	{
+		printf("%s:%s:%s\n", probemod, probefunc, probename);
+	}
+EOF
+}
+
+script
+status=$?
+
+exit $status
-- 
2.39.2




More information about the DTrace-devel mailing list