[asan] Reify ErrorNewDeleteSizeMismatch

[filcab]

Summary

Keep reifying other errors.

[filcab]

Updating with current tree.

[filcab]

Minimizing commit

[vitalybuka]

Can you add some PR or github issue into all these issues with general overview of what are you trying to achieve?

Also please use [asan] prefix in the title.

[filcab]

The rationale is here:
http://lists.llvm.org/pipermail/llvm-dev/2016-July/101933.html

I want to not rely on "print as you get any piece of information" when reporting errors, and convert to structured error reporting, which can then be printed if needed. Or be passed to a debugger.

[filcab]

Updated title.

[vitalybuka]

In D23874#534180, @filcab wrote:

The rationale is here:
http://lists.llvm.org/pipermail/llvm-dev/2016-July/101933.html

I want to not rely on "print as you get any piece of information" when reporting errors, and convert to structured error reporting, which can then be printed if needed. Or be passed to a debugger.

I'd advise to copy/past that message into some llvm bug and attach to every related patch.
But I am not going to insist.

[vitalybuka]

lib/asan/asan_errors.cc

105 ↗

(On Diff #70337)

is this clang-formatted?

lib/asan/asan_errors.h

144 ↗

(On Diff #70337)

Just asking: can we make it a "class" with private data and add
some

const ErrorStackOverflow* AsStackOverflow() {

if (kind does not math)
  return nulltpr;
return stack_overflow;

}

is this going to work for debugger?

[vitalybuka]

LGTM if it's clang-formatted

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

if (m->alloc_type != alloc_type) {
       if (atomic_load(&alloc_dealloc_mismatch, memory_order_acquire)) {
         ReportAllocTypeMismatch((uptr)ptr, stack, (AllocType)m->alloc_type,
                                 (AllocType)alloc_type);
       }
     }
 
     if (delete_size && flags()->new_delete_type_mismatch &&
         delete_size != m->UsedSize()) {
-      ReportNewDeleteSizeMismatch(p, m->UsedSize(), delete_size, stack);
+      ReportNewDeleteSizeMismatch(p, delete_size, stack);
     }
 
     QuarantineChunk(m, ptr, stack, alloc_type);
   }
 
   void *Reallocate(void *old_ptr, uptr new_size, BufferedStackTrace *stack) {
     CHECK(old_ptr && new_size);
     uptr p = reinterpret_cast<uptr>(old_ptr);

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

ErrorDoubleFree(uptr addr, u32 tid_, BufferedStackTrace *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;
+  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) {
+    GetHeapAddressInformation(addr, 1, &addr_description);
+    scariness.Clear();
+    scariness.Scare(10, "new-delete-type-mismatch");
+  }
+  void Print();
+};
+
 enum ErrorKind {
   kErrorKindInvalid = 0,
   kErrorKindStackOverflow,
   kErrorKindDoubleFree,
+  kErrorKindNewDeleteSizeMismatch,
 };
 
 struct ErrorDescription {
   ErrorKind kind;
   // We're using a tagged union because it allows us to have a trivially
   // copiable type and use the same structures as the public interface.
   //
   // We can add a wrapper around it to make it "more c++-like", but that would
   // add a lot of code and the benefit wouldn't be that big.
   union {
     ErrorStackOverflow stack_overflow;
     ErrorDoubleFree double_free;
+    ErrorNewDeleteSizeMismatch new_delete_size_mismatch;
   };
   ErrorDescription() { internal_memset(this, 0, sizeof(*this)); }
   ErrorDescription(const ErrorStackOverflow &e)  // NOLINT
       : kind(kErrorKindStackOverflow),
         stack_overflow(e) {}
   ErrorDescription(const ErrorDoubleFree &e)  // NOLINT
       : kind(kErrorKindDoubleFree),
         double_free(e) {}
+  ErrorDescription(const ErrorNewDeleteSizeMismatch &e)  // NOLINT
+      : kind(kErrorKindNewDeleteSizeMismatch),
+        new_delete_size_mismatch(e) {}
 
   bool IsValid() { return kind != kErrorKindInvalid; }
   void Print() {
     switch (kind) {
       case kErrorKindStackOverflow:
         stack_overflow.Print();
         return;
       case kErrorKindDoubleFree:
         double_free.Print();
         return;
+      case kErrorKindNewDeleteSizeMismatch:
+        new_delete_size_mismatch.Print();
+        return;
       case kErrorKindInvalid:
         CHECK(0);
     }
     CHECK(0);
   }
 };
 
 }  // namespace __asan
 
 #endif  // ASAN_ERRORS_H

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

void ErrorDoubleFree::Print() {
   scariness.Print();
   GET_STACK_TRACE_FATAL(second_free_stack->trace[0],
                         second_free_stack->top_frame_bp);
   stack.Print();
   addr_description.Print();
   ReportErrorSummary("double-free", &stack);
 }
 
