[Bug sanitizer/67575] New: tsan: double instrumentation of x++

[Bug sanitizer/67575] New: tsan: double instrumentation of x++

[dvyukov at google dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67575

            Bug ID: 67575
           Summary: tsan: double instrumentation of x++
           Product: gcc
           Version: 6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: sanitizer
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dvyukov at google dot com
                CC: dodji at gcc dot gnu.org, dvyukov at gcc dot gnu.org,
                    jakub at gcc dot gnu.org, kcc at gcc dot gnu.org
  Target Milestone: ---

This come up during work on KernelThreadSanitizer for functions like:

static __always_inline void pagefault_disabled_inc(void)
{
        current->pagefault_disabled++;
}

The memory access is instrumented twice in such functions.
Here is a standalone reproducer:

long x;

void foo(long *p) {
        (*p)++;
}

void bar(void) {
        x++;
}

int main() {
        return 0;
}

$ gcc test.c -fsanitize=thread -O2
$ objdump -d a.out | less

0000000000400760 <foo>:
  400760:       55                      push   %rbp
  400761:       53                      push   %rbx
  400762:       48 89 fb                mov    %rdi,%rbx
  400765:       48 83 ec 08             sub    $0x8,%rsp
  400769:       48 8b 7c 24 18          mov    0x18(%rsp),%rdi
  40076e:       e8 8d fe ff ff          callq  400600 <__tsan_func_entry@plt>
  400773:       48 89 df                mov    %rbx,%rdi
  400776:       e8 95 fe ff ff          callq  400610 <__tsan_read8@plt>
  40077b:       48 8b 03                mov    (%rbx),%rax
  40077e:       48 89 df                mov    %rbx,%rdi
  400781:       48 8d 68 01             lea    0x1(%rax),%rbp
  400785:       e8 96 fe ff ff          callq  400620 <__tsan_write8@plt>
  40078a:       48 89 2b                mov    %rbp,(%rbx)
  40078d:       48 83 c4 08             add    $0x8,%rsp
  400791:       5b                      pop    %rbx
  400792:       5d                      pop    %rbp
  400793:       e9 98 fe ff ff          jmpq   400630 <__tsan_func_exit@plt>
  400798:       0f 1f 84 00 00 00 00    nopl   0x0(%rax,%rax,1)
  40079f:       00 

00000000004007a0 <bar>:
  4007a0:       53                      push   %rbx
  4007a1:       48 8b 7c 24 08          mov    0x8(%rsp),%rdi
  4007a6:       e8 55 fe ff ff          callq  400600 <__tsan_func_entry@plt>
  4007ab:       bf 40 0c 60 00          mov    $0x600c40,%edi
  4007b0:       e8 5b fe ff ff          callq  400610 <__tsan_read8@plt>
  4007b5:       48 8b 05 84 04 20 00    mov    0x200484(%rip),%rax        #
600c40 <x>
  4007bc:       bf 40 0c 60 00          mov    $0x600c40,%edi
  4007c1:       48 8d 58 01             lea    0x1(%rax),%rbx
  4007c5:       e8 56 fe ff ff          callq  400620 <__tsan_write8@plt>
  4007ca:       48 89 1d 6f 04 20 00    mov    %rbx,0x20046f(%rip)        #
600c40 <x>
  4007d1:       5b                      pop    %rbx
  4007d2:       e9 59 fe ff ff          jmpq   400630 <__tsan_func_exit@plt>
  4007d7:       66 0f 1f 84 00 00 00    nopw   0x0(%rax,%rax,1)
  4007de:       00 00 

$ gcc -v
gcc version 6.0.0 20150831 (experimental) (GCC) 

For both functions there is both __tsan_read8 and __tsan_write8 callbacks.
Only __tsan_write8 should be emitted for performance reasons.

[Bug sanitizer/67575] tsan: double instrumentation of x++

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67575

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This is needed to be done for thread.  There was a thread (so no pun intended)
about this on the mailing list IIRC.

[Bug sanitizer/67575] tsan: double instrumentation of x++

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67575

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-09-14
     Ever confirmed|0                           |1

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Actually I was wrong, it was discussed but the discussion about having it:
> TSAN will badly need similar optimization pass to what ASAN needs (after
> deferring expansion of the shadow memory checks), e.g. var++ right now
> results in __tsan_read4 (&var); followed soon by __tsan_write4 (&var);.
> With no intervening calls (we could ignore many string/memory builtins
> I guess) and no intervening atomics it should be fine to just use
> __tsan_write4 (&var); for that, right?

Right.

https://gcc.gnu.org/ml/gcc-patches/2012-11/msg02430.html