Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile: omit racefuncentry/exit when they are not needed #24662

Closed
dvyukov opened this issue Apr 3, 2018 · 1 comment
Closed

cmd/compile: omit racefuncentry/exit when they are not needed #24662

dvyukov opened this issue Apr 3, 2018 · 1 comment

Comments

@dvyukov
Copy link
Member

@dvyukov dvyukov commented Apr 3, 2018

We only need racefuncentry/exit if the function also contains instrumented memory accesses and/or other calls (racefuncentry/exit are used to restore stack traces, if there is nothing that will require stack traces, then no point in these calls as well).
This was done in LLVM from day one:
https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/Instrumentation/ThreadSanitizer.cpp#L471
and I was under impression that we are doing this in gc too. But it seems that we don't. A good example is internal/bytealg.countGenericString:

0000000000425420 <internal/bytealg.countGenericString>:
  425420:       64 48 8b 0c 25 f8 ff    mov    %fs:0xfffffffffffffff8,%rcx
  425427:       ff ff 
  425429:       48 3b 61 10             cmp    0x10(%rcx),%rsp
  42542d:       76 6b                   jbe    42549a <internal/bytealg.countGenericString+0x7a>
  42542f:       48 83 ec 18             sub    $0x18,%rsp
  425433:       48 89 6c 24 10          mov    %rbp,0x10(%rsp)
  425438:       48 8d 6c 24 10          lea    0x10(%rsp),%rbp
  42543d:       48 8b 44 24 18          mov    0x18(%rsp),%rax
  425442:       48 89 04 24             mov    %rax,(%rsp)
  425446:       e8 d5 85 05 00          callq  47da20 <runtime.racefuncenter>
  42544b:       48 8b 44 24 28          mov    0x28(%rsp),%rax
  425450:       0f b6 4c 24 30          movzbl 0x30(%rsp),%ecx
  425455:       48 8b 54 24 20          mov    0x20(%rsp),%rdx
  42545a:       31 db                   xor    %ebx,%ebx
  42545c:       31 f6                   xor    %esi,%esi
  42545e:       eb 17                   jmp    425477 <internal/bytealg.countGenericString+0x57>
  425460:       48 8d 7b 01             lea    0x1(%rbx),%rdi
  425464:       44 0f b6 04 1a          movzbl (%rdx,%rbx,1),%r8d
  425469:       4c 8d 4e 01             lea    0x1(%rsi),%r9
  42546d:       41 38 c8                cmp    %cl,%r8b
  425470:       49 0f 44 f1             cmove  %r9,%rsi
  425474:       48 89 fb                mov    %rdi,%rbx
  425477:       48 39 c3                cmp    %rax,%rbx
  42547a:       7c e4                   jl     425460 <internal/bytealg.countGenericString+0x40>
  42547c:       48 89 74 24 08          mov    %rsi,0x8(%rsp)
  425481:       e8 da 85 05 00          callq  47da60 <runtime.racefuncexit>
  425486:       48 8b 44 24 08          mov    0x8(%rsp),%rax
  42548b:       48 89 44 24 38          mov    %rax,0x38(%rsp)
  425490:       48 8b 6c 24 10          mov    0x10(%rsp),%rbp
  425495:       48 83 c4 18             add    $0x18,%rsp
  425499:       c3                      retq   
  42549a:       e8 51 55 05 00          callq  47a9f0 <runtime.morestack_noctxt>
  42549f:       e9 7c ff ff ff          jmpq   425420 <internal/bytealg.countGenericString>

@mdempsky @josharian

@dvyukov dvyukov added the RaceDetector label Apr 3, 2018
@josharian josharian added this to the Unplanned milestone Apr 3, 2018
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jun 27, 2018

Change https://golang.org/cl/121235 mentions this issue: cmd/compile: omit racefuncentry/exit when they are not needed

@gopherbot gopherbot closed this in 4201c20 Aug 20, 2018
@golang golang locked and limited conversation to collaborators Aug 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.