Laszlo Ersek
2022-Apr-07 09:20 UTC
[Libguestfs] [PATCH nbdkit 2/3] configure: Add --disable-linker-script flag
On 04/06/22 18:28, Richard W.M. Jones wrote:> This flag allows you to disable the linker script (for the server > only). This is necessary for ASAN to work as explained in the > documentation. > > Thanks Eric Blake for helping me with working out the root cause. > --- > configure.ac | 30 ++++++++++++++++++++++++------ > server/Makefile.am | 5 ----- > README | 13 +++++++++++++ > 3 files changed, 37 insertions(+), 11 deletions(-) > > diff --git a/configure.ac b/configure.ac > index f86215d9..404d4e23 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -502,17 +502,19 @@ main (int argc, char *argv[]) > ]) > AM_CONDITIONAL([HAVE_ICONV], [test "x$iconv_working" = "xyes"]) > > +use_linker_script_for_server=yes > + > dnl Don't use linker script for the server on FreeBSD because > dnl FreeBSD's linker is broken. See eg: > dnl https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=190851 > -AC_MSG_CHECKING([if we should use a linker script for the server]) > +AC_MSG_CHECKING([if we should disable the linker script (FreeBSD only)]) > AS_CASE([$host_os], > - [freebsd*], [use_linker_script_for_server=no], > - [use_linker_script_for_server=yes] > + [freebsd*], [ > + use_linker_script_for_server=no > + AC_MSG_RESULT([yes]) > + ], > + [AC_MSG_RESULT([no])] > ) > -AC_MSG_RESULT([$use_linker_script_for_server]) > -AM_CONDITIONAL([USE_LINKER_SCRIPT_FOR_SERVER], > - [test "x$use_linker_script_for_server" = "xyes"]) > > dnl Check if -rdynamic linker flag works. > acx_nbdkit_save_LDFLAGS="${LDFLAGS}"Hunk 1.> @@ -691,9 +693,23 @@ AC_ARG_ENABLE([libfuzzer], > [enable_libfuzzer=no]) > AS_IF([test "x$enable_libfuzzer" = "xyes"],[ > AC_DEFINE([ENABLE_LIBFUZZER],[1],[Enable special libFuzzer binary]) > + # We have to disable the linker script for libFuzzer because Clang > + # adds loads of fuzzer and ASAN-related symbols that are required > + # by the plugins but which our linker script tries to hide. > + use_linker_script_for_server=no > ]) > AM_CONDITIONAL([ENABLE_LIBFUZZER],[test "x$enable_libfuzzer" = "xyes"]) >Hunk 2.> +dnl Should we use the linker script with the server? Note > +dnl some tests above may set this variable. > +AC_ARG_ENABLE([linker-script], > + [AS_HELP_STRING([--disable-linker-script], > + [disable linker script for server (developers only)])], > + [use_linker_script_for_server=$enableval], > + [])Hunk 3.> +AM_CONDITIONAL([USE_LINKER_SCRIPT_FOR_SERVER], > + [test "x$use_linker_script_for_server" = "xyes"]) > + > dnl Bash completion. > PKG_CHECK_MODULES([BASH_COMPLETION], [bash-completion >= 2.0], [ > bash_completion=yesHunk 4.> @@ -1419,6 +1435,8 @@ feature "bash-completion ........................ " \ > test "x$HAVE_BASH_COMPLETION_TRUE" = "x" > feature "libfuzzer .............................. " \ > test "x$ENABLE_LIBFUZZER_TRUE" = "x" > +feature "linker script .......................... " \ > + test "x$USE_LINKER_SCRIPT_FOR_SERVER_TRUE" = "x" > feature "manual pages ........................... " \ > test "x$HAVE_POD_TRUE" = "x" > feature "SELinux ................................ " \Hunk 5.> diff --git a/server/Makefile.am b/server/Makefile.am > index 0039d1da..9ab621d5 100644 > --- a/server/Makefile.am > +++ b/server/Makefile.am > @@ -108,13 +108,8 @@ nbdkit_LDFLAGS = \ > $(NULL) > EXTRA_nbdkit_DEPENDENCIES = nbdkit.syms > if USE_LINKER_SCRIPT_FOR_SERVER > -# We have to disable the linker script for libFuzzer because Clang > -# adds loads of fuzzer and ASAN-related symbols that are required by > -# the plugins but which our linker script tries to hide. > -if !ENABLE_LIBFUZZER > nbdkit_LDFLAGS += -Wl,--version-script=$(srcdir)/nbdkit.syms > endif > -endif > > if IS_WINDOWS > # On Windows, generate an import library so that plugins can linkHunk 6.> diff --git a/README b/README > index 3480d561..7aa93b1d 100644 > --- a/README > +++ b/README > @@ -324,6 +324,19 @@ For further information, see: > https://libguestfs.org/ > https://github.com/NetworkBlockDevice/nbd/blob/master/doc/proto.md > > +Address sanitizer (ASAN) > +------------------------ > + > +You can compile nbdkit with clang and ASAN with: > + > + ./configure CC=clang CXX=clang++ \ > + CFLAGS="-O0 -g -fsanitize=address -fno-omit-frame-pointer" \ > + --disable-linker-script \ > + --disable-golang > + make clean > + make > + ASAN_OPTIONS="allocator_may_return_null=1 detect_leaks=false" make check > + > Test coverage > ------------- > >Hunk 7. --*-- I find this patch confusing; IMO it does three things at once. The first patch should be the reversal of how we set "use_linker_script_for_server" when building on FreeBSD. This is a refactoring. This patch should consist of hunks #1, #4 and #5. The second patch should be related to libfuzzer. It should contain hunks #2 and #6. It is also a refactoring. Notably though, the comment in hunk #2 should *not* refer to ASAN, only to libfuzzer. The third patch should be realted to ASAN. It is a feature. It should consist of hunks #3 and #7. Basically we have three separate reasons for disabling the linker script (building on FreeBSD, using libfuzzer, using clang+ASAN). The patches should reflect that, in my opinion. Thanks Laszlo
Richard W.M. Jones
2022-Apr-07 09:37 UTC
[Libguestfs] [PATCH nbdkit 2/3] configure: Add --disable-linker-script flag
I have split this into three patches in my local copy, but ... On Thu, Apr 07, 2022 at 11:20:47AM +0200, Laszlo Ersek wrote:> The second patch should be related to libfuzzer. It should contain hunks > #2 and #6. It is also a refactoring. Notably though, the comment in hunk > #2 should *not* refer to ASAN, only to libfuzzer.in the second patch I just copied the existing message. I think it's OK to keep the ASAN reference since libFuzzer separately recommends using ASAN (https://www.llvm.org/docs/LibFuzzer.html#fuzzer-usage).> Basically we have three separate reasons for disabling the linker script > (building on FreeBSD, using libfuzzer, using clang+ASAN). The patches > should reflect that, in my opinion.Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW