* [PATCH v2 1/7] libasm: Fix xdefault_pattern initialization
2023-02-08 19:52 [PATCH v2 0/7] Add Memory Sanitizer support Ilya Leoshkevich
@ 2023-02-08 19:52 ` Ilya Leoshkevich
2023-02-09 13:14 ` Mark Wielaard
2023-02-08 19:52 ` [PATCH v2 2/7] printversion: Fix unused variable Ilya Leoshkevich
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Ilya Leoshkevich @ 2023-02-08 19:52 UTC (permalink / raw)
To: Mark Wielaard; +Cc: elfutils-devel, 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/7] libasm: Fix xdefault_pattern initialization
2023-02-08 19:52 ` [PATCH v2 1/7] libasm: Fix xdefault_pattern initialization Ilya Leoshkevich
@ 2023-02-09 13:14 ` Mark Wielaard
0 siblings, 0 replies; 15+ messages in thread
From: Mark Wielaard @ 2023-02-09 13:14 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: elfutils-devel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/7] printversion: Fix unused variable
2023-02-08 19:52 [PATCH v2 0/7] Add Memory Sanitizer support Ilya Leoshkevich
2023-02-08 19:52 ` [PATCH v2 1/7] libasm: Fix xdefault_pattern initialization Ilya Leoshkevich
@ 2023-02-08 19:52 ` Ilya Leoshkevich
2023-02-08 19:52 ` [PATCH v2 3/7] readelf: Fix set but not used parameter Ilya Leoshkevich
` (4 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Ilya Leoshkevich @ 2023-02-08 19:52 UTC (permalink / raw)
To: Mark Wielaard; +Cc: elfutils-devel, 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 3/7] readelf: Fix set but not used parameter
2023-02-08 19:52 [PATCH v2 0/7] Add Memory Sanitizer support Ilya Leoshkevich
2023-02-08 19:52 ` [PATCH v2 1/7] libasm: Fix xdefault_pattern initialization Ilya Leoshkevich
2023-02-08 19:52 ` [PATCH v2 2/7] printversion: Fix unused variable Ilya Leoshkevich
@ 2023-02-08 19:52 ` Ilya Leoshkevich
2023-02-09 13:17 ` Mark Wielaard
2023-02-08 19:52 ` [PATCH v2 4/7] x86_64_return_value_location: Support lvalue and rvalue references Ilya Leoshkevich
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Ilya Leoshkevich @ 2023-02-08 19:52 UTC (permalink / raw)
To: Mark Wielaard; +Cc: elfutils-devel, 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/7] readelf: Fix set but not used parameter
2023-02-08 19:52 ` [PATCH v2 3/7] readelf: Fix set but not used parameter Ilya Leoshkevich
@ 2023-02-09 13:17 ` Mark Wielaard
0 siblings, 0 replies; 15+ messages in thread
From: Mark Wielaard @ 2023-02-09 13:17 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: elfutils-devel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 4/7] x86_64_return_value_location: Support lvalue and rvalue references
2023-02-08 19:52 [PATCH v2 0/7] Add Memory Sanitizer support Ilya Leoshkevich
` (2 preceding siblings ...)
2023-02-08 19:52 ` [PATCH v2 3/7] readelf: Fix set but not used parameter Ilya Leoshkevich
@ 2023-02-08 19:52 ` Ilya Leoshkevich
2023-02-09 13:45 ` Mark Wielaard
2023-02-08 19:52 ` [PATCH v2 5/7] configure: Use -fno-addrsig if possible Ilya Leoshkevich
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Ilya Leoshkevich @ 2023-02-08 19:52 UTC (permalink / raw)
To: Mark Wielaard; +Cc: elfutils-devel, 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 4/7] x86_64_return_value_location: Support lvalue and rvalue references
2023-02-08 19:52 ` [PATCH v2 4/7] x86_64_return_value_location: Support lvalue and rvalue references Ilya Leoshkevich
@ 2023-02-09 13:45 ` Mark Wielaard
2023-02-10 1:20 ` Ilya Leoshkevich
0 siblings, 1 reply; 15+ messages in thread
From: Mark Wielaard @ 2023-02-09 13:45 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: elfutils-devel
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,
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 4/7] x86_64_return_value_location: Support lvalue and rvalue references
2023-02-09 13:45 ` Mark Wielaard
@ 2023-02-10 1:20 ` Ilya Leoshkevich
2023-02-10 16:22 ` Frank Ch. Eigler
0 siblings, 1 reply; 15+ messages in thread
From: Ilya Leoshkevich @ 2023-02-10 1:20 UTC (permalink / raw)
To: Mark Wielaard; +Cc: elfutils-devel
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,
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 4/7] x86_64_return_value_location: Support lvalue and rvalue references
2023-02-10 1:20 ` Ilya Leoshkevich
@ 2023-02-10 16:22 ` Frank Ch. Eigler
0 siblings, 0 replies; 15+ messages in thread
From: Frank Ch. Eigler @ 2023-02-10 16:22 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: Mark Wielaard, elfutils-devel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 5/7] configure: Use -fno-addrsig if possible
2023-02-08 19:52 [PATCH v2 0/7] Add Memory Sanitizer support Ilya Leoshkevich
` (3 preceding siblings ...)
2023-02-08 19:52 ` [PATCH v2 4/7] x86_64_return_value_location: Support lvalue and rvalue references Ilya Leoshkevich
@ 2023-02-08 19:52 ` Ilya Leoshkevich
2023-02-09 13:28 ` Mark Wielaard
2023-02-08 19:52 ` [PATCH v2 6/7] configure: Add --disable-demangler Ilya Leoshkevich
2023-02-08 19:52 ` [PATCH v2 7/7] configure: Add --enable-sanitize-memory Ilya Leoshkevich
6 siblings, 1 reply; 15+ messages in thread
From: Ilya Leoshkevich @ 2023-02-08 19:52 UTC (permalink / raw)
To: Mark Wielaard; +Cc: elfutils-devel, 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 5/7] configure: Use -fno-addrsig if possible
2023-02-08 19:52 ` [PATCH v2 5/7] configure: Use -fno-addrsig if possible Ilya Leoshkevich
@ 2023-02-09 13:28 ` Mark Wielaard
0 siblings, 0 replies; 15+ messages in thread
From: Mark Wielaard @ 2023-02-09 13:28 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: elfutils-devel
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"
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 6/7] configure: Add --disable-demangler
2023-02-08 19:52 [PATCH v2 0/7] Add Memory Sanitizer support Ilya Leoshkevich
` (4 preceding siblings ...)
2023-02-08 19:52 ` [PATCH v2 5/7] configure: Use -fno-addrsig if possible Ilya Leoshkevich
@ 2023-02-08 19:52 ` Ilya Leoshkevich
2023-02-09 13:40 ` Mark Wielaard
2023-02-08 19:52 ` [PATCH v2 7/7] configure: Add --enable-sanitize-memory Ilya Leoshkevich
6 siblings, 1 reply; 15+ messages in thread
From: Ilya Leoshkevich @ 2023-02-08 19:52 UTC (permalink / raw)
To: Mark Wielaard; +Cc: elfutils-devel, 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 6/7] configure: Add --disable-demangler
2023-02-08 19:52 ` [PATCH v2 6/7] configure: Add --disable-demangler Ilya Leoshkevich
@ 2023-02-09 13:40 ` Mark Wielaard
0 siblings, 0 replies; 15+ messages in thread
From: Mark Wielaard @ 2023-02-09 13:40 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: elfutils-devel
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],
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 7/7] configure: Add --enable-sanitize-memory
2023-02-08 19:52 [PATCH v2 0/7] Add Memory Sanitizer support Ilya Leoshkevich
` (5 preceding siblings ...)
2023-02-08 19:52 ` [PATCH v2 6/7] configure: Add --disable-demangler Ilya Leoshkevich
@ 2023-02-08 19:52 ` Ilya Leoshkevich
6 siblings, 0 replies; 15+ messages in thread
From: Ilya Leoshkevich @ 2023-02-08 19:52 UTC (permalink / raw)
To: Mark Wielaard; +Cc: elfutils-devel, 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
^ permalink raw reply [flat|nested] 15+ messages in thread