diff mbox

[RFC,lttng-ust,2/2] dl instrumentation: add dlmopen event, trace dlopen flags

Message ID 1467672828-19184-2-git-send-email-mathieu.desnoyers@efficios.com
State Accepted, archived
Headers show

Commit Message

Mathieu Desnoyers July 4, 2016, 10:53 p.m. UTC
Add missing dlmopen(3) instrumentation, and add the "flags" field to
dlopen instrumentation.

Link: http://man7.org/linux/man-pages/man3/dlopen.3.html
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
---
 include/lttng/ust-dlfcn.h      |  10 ++++
 liblttng-ust-dl/lttng-ust-dl.c | 114 ++++++++++++++++++++++++++++++++++++++---
 liblttng-ust-dl/ust_dl.c       |   1 +
 liblttng-ust-dl/ust_dl.h       |  19 ++++++-
 4 files changed, 135 insertions(+), 9 deletions(-)

Comments

Thomas Petazzoni Feb. 21, 2017, 9:03 a.m. UTC | #1
Hello,

On Mon,  4 Jul 2016 18:53:48 -0400, Mathieu Desnoyers wrote:
> Add missing dlmopen(3) instrumentation, and add the "flags" field to
> dlopen instrumentation.
> 
> Link: http://man7.org/linux/man-pages/man3/dlopen.3.html
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>

FYI, this commit (7b0fdd83c137bea1d7302df1a45252dfdccd8172) breaks the
build with the musl C library, because it doesn't implement the
dlmopen() extension.

Perhaps there should be an autoconf test to check if the C library
provides dlmopen() functionality or not?

Best regards,

Thomas Petazzoni
Mathieu Desnoyers Feb. 21, 2017, 2:08 p.m. UTC | #2
----- On Feb 21, 2017, at 4:03 AM, Thomas Petazzoni thomas.petazzoni at free-electrons.com wrote:

> Hello,
> 
> On Mon,  4 Jul 2016 18:53:48 -0400, Mathieu Desnoyers wrote:
>> Add missing dlmopen(3) instrumentation, and add the "flags" field to
>> dlopen instrumentation.
>> 
>> Link: http://man7.org/linux/man-pages/man3/dlopen.3.html
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> 
> FYI, this commit (7b0fdd83c137bea1d7302df1a45252dfdccd8172) breaks the
> build with the musl C library, because it doesn't implement the
> dlmopen() extension.
> 
> Perhaps there should be an autoconf test to check if the C library
> provides dlmopen() functionality or not?

My expectation is that the dlmopen override symbol gets generated,
but it is never invoked in builds where the libc does not implement
it, so we never have to look it up dynamically through dlsym().

How does the build fail with musl ? Can you provide the build
output ?

Thanks,

Mathieu

> 
> Best regards,
> 
> Thomas Petazzoni
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
Thomas Petazzoni Feb. 21, 2017, 2:19 p.m. UTC | #3
Hello,

On Tue, 21 Feb 2017 14:08:08 +0000 (UTC), Mathieu Desnoyers wrote:

> My expectation is that the dlmopen override symbol gets generated,
> but it is never invoked in builds where the libc does not implement
> it, so we never have to look it up dynamically through dlsym().

The main issue is that the Lmid_t type doesn't exist I believe.

> How does the build fail with musl ? Can you provide the build
> output ?

Sure:

  http://autobuild.buildroot.net/results/113/113fa95a054df183014b43ec36b329f9c59bd356/build-end.log

If you want to reproduce:

$ git clone git://git.busybox.net/buildroot
$ cd buildroot/
$ git checkout 2017.02-rc2
$ cat > .config
BR2_arm=y
BR2_cortex_a9=y
BR2_ARM_ENABLE_VFP=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-arm-cortex-a9-musl-2017.02-rc1-2-g133c5ac.tar.bz2"
BR2_TOOLCHAIN_EXTERNAL_GCC_5=y
BR2_TOOLCHAIN_EXTERNAL_HEADERS_4_9=y
BR2_TOOLCHAIN_EXTERNAL_CUSTOM_MUSL=y
BR2_TOOLCHAIN_EXTERNAL_CXX=y
BR2_INIT_NONE=y
BR2_SYSTEM_BIN_SH_NONE=y
# BR2_PACKAGE_BUSYBOX is not set
BR2_PACKAGE_LTTNG_LIBUST=y
# BR2_TARGET_ROOTFS_TAR is not set
$ make olddefconfig
$ make

