[asan] Cleanup: Move tid into ErrorBase, add const to BufferedStackTrace, be consistent in constructor arguments and member order.

[filcab]

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.

[filcab]

Updated with more cleanup

[vitalybuka]

LGTM

lib/asan/asan_errors.h

53 ↗

(On Diff #71009)

const?

compiler-rt/trunk/lib/asan/asan_errors.h

 
 #include "asan_descriptions.h"
 #include "asan_scariness_score.h"
 #include "sanitizer_common/sanitizer_common.h"
 
 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),
         sp(sig.sp),
         context(sig.context) {
     scariness.Clear();
     scariness.Scare(10, "stack-overflow");
   }
   void Print();
 };
 
 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()) {
         scariness.Scare(10, "null-deref");
       } else if (addr == pc) {
         scariness.Scare(60, "wild-jump");
       } else if (write_flag == SignalContext::WRITE) {
         scariness.Scare(30, "wild-addr-write");
       } else if (write_flag == SignalContext::READ) {
         scariness.Scare(20, "wild-addr-read");
       } else {
         scariness.Scare(25, "wild-addr");
       }
     } else {
       scariness.Scare(10, "signal");
     }
   }
   void Print();
 };
 
 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();
     scariness.Scare(42, "double-free");
   }
   void Print();
 };
 
 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");
   }
   void Print();
 };
 
 enum ErrorKind {

compiler-rt/trunk/lib/asan/asan_report.cc

 };
 
 StaticSpinMutex ScopedInErrorReport::lock_;
 u32 ScopedInErrorReport::reporting_thread_tid_ = kInvalidTid;
 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);
 }
 
 void ReportFreeNotMalloced(uptr addr, BufferedStackTrace *free_stack) {
   ScopedInErrorReport in_report;
   Decorator d;
   Printf("%s", d.Warning());
   char tname[128];

compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h

 // Construct a one-line string:
 //   SUMMARY: SanitizerToolName: error_message
 // and pass it to __sanitizer_report_error_summary.
 void ReportErrorSummary(const char *error_message);
 // Same as above, but construct error_message as:
 //   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__)
 extern "C" {
 unsigned char _BitScanForward(unsigned long *index, unsigned long mask);  // NOLINT
 unsigned char _BitScanReverse(unsigned long *index, unsigned long mask);  // NOLINT
 #if defined(_WIN64)
 unsigned char _BitScanForward64(unsigned long *index, unsigned __int64 mask);  // NOLINT

compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_libcdep.cc

return internal_strcmp(flag, "always") == 0 ||
          (internal_strcmp(flag, "auto") == 0 && report_file.SupportsColors());
 }
 
 static void (*sandboxing_callback)();
 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;
   if (stack->size == 0) {
     ReportErrorSummary(error_type);
     return;
   }
   // Currently, we include the first stack frame into the report summary.