diff mbox series

[v2] build: allow turning off --no-undefined and -z,defs

Message ID 20211203141721.987169-1-evvers@ya.ru
State Superseded
Headers show
Series [v2] build: allow turning off --no-undefined and -z,defs | expand

Commit Message

Evgeny Vereshchagin Dec. 3, 2021, 2:17 p.m. UTC
ASan, UBSan and MSan provided by clang aren't compatible with --no-undefined and -z,defs:
https://clang.llvm.org/docs/AddressSanitizer.html#usage
https://github.com/google/sanitizers/issues/380
so to build elfutils with clang with the sanitizers it should be possible
to turn them off.

Without this patch something like

sed -i 's/^\(ZDEFS_LDFLAGS=\).*/\1/' configure.ac
find -name Makefile.am | xargs sed -i 's/,--no-undefined//'

should be used to make elfutils compile.

Issues like https://bugs.llvm.org/show_bug.cgi?id=30333 have been
open since at least 2016 so it seems it's safe to say that it isn't
going to be fixed anytime soon. It's so ingrained that some build
systems complain when `-fsanitize=...` is passed to clang without
turning off no-undefined.

The patch was tested in https://github.com/evverx/elfutils/pull/24 by
compiling elfutils with both gcc and clang with and without ASan/UBsan
and running `make check && make distcheck`. --no-undefined and -z,defs
are still passed by default as expected.

Signed-off-by: Evgeny Vereshchagin <evvers@ya.ru>
---
 ChangeLog              |  5 +++++
 configure.ac           | 31 ++++++++++++++++++++++---------
 debuginfod/Makefile.am |  2 +-
 libasm/Makefile.am     |  2 +-
 libdw/Makefile.am      |  2 +-
 libelf/Makefile.am     |  2 +-
 6 files changed, 31 insertions(+), 13 deletions(-)

Comments

Mark Wielaard Dec. 4, 2021, 8:26 p.m. UTC | #1
Hi Evgeny,

On Fri, Dec 03, 2021 at 02:17:21PM +0000, Evgeny Vereshchagin wrote:
> ASan, UBSan and MSan provided by clang aren't compatible with --no-undefined and -z,defs:
> https://clang.llvm.org/docs/AddressSanitizer.html#usage
> https://github.com/google/sanitizers/issues/380
> so to build elfutils with clang with the sanitizers it should be possible
> to turn them off.

I have to agree with Florian, this really is a bug in the compiler you
are using.  Adding -fsanitize=address to CFLAGS/CXXFLAGS works just
fine with gcc.  I have been using it in the past in combination with
the afl fuzzer (32bit only).  It doesn't work together with
--enable-valgrind though.

That said, I really would like to add address sanitizer support. It
would be great to add this to our buildbot CI to catch more issues
early. We already support --enable-sanitize-undefined and
--enable-valgrind. But sadly we have to disable valgrind in a couple
of testcases, specifically when testing the debuginfod server.

I just testing with gcc (Debian 10.2.1-6) 10.2.1 20210110 on arm64 and
it actually found some issues. I'll post patches for those.

There is one issue with the test-nlist test because we use special
CFLAGS for that. But if we introduce an --enable-sanitize-address we
could work around that.

If clang really cannot be fixed then your patch in combination with an
--enable-sanitize-address might be a good idea. But I don't think it
makes sense as a standalone option. In the past we made the mistake of
adding configure options to disable some necessary flags, like
--disable-symbol-versioning, which was a mistake. There are now
distros shipping elfutils libraries with broken abis while using the
same SONAMEs.

Cheers,

Mark
diff mbox series

Patch

diff --git a/ChangeLog b/ChangeLog
index d61b21c7..33d20be5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@ 
+2021-12-03  Evgeny Vereshchagin  <evvers@ya.ru>
+
+	* configure.ac [--disable-no-undefined]: Allow turning off
+	--no-undefined and -z,defs to build elfutils with clang sanitizers.
+
 2021-11-10  Mark Wielaard  <mark@klomp.org>
 
 	* configure.ac (AC_INIT): Set version to 0.186.
diff --git a/configure.ac b/configure.ac
index ff9581d2..14cd2e6f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -153,16 +153,29 @@  AC_SUBST([fpie_CFLAGS])
 
 dso_LDFLAGS="-shared"
 