Takes ~5 minutes maybe for the all build to proceed until it breaks on
lttng-ust.

Thanks!

Thomas
Mathieu Desnoyers Feb. 21, 2017, 2:40 p.m. UTC | #4
----- On Feb 21, 2017, at 9:19 AM, Thomas Petazzoni thomas.petazzoni at free-electrons.com wrote:

> Hello,
> 
> On Tue, 21 Feb 2017 14:08:08 +0000 (UTC), Mathieu Desnoyers wrote:
> 
>> My expectation is that the dlmopen override symbol gets generated,
>> but it is never invoked in builds where the libc does not implement
>> it, so we never have to look it up dynamically through dlsym().
> 
> The main issue is that the Lmid_t type doesn't exist I believe.

Ah yes, good point. This typedef is only there with glibc header
files.

So we'd have to add a configure.ac test that would detect if dlmopen()
is there, and expose a define to lttng-ust to skip the dlmopen() bits
accordingly.

Jonathan, can you have a look and prepare a patch please ?

Thanks,

Mathieu

> 
>> How does the build fail with musl ? Can you provide the build
>> output ?
> 
> Sure:
> 
>  http://autobuild.buildroot.net/results/113/113fa95a054df183014b43ec36b329f9c59bd356/build-end.log
> 
> If you want to reproduce:
> 
> $ git clone git://git.busybox.net/buildroot
> $ cd buildroot/
> $ git checkout 2017.02-rc2
> $ cat > .config
> BR2_arm=y
> BR2_cortex_a9=y
> BR2_ARM_ENABLE_VFP=y
> BR2_TOOLCHAIN_EXTERNAL=y
> BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
> BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
> BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-arm-cortex-a9-musl-2017.02-rc1-2-g133c5ac.tar.bz2"
> BR2_TOOLCHAIN_EXTERNAL_GCC_5=y
> BR2_TOOLCHAIN_EXTERNAL_HEADERS_4_9=y
> BR2_TOOLCHAIN_EXTERNAL_CUSTOM_MUSL=y
> BR2_TOOLCHAIN_EXTERNAL_CXX=y
> BR2_INIT_NONE=y
> BR2_SYSTEM_BIN_SH_NONE=y
> # BR2_PACKAGE_BUSYBOX is not set
> BR2_PACKAGE_LTTNG_LIBUST=y
> # BR2_TARGET_ROOTFS_TAR is not set
> $ make olddefconfig
> $ make
> 
> Takes ~5 minutes maybe for the all build to proceed until it breaks on
> lttng-ust.
> 
> Thanks!
> 
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
Jonathan Rajotte Feb. 21, 2017, 3:45 p.m. UTC | #5
On Tue, Feb 21, 2017 at 02:40:28PM +0000, Mathieu Desnoyers wrote:
> ----- On Feb 21, 2017, at 9:19 AM, Thomas Petazzoni thomas.petazzoni at free-electrons.com wrote:
> 
> > Hello,
> > 
> > On Tue, 21 Feb 2017 14:08:08 +0000 (UTC), Mathieu Desnoyers wrote:
> > 
> >> My expectation is that the dlmopen override symbol gets generated,
> >> but it is never invoked in builds where the libc does not implement
> >> it, so we never have to look it up dynamically through dlsym().
> > 
> > The main issue is that the Lmid_t type doesn't exist I believe.
> 
> Ah yes, good point. This typedef is only there with glibc header
> files.
> 
> So we'd have to add a configure.ac test that would detect if dlmopen()
> is there, and expose a define to lttng-ust to skip the dlmopen() bits
> accordingly.
> 
> Jonathan, can you have a look and prepare a patch please ?

Sure.

