[PATCH v2 0/7] Add Memory Sanitizer support

[PATCH v2 0/7] Add Memory Sanitizer support

[Ilya Leoshkevich]

Hi,

I've made the updates suggested so far and rebased on top of the latest
master. Please take a look.

v1: https://sourceware.org/pipermail/elfutils-devel/2023q1/005831.html
v1 -> v2:
* Drop the unnecessary and the integrated patches.
* Add a comment to the xdefault_pattern patch.
* Add extern to the printversion patch.
* Use the fix from Mark for the handle_bit_registers() issue.
* Fix the --disable-demangle default value.

Best regards,
Ilya

Ilya Leoshkevich (7):
  libasm: Fix xdefault_pattern initialization
  printversion: Fix unused variable
  readelf: Fix set but not used parameter
  x86_64_return_value_location: Support lvalue and rvalue references
  configure: Use -fno-addrsig if possible
  configure: Add --disable-demangler
  configure: Add --enable-sanitize-memory

 backends/x86_64_retval.c  |  2 ++
 configure.ac              | 40 ++++++++++++++++++++++++++++++++++++++-
 debuginfod/Makefile.am    |  3 ++-
 lib/printversion.h        |  7 +++++--
 libasm/Makefile.am        |  3 ++-
 libasm/asm_newscn.c       | 16 +++++++++++-----
 libdw/Makefile.am         |  3 ++-
 libelf/Makefile.am        |  3 ++-
 src/readelf.c             | 17 +++++------------
 tests/Makefile.am         | 10 +++++++++-
 tests/run-readelf-self.sh |  5 +++++
 tests/run-strip-reloc.sh  |  5 +++++
 tests/run-varlocs-self.sh |  5 +++++
 13 files changed, 94 insertions(+), 25 deletions(-)

-- 
2.39.1

[PATCH v2 1/7] libasm: Fix xdefault_pattern initialization

[Ilya Leoshkevich]

clang complains:

    asm_newscn.c:48:22: error: field 'pattern' with variable sized type 'struct FillPattern' not at the end of a struct or class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
      struct FillPattern pattern;
                         ^

Fix by using a union instead. Define the second union member to be a
char array 1 byte larger than struct FillPattern. This should be legal
according to 6.7.9:

    If an object that has static or thread storage duration is not
    initialized explicitly, then ... if it is a union, the first named
    member is initialized (recursively) according to these rules, and
    any padding is initialized to zero bits.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 libasm/asm_newscn.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/libasm/asm_newscn.c b/libasm/asm_newscn.c
index d258d969..f28c40f9 100644
--- a/libasm/asm_newscn.c
+++ b/libasm/asm_newscn.c
@@ -41,19 +41,25 @@
 
 
 /* Memory for the default pattern.  The type uses a flexible array
-   which does work well with a static initializer.  So we play some
-   dirty tricks here.  */
-static const struct
+   which does work well with a static initializer.  Work around this by
+   wrapping it in a union, whose second member is a char array 1 byte larger
+   than struct FillPattern.  According to 6.7.9, this does what we need:
+
+        If an object that has static or thread storage duration is not
+        initialized explicitly, then ... if it is a union, the first named
+        member is initialized (recursively) according to these rules, and
+        any padding is initialized to zero bits.  */
+
+static const union
 {
   struct FillPattern pattern;
-  char zero;
+  char zeroes[sizeof(struct FillPattern) + 1];
 } xdefault_pattern =
   {
     .pattern =
     {
       .len = 1
     },
-    .zero = '\0'
   };
 const struct FillPattern *__libasm_default_pattern = &xdefault_pattern.pattern;
 
-- 
2.39.1

[PATCH v2 2/7] printversion: Fix unused variable

[Ilya Leoshkevich]

clang complains:

    debuginfod.cxx:354:1: error: unused variable 'apba__' [-Werror,-Wunused-const-variable]
    ARGP_PROGRAM_BUG_ADDRESS_DEF = PACKAGE_BUGREPORT;
    ^
    ../lib/printversion.h:47:21: note: expanded from macro 'ARGP_PROGRAM_BUG_ADDRESS_DEF'
      const char *const apba__ __asm ("argp_program_bug_address")
                        ^

