From d07d6c1b5c2e747fc6c20c32f53cf8a7e7d4e87c Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Thu, 16 Aug 2018 11:35:50 -0700 Subject: [PATCH] Bug 1483865: Make NotStackAllocated assertion a bit safer. r=froydnj This assertion was always meant to be a best-effort thing to catch obvious errors, but the cases where the assumptions it makes fail have been growing. I could remove it entirely, but I'd be happier keeping at least some basic sanity checks. This compromise continues allowing any address below the first argument pointer, and extends the assertion to also allow anything more than 2KiB above it. We could probably get away with stretching that to at least 4KiB, but 2 seems safer, and likely enough to catch the obvious cases. Differential Revision: https://phabricator.services.mozilla.com/D3542 --HG-- extra : rebase_source : 9e17aa3d751f75deff67e40d223fda7aaa4f84f0 extra : amend_source : 86ada4dd01584defeaa5dcb093ad9a73b34f0318 --- xpcom/components/nsComponentManager.cpp | 33 +++++++++++++++++++------ 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/xpcom/components/nsComponentManager.cpp b/xpcom/components/nsComponentManager.cpp index 654f42099154..9cc16ae70f68 100644 --- a/xpcom/components/nsComponentManager.cpp +++ b/xpcom/components/nsComponentManager.cpp @@ -480,14 +480,31 @@ template static void AssertNotStackAllocated(T* aPtr) { - // The main thread's stack should be allocated at the top of our address - // space. Anything stack allocated should be above us on the stack, and - // therefore above our first argument pointer. - // Only this is apparently not the case on Windows. -#if !(defined(XP_WIN) || defined(ANDROID)) - MOZ_ASSERT(NS_IsMainThread()); - MOZ_ASSERT(uintptr_t(aPtr) < uintptr_t(&aPtr)); -#endif + // On all of our supported platforms, the stack grows down. Any address + // located below the address of our argument is therefore guaranteed not to be + // stack-allocated by the caller. + // + // For addresses above our argument, things get trickier. The main thread + // stack is traditionally placed at the top of the program's address space, + // but that is becoming less reliable as more and more systems adopt address + // space layout randomization strategies, so we have to guess how much space + // above our argument pointer we need to care about. + // + // On most systems, we're guaranteed at least several KiB at the top of each + // stack for TLS. We'd probably be safe assuming at least 4KiB in the stack + // segment above our argument address, but safer is... well, safer. + // + // For threads with huge stacks, it's theoretically possible that we could + // wind up being passed a stack-allocated string from farther up the stack, + // but this is a best-effort thing, so we'll assume we only care about the + // immediate caller. For that case, max 2KiB per stack frame is probably a + // reasonable guess most of the time, and is less than the ~4KiB that we + // expect for TLS, so go with that to avoid the risk of bumping into heap + // data just above the stack. + static constexpr size_t kFuzz = 2048; + + MOZ_ASSERT(uintptr_t(aPtr) < uintptr_t(&aPtr) || + uintptr_t(aPtr) > uintptr_t(&aPtr) + kFuzz); } static inline nsCString