> 
> Thanks,
> 
> Mathieu
> 
> > 
> >> How does the build fail with musl ? Can you provide the build
> >> output ?
> > 
> > Sure:
> > 
> >  http://autobuild.buildroot.net/results/113/113fa95a054df183014b43ec36b329f9c59bd356/build-end.log
> > 
> > If you want to reproduce:
> > 
> > $ git clone git://git.busybox.net/buildroot
> > $ cd buildroot/
> > $ git checkout 2017.02-rc2
> > $ cat > .config
> > BR2_arm=y
> > BR2_cortex_a9=y
> > BR2_ARM_ENABLE_VFP=y
> > BR2_TOOLCHAIN_EXTERNAL=y
> > BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
> > BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
> > BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-arm-cortex-a9-musl-2017.02-rc1-2-g133c5ac.tar.bz2"
> > BR2_TOOLCHAIN_EXTERNAL_GCC_5=y
> > BR2_TOOLCHAIN_EXTERNAL_HEADERS_4_9=y
> > BR2_TOOLCHAIN_EXTERNAL_CUSTOM_MUSL=y
> > BR2_TOOLCHAIN_EXTERNAL_CXX=y
> > BR2_INIT_NONE=y
> > BR2_SYSTEM_BIN_SH_NONE=y
> > # BR2_PACKAGE_BUSYBOX is not set
> > BR2_PACKAGE_LTTNG_LIBUST=y
> > # BR2_TARGET_ROOTFS_TAR is not set
> > $ make olddefconfig
> > $ make
> > 
> > Takes ~5 minutes maybe for the all build to proceed until it breaks on
> > lttng-ust.
> > 
> > Thanks!
> > 
> > Thomas
> > --
> > Thomas Petazzoni, CTO, Free Electrons
> > Embedded Linux and Kernel engineering
> > http://free-electrons.com
> 
> -- 
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
diff mbox

Patch

diff --git a/include/lttng/ust-dlfcn.h b/include/lttng/ust-dlfcn.h
index 7471d94..7cd5bf4 100644
--- a/include/lttng/ust-dlfcn.h
+++ b/include/lttng/ust-dlfcn.h
@@ -47,7 +47,11 @@ 
 #define dlclose glibc_dlclose_proto_lies_about_leafness
 #define dlsym glibc_dlsym_proto_lies_about_leafness
 #define dlerror glibc_dlerror_proto_lies_about_leafness
+#define dlmopen glibc_dlmopen_proto_lies_about_leafness
+#define dlvsym glibc_dlvsym_proto_lies_about_leafness
 #include <dlfcn.h>
+#undef dlvsym
+#undef dlmopen
 #undef dlerror
 #undef dlsym
 #undef dlclose
@@ -58,6 +62,12 @@  extern int dlclose(void *__handle) __nonnull ((1));
 extern void *dlsym(void *__restrict __handle,
 		__const char *__restrict __name) __nonnull ((2));
 extern char *dlerror(void);
+#ifdef __USE_GNU
+extern void *dlmopen(Lmid_t __nsid, const char *__file, int __mode);
+extern void *dlvsym(void *__restrict __handle,
+		__const char *__restrict __name,
+		__const char *__restrict __version);
+#endif
 #else
 #include <dlfcn.h>
 #endif /* __GLIBC__ */
diff --git a/liblttng-ust-dl/lttng-ust-dl.c b/liblttng-ust-dl/lttng-ust-dl.c
index 81b5bec..b0737b6 100644
--- a/liblttng-ust-dl/lttng-ust-dl.c
+++ b/liblttng-ust-dl/lttng-ust-dl.c
@@ -37,11 +37,13 @@ 
 #define TRACEPOINT_DEFINE
 #include "ust_dl.h"
 
-static void *(*__lttng_ust_plibc_dlopen)(const char *filename, int flag);
+static void *(*__lttng_ust_plibc_dlopen)(const char *filename, int flags);
+static void *(*__lttng_ust_plibc_dlmopen)(Lmid_t nsid, const char *filename,
+		int flags);
 static int (*__lttng_ust_plibc_dlclose)(void *handle);
 
 static
