odyaka341 / thread-sanitizer

Automatically exported from code.google.com/p/thread-sanitizer
0 stars 0 forks source link

tsan sometimes misses racey use-after-return #27

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
tsan may miss a racy use-after-return: 

  1. Thread1 passes a pointer to a local var to Thread2
  2. Thread1 exits the current function
  3a. Thread2 touches the now-stale pointer
  3b. Thread1 enters or exits a function thus writing/reading stack 

3a and 3b may race.

A possible solution is to treat __tsan_func_entry as write and __tsan_func_exit 
as read.
The tricky parts are what memory range to access on entry.exit,
how that will affect performance,
and how to test the feature reliably. 
We'll need to make sure that even an empty function gets instrumented. 

This feature should be guarder by a run-time flag. 

% cat ~/tmp/racy_uar.cc
#include <pthread.h>
#include <assert.h>
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>

int *g, *g1;
pthread_t t;

void *Thread(void*) {
  assert(g);
  sleep(1);
  *g = 0xabcd1234;
  return 0;
}

void foo() {
  int x, y, z;
  g = &x;
  printf("& x,y,z: %p %p %p; Thread pc: %p\n", &x, &y, &z, Thread);
  sleep(1);
  pthread_create(&t, 0, Thread, 0);
//  printf("x: %x\n", x);
}

void bar() {
  int x, y, z = 0;
  g1 = &x;
  printf("& x,y,z: %p %p %p\n", &x, &y, &z);
  sleep(2);
  pthread_join(t, 0);
  printf("x: %x\n", x);
}

int main() {
  foo();
  bar();
}
% clang++ -g  -fPIE -pie -O1 -fsanitize=thread  ~/tmp/racy_uar.cc -lpthread  && 
./a.out 
& x,y,z: 0x7fffd7c7114c 0x7fffd7c71148 0x7fffd7c71144; Thread pc: 0x7fbcbc83f390
& x,y,z: 0x7fffd7c7113c 0x7fffd7c71138 0x7fffd7c71134
x: 7fbc
% 

Note: x should be 0, instead it contains part of the pc (7fbc)

Note: AddressSanitizer finds this bug reliably in use-after-return mode 
and less reliably in the default mode.

Original issue reported on code.google.com by konstant...@gmail.com on 2 Sep 2013 at 8:46

GoogleCodeExporter commented 9 years ago
Why should x be 0? As far as I see, it's unitialized.

Original comment by dvyu...@google.com on 2 Sep 2013 at 8:57

GoogleCodeExporter commented 9 years ago
my fault, the initial test case is broken. But the problem remains. 
Here is another test case: 

% cat ~/tmp/racy_uar.cc 
#include <pthread.h>
#include <assert.h>
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>

long *g;
int idx = 7;
pthread_t t;

void NeverCalled() {
  printf("I can never be called!!!!\n");
}

void *Thread(void*) {
  sleep(1);
  assert(g);
  *g = (long)&NeverCalled;
  printf("Thread: %p %lx\n", g, *g);
  return 0;
}

void foo() {
  long x[10] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9};
  g = &x[idx];
  printf("x     : %p %lx\n", g, *g);
  pthread_create(&t, 0, Thread, 0);
}

__attribute__((noinline)) void call4() {
  long local;
  sleep(2);
  printf("call4 : %p\n", &local);
}
__attribute__((noinline)) void call3() {
  long local;
  printf("call3 : %p\n", &local);
  call4();
}
__attribute__((noinline)) void call2() {
  long local;
  printf("call2 : %p\n", &local);
  call3();
}
__attribute__((noinline)) void call1() {
  long local;
  printf("call1 : %p\n", &local);
  call2();
}

int main(int argc, char **argv) {
  foo();
  call1();
}
% clang++ -g  -fPIE -pie -O1 -fsanitize=thread  ~/tmp/racy_uar.cc -lpthread ; 
./a.out 
x     : 0x7fff6c9b3e98 7
call1 : 0x7fff6c9b3ec0
call2 : 0x7fff6c9b3eb0
call3 : 0x7fff6c9b3ea0
Thread: 0x7fff6c9b3e98 7f1aee5703d0
call4 : 0x7fff6c9b3e90
I can never be called!!!!
Segmentation fault (core dumped)

Original comment by konstant...@gmail.com on 2 Sep 2013 at 9:43

Attachments:

GoogleCodeExporter commented 9 years ago
OK, this looks valid.

I think we do not want to quarantine stack frames as in asan use-after-return, 
because of the additional memory consumption. Right?
We can make read/write of return address visible to tsan. And additionally 
emulate writes to addressable stack vars, when they go out of scope.

Original comment by dvyu...@google.com on 2 Sep 2013 at 2:37

GoogleCodeExporter commented 9 years ago
Unassigning from myself as this requires some llvm expertise. CCing more llvm 
experts.

Original comment by dvyu...@google.com on 2 Sep 2014 at 2:26