This is as expected: it's used by argp via the
"argp_program_bug_address" name, which is not visible on the C level.
Add __attribute__ ((used)) to make sure that the compiler emits it.

While at it, fix debuginfod not printing the bug report address.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 lib/printversion.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/printversion.h b/lib/printversion.h
index a9e059ff..37adff7e 100644
--- a/lib/printversion.h
+++ b/lib/printversion.h
@@ -39,11 +39,14 @@ void print_version (FILE *stream, struct argp_state *state);
    argp_program_bug_address, in all programs.  argp.h declares these
    variables as non-const (which is correct in general).  But we can
    do better, it is not going to change.  So we want to move them into
-   the .rodata section.  Define macros to do the trick.  */
+   the .rodata section.  Define macros to do the trick.  The default
+   linkage for consts in C++ is internal, so declare them extern.  */
 #define ARGP_PROGRAM_VERSION_HOOK_DEF \
   void (*const apvh) (FILE *, struct argp_state *) \
    __asm ("argp_program_version_hook")
 #define ARGP_PROGRAM_BUG_ADDRESS_DEF \
-  const char *const apba__ __asm ("argp_program_bug_address")
+  extern const char *const apba__; \
+  const char *const apba__ __asm ("argp_program_bug_address") \
+  __attribute__ ((used))
 
 #endif // PRINTVERSION_H
-- 
2.39.1

[PATCH v2 3/7] readelf: Fix set but not used parameter

[Ilya Leoshkevich]

