[compiler-rt] r219600 - tsan: better reporting for virtual-call-after-free

[David Blaikie]

On Mon, Oct 13, 2014 at 1:46 AM, Dmitry Vyukov <dvyukov at google.com> wrote:

> Author: dvyukov
> Date: Mon Oct 13 03:46:25 2014
> New Revision: 219600
>
> URL: http://llvm.org/viewvc/llvm-project?rev=219600&view=rev
> Log:
> tsan: better reporting for virtual-call-after-free
> Previously we said that it's a data race, which is confusing
> if it happens in the same thread.
>
>
>
> Added:
>     compiler-rt/trunk/test/tsan/vptr_harmful_race4.cc
> Modified:
>     compiler-rt/trunk/lib/tsan/rtl/tsan_report.cc
>     compiler-rt/trunk/lib/tsan/rtl/tsan_report.h
>     compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_report.cc
>     compiler-rt/trunk/lib/tsan/rtl/tsan_suppressions.cc
>
> Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_report.cc
> URL:
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_report.cc?rev=219600&r1=219599&r2=219600&view=diff
>
> ==============================================================================
> --- compiler-rt/trunk/lib/tsan/rtl/tsan_report.cc (original)
> +++ compiler-rt/trunk/lib/tsan/rtl/tsan_report.cc Mon Oct 13 03:46:25 2014
> @@ -70,6 +70,8 @@ static const char *ReportTypeString(Repo
>      return "data race on vptr (ctor/dtor vs virtual call)";
>    if (typ == ReportTypeUseAfterFree)
>      return "heap-use-after-free";
> +  if (typ == ReportTypeVptrUseAfterFree)
> +    return "heap-use-after-free (virtual call vs free)";
>    if (typ == ReportTypeThreadLeak)
>      return "thread leak";
>    if (typ == ReportTypeMutexDestroyLocked)
>
> Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_report.h
> URL:
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_report.h?rev=219600&r1=219599&r2=219600&view=diff
>
> ==============================================================================
> --- compiler-rt/trunk/lib/tsan/rtl/tsan_report.h (original)
> +++ compiler-rt/trunk/lib/tsan/rtl/tsan_report.h Mon Oct 13 03:46:25 2014
> @@ -22,6 +22,7 @@ enum ReportType {
>    ReportTypeRace,
>    ReportTypeVptrRace,
>    ReportTypeUseAfterFree,
> +  ReportTypeVptrUseAfterFree,
>    ReportTypeThreadLeak,
>    ReportTypeMutexDestroyLocked,
>    ReportTypeMutexDoubleLock,
>
> Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_report.cc
> URL:
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_report.cc?rev=219600&r1=219599&r2=219600&view=diff
>
> ==============================================================================
> --- compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_report.cc (original)
> +++ compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_report.cc Mon Oct 13 03:46:25
> 2014
> @@ -627,7 +627,9 @@ void ReportRace(ThreadState *thr) {
>    ThreadRegistryLock l0(ctx->thread_registry);
>
>    ReportType typ = ReportTypeRace;
> -  if (thr->is_vptr_access)
> +  if (thr->is_vptr_access && freed)
> +    typ = ReportTypeVptrUseAfterFree;
> +  else if (thr->is_vptr_access)
>      typ = ReportTypeVptrRace;
>    else if (freed)
>      typ = ReportTypeUseAfterFree;
>
> Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_suppressions.cc
> URL:
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_suppressions.cc?rev=219600&r1=219599&r2=219600&view=diff
>
> ==============================================================================
> --- compiler-rt/trunk/lib/tsan/rtl/tsan_suppressions.cc (original)
> +++ compiler-rt/trunk/lib/tsan/rtl/tsan_suppressions.cc Mon Oct 13
> 03:46:25 2014
> @@ -60,6 +60,8 @@ SuppressionType conv(ReportType typ) {
>      return SuppressionRace;
>    else if (typ == ReportTypeUseAfterFree)
>      return SuppressionRace;
> +  else if (typ == ReportTypeVptrUseAfterFree)
> +    return SuppressionRace;
>    else if (typ == ReportTypeThreadLeak)
>      return SuppressionThread;
>    else if (typ == ReportTypeMutexDestroyLocked)
>
> Added: compiler-rt/trunk/test/tsan/vptr_harmful_race4.cc
> URL:
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/vptr_harmful_race4.cc?rev=219600&view=auto
>
> ==============================================================================
> --- compiler-rt/trunk/test/tsan/vptr_harmful_race4.cc (added)
> +++ compiler-rt/trunk/test/tsan/vptr_harmful_race4.cc Mon Oct 13 03:46:25
> 2014
> @@ -0,0 +1,34 @@
> +// RUN: %clangxx_tsan -O1 %s -o %t && %deflake %run %t | FileCheck %s
> +#include <pthread.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +struct A {
> +  virtual void F() {
> +  }
> +
> +  virtual ~A() {
> +  }
> +};
> +
> +struct B : A {
> +  virtual void F() {
> +  }
> +};
> +
> +void *Thread(void *x) {
> +  sleep(1);
> +  ((A*)x)->F();
> +  return 0;
> +}
> +
> +int main() {
> +  A *obj = new B;
> +  pthread_t t;
> +  pthread_create(&t, 0, Thread, obj);
> +  delete obj;
> +  pthread_join(t, 0);
> +}
> +
> +// CHECK: WARNING: ThreadSanitizer: heap-use-after-free (virtual call vs
> free)
>

Could/should we be more accurate, or at least more vague, about which kind
of allocation was done? (this example uses 'delete', yet the diagnostic
says 'free') "virtual call after deallocation"? Not sure, maybe free is
sufficient. Just a thought.


> +
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141013/83c6a481/attachment.html>