[asan] Cleanup: Move tid into ErrorBase, add const to BufferedStackTrace, be consiste...

Summary: As mentioned in D24394, I'm moving tid to ErrorBase, since basically all errors need it.
Also mentioned in the same review are other cleanups like adding const
to BufferedStackTrace and make sure constructor orders are consistent.

Reviewers: vitalybuka, kcc, eugenis

Subscribers: llvm-commits, kubabrecka

Differential Revision: https://reviews.llvm.org/D24455

llvm-svn: 281236

diff --git a/compiler-rt/lib/asan/asan_errors.h b/compiler-rt/lib/asan/asan_errors.h
index 3ac4d95..afa96de 100644 (file)
--- a/compiler-rt/lib/asan/asan_errors.h
+++ b/compiler-rt/lib/asan/asan_errors.h
@@ -21,19 +21,21 @@
 namespace __asan {
 
 struct ErrorBase {
+  ErrorBase() = default;
+  explicit ErrorBase(u32 tid_) : tid(tid_) {}
   ScarinessScoreBase scariness;
+  u32 tid;
 };
 
 struct ErrorStackOverflow : ErrorBase {
-  u32 tid;
   uptr addr, pc, bp, sp;
   // ErrorStackOverflow never owns the context.
   void *context;
   // VS2013 doesn't implement unrestricted unions, so we need a trivial default
   // constructor
   ErrorStackOverflow() = default;
-  ErrorStackOverflow(const SignalContext &sig, u32 tid_)
-      : tid(tid_),
+  ErrorStackOverflow(u32 tid, const SignalContext &sig)
+      : ErrorBase(tid),
         addr(sig.addr),
         pc(sig.pc),
         bp(sig.bp),
@@ -46,26 +48,25 @@ struct ErrorStackOverflow : ErrorBase {
 };
 
 struct ErrorDeadlySignal : ErrorBase {
-  u32 tid;
   uptr addr, pc, bp, sp;
+  // ErrorDeadlySignal never owns the context.
+  void *context;
   int signo;
   SignalContext::WriteFlag write_flag;
   bool is_memory_access;
-  // ErrorDeadlySignal never owns the context.
-  void *context;
   // VS2013 doesn't implement unrestricted unions, so we need a trivial default
   // constructor
   ErrorDeadlySignal() = default;
-  ErrorDeadlySignal(int signo_, const SignalContext &sig, u32 tid_)
-      : tid(tid_),
+  ErrorDeadlySignal(u32 tid, const SignalContext &sig, int signo_)
+      : ErrorBase(tid),
         addr(sig.addr),
         pc(sig.pc),
         bp(sig.bp),
         sp(sig.sp),
+        context(sig.context),
         signo(signo_),
         write_flag(sig.write_flag),
-        is_memory_access(sig.is_memory_access),
-        context(sig.context) {
+        is_memory_access(sig.is_memory_access) {
     scariness.Clear();
     if (is_memory_access) {
       if (addr < GetPageSizeCached()) {
@@ -87,15 +88,14 @@ struct ErrorDeadlySignal : ErrorBase {
 };
 
 struct ErrorDoubleFree : ErrorBase {
-  u32 tid;
-  HeapAddressDescription addr_description;
   // ErrorDoubleFree doesn't own the stack trace.
-  BufferedStackTrace *second_free_stack;
+  const BufferedStackTrace *second_free_stack;
+  HeapAddressDescription addr_description;
   // VS2013 doesn't implement unrestricted unions, so we need a trivial default
   // constructor
   ErrorDoubleFree() = default;
-  ErrorDoubleFree(uptr addr, u32 tid_, BufferedStackTrace *stack)
-      : tid(tid_), second_free_stack(stack) {
+  ErrorDoubleFree(u32 tid, BufferedStackTrace *stack, uptr addr)
+      : ErrorBase(tid), second_free_stack(stack) {
     CHECK_GT(second_free_stack->size, 0);
     GetHeapAddressInformation(addr, 1, &addr_description);
     scariness.Clear();
@@ -105,17 +105,16 @@ struct ErrorDoubleFree : ErrorBase {
 };
 
 struct ErrorNewDeleteSizeMismatch : ErrorBase {
-  u32 tid;
+  // ErrorNewDeleteSizeMismatch doesn't own the stack trace.
+  const BufferedStackTrace *free_stack;
   HeapAddressDescription addr_description;
   uptr delete_size;
-  // ErrorNewDeleteSizeMismatch doesn't own the stack trace.
-  BufferedStackTrace *free_stack;
   // VS2013 doesn't implement unrestricted unions, so we need a trivial default
   // constructor
   ErrorNewDeleteSizeMismatch() = default;
-  ErrorNewDeleteSizeMismatch(uptr addr, u32 tid_, uptr delete_size_,
-                             BufferedStackTrace *stack)
-      : tid(tid_), delete_size(delete_size_), free_stack(stack) {
+  ErrorNewDeleteSizeMismatch(u32 tid, BufferedStackTrace *stack, uptr addr,
+                             uptr delete_size_)
+      : ErrorBase(tid), free_stack(stack), delete_size(delete_size_) {
     GetHeapAddressInformation(addr, 1, &addr_description);
     scariness.Clear();
     scariness.Scare(10, "new-delete-type-mismatch");

diff --git a/compiler-rt/lib/asan/asan_report.cc b/compiler-rt/lib/asan/asan_report.cc
index d091cc8..92214df 100644 (file)
--- a/compiler-rt/lib/asan/asan_report.cc
+++ b/compiler-rt/lib/asan/asan_report.cc
@@ -324,27 +324,27 @@ ErrorDescription ScopedInErrorReport::current_error_;
 
 void ReportStackOverflow(const SignalContext &sig) {
   ScopedInErrorReport in_report(/*report*/ nullptr, /*fatal*/ true);
-  ErrorStackOverflow error{sig, GetCurrentTidOrInvalid()};  // NOLINT
+  ErrorStackOverflow error(GetCurrentTidOrInvalid(), sig);
   in_report.ReportError(error);
 }
 
 void ReportDeadlySignal(int signo, const SignalContext &sig) {
   ScopedInErrorReport in_report(/*report*/ nullptr, /*fatal*/ true);
-  ErrorDeadlySignal error(signo, sig, GetCurrentTidOrInvalid());
+  ErrorDeadlySignal error(GetCurrentTidOrInvalid(), sig, signo);
   in_report.ReportError(error);
 }
 
 void ReportDoubleFree(uptr addr, BufferedStackTrace *free_stack) {
   ScopedInErrorReport in_report;
-  ErrorDoubleFree error{addr, GetCurrentTidOrInvalid(), free_stack};  // NOLINT
+  ErrorDoubleFree error(GetCurrentTidOrInvalid(), free_stack, addr);
   in_report.ReportError(error);
 }
 
 void ReportNewDeleteSizeMismatch(uptr addr, uptr delete_size,
                                  BufferedStackTrace *free_stack) {
   ScopedInErrorReport in_report;
-  ErrorNewDeleteSizeMismatch error(addr, GetCurrentTidOrInvalid(), delete_size,
-                                   free_stack);
+  ErrorNewDeleteSizeMismatch error(GetCurrentTidOrInvalid(), free_stack, addr,
+                                   delete_size);
   in_report.ReportError(error);
 }
 

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
index 0df4b79..2e8039b 100644 (file)
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
@@ -396,7 +396,7 @@ void ReportErrorSummary(const char *error_message);
 //   error_type file:line[:column][ function]
 void ReportErrorSummary(const char *error_type, const AddressInfo &info);
 // Same as above, but obtains AddressInfo by symbolizing top stack trace frame.
-void ReportErrorSummary(const char *error_type, StackTrace *trace);
+void ReportErrorSummary(const char *error_type, const StackTrace *trace);
 
 // Math
 #if SANITIZER_WINDOWS && !defined(__clang__) && !defined(__GNUC__)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common_libcdep.cc b/compiler-rt/lib/sanitizer_common/sanitizer_common_libcdep.cc
index 1727f24..bed8fab 100644 (file)
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common_libcdep.cc
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common_libcdep.cc
@@ -46,7 +46,7 @@ void SetSandboxingCallback(void (*f)()) {
   sandboxing_callback = f;
 }
 
-void ReportErrorSummary(const char *error_type, StackTrace *stack) {
+void ReportErrorSummary(const char *error_type, const StackTrace *stack) {
 #if !SANITIZER_GO
   if (!common_flags()->print_summary)
     return;