-void *_lttng_ust_dl_libc_dlopen(const char *filename, int flag)
+void *_lttng_ust_dl_libc_dlopen(const char *filename, int flags)
 {
 	if (!__lttng_ust_plibc_dlopen) {
 		__lttng_ust_plibc_dlopen = dlsym(RTLD_NEXT, "dlopen");
@@ -50,7 +52,21 @@  void *_lttng_ust_dl_libc_dlopen(const char *filename, int flag)
 			return NULL;
 		}
 	}
-	return __lttng_ust_plibc_dlopen(filename, flag);
+	return __lttng_ust_plibc_dlopen(filename, flags);
+}
+
+static
+void *_lttng_ust_dl_libc_dlmopen(Lmid_t nsid, const char *filename,
+		int flags)
+{
+	if (!__lttng_ust_plibc_dlmopen) {
+		__lttng_ust_plibc_dlmopen = dlsym(RTLD_NEXT, "dlmopen");
+		if (!__lttng_ust_plibc_dlmopen) {
+			fprintf(stderr, "%s\n", dlerror());
+			return NULL;
+		}
+	}
+	return __lttng_ust_plibc_dlmopen(nsid, filename, flags);
 }
 
 static
@@ -67,7 +83,8 @@  int _lttng_ust_dl_libc_dlclose(void *handle)
 }
 
 static
-void lttng_ust_dl_dlopen(void *so_base, const char *so_name, void *ip)
+void lttng_ust_dl_dlopen(void *so_base, const char *so_name,
+		int flags, void *ip)
 {
 	char resolved_path[PATH_MAX];
 	struct lttng_ust_elf *elf;
@@ -106,7 +123,7 @@  void lttng_ust_dl_dlopen(void *so_base, const char *so_name, void *ip)
 	}
 
 	tracepoint(lttng_ust_dl, dlopen,
-		ip, so_base, resolved_path, memsz,
+		ip, so_base, resolved_path, flags, memsz,
 		has_build_id, has_debug_link);
 
 	if (has_build_id) {
@@ -126,11 +143,72 @@  end:
 	return;
 }
 
-void *dlopen(const char *filename, int flag)
+static
+void lttng_ust_dl_dlmopen(void *so_base, Lmid_t nsid, const char *so_name,
+		int flags, void *ip)
+{
+	char resolved_path[PATH_MAX];
+	struct lttng_ust_elf *elf;
+	uint64_t memsz;
+	uint8_t *build_id = NULL;
+	size_t build_id_len;
+	char *dbg_file = NULL;
+	uint32_t crc;
+	int has_build_id = 0, has_debug_link = 0;
+	int ret;
+
+	if (!realpath(so_name, resolved_path)) {
+		ERR("could not resolve path '%s'", so_name);
+		return;
+	}
+
+	elf = lttng_ust_elf_create(resolved_path);
+	if (!elf) {
+		ERR("could not acces file %s", resolved_path);
+		return;
+	}
+
+	ret = lttng_ust_elf_get_memsz(elf, &memsz);
+	if (ret) {
+		goto end;
+	}
+	ret = lttng_ust_elf_get_build_id(
+		elf, &build_id, &build_id_len, &has_build_id);
+	if (ret) {
+		goto end;
+	}
+	ret = lttng_ust_elf_get_debug_link(
+		elf, &dbg_file, &crc, &has_debug_link);
+	if (ret) {
+		goto end;
+	}
+
+	tracepoint(lttng_ust_dl, dlmopen,
+		ip, so_base, nsid, resolved_path, flags, memsz,
+		has_build_id, has_debug_link);
+
+	if (has_build_id) {
+		tracepoint(lttng_ust_dl, build_id,
+			ip, so_base, build_id, build_id_len);
+	}
+
+	if (has_debug_link) {
+		tracepoint(lttng_ust_dl, debug_link,
+			ip, so_base, dbg_file, crc);
+	}
+
+end:
+	free(dbg_file);
+	free(build_id);
+	lttng_ust_elf_destroy(elf);
+	return;
+}
+
+void *dlopen(const char *filename, int flags)
 {
 	void *handle;
 
-	handle = _lttng_ust_dl_libc_dlopen(filename, flag);
+	handle = _lttng_ust_dl_libc_dlopen(filename, flags);
 	if (__tracepoint_ptrs_registered && handle) {
 		struct link_map *p = NULL;
 		int ret;
@@ -138,12 +216,32 @@  void *dlopen(const char *filename, int flag)
 		ret = dlinfo(handle, RTLD_DI_LINKMAP, &p);
 		if (ret != -1 && p != NULL && p->l_addr != 0) {
 			lttng_ust_dl_dlopen((void *) p->l_addr,
-				p->l_name,
+				p->l_name, flags, LTTNG_UST_CALLER_IP());
+		}
+	}
+	lttng_ust_dl_update(LTTNG_UST_CALLER_IP());
+	return handle;
+}
+
+void *dlmopen(Lmid_t nsid, const char *filename, int flags)
+{
+	void *handle;
+
+	handle = _lttng_ust_dl_libc_dlmopen(nsid, filename, flags);
+	if (__tracepoint_ptrs_registered && handle) {
+		struct link_map *p = NULL;
+		int ret;
+
+		ret = dlinfo(handle, RTLD_DI_LINKMAP, &p);
+		if (ret != -1 && p != NULL && p->l_addr != 0) {
+			lttng_ust_dl_dlmopen((void *) p->l_addr,
+				nsid, p->l_name, flags,
 				LTTNG_UST_CALLER_IP());
 		}
 	}
 	lttng_ust_dl_update(LTTNG_UST_CALLER_IP());
 	return handle;
+
 }
 
 int dlclose(void *handle)
