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

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

[Evgeny Vereshchagin]

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.

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(-)

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)
-- 
2.31.1

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

[Evgeny Vereshchagin]

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(-)

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)
-- 
2.31.1

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

[Evgeny Vereshchagin]

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.

It's implemented as a standalone option because there are places like
OSS-Fuzz for example where all the sanitizer flags are passed via CFLAGS
and CXXFLAGS: https://google.github.io/oss-fuzz/getting-started/new-project-guide/#Requirements
and it should be possible to just turn off --no-undefined and -z,defs
withut flipping other "configure" options and interfering with all
those fine-grained -fsanitize=... and -fno-sanitize-recover=... compiler flags.  Other than that, while
options like --enable-sanitize-undefined are helpful shortcuts, they
simply can't cover all the usecases and it's still necessary to pass
additional compiler flags to clang via CFLAGS to for example get around issues like
https://github.com/evverx/elfutils/issues/16 and
https://github.com/evverx/elfutils/issues/15.

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.

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.

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(-)

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)
-- 
2.31.1

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

[Florian Weimer]

* Evgeny Vereshchagin:

> 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.
>
> 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.

Why isn't this a bug in the compiler driver?  Nowadays, GCC passes
-lasan if -fsanitize=address is used.  I think that's quite reasonable.

Thanks,
Florian

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

[Mark Wielaard]

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

Re: [PATCH v3] build: allow turning off --no-undefined and -z,defs

[Mark Wielaard]

Hi Evgeny,

I really appreciate you helping us using more analyzers and fuzzers to
improve the code base. I think the address sanitizer already helped
improve the code. But it would help if you replied to the original
reviews and/or mentioned how the different versions of your patch have
changed since the last time. As far as I can see you only changed the
commit message a little this time.

On Fri, Dec 03, 2021 at 06:02:09PM +0000, Evgeny Vereshchagin wrote:
> Other than that, while options like --enable-sanitize-undefined are
> helpful shortcuts, they simply can't cover all the usecases and it's
> still necessary to pass additional compiler flags to clang via
> CFLAGS to for example get around issues like
> https://github.com/evverx/elfutils/issues/16 and
> https://github.com/evverx/elfutils/issues/15.

I really do believe that working from the now proposed
--enable-sanatize-address will be easiest to integrate address
sanitizer support. See how I used it to workaround isssues with the
gcc address sanitizer. You can use it likewise to work around issues
with clang. e.g. the configure check should detect the issue with
--no-undefined and could try if adding -lasan to LDFLAGS helps.

So the other isssues you are seeing with the clang address sanitizer
is "applying non-zero offset ... to null pointer" and "variable length
array bound evaluates to non-positive value 0". I am not sure how
those are actual problems. In the first case the calculated addresses
aren't actually used as (dereferenced) pointers, in the second the
array bound being zero also means the array content isn't used. Do you
know why these issues are flagged? Are there any extra ASAN_OPTIONS
set in these cases?

Cheers,

Mark

Re: [PATCH v3] build: allow turning off --no-undefined and -z,defs

[Evgeny Vereshchagin]

Hi Mark,

>  But it would help if you replied to the original
> reviews and/or mentioned how the different versions of your patch have
> changed since the last time.

I did but it looks like those emails didn't pass the spam filter. I'll try to figure out what happened there. Sorry about that!

>  As far as I can see you only changed the
> commit message a little this time.

That's correct. I tried to explain in the commit message why `--disable-undefined` is implemented as a standalone option.

> See how I used it to workaround isssues with the
> gcc address sanitizer. You can use it likewise to work around issues
> with clang. e.g. the configure check should detect the issue with
> --no-undefined and could try if adding -lasan to LDFLAGS helps

I saw that patch and I think it should make building elfutils with gcc and running the unit tests under ASan easier. Thanks! But it's based on the assumption that configure controls ASan flags and can change CFLAGS/LDFLAGS however it needs. Unfortunately I can't do that on OSS-Fuzz because all the sanitizer options are passed via CFLAGS there and I can't interfere with those CFLAGS. FWIW it isn't a theoretical issue because elfutils was integrated into OSS-Fuzz in https://github.com/google/oss-fuzz/pull/6944 and has been fuzzed there since then. And there elfutils is also built with MSan as well (which has never been implemented in gcc) and I'm not sure how additional configure options can cover that.

I agree that it would be great to make `--enable-sanitize-{undefined,address}` work with clang as well but I think it can be done later on top of `--disable-undefined`.

>  Do you
> know why these issues are flagged? Are there any extra ASAN_OPTIONS
> set in these cases?

No, there aren't. Those issues are flagged because -fsanitize=undefined in clang by default includes "pointer-overflow" and "vla-bound" (which as far as I know aren't available in gcc)

