[PATCH] btrfs-progs: Enable ThreadSanitizer, using D=tsan.

[PATCH] btrfs-progs: Enable ThreadSanitizer, using D=tsan.

[Adam Buchbinder]

Tested with clang-3.9.

Signed-off-by: Adam Buchbinder <abuchbinder@google.com>
---
 Makefile | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Makefile b/Makefile
index 81598df..8948301 100644
--- a/Makefile
+++ b/Makefile
@@ -17,6 +17,7 @@
 #                  abort   - call abort() on first error (dumps core)
 #                  all     - shortcut for all of the above
 #                  asan    - enable address sanitizer compiler feature
+#                  tsan    - enable thread sanitizer compiler feature
 #                  ubsan   - undefined behaviour sanitizer compiler feature
 #                  bcheck  - extended build checks
 #   W=123          build with warnings (default: off)
@@ -157,6 +158,11 @@ ifneq (,$(findstring asan,$(D)))
   DEBUG_CFLAGS_INTERNAL += -fsanitize=address
 endif
 
+ifneq (,$(findstring tsan,$(D)))
+  DEBUG_CFLAGS_INTERNAL += -fsanitize=thread -fPIE
+  LD_FLAGS += -fsanitize=thread -ltsan -pie
+endif
+
 ifneq (,$(findstring ubsan,$(D)))
   DEBUG_CFLAGS_INTERNAL += -fsanitize=undefined
 endif
-- 
2.13.2.932.g7449e964c-goog

Re: [PATCH] btrfs-progs: Enable ThreadSanitizer, using D=tsan.

[David Sterba]

On Wed, Jul 12, 2017 at 01:04:49PM -0700, Adam Buchbinder wrote:
> Tested with clang-3.9.
> 
> Signed-off-by: Adam Buchbinder <abuchbinder@google.com>
> ---
>  Makefile | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 81598df..8948301 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -17,6 +17,7 @@
>  #                  abort   - call abort() on first error (dumps core)
>  #                  all     - shortcut for all of the above
>  #                  asan    - enable address sanitizer compiler feature
> +#                  tsan    - enable thread sanitizer compiler feature
>  #                  ubsan   - undefined behaviour sanitizer compiler feature
>  #                  bcheck  - extended build checks
>  #   W=123          build with warnings (default: off)
> @@ -157,6 +158,11 @@ ifneq (,$(findstring asan,$(D)))
>    DEBUG_CFLAGS_INTERNAL += -fsanitize=address
>  endif
>  
> +ifneq (,$(findstring tsan,$(D)))
> +  DEBUG_CFLAGS_INTERNAL += -fsanitize=thread -fPIE
> +  LD_FLAGS += -fsanitize=thread -ltsan -pie

Why do you need to set PIE here? Is is necessary for tsan?

> +endif
> +
>  ifneq (,$(findstring ubsan,$(D)))
>    DEBUG_CFLAGS_INTERNAL += -fsanitize=undefined
>  endif
> -- 
> 2.13.2.932.g7449e964c-goog
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] btrfs-progs: Enable ThreadSanitizer, using D=tsan.

[David Sterba]

On Wed, Jul 12, 2017 at 03:45:46PM -0700, Adam Buchbinder wrote:
> On Wed, Jul 12, 2017 at 2:51 PM, David Sterba <dsterba@suse.cz> wrote:
> >
> > On Wed, Jul 12, 2017 at 01:04:49PM -0700, Adam Buchbinder wrote:
> > > Tested with clang-3.9.
> > >
> > > Signed-off-by: Adam Buchbinder <abuchbinder@google.com>
> > > ---
> > >  Makefile | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 81598df..8948301 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -17,6 +17,7 @@
> > >  #                  abort   - call abort() on first error (dumps core)
> > >  #                  all     - shortcut for all of the above
> > >  #                  asan    - enable address sanitizer compiler feature
> > > +#                  tsan    - enable thread sanitizer compiler feature
> > >  #                  ubsan   - undefined behaviour sanitizer compiler
> feature
> > >  #                  bcheck  - extended build checks
> > >  #   W=123          build with warnings (default: off)
> > > @@ -157,6 +158,11 @@ ifneq (,$(findstring asan,$(D)))
> > >    DEBUG_CFLAGS_INTERNAL += -fsanitize=address
> > >  endif
> > >
> > > +ifneq (,$(findstring tsan,$(D)))
> > > +  DEBUG_CFLAGS_INTERNAL += -fsanitize=thread -fPIE
> > > +  LD_FLAGS += -fsanitize=thread -ltsan -pie
> >
> > Why do you need to set PIE here? Is is necessary for tsan?
> 
> Yes; see https://clang.llvm.org/docs/ThreadSanitizer.html:
> "Non-position-independent executables are not supported."