diff --git a/liblttng-ust-dl/ust_dl.c b/liblttng-ust-dl/ust_dl.c
index b25ba7b..2324143 100644
--- a/liblttng-ust-dl/ust_dl.c
+++ b/liblttng-ust-dl/ust_dl.c
@@ -16,6 +16,7 @@ 
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
  */
 
+#define _GNU_SOURCE
 #define _LGPL_SOURCE
 #define TRACEPOINT_CREATE_PROBES
 #define TP_IP_PARAM ip
diff --git a/liblttng-ust-dl/ust_dl.h b/liblttng-ust-dl/ust_dl.h
index 24c9b70..b8cfe82 100644
--- a/liblttng-ust-dl/ust_dl.h
+++ b/liblttng-ust-dl/ust_dl.h
@@ -38,12 +38,29 @@  extern "C" {
 #include <lttng/tracepoint.h>
 
 TRACEPOINT_EVENT(lttng_ust_dl, dlopen,
-	TP_ARGS(void *, ip, void *, baddr, const char*, path,
+	TP_ARGS(void *, ip, void *, baddr, const char *, path,
+		int, flags, uint64_t, memsz, uint8_t, has_build_id,
+		uint8_t, has_debug_link),
+	TP_FIELDS(
+		ctf_integer_hex(void *, baddr, baddr)
+		ctf_integer(uint64_t, memsz, memsz)
+		ctf_integer_hex(int, flags, flags)
+		ctf_string(path, path)
+		ctf_integer(uint8_t, has_build_id, has_build_id)
+		ctf_integer(uint8_t, has_debug_link, has_debug_link)
+	)
+)
+
+TRACEPOINT_EVENT(lttng_ust_dl, dlmopen,
+	TP_ARGS(void *, ip, void *, baddr, Lmid_t, nsid,
+		const char *, path, int, flags,
 		uint64_t, memsz, uint8_t, has_build_id,
 		uint8_t, has_debug_link),
 	TP_FIELDS(
 		ctf_integer_hex(void *, baddr, baddr)
 		ctf_integer(uint64_t, memsz, memsz)
+		ctf_integer(Lmid_t, nsid, nsid)
+		ctf_integer_hex(int, flags, flags)
 		ctf_string(path, path)
 		ctf_integer(uint8_t, has_build_id, has_build_id)
 		ctf_integer(uint8_t, has_debug_link, has_debug_link)