Re: [PATCH v3] build: allow turning off --no-undefined and -z,defs

[Mark Wielaard]

Hi Evgeny,

On Sun, 2021-12-05 at 19:52 +0300, Evgeny Vereshchagin wrote:
> > See how I used it to workaround isssues with the
> > gcc address sanitizer. You can use it likewise to work around
> > issues
> > with clang. e.g. the configure check should detect the issue with
> > --no-undefined and could try if adding -lasan to LDFLAGS helps
> 
> I saw that patch and I think it should make building elfutils with
> gcc and running the unit tests under ASan easier. Thanks! But it's
> based on the assumption that configure controls ASan flags and can
> change CFLAGS/LDFLAGS however it needs. Unfortunately I can't do that
> on OSS-Fuzz because all the sanitizer options are passed via CFLAGS
> there and I can't interfere with those CFLAGS.

But that doesn't really work if you use clang. It would actually work
as is if you used gcc. But I am not sure trying to use arbitrary
sanitizer flags that aren't tested in the upstream project is a good
idea.

I am not against OSS-Fuzz. I have had good experiences with using
fuzzers on the elfutils code base. But I find the project slightly
annoying. It requires a github and a google account and it hides the
results from the upstream project. Also the way they setup the fuzzers
feels odd (like how they try to cram everything through the CFLAGS and
how they try to link against a C++ library even for plain C projects).
I really would like to have any fuzzing targets be part of the upstream
project so we can all run the fuzzers instead of having to rely of
Google.

> I agree that it would be great to make `--enable-sanitize-
> {undefined,address}` work with clang as well but I think it can be
> done later on top of `--disable-undefined`.

I think it should be done as part of --enable-sanitize-address.

> >  Do you
> > know why these issues are flagged? Are there any extra ASAN_OPTIONS
> > set in these cases?
> 
> No, there aren't. Those issues are flagged because
> -fsanitize=undefined in clang by default includes "pointer-overflow"
> and "vla-bound" (which as far as I know aren't available in gcc)

But those seem to report bogus issues. At least in these cases, it
seems the code is fine.

Cheers,

Mark

Re: [PATCH v3] build: allow turning off --no-undefined and -z,defs

[Evgeny Vereshchagin]

Hi Mark,

> But that doesn't really work if you use clang.

It kind of does because almost everybody who builds their projects with clang sanitizers
turns off `z,defs` and `--no-undefined`. I agree it looks weird (and it's probably weird) but
it's just how it has been done for I don't know how many years. My understanding is
that it will never be fixed mostly because unlike gcc, clang doesn't support "shared" ASan/UBSan/MSan
(or, more precisely it isn't actively maintained there and it isn't used in general).

> sanitizer flags that aren't tested in the upstream project is a good
> idea.
> 

I wouldn't say that they are arbitrary. They have been tested with about 400 projects I think
and new flags are rolled out only if they don't break anything.

> It requires a github and a google account and it hides the
> results from the upstream project.

I don't think github accounts are required there but to due to some limitations gmail accounts
have to be used unfortunately. There is a flag there I can flip to prevent OSS-Fuzz from
restricting bug reports in the first place but I think I wrote it elsewhere already (after this email was sent)
and it'd probably make sense to keep discussing it there.

> Also the way they setup the fuzzers
> feels odd (like how they try to cram everything through the CFLAGS and
> how they try to link against a C++ library even for plain C projects).

They have to support a lot of different build systems there and I think it was decided that
CFLAGS is the only thing that they can use to affect them (which I think makes sense).
clang++ and stlibc++ have something to do with UBSan as far as I know.

> I really would like to have any fuzzing targets be part of the upstream
> project so we can all run the fuzzers instead of having to rely of
> Google.

I'm planning to move the fuzz targets upstream eventually and include them in the test suite
at least but I think they should be
compatible with OSS-Fuzz either way (because it's kind of hard to set up continuous
fuzzing manually)

>> I agree that it would be great to make `--enable-sanitize-
>> {undefined,address}` work with clang as well but I think it can be
>> done later on top of `--disable-undefined`.
> 
> I think it should be done as part of --enable-sanitize-address.

If --enable-sanitize-address controlled it, I'm not sure how I would build elfutils with Memory Sanitizer
(where I have to turn z,defs and no-undefined as well).

> But those seem to report bogus issues. At least in these cases, it
> seems the code is fine.

The rationale behind those checks (and why they are enabled by default) can be found
at https://reviews.llvm.org/D67122. My understanding is that code with that kind of UB
is known to be miscompiled from time to time.

Thanks,
Evgeny Vereshchagin