Thanks, patch applied.

Re: [PATCH] btrfs-progs: Enable ThreadSanitizer, using D=tsan.

[David Sterba]

On Wed, Jul 12, 2017 at 01:04:49PM -0700, Adam Buchbinder wrote:
> Tested with clang-3.9.
> 
> Signed-off-by: Adam Buchbinder <abuchbinder@google.com>
> ---
>  Makefile | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 81598df..8948301 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -17,6 +17,7 @@
>  #                  abort   - call abort() on first error (dumps core)
>  #                  all     - shortcut for all of the above
>  #                  asan    - enable address sanitizer compiler feature
> +#                  tsan    - enable thread sanitizer compiler feature
>  #                  ubsan   - undefined behaviour sanitizer compiler feature
>  #                  bcheck  - extended build checks
>  #   W=123          build with warnings (default: off)
> @@ -157,6 +158,11 @@ ifneq (,$(findstring asan,$(D)))
>    DEBUG_CFLAGS_INTERNAL += -fsanitize=address
>  endif
>  
> +ifneq (,$(findstring tsan,$(D)))
> +  DEBUG_CFLAGS_INTERNAL += -fsanitize=thread -fPIE
> +  LD_FLAGS += -fsanitize=thread -ltsan -pie

I did not notice that first, but there's no variable LD_FLAGS, it's
LDFLAGS. So either clang assumes this variable and used it or this
magically worked independent of the flags you set here.

The LDFLAGS should be set in a similar way as DEBUG_CFLAGS_INTERNAL,
I'll fix that myself. Then the linker flags are then passed and it also
works with gcc.

The libbtrfs library still fails to build, complaining about the missing
fPIC flags, which is a bit puzzling as the flags are there.

    [CC]     send-stream.o
gcc -g -O1 -Wall -D_FORTIFY_SOURCE=2 -std=gnu90 -include config.h -DBTRFS_FLAT_INCLUDES -D_XOPEN_SOURCE=700 -fno-strict-aliasing -fPIC -I/labs/dsterba/gits/btrfs-progs -I/labs/dsterba/gits/btrfs-progs/kernel-lib  -O0 -U_FORTIFY_SOURCE -ggdb3  -fsanitize=thread -fPIE  -c send-stream.c -o send-stream.o
    [LD]     libbtrfs.so.0.1
gcc -g -O1 -Wall -D_FORTIFY_SOURCE=2 -std=gnu90 -include config.h -DBTRFS_FLAT_INCLUDES -D_XOPEN_SOURCE=700 -fno-strict-aliasing -fPIC -I/labs/dsterba/gits/btrfs-progs -I/labs/dsterba/gits/btrfs-progs/kernel-lib  -O0 -U_FORTIFY_SOURCE -ggdb3  -fsanitize=thread -fPIE  send-stream.o send-utils.o kernel-lib/rbtree.o btrfs-list.o kernel-lib/crc32c.o messages.o uuid-tree.o utils-lib.o rbtree-utils.o  -rdynamic -L/labs/dsterba/gits/btrfs-progs -fsanitize=thread -ltsan -pie  -luuid -lblkid -L. -pthread \
        -shared -Wl,-soname,libbtrfs.so.0 -o libbtrfs.so.0.1
/usr/lib64/gcc/x86_64-suse-linux/7/../../../../x86_64-suse-linux/bin/ld: send-stream.o: relocation R_X86_64_PC32 against symbol `stderr@@GLIBC_2.2.5' can not be used when making a shared object; recompile with -fPIC
/usr/lib64/gcc/x86_64-suse-linux/7/../../../../x86_64-suse-linux/bin/ld: final link failed: Bad value
collect2: error: ld returned 1 exit status
make: *** [Makefile:347: libbtrfs.so.0.1] Error 1