Libguestfs - Apr 2022 - [PATCH nbdkit 2/3] configure: Add --disable-linker-script flag

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}"
@@ -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"])
 
+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],
+    [])
+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=yes
@@ -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 ................................ " \
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 link
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
 -------------
 
-- 
2.35.1

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=yes
Hunk 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 link
Hunk 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