+void ErrorNewDeleteSizeMismatch::Print() {
+  Decorator d;
+  Printf("%s", d.Warning());
+  char tname[128];
+  Report(
+      "ERROR: AddressSanitizer: new-delete-type-mismatch on %p in thread "
+      "T%d%s:\n",
+      addr_description.addr, tid,
+      ThreadNameWithParenthesis(tid, tname, sizeof(tname)));
+  Printf("%s  object passed to delete has wrong type:\n", d.EndWarning());
+  Printf(
+      "  size of the allocated type:   %zd bytes;\n"
+      "  size of the deallocated type: %zd bytes.\n",
+      addr_description.chunk_access.chunk_size, delete_size);
+  CHECK_GT(free_stack->size, 0);
+  scariness.Print();
+  GET_STACK_TRACE_FATAL(free_stack->trace[0], free_stack->top_frame_bp);
+  stack.Print();
+  addr_description.Print();
+  ReportErrorSummary("new-delete-type-mismatch", &stack);
+  Report(
+      "HINT: if you don't care about these errors you may set "
+      "ASAN_OPTIONS=new_delete_type_mismatch=0\n");
+}
+
 }  // namespace __asan

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

 bool ParseFrameDescription(const char *frame_descr,
                            InternalMmapVector<StackVarDescr> *vars);
 
 // Different kinds of error reports.
 void ReportGenericError(uptr pc, uptr bp, uptr sp, uptr addr, bool is_write,
                         uptr access_size, u32 exp, bool fatal);
 void ReportStackOverflow(const SignalContext &sig);
 void ReportDeadlySignal(const char *description, const SignalContext &sig);
-void ReportNewDeleteSizeMismatch(uptr addr, uptr alloc_size, uptr delete_size,
+void ReportNewDeleteSizeMismatch(uptr addr, uptr delete_size,
                                  BufferedStackTrace *free_stack);
 void ReportDoubleFree(uptr addr, BufferedStackTrace *free_stack);
 void ReportFreeNotMalloced(uptr addr, BufferedStackTrace *free_stack);
 void ReportAllocTypeMismatch(uptr addr, BufferedStackTrace *free_stack,
                              AllocType alloc_type,
                              AllocType dealloc_type);
 void ReportMallocUsableSizeNotOwned(uptr addr, BufferedStackTrace *stack);
 void ReportSanitizerGetAllocatedSizeNotOwned(uptr addr,

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

 }
 
 void ReportDoubleFree(uptr addr, BufferedStackTrace *free_stack) {
   ScopedInErrorReport in_report;
   ErrorDoubleFree error{addr, GetCurrentTidOrInvalid(), free_stack};  // NOLINT
   in_report.ReportError(error);
 }
 
-void ReportNewDeleteSizeMismatch(uptr addr, uptr alloc_size, uptr delete_size,
+void ReportNewDeleteSizeMismatch(uptr addr, uptr delete_size,
                                  BufferedStackTrace *free_stack) {
   ScopedInErrorReport in_report;
-  Decorator d;
-  Printf("%s", d.Warning());
-  char tname[128];
-  u32 curr_tid = GetCurrentTidOrInvalid();
-  Report("ERROR: AddressSanitizer: new-delete-type-mismatch on %p in "
-         "thread T%d%s:\n",
-         addr, curr_tid,
-         ThreadNameWithParenthesis(curr_tid, tname, sizeof(tname)));
-  Printf("%s  object passed to delete has wrong type:\n", d.EndWarning());
-  Printf("  size of the allocated type:   %zd bytes;\n"
-         "  size of the deallocated type: %zd bytes.\n",
-         alloc_size, delete_size);
-  CHECK_GT(free_stack->size, 0);
-  ScarinessScore::PrintSimple(10, "new-delete-type-mismatch");
-  GET_STACK_TRACE_FATAL(free_stack->trace[0], free_stack->top_frame_bp);
-  stack.Print();
-  DescribeAddressIfHeap(addr);
-  ReportErrorSummary("new-delete-type-mismatch", &stack);
-  Report("HINT: if you don't care about these errors you may set "
-         "ASAN_OPTIONS=new_delete_type_mismatch=0\n");
+  ErrorNewDeleteSizeMismatch error(addr, GetCurrentTidOrInvalid(), delete_size,
+                                   free_stack);
+  in_report.ReportError(error);
 }
 
 void ReportFreeNotMalloced(uptr addr, BufferedStackTrace *free_stack) {
   ScopedInErrorReport in_report;
   Decorator d;
   Printf("%s", d.Warning());
   char tname[128];
   u32 curr_tid = GetCurrentTidOrInvalid();