- Unpoison stack for ASan when tearing down stack frames

runtime/vm/exceptions.cc

 // Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/exceptions.h"
 
 #include "vm/dart_api_impl.h"
 #include "vm/dart_entry.h"
 #include "vm/debugger.h"
 #include "vm/flags.h"
 #include "vm/object.h"
 #include "vm/object_store.h"
 #include "vm/stack_frame.h"
 #include "vm/stub_code.h"
 #include "vm/symbols.h"
 
+// Allow the use of ASan (AddressSanitizer). This is needed as ASan needs to be
+// told about areas where the VM does the equivalent of a long-jump.
+#if defined(__has_feature)
+#if __has_feature(address_sanitizer)
+extern "C" void __asan_unpoison_memory_region(void *, size_t);
+#else  // __has_feature(address_sanitizer)
+void __asan_unpoison_memory_region(void* ignore1, size_t ignore2) {}
+#endif  // __has_feature(address_sanitizer)
+#else  // defined(__has_feature)
+void __asan_unpoison_memory_region(void* ignore1, size_t ignore2) {}
+#endif  // defined(__has_feature)
+
+
 namespace dart {
 
 DEFINE_FLAG(bool, print_stacktrace_at_throw, false,
             "Prints a stack trace everytime a throw occurs.");
 DEFINE_FLAG(bool, heap_profile_out_of_memory, false,
             "Writes a heap profile on unhandled out-of-memory exceptions.");
 DEFINE_FLAG(bool, verbose_stacktrace, false,
     "Stack traces will include methods marked invisible.");
 
 const char* Exceptions::kCastErrorDstName = "type cast";
@@ skipping 209 matching lines @@
          (reinterpret_cast<uword>(isolate->top_resource()) < stack_pointer)) {
     isolate->top_resource()->~StackResource();
   }
 
   // Call a stub to set up the exception object in kExceptionObjectReg,
   // to set up the stacktrace object in kStackTraceObjectReg, and to
   // continue execution at the given pc in the given frame.
   typedef void (*ExcpHandler)(uword, uword, uword, RawObject*, RawObject*);
   ExcpHandler func = reinterpret_cast<ExcpHandler>(
       StubCode::JumpToExceptionHandlerEntryPoint());
+
+  // Unpoison the stack before we tear it down in the generated stub code.
+  uword current_sp = reinterpret_cast<uword>(&program_counter) - 1024;
+  __asan_unpoison_memory_region(reinterpret_cast<void*>(current_sp),

[Anton Muhin, 2013/06/03 12:55:07]

nit: as per ASAN source code, one is supposed to use macros instead function
calls, up to you if you'd like to follow this advice or not.

[Anton Muhin, 2013/06/03 12:55:07]

one hypothetical scenario I am concerned with:

1) imagine stacks of two threads allocated side by side;
2) the thread with bigger stack address almost exhausts its stack and throws an
exception, and your overestimate of the region to unpoison intersects with part
of lower address stack and this region is being poisoned/unpoised right now
3) race condition: according to the spec one shouldn't poison/unpoison the same
memory address concurrently.

I am not sure if this scenario is real: ASAN's runtime itself employes pretty
similar trick.

[Anton Muhin, 2013/06/03 12:55:07]

who will poison this region back?

I suspect ASAN will poison some part of stack on function invocation, but
probably only the frame needed, so it looks like low addresses may stay
unpoisned for a long time.

Also, I don't know details, but ASAM runtime may assume that unused part of
stack is poisoned.  For example, when allocating new frame, it may only unpoison
allocated locals and not poison guards.

[Ivan Posva, 2013/06/06 17:17:05]

Since there are guard pages between stacks, and the slack being added is less
than a page, this code will not touch a different stack's memory.

[Ivan Posva, 2013/06/06 17:17:05]

As far as I understand, function entries that are called after we unwind the
stack will poison the area they need poisoned. What we are doing here is just
accounting for all the missed function exits which we are skipping due to the
fact that we are effectively executing a long jump.

[kcc1, 2013/06/07 06:38:45]

If you include the asan's header, the macro makes your life simpler.
But in dart I guess you don't want to depend on asan headers.

[kcc1, 2013/06/07 06:38:45]

In theory this may happen, but given the relatively small constant
(1024) and the implementation details of glibc's threads' stacks
this will not happen. 

[kcc1, 2013/06/07 06:38:45]

Once we enter another function it will poison its portion of stack. 
I assume that "stack_pointer" is the place to which we unwind, and not the
beginning of the entire thread's stack.
If so, the code is just fine. 
No. The asan's protocol is that all memory is unpoisoned unless we poisoned it
on purpose 
(e.g. on function entry).

[Ivan Posva, 2013/06/07 15:23:12]

That is correct. stack_pointer is the value that will be set as for example the
ESP on ia32. It is the stack pointer when we resume execution at the exception
handler.

+                                stack_pointer - current_sp);
   func(program_counter, stack_pointer, frame_pointer,
        raw_exception, raw_stacktrace);
 #endif
   UNREACHABLE();
 }
 
 
 static void ThrowExceptionHelper(const Instance& incoming_exception,
                                  const Instance& existing_stacktrace) {
   bool use_preallocated_stacktrace = false;
@@ skipping 350 matching lines @@
       break;
   }
 
   return DartLibraryCalls::ExceptionCreate(library,
                                            *class_name,
                                            *constructor_name,
                                            arguments);
 }
 
 }  // namespace dart