-ZDEFS_LDFLAGS="-Wl,-z,defs"
-AC_CACHE_CHECK([whether gcc supports $ZDEFS_LDFLAGS], ac_cv_zdefs, [dnl
-save_LDFLAGS="$LDFLAGS"
-LDFLAGS="$ZDEFS_LDFLAGS $save_LDFLAGS"
-AC_LINK_IFELSE([AC_LANG_PROGRAM()], ac_cv_zdefs=yes, ac_cv_zdefs=no)
-LDFLAGS="$save_LDFLAGS"
-])
-if test "$ac_cv_zdefs" = "yes"; then
-	dso_LDFLAGS="$dso_LDFLAGS $ZDEFS_LDFLAGS"
+# ASan, UBSan and MSan provided by clang aren't compatible with --no-undefined and -z,defs:
+# https://clang.llvm.org/docs/AddressSanitizer.html#usage
+# https://github.com/google/sanitizers/issues/380
+# so to build elfutils with clang with the sanitizers it should be possible
+# to turn them off.
+AC_ARG_ENABLE([no-undefined],
+AS_HELP_STRING([--disable-no-undefined],[disable --no-undefined and -z,defs]),
+[use_no_undefined=$enableval], [use_no_undefined=yes])
+if test "$use_no_undefined" = yes; then
+	ZDEFS_LDFLAGS="-Wl,-z,defs"
+	AC_CACHE_CHECK([whether gcc supports $ZDEFS_LDFLAGS], ac_cv_zdefs, [dnl
+	save_LDFLAGS="$LDFLAGS"
+	LDFLAGS="$ZDEFS_LDFLAGS $save_LDFLAGS"
+	AC_LINK_IFELSE([AC_LANG_PROGRAM()], ac_cv_zdefs=yes, ac_cv_zdefs=no)
+	LDFLAGS="$save_LDFLAGS"
+	])
+	if test "$ac_cv_zdefs" = "yes"; then
+		dso_LDFLAGS="$dso_LDFLAGS $ZDEFS_LDFLAGS"
+	fi
+
+	NO_UNDEFINED=",--no-undefined"
 fi
+AC_SUBST([NO_UNDEFINED])
 
 # We really want build-ids. Warn and force generating them if gcc was
 # configure without --enable-linker-build-id
diff --git a/debuginfod/Makefile.am b/debuginfod/Makefile.am
index 3adb2755..58bf71d3 100644
--- a/debuginfod/Makefile.am
+++ b/debuginfod/Makefile.am
@@ -102,7 +102,7 @@  endif
 $(LIBDEBUGINFOD_SONAME): $(srcdir)/libdebuginfod.map $(libdebuginfod_so_LIBS)
 	$(AM_V_CCLD)$(LINK) $(dso_LDFLAGS) -o $@ \
 		-Wl,--soname,$(LIBDEBUGINFOD_SONAME) \
-		-Wl,--version-script,$<,--no-undefined \
+		-Wl,--version-script,$<$(NO_UNDEFINED) \
 		-Wl,--whole-archive $(libdebuginfod_so_LIBS) -Wl,--no-whole-archive \
 		$(libdebuginfod_so_LDLIBS)
 	@$(textrel_check)
diff --git a/libasm/Makefile.am b/libasm/Makefile.am
index c2b54811..683c9847 100644
--- a/libasm/Makefile.am
+++ b/libasm/Makefile.am
@@ -64,7 +64,7 @@  libasm_so_LIBS = libasm_pic.a
 libasm.so: $(srcdir)/libasm.map $(libasm_so_LIBS) $(libasm_so_DEPS)
 	$(AM_V_CCLD)$(LINK) $(dso_LDFLAGS) -o $@ \
 		-Wl,--soname,$@.$(VERSION) \
-		-Wl,--version-script,$<,--no-undefined \
+		-Wl,--version-script,$<$(NO_UNDEFINED) \
 		-Wl,--whole-archive $(libasm_so_LIBS) -Wl,--no-whole-archive \
 		$(libasm_so_LDLIBS)
 	@$(textrel_check)
diff --git a/libdw/Makefile.am b/libdw/Makefile.am
index 4fda33bd..534e5cc7 100644
--- a/libdw/Makefile.am
+++ b/libdw/Makefile.am
@@ -114,7 +114,7 @@  libdw_so_LDLIBS = $(libdw_so_DEPS) -ldl -lz $(argp_LDADD) $(fts_LIBS) $(obstack_
 libdw.so: $(srcdir)/libdw.map $(libdw_so_LIBS) $(libdw_so_DEPS)
 	$(AM_V_CCLD)$(LINK) $(dso_LDFLAGS) -o $@ \
 		-Wl,--soname,$@.$(VERSION),--enable-new-dtags \
-		-Wl,--version-script,$<,--no-undefined \
+		-Wl,--version-script,$<$(NO_UNDEFINED) \
 		-Wl,--whole-archive $(libdw_so_LIBS) -Wl,--no-whole-archive \
 		$(libdw_so_LDLIBS)
 	@$(textrel_check)
diff --git a/libelf/Makefile.am b/libelf/Makefile.am
index 560ed45f..034b7a0d 100644
--- a/libelf/Makefile.am
+++ b/libelf/Makefile.am
@@ -115,7 +115,7 @@  libelf_so_LIBS = libelf_pic.a
 libelf.so: $(srcdir)/libelf.map $(libelf_so_LIBS) $(libelf_so_DEPS)
 	$(AM_V_CCLD)$(LINK) $(dso_LDFLAGS) -o $@ \
 		-Wl,--soname,$@.$(VERSION) \
-		-Wl,--version-script,$<,--no-undefined \
+		-Wl,--version-script,$<$(NO_UNDEFINED) \
 		-Wl,--whole-archive $(libelf_so_LIBS) -Wl,--no-whole-archive \
 		$(libelf_so_LDLIBS)
 	@$(textrel_check)