clang complains:

    readelf.c:12205:72: error: parameter 'desc' set but not used [-Werror,-Wunused-but-set-parameter]
    handle_bit_registers (const Ebl_Register_Location *regloc, const void *desc,
                                                                           ^

Mark Wielaard says:

    It is never really used since as far as I can see we don't have any
    backend with a core register sets where a register doesn't have a
    number of bits which isn't a multiple of 8 (only ia64 has some 1
    bit registers, but those don't seem part of the core register set).

    If we do accidentally try to handle such a register having an abort
    is also not very nice. Lets just warn and return/continue.

https://sourceware.org/bugzilla/show_bug.cgi?id=30084

Co-developed-by: Mark Wielaard <mark@klomp.org>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 src/readelf.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/src/readelf.c b/src/readelf.c
index 0bbd708e..5b3319c2 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -12199,24 +12199,17 @@ handle_core_items (Elf *core, const void *desc, size_t descsz,
   return colno;
 }
 
-static unsigned int
-handle_bit_registers (const Ebl_Register_Location *regloc, const void *desc,
-		      unsigned int colno)
-{
-  desc += regloc->offset;
-
-  abort ();			/* XXX */
-  return colno;
-}
-
-
 static unsigned int
 handle_core_register (Ebl *ebl, Elf *core, int maxregname,
 		      const Ebl_Register_Location *regloc, const void *desc,
 		      unsigned int colno)
 {
   if (regloc->bits % 8 != 0)
-    return handle_bit_registers (regloc, desc, colno);
+    {
+      error (0, 0, "Warning: Cannot handle register with %" PRIu8 "bits\n",
+	     regloc->bits);
+      return colno;
+    }
 
   desc += regloc->offset;
 
-- 
2.39.1

[PATCH v2 4/7] x86_64_return_value_location: Support lvalue and rvalue references

[Ilya Leoshkevich]

On the low level, they are the same as pointers.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 backends/x86_64_retval.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/backends/x86_64_retval.c b/backends/x86_64_retval.c
index f9114cb1..e668eacc 100644
--- a/backends/x86_64_retval.c
+++ b/backends/x86_64_retval.c
@@ -106,6 +106,8 @@ x86_64_return_value_location (Dwarf_Die *functypedie, const Dwarf_Op **locp)
     case DW_TAG_enumeration_type:
     case DW_TAG_pointer_type:
     case DW_TAG_ptr_to_member_type:
+    case DW_TAG_reference_type:
+    case DW_TAG_rvalue_reference_type:
       {
 	Dwarf_Attribute attr_mem;
 	if (dwarf_formudata (dwarf_attr_integrate (typedie, DW_AT_byte_size,
-- 
2.39.1

[PATCH v2 5/7] configure: Use -fno-addrsig if possible

[Ilya Leoshkevich]

By default, clang produces .llvm_addrsig sections [1]. The GNU
toolchain does not know how to handle them yet [2], so just ask clang
not to generate them for the time being.

[1] https://llvm.org/docs/Extensions.html#sht-llvm-addrsig-section-address-significance-table
[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105625

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 configure.ac | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/configure.ac b/configure.ac
index 8fe8baee..7dc9be63 100644
--- a/configure.ac
+++ b/configure.ac
@@ -588,6 +588,14 @@ CFLAGS="$old_CFLAGS"])
 AM_CONDITIONAL(HAVE_NO_PACKED_NOT_ALIGNED_WARNING,
 	       [test "x$ac_cv_no_packed_not_aligned" != "xno"])
 
+AC_CACHE_CHECK([whether the compiler accepts -fno-addrsig], ac_cv_fno_addrsig, [dnl
+old_CFLAGS="$CFLAGS"
+CFLAGS="$CFLAGS -fno-addrsig -Werror"
+AC_COMPILE_IFELSE([AC_LANG_SOURCE([])],
+                  ac_cv_fno_addrsig=yes, ac_cv_fno_addrsig=no)
+CFLAGS="$old_CFLAGS"])
+AS_IF([test "x$ac_cv_fno_addrsig" = "xyes"], CFLAGS="$CFLAGS -fno-addrsig")
+
 saved_LIBS="$LIBS"
 AC_SEARCH_LIBS([argp_parse], [argp])
 LIBS="$saved_LIBS"
-- 
2.39.1

[PATCH v2 6/7] configure: Add --disable-demangler

[Ilya Leoshkevich]

__cxa_demangle is normally implemented in the C++ runtime library,
instrumenting which for MSan is a hassle. Add a knob for disbling it.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 configure.ac | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 7dc9be63..62a4c8a7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -466,11 +466,17 @@ CFLAGS="$CFLAGS -D_GNU_SOURCE"
 AC_FUNC_STRERROR_R()
 CFLAGS="$old_CFLAGS"
 
+AC_ARG_ENABLE([demangler],
+AS_HELP_STRING([--disable-demangler],
+	       [Disable libstdc++ demangle support]),
+	       [], [enable_demangler=yes])
+AS_IF([test "x$enable_demangler" == xyes],
 AC_CHECK_LIB([stdc++], [__cxa_demangle], [dnl
 AC_DEFINE([USE_DEMANGLE], [1], [Defined if demangling is enabled])])
 AM_CONDITIONAL(DEMANGLE, test "x$ac_cv_lib_stdcpp___cxa_demangle" = "xyes")
 AS_IF([test "x$ac_cv_lib_stdcpp___cxa_demangle" = "xyes"],
-      [enable_demangler=yes],[enable_demangler=no])
+      [enable_demangler=yes],[enable_demangler=no]),
+AM_CONDITIONAL(DEMANGLE, false))
 
 AC_ARG_ENABLE([textrelcheck],
 AS_HELP_STRING([--disable-textrelcheck],
-- 
2.39.1

[PATCH v2 7/7] configure: Add --enable-sanitize-memory

[Ilya Leoshkevich]

Add support for clang Memory Sanitizer [1], which detects the usage of
uninitialized values. While elfutils itself is already checked with
valgrind, checking code that depends on elfutils requires elfutils to
be built with MSan.

MSan is not linked into shared libraries, and is linked into
executables statically. Therefore, unlike the other sanitizers, MSan
needs to be configured fairly early, since we need to drop
-D_FORTIFY_SOURCE [2], -Wl,-z,defs and --no-undefined.

Disable a few tests that run for more than 5 minutes due to test files
being statically linked with MSan.

[1] https://clang.llvm.org/docs/MemorySanitizer.html
[2] https://github.com/google/sanitizers/issues/247

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 configure.ac              | 24 ++++++++++++++++++++++++
 debuginfod/Makefile.am    |  3 ++-
 libasm/Makefile.am        |  3 ++-
 libdw/Makefile.am         |  3 ++-
 libelf/Makefile.am        |  3 ++-
 tests/Makefile.am         | 10 +++++++++-
 tests/run-readelf-self.sh |  5 +++++
 tests/run-strip-reloc.sh  |  5 +++++
 tests/run-varlocs-self.sh |  5 +++++
 9 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/configure.ac b/configure.ac
index 62a4c8a7..0eb309cf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -155,6 +155,29 @@ AC_SUBST([fpie_CFLAGS])
 
 dso_LDFLAGS="-shared"
 
+NO_UNDEFINED=-Wl,--no-undefined
+AC_ARG_ENABLE([sanitize-memory],
+              AS_HELP_STRING([--enable-sanitize-memory],
+                             [Use clang memory sanitizer]),
+                             [use_msan=$enableval], [use_msan=no])
+if test "$use_msan" = yes; then
+  old_CFLAGS="$CFLAGS"
+  old_CXXFLAGS="$CXXFLAGS"
+  old_LDFLAGS="$LDFLAGS"
+  # -fsanitize=memory is not compatible with -D_FORTIFY_SOURCE, -Wl,-z,defs and --no-undefined
+  CFLAGS="$CFLAGS -fsanitize=memory -fsanitize-memory-track-origins -D_FORTIFY_SOURCE=0"
+  CXXFLAGS="$CXXFLAGS -fsanitize=memory -fsanitize-memory-track-origins -D_FORTIFY_SOURCE=0"
+  LDFLAGS="-shared"
+  AC_LINK_IFELSE([AC_LANG_SOURCE([int main (int argc, char **argv) { return 0; }])], use_msan=yes, use_msan=no)
+  AS_IF([test "x$use_msan" == xyes],
+        ac_cv_zdefs=no NO_UNDEFINED=,
+        AC_MSG_WARN([clang memory sanitizer not available])
+        CFLAGS="$old_CFLAGS" CXXFLAGS="$old_CXXFLAGS")
+  LDFLAGS="$old_LDFLAGS"
+fi
+AC_SUBST(NO_UNDEFINED)
+AM_CONDITIONAL(USE_MEMORY_SANITIZER, test "$use_msan" = yes)
+
 ZDEFS_LDFLAGS="-Wl,-z,defs"
 AC_CACHE_CHECK([whether gcc supports $ZDEFS_LDFLAGS], ac_cv_zdefs, [dnl
 save_LDFLAGS="$LDFLAGS"
@@ -887,6 +910,7 @@ AC_MSG_NOTICE([
     run all tests under valgrind       : ${use_valgrind}
     gcc undefined behaviour sanitizer  : ${use_undefined}
     gcc address sanitizer              : ${use_address}
+    clang memory sanitizer             : ${use_msan}
     use rpath in tests                 : ${tests_use_rpath}
     test biarch                        : ${utrace_cv_cc_biarch}
 ])
diff --git a/debuginfod/Makefile.am b/debuginfod/Makefile.am
index f27d6e2e..125be97b 100644
--- a/debuginfod/Makefile.am
+++ b/debuginfod/Makefile.am
@@ -102,7 +102,8 @@ 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..1e6b63e8 100644
--- a/libasm/Makefile.am
+++ b/libasm/Makefile.am
@@ -64,7 +64,8 @@ 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 1b6fead4..e548f38c 100644
--- a/libdw/Makefile.am
+++ b/libdw/Makefile.am
@@ -114,7 +114,8 @@ 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 24c25cf8..aabce43e 100644
--- a/libelf/Makefile.am
+++ b/libelf/Makefile.am
@@ -115,7 +115,8 @@ 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)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 36823d94..31dd2f67 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -88,12 +88,16 @@ endif
 
 # test_nlist checks its own symbol table, and expects various symbols
 # to be in the order as specified in the source file. Explicitly set
-# minimal CFLAGS. But add address sanitizer if in use.
+# minimal CFLAGS. But add sanitizers if in use.
 if USE_ADDRESS_SANITIZER
 EXTRA_NLIST_CFLAGS=-fsanitize=address
 else
+if USE_MEMORY_SANITIZER
+EXTRA_NLIST_CFLAGS=-fsanitize=memory -fsanitize-memory-track-origins
+else
 EXTRA_NLIST_CFLAGS=
 endif
+endif
 
 test-nlist$(EXEEXT): test-nlist.c
 	$(AM_V_CC)$(CC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) \
@@ -225,6 +229,10 @@ if USE_ZSTD_COMPRESS
 export ELFUTILS_ZSTD = 1
 endif
 
+if USE_MEMORY_SANITIZER
+export ELFUTILS_MEMORY_SANITIZER = 1
+endif
+
 if DEBUGINFOD
 check_PROGRAMS += debuginfod_build_id_find
 # With the dummy delegation doesn't work
diff --git a/tests/run-readelf-self.sh b/tests/run-readelf-self.sh
index 7ffb3577..61f803fb 100755
--- a/tests/run-readelf-self.sh
+++ b/tests/run-readelf-self.sh
@@ -17,5 +17,10 @@
 
 . $srcdir/test-subr.sh
 
+if test -n "$ELFUTILS_MEMORY_SANITIZER"; then
+  echo "binaries statically linked memory sanitizer are too big"
+  exit 77
+fi
+
 # Just makes sure readelf doesn't crash
 testrun_on_self_quiet ${abs_top_builddir}/src/readelf -a -w
diff --git a/tests/run-strip-reloc.sh b/tests/run-strip-reloc.sh
index 033ed278..31a11fa2 100755
--- a/tests/run-strip-reloc.sh
+++ b/tests/run-strip-reloc.sh
@@ -17,6 +17,11 @@
 
 . $srcdir/test-subr.sh
 
+if test -n "$ELFUTILS_MEMORY_SANITIZER"; then
+  echo "binaries statically linked memory sanitizer are too big"
+  exit 77
+fi
+
 testfiles hello_i386.ko hello_x86_64.ko hello_ppc64.ko hello_s390.ko \
 	hello_aarch64.ko hello_m68k.ko hello_riscv64.ko hello_csky.ko \
 	hello_arc_hs4.ko
diff --git a/tests/run-varlocs-self.sh b/tests/run-varlocs-self.sh
index 5454fc70..7d79f70e 100755
--- a/tests/run-varlocs-self.sh
+++ b/tests/run-varlocs-self.sh
@@ -17,6 +17,11 @@
 
 . $srcdir/test-subr.sh
 
+if test -n "$ELFUTILS_MEMORY_SANITIZER"; then
+  echo "binaries statically linked memory sanitizer are too big"
+  exit 77
+fi
+
 # Make sure varlocs doesn't crash, doesn't trigger self-check/asserts
 # or leaks running under valgrind.
 testrun_on_self_exe ${abs_top_builddir}/tests/varlocs -e
-- 
2.39.1

Re: [PATCH v2 1/7] libasm: Fix xdefault_pattern initialization

[Mark Wielaard]

Hi Ilya,

On Wed, 2023-02-08 at 20:52 +0100, Ilya Leoshkevich wrote:
> clang complains:
> 
>     asm_newscn.c:48:22: error: field 'pattern' with variable sized type 'struct FillPattern' not at the end of a struct or class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
>       struct FillPattern pattern;
>                          ^
> 
> Fix by using a union instead. Define the second union member to be a
> char array 1 byte larger than struct FillPattern. This should be legal
> according to 6.7.9:
> 
>     If an object that has static or thread storage duration is not
>     initialized explicitly, then ... if it is a union, the first named
>     member is initialized (recursively) according to these rules, and
>     any padding is initialized to zero bits.
> 

Thanks, pushed.

Mark

Re: [PATCH v2 3/7] readelf: Fix set but not used parameter

[Mark Wielaard]

Hi Ilya,

On Wed, 2023-02-08 at 20:52 +0100, Ilya Leoshkevich wrote:
> clang complains:
> 
>     readelf.c:12205:72: error: parameter 'desc' set but not used [-Werror,-Wunused-but-set-parameter]
>     handle_bit_registers (const Ebl_Register_Location *regloc, const void *desc,
>                                                                            ^
> 
> Mark Wielaard says:
> 
>     It is never really used since as far as I can see we don't have any
>     backend with a core register sets where a register doesn't have a
>     number of bits which isn't a multiple of 8 (only ia64 has some 1
>     bit registers, but those don't seem part of the core register set).
> 
>     If we do accidentally try to handle such a register having an abort
>     is also not very nice. Lets just warn and return/continue.
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=30084
> 
> Co-developed-by: Mark Wielaard <mark@klomp.org>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>

Thanks, pushed.

Mark

Re: [PATCH v2 5/7] configure: Use -fno-addrsig if possible

[Mark Wielaard]

Hi Ilya,

On Wed, 2023-02-08 at 20:52 +0100, Ilya Leoshkevich wrote:
> By default, clang produces .llvm_addrsig sections [1]. The GNU
> toolchain does not know how to handle them yet [2], so just ask clang
> not to generate them for the time being.
> 
> [1] https://llvm.org/docs/Extensions.html#sht-llvm-addrsig-section-address-significance-table
> [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105625

That looks like a good generic configure check.
Pushed.

Thanks,

Mark

> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  configure.ac | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/configure.ac b/configure.ac
> index 8fe8baee..7dc9be63 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -588,6 +588,14 @@ CFLAGS="$old_CFLAGS"])
>  AM_CONDITIONAL(HAVE_NO_PACKED_NOT_ALIGNED_WARNING,
>  	       [test "x$ac_cv_no_packed_not_aligned" != "xno"])
>  
> +AC_CACHE_CHECK([whether the compiler accepts -fno-addrsig], ac_cv_fno_addrsig, [dnl
> +old_CFLAGS="$CFLAGS"
> +CFLAGS="$CFLAGS -fno-addrsig -Werror"
> +AC_COMPILE_IFELSE([AC_LANG_SOURCE([])],
> +                  ac_cv_fno_addrsig=yes, ac_cv_fno_addrsig=no)
> +CFLAGS="$old_CFLAGS"])
> +AS_IF([test "x$ac_cv_fno_addrsig" = "xyes"], CFLAGS="$CFLAGS -fno-addrsig")
> +
>  saved_LIBS="$LIBS"
>  AC_SEARCH_LIBS([argp_parse], [argp])
>  LIBS="$saved_LIBS"

Re: [PATCH v2 6/7] configure: Add --disable-demangler

[Mark Wielaard]

Hi Ilya,

On Wed, 2023-02-08 at 20:52 +0100, Ilya Leoshkevich wrote:
> __cxa_demangle is normally implemented in the C++ runtime library,
> instrumenting which for MSan is a hassle. Add a knob for disbling it.

Thanks for the fixes.

Pushed,

Mark

> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  configure.ac | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 7dc9be63..62a4c8a7 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -466,11 +466,17 @@ CFLAGS="$CFLAGS -D_GNU_SOURCE"
>  AC_FUNC_STRERROR_R()
>  CFLAGS="$old_CFLAGS"
>  
> +AC_ARG_ENABLE([demangler],
> +AS_HELP_STRING([--disable-demangler],
> +	       [Disable libstdc++ demangle support]),
> +	       [], [enable_demangler=yes])
> +AS_IF([test "x$enable_demangler" == xyes],
>  AC_CHECK_LIB([stdc++], [__cxa_demangle], [dnl
>  AC_DEFINE([USE_DEMANGLE], [1], [Defined if demangling is enabled])])
>  AM_CONDITIONAL(DEMANGLE, test "x$ac_cv_lib_stdcpp___cxa_demangle" = "xyes")
>  AS_IF([test "x$ac_cv_lib_stdcpp___cxa_demangle" = "xyes"],
> -      [enable_demangler=yes],[enable_demangler=no])
> +      [enable_demangler=yes],[enable_demangler=no]),
> +AM_CONDITIONAL(DEMANGLE, false))
>  
>  AC_ARG_ENABLE([textrelcheck],
>  AS_HELP_STRING([--disable-textrelcheck],

Re: [PATCH v2 4/7] x86_64_return_value_location: Support lvalue and rvalue references

[Mark Wielaard]

Hi Ilya,

On Wed, 2023-02-08 at 20:52 +0100, Ilya Leoshkevich wrote:
> On the low level, they are the same as pointers.

Yes, I can see how that would work for return types.
Do you happen to have a testcase for this?

Right below this code is a check whether the type has a
DW_AT_byte_size, and if not we'll assume 8 as (address) size if the
type is either DW_TAG_pointer_type or DW_TAG_ptr_to_member_type.
Should the same be done for DW_TAG_reference_type and
DW_TAG_rvalue_reference_type?

This doesn't seem x86_64 specific, other backends have similar code, I
assume they all need a similar update?

Thanks,

Mark

> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  backends/x86_64_retval.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/backends/x86_64_retval.c b/backends/x86_64_retval.c
> index f9114cb1..e668eacc 100644
> --- a/backends/x86_64_retval.c
> +++ b/backends/x86_64_retval.c
> @@ -106,6 +106,8 @@ x86_64_return_value_location (Dwarf_Die *functypedie, const Dwarf_Op **locp)
>      case DW_TAG_enumeration_type:
>      case DW_TAG_pointer_type:
>      case DW_TAG_ptr_to_member_type:
> +    case DW_TAG_reference_type:
> +    case DW_TAG_rvalue_reference_type:
>        {
>  	Dwarf_Attribute attr_mem;
>  	if (dwarf_formudata (dwarf_attr_integrate (typedie, DW_AT_byte_size,

Re: [PATCH v2 4/7] x86_64_return_value_location: Support lvalue and rvalue references

[Ilya Leoshkevich]

On Thu, 2023-02-09 at 14:45 +0100, Mark Wielaard wrote:
> Hi Ilya,
> 
> On Wed, 2023-02-08 at 20:52 +0100, Ilya Leoshkevich wrote:
> > On the low level, they are the same as pointers.
> 
> Yes, I can see how that would work for return types.
> Do you happen to have a testcase for this?

You can trigger the issue by compiling the following:

$ cat 1.cpp
int &foo() { throw; }
int &&bar() { throw; }

$ g++ -g 1.cpp -c

and then running

$ LD_LIBRARY_PATH=../libelf:../libdw ./funcretval -e 1.o

What would be a good way to integrate such a testcase?
Currently all tests are built with gcc, is it okay to add one
built with g++? If not, I guess the alternative would be to check in
multiple compressed object files built for different platforms?

> Right below this code is a check whether the type has a
> DW_AT_byte_size, and if not we'll assume 8 as (address) size if the
> type is either DW_TAG_pointer_type or DW_TAG_ptr_to_member_type.
> Should the same be done for DW_TAG_reference_type and
> DW_TAG_rvalue_reference_type?

Sounds reasonable. Here is what I have on x86_64:

 <1><5a>: Abbrev Number: 1 (DW_TAG_reference_type)
    <5b>   DW_AT_byte_size   : 8
 <1><80>: Abbrev Number: 7 (DW_TAG_rvalue_reference_type)
    <81>   DW_AT_byte_size   : 8

IIUC these checks handle weird compilers that do not emit
DW_AT_byte_size.

> This doesn't seem x86_64 specific, other backends have similar code,
> I
> assume they all need a similar update?

Oh, right, this issue seems to affect all of them.

> Thanks,
> 
> Mark
> 
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >  backends/x86_64_retval.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/backends/x86_64_retval.c b/backends/x86_64_retval.c
> > index f9114cb1..e668eacc 100644
> > --- a/backends/x86_64_retval.c
> > +++ b/backends/x86_64_retval.c
> > @@ -106,6 +106,8 @@ x86_64_return_value_location (Dwarf_Die
> > *functypedie, const Dwarf_Op **locp)
> >      case DW_TAG_enumeration_type:
> >      case DW_TAG_pointer_type:
> >      case DW_TAG_ptr_to_member_type:
> > +    case DW_TAG_reference_type:
> > +    case DW_TAG_rvalue_reference_type:
> >        {
> >         Dwarf_Attribute attr_mem;
> >         if (dwarf_formudata (dwarf_attr_integrate (typedie,
> > DW_AT_byte_size,
> 

Re: [PATCH v2 4/7] x86_64_return_value_location: Support lvalue and rvalue references

[Frank Ch. Eigler]

Hi -

> $ cat 1.cpp
> int &foo() { throw; }
> int &&bar() { throw; }
> $ g++ -g 1.cpp -c
> and then running
> 
> $ LD_LIBRARY_PATH=../libelf:../libdw ./funcretval -e 1.o
> 
> What would be a good way to integrate such a testcase?
> Currently all tests are built with gcc, is it okay to add one
> built with g++? [...]

Sure.


- FChE