1467116 - Add padding between LifoAlloc allocation to improve Valgrind/ASan reports

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P2)

Description

[Nicolas B. Pierron [:nbp]]

LifoAlloc is a bump allocator, the fact that we do not add any padding might prevent Valgrind and ASan from reporting buffer overflows, unless we overflow the LifoAlloc chunks.

Comment 1

[Jan de Mooij [:jandem]]

I've been thinking about adding a LifoAlloc-compatible class that just uses malloc for each allocation and stores all of them in a Vector<> (so the fake-LifoAlloc destructor can free them).

That way it's just plain malloc/free and tools already know about that. It will also completely change the memory layout so it might uncover interesting things.

Comment 2

[Nicolas B. Pierron [:nbp]]

> I've been thinking about adding a LifoAlloc-compatible class that just uses
> malloc for each allocation and stores all of them in a Vector<> (so the
> fake-LifoAlloc destructor can free them).

This is a good idea!

However, we expect to have inflallible allocations in many places and a real OOM would fallback on likely crashes, or worse.

Comment 3

[Nicolas B. Pierron [:nbp]]

Attachment: Add a red-zone for each LifoAlloc allocation.

This patch centralize the computation around each allocation in 2 methods,
nextAllocBase and nextAllocEnd. Then, nextAllocEnd is modified to account for
an additional red-zone of 16 bytes for each allocation.

Note, for debug / ASan builds this will cause LSprinter (JitSpew) to behave
inefficiently in terms of memory allocations, which should be addressed in a
follow-up bug to follow an exponential growth of reserved space instead of
coalescing adjacent allocations.

Comment 4

[Nicolas B. Pierron [:nbp]]

Since mprotect is going to be disabled by default and did not report any actionable data, raise the priority of this issue to potentially catch these issues.

Comment 5

[Nicolas B. Pierron [:nbp]]

The current patch seems to have caught an issue in wasm/gc/ref.js and wasm/gc/struct.js locally. I am investigating.

Comment 6

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8993713 [details] [diff] [review]
Add a red-zone for each LifoAlloc allocation.

Review of attachment 8993713 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/ds/LifoAlloc.h
@@ +329,5 @@
>              LIFO_MAKE_MEM_NOACCESS(newBump, bump_ - newBump);
> +        else if (newBump > bump_) {
> +            MOZ_ASSERT(newBump - redZoneSize > bump_);
> +            LIFO_MAKE_MEM_UNDEFINED(bump_, newBump - bump_ - redZoneSize);
> +            LIFO_MAKE_MEM_NOACCESS(newBump, redZoneSize);

self-nit: this should be  …MEM_NOACCESS(newBump - redZoneSize, redZoneSize);

Comment 7

[Nicolas B. Pierron [:nbp]]

Attachment: Add a red-zone for each LifoAlloc allocation.

Comment 8

[Jan de Mooij [:jandem]]

Comment on attachment 8993757 [details] [diff] [review]
Add a red-zone for each LifoAlloc allocation.

Review of attachment 8993757 [details] [diff] [review]:
-----------------------------------------------------------------

This is cool. Some drive-by nits.

::: js/src/ds/LifoAlloc.h
@@ +245,5 @@
>      static constexpr int undefinedChunkMemory = 0xcd;
>      // Byte used for poisoning uninitialized memory after reserving memory.
>      static constexpr int uninitializedChunkMemory = 0xce;
> +    // Red zone reserved after each allocation.
> +    static constexpr size_t redZoneSize = 16;

Nit: it's a constant so RedZoneSize, to make it clear it's a constant

@@ +325,5 @@
>          MOZ_ASSERT(begin() <= newBump);
>          MOZ_ASSERT(newBump <= capacity_);
>  #if defined(LIFO_HAVE_MEM_CHECKS)
>          // Poison/Unpoison memory that we just free'd/allocated.
>          if (bump_ > newBump)

Nit: add {}

Comment 9

[Jan de Mooij [:jandem]]

(In reply to Nicolas B. Pierron [:nbp] {backlog: 39} from comment #2)
> However, we expect to have inflallible allocations in many places and a real
> OOM would fallback on likely crashes, or worse.

ensureBallast et al could just make sure we have X smallish allocations available for when we need it so I don't think this is a dealbreaker. I think the more serious issue is that it's quite a lot of work and your patch might give us most of the same benefits.

Comment 10

[Nicolas B. Pierron [:nbp]]

Attachment: Add a red-zone for each LifoAlloc allocation.

Delta:
 - Capitalize redZoneSize.
 - Remove the red-zone out of the chunk free space (LifoAlloc::newChunkWithCapacity)
   Without this, an allocation can fail after allocating a new chunk dedicated for it.

Comment 11

[Ted Campbell [:tcampbell]]

Sorry for delays. I'm still a little unsure about this implementation. I'm rewriting a version of your patch to try and simplify a few things. I should have it up tomorrow to discuss.

Comment 12

[Ted Campbell [:tcampbell]]

Comment on attachment 8994255 [details] [diff] [review]
Add a red-zone for each LifoAlloc allocation.

Review of attachment 8994255 [details] [diff] [review]:
-----------------------------------------------------------------

This is a nice technique and my main complaints were pre-existing, so I'll stop interfering and approve this patch (with a few tweaks) and offer a separate follow-up patch later.

::: js/src/ds/LifoAlloc.h
@@ +223,5 @@
>      const uintptr_t magic_ : 24;
>      static constexpr uintptr_t magicNumber = uintptr_t(0x4c6966);
>  #endif
>  
> +#if defined(DEBUG)

Is this intentional? I'm not understanding the reasoning for this change.

@@ +245,5 @@
>      static constexpr int undefinedChunkMemory = 0xcd;
>      // Byte used for poisoning uninitialized memory after reserving memory.
>      static constexpr int uninitializedChunkMemory = 0xce;
> +    // Red zone reserved after each allocation.
> +    static constexpr size_t RedZoneSize = 16;

I'd prefer this RedZoneSize definition be guarded by LIFO_HAVE_MEM_CHECKS (or add a new LIFO_ADD_REDZONE) instead of having three definitions.

@@ +251,5 @@
>  # define LIFO_MAKE_MEM_NOACCESS(addr, size)      \
>      do {                                         \
>          uint8_t* base = (addr);                  \
>          size_t sz = (size);                      \
> +        MOZ_MAKE_MEM_UNDEFINED(base, sz);        \

If you fix the double-NO_ACCESS call below, we can avoid this change.

@@ +330,5 @@
>              LIFO_MAKE_MEM_NOACCESS(newBump, bump_ - newBump);
> +        } else if (newBump > bump_) {
> +            MOZ_ASSERT(newBump - RedZoneSize >= bump_);
> +            LIFO_MAKE_MEM_UNDEFINED(bump_, newBump - RedZoneSize - bump_);
> +            LIFO_MAKE_MEM_NOACCESS(newBump - RedZoneSize, RedZoneSize);

The redzone is already MEM_NOACCESS due to chunk allocation or the call a few lines above. Re-calling it here is misleading to the reader. I'd add a comment to mention that the redzone space is already NOACCESS.

@@ +433,5 @@
> +    static uint8_t* nextAllocBase(uint8_t* e) {
> +        return detail::AlignPtr(e);
> +    }
> +    static uint8_t* nextAllocEnd(uint8_t* b, size_t n) {
> +        return b + n + RedZoneSize;

I would still like to change this in the future so start and end of internal allocation are aligned, but this complicates RedZone marking so I will withdraw my comments and create a follow up patch to rework some of the LifoAlloc stuff.

Comment 13

[Nicolas B. Pierron [:nbp]]

(In reply to Ted Campbell [:tcampbell] from comment #12)
> ::: js/src/ds/LifoAlloc.h
> >  
> > +#if defined(DEBUG)
> 
> Is this intentional? I'm not understanding the reasoning for this change.

Yes. The reasoning is that the memory protection mechanism is known for being slow, and the only added value compared to ASan is when we are moving Ion compilation off-thread.  Based on the number of [@ js::jit::MakeMRegExpHoistable ] reports, I would have expect it to be more efficient. So I think this is not worth the cost, and ASan red-zones should provide the same information with a bit less coverage.

Comment 14

[Nicolas B. Pierron [:nbp]]

(In reply to Ted Campbell [:tcampbell] from comment #12)
> @@ +251,5 @@
> >  # define LIFO_MAKE_MEM_NOACCESS(addr, size)      \
> >      do {                                         \
> >          uint8_t* base = (addr);                  \
> >          size_t sz = (size);                      \
> > +        MOZ_MAKE_MEM_UNDEFINED(base, sz);        \
> 
> If you fix the double-NO_ACCESS call below, we can avoid this change.

No, this is not enough, because when we exit a LifoAllocScope, we poison all the memory as no-access, including the red-zones which are already marked as no-access.

> __asan_memset at ??:?
> js::detail::BumpChunk::setBump(unsigned char*) at LifoAlloc.h:330
> js::detail::BumpChunk::release() at LifoAlloc.h:422
> js::LifoAlloc::release(js::detail::BumpChunk::Mark) at LifoAlloc.h:802
> ~LifoAllocScope at LifoAlloc.h:997
> ~TempAllocator at JitAllocPolicy.h:25
> mozilla::Maybe<js::jit::AutoJitContextAlloc>::reset() at Maybe.h:496
> ~Maybe at Maybe.h:188
> ~MacroAssembler at MacroAssembler.h:238
> ~StackMacroAssembler at MacroAssembler.h:2624
> js::jit::JitRuntime::initialize(JSContext*) at Ion.cpp:336

Comment 15

[Ted Campbell [:tcampbell]]

Both of your responses make sense to me. Thanks.

Comment 16

[Nicolas B. Pierron [:nbp]]

Attachment: Add a red-zone for each LifoAlloc allocation.

Delta:
 - Replace red-zone flagging as no-access by a descriptive comment.

 - Factor the RedZoneSize constant into its own section guarded by
   LIFO_HAVE_MEM_CHECKS.

 - Fix LifoAlloc::newChunkWithCapcity, as nextAllocEnd already account for the
   allocation size. This reverses the logic by computing the
   allocSizeWithCanaries (n is included) instead of the defaultChunkFreeSpace (n
   is not included).

Asking feedback from fuzzers to know if they can find opt-ASan, debug-ASan or
debug failures with this new patch before landing it.

Comment 17

[Ted Campbell [:tcampbell]]

Comment on attachment 8995252 [details] [diff] [review]
Add a red-zone for each LifoAlloc allocation.

Review of attachment 8995252 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/ds/LifoAlloc.cpp
@@ +180,5 @@
> +    // bytes in a newly allocated chunk, or default to |defaultChunkSize_|.
> +    uint8_t* u8begin = nullptr;
> +    uint8_t* u8end = u8begin + detail::BumpChunkReservedSpace;
> +    u8end = detail::BumpChunk::nextAllocEnd(detail::BumpChunk::nextAllocBase(u8end), n);
> +    size_t allocSizeWithCanaries = u8end - u8begin;

This is much nicer :)

Comment 18

[Christian Holler (:decoder)]

The patch shows multiple issues that seem to be hard to reproduce:


1. Several stack-overflow reports, e.g.:

==23930==ERROR: AddressSanitizer: stack-overflow on address 0x7ffc8aa17ff8 (pc 0x0000005234da bp 0x7ffc8aa18890 sp 0x7ffc8aa18000 T0)
    #0 0x5234d9 in __asan_region_is_poisoned (js+0x5234d9)
    #1 0x5097ea in __asan_memcpy (js+0x5097ea)
    #2 0x6c36a5 in js_memcpy(void*, void const*, unsigned long) js/src/jsutil.h:42:12
    #3 0x6c36a5 in js::NewObjectCache::copyCachedToObject(js::NativeObject*, js::NativeObject*, js::gc::AllocKind) js/src/vm/Caches.h:233
    #4 0x6c36a5 in js::NewObjectCache::newObjectFromHit(JSContext*, int, js::gc::InitialHeap) js/src/vm/Caches-inl.h:71
    #5 0x69c18c in js::ArrayObject* NewArray<4294967295u>(JSContext*, unsigned int, JS::Handle<JSObject*>, js::NewObjectKind) js/src/builtin/Array.cpp:3808:35
    #6 0x69c18c in js::NewDenseFullyAllocatedArray(JSContext*, unsigned int, JS::Handle<JSObject*>, js::NewObjectKind) js/src/builtin/Array.cpp:3882
    #7 0x939f78 in js::NewArrayOperationWithTemplate(JSContext*, JS::Handle<JSObject*>) js/src/vm/Interpreter.cpp:5260:24
    #8 0x140067a in js::jit::DoNewArray(JSContext*, void*, js::jit::ICNewArray_Fallback*, unsigned int, JS::MutableHandle<JS::Value>) js/src/jit/SharedIC.cpp:2315:15
    #9 0x104ddc9b8dac  (<unknown module>)

==18699==ERROR: AddressSanitizer: stack-overflow on address 0x7ffe7fd2df98 (pc 0x0000004b020a bp 0x7ffe7fd2e810 sp 0x7ffe7fd2dfa0 T0)
    #0 0x4b0209 in __interceptor_memchr.part.37 (js+0x4b0209)
    #1 0x1231169 in FirstCharMatcher8bit(char const*, unsigned int, char) js/src/builtin/String.cpp:1867:42
    #2 0x1231169 in int Matcher<ManualCmp<unsigned char, unsigned char>, unsigned char, unsigned char>(unsigned char const*, unsigned int, unsigned char const*, unsigned int) js/src/builtin/String.cpp:1888
    #3 0x1231169 in int StringMatch<unsigned char, unsigned char>(unsigned char const*, unsigned int, unsigned char const*, unsigned int) js/src/builtin/String.cpp:1957
    #4 0x1231169 in StringMatch(JSLinearString*, JSLinearString*, unsigned int) js/src/builtin/String.cpp:1972
    #5 0x1233351 in js::str_indexOf(JSContext*, unsigned int, JS::Value*) js/src/builtin/String.cpp:2296:26
    #6 0x91af10 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) js/src/vm/Interpreter.cpp:444:15
    #7 0x91af10 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) js/src/vm/Interpreter.cpp:532
    #8 0x91cbc2 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) js/src/vm/Interpreter.cpp:602:10
    #9 0x1432c61 in js::jit::InvokeFunction(JSContext*, JS::Handle<JSObject*>, bool, bool, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) js/src/jit/VMFunctions.cpp:106:12
    #10 0x2b34b3c91b25  (<unknown module>)


2. This weird crash on AsanCheckFailed (but I don't see the CHECK message for some reason):


==29339==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x00000100688c bp 0x7f9a8f8c9e50 sp 0x7f9a8f8c9d40 T69)
==29339==The signal is caused by a WRITE memory access.
==29339==Hint: address points to the zero page.
    #0 0x52aa8f in __asan::AsanCheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) (js+0x52aa8f)
    #1 0x546b65 in __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) (js+0x546b65)
    #2 0x535b31 in __sanitizer::ReportMmapFailureAndDie(unsigned long, char const*, char const*, int, bool) (js+0x535b31)
    #3 0x53f989 in __sanitizer::MmapOrDie(unsigned long, char const*, bool) (js+0x53f989)
    #4 0x548b0a in __sanitizer::SetAlternateSignalStack() (js+0x548b0a)
    #5 0x52f034 in __asan::AsanThread::ThreadStart(unsigned long, __sanitizer::atomic_uintptr_t*) (js+0x52f034)
    #6 0x7f9aa3cc46b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
    #7 0x7f9aa2b2c41c in clone (/lib/x86_64-linux-gnu/libc.so.6+0x10741c)



3. This heap-buffer-overflow in our readline function, this might be a legit bug:


==27377==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62100036d500 at pc 0x00000223bc88 bp 0x7fff980f1110 sp 0x7fff980f1108
READ of size 1 at 0x62100036d500 thread T0
    #0 0x223bc87 in getc_unlocked /usr/include/x86_64-linux-gnu/bits/stdio.h:65:10
    #1 0x223bc87 in js_fgets(char*, int, _IO_FILE*) js/src/frontend/TokenStream.cpp:2744
    #2 0x58fcfd in ReadLine(JSContext*, unsigned int, JS::Value*) js/src/shell/js.cpp:2239:25
    #3 0x7fd71f4f9f57  (<unknown module>)

0x62100036d500 is located 0 bytes to the right of 4096-byte region [0x62100036c500,0x62100036d500)
allocated by thread T0 here:
    #0 0x520628 in __interceptor_malloc (js+0x520628)
    #1 0x7fd7622101d4 in _IO_file_doallocate (/lib/x86_64-linux-gnu/libc.so.6+0x6d1d4)

SUMMARY: AddressSanitizer: heap-buffer-overflow /usr/include/x86_64-linux-gnu/bits/stdio.h:65:10 in getc_unlocked
Shadow bytes around the buggy address:
  0x0c4280065a90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c4280065aa0:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4280065ab0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Heap left redzone:       fa
==27377==ABORTING

Comment 19

[Christian Holler (:decoder)]

(In reply to Christian Holler (:decoder) from comment #18)
> 
> 2. This weird crash on AsanCheckFailed (but I don't see the CHECK message
> for some reason):


Ok the CHECK failures seem to be OOM. I don't know why this patch would trigger that condition in particular (I didn't see any of these crashes on regular mozilla-central). I will update the toolchain to see if the error goes away.

Comment 20

[Nicolas B. Pierron [:nbp]]

Decoder, what would be the next steps?

(In reply to Christian Holler (:decoder) from comment #18)
> The patch shows multiple issues that seem to be hard to reproduce:
> 
> 
> 1. Several stack-overflow reports, e.g.:

None of these 2 OOMs seems to be related to the addition of LifoAlloc red-zone.


> 2. This weird crash on AsanCheckFailed (but I don't see the CHECK message
> for some reason):
> 
> 
> ==29339==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc
> 0x00000100688c bp 0x7f9a8f8c9e50 sp 0x7f9a8f8c9d40 T69)
> ==29339==The signal is caused by a WRITE memory access.
> ==29339==Hint: address points to the zero page.
>     #0 0x52aa8f in __asan::AsanCheckFailed(char const*, int, char const*,
> unsigned long long, unsigned long long) (js+0x52aa8f)
>     #1 0x546b65 in __sanitizer::CheckFailed(char const*, int, char const*,
> unsigned long long, unsigned long long) (js+0x546b65)
>     #2 0x535b31 in __sanitizer::ReportMmapFailureAndDie(unsigned long, char
> const*, char const*, int, bool) (js+0x535b31)
>     #3 0x53f989 in __sanitizer::MmapOrDie(unsigned long, char const*, bool)
> (js+0x53f989)
>     #4 0x548b0a in __sanitizer::SetAlternateSignalStack() (js+0x548b0a)
>     #5 0x52f034 in __asan::AsanThread::ThreadStart(unsigned long,
> __sanitizer::atomic_uintptr_t*) (js+0x52f034)
>     #6 0x7f9aa3cc46b9 in start_thread
> (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
>     #7 0x7f9aa2b2c41c in clone (/lib/x86_64-linux-gnu/libc.so.6+0x10741c)

This crash seems to be some internal issue within ASan. I do not see how it relates to the current patch.

> 3. This heap-buffer-overflow in our readline function, this might be a legit
> bug:
> 
> 
> ==27377==ERROR: AddressSanitizer: heap-buffer-overflow on address
> 0x62100036d500 at pc 0x00000223bc88 bp 0x7fff980f1110 sp 0x7fff980f1108
> READ of size 1 at 0x62100036d500 thread T0
>     #0 0x223bc87 in getc_unlocked
> /usr/include/x86_64-linux-gnu/bits/stdio.h:65:10
>     #1 0x223bc87 in js_fgets(char*, int, _IO_FILE*)
> js/src/frontend/TokenStream.cpp:2744
>     #2 0x58fcfd in ReadLine(JSContext*, unsigned int, JS::Value*)
> js/src/shell/js.cpp:2239:25
>     #3 0x7fd71f4f9f57  (<unknown module>)
> 
> 0x62100036d500 is located 0 bytes to the right of 4096-byte region
> [0x62100036c500,0x62100036d500)
> allocated by thread T0 here:
>     #0 0x520628 in __interceptor_malloc (js+0x520628)
>     #1 0x7fd7622101d4 in _IO_file_doallocate
> (/lib/x86_64-linux-gnu/libc.so.6+0x6d1d4)
> 
> SUMMARY: AddressSanitizer: heap-buffer-overflow
> /usr/include/x86_64-linux-gnu/bits/stdio.h:65:10 in getc_unlocked
> Shadow bytes around the buggy address:
>   0x0c4280065a90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> =>0x0c4280065aa0:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>   0x0c4280065ab0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> Shadow byte legend (one shadow byte represents 8 application bytes):
>   Addressable:           00
>   Heap left redzone:       fa
> ==27377==ABORTING

This does not seems related to LifoAlloc red-zones.  LifoAlloc red-zones are between 16 to 23 bytes, which should highlight the small red-zones as "00 fa fa 00" or as "00 07 fa fa 00".

So, this seems to be an existing issue.

Comment 21

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Comment on attachment 8995252 [details] [diff] [review]
Add a red-zone for each LifoAlloc allocation.

Review of attachment 8995252 [details] [diff] [review]:
-----------------------------------------------------------------

Not sure I'll have time to get to this...

Comment 22

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Comment on attachment 8995252 [details] [diff] [review]
Add a red-zone for each LifoAlloc allocation.

Now testing this at least with debug builds for jsfunfuzz.

Comment 23

[Christian Holler (:decoder)]

I would suggest we land this after :gkw has checked it.


(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #22)
> Now testing this at least with debug builds for jsfunfuzz.


Is that ASan builds? Because only those will be useful for finding regressions with this patch. We need all the fuzzing coverage we can get to avoid instabilities with the ASan Nightly Project.

Comment 24

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Christian Holler (:decoder) from comment #23)
> Is that ASan builds? Because only those will be useful for finding
> regressions with this patch. We need all the fuzzing coverage we can get to
> avoid instabilities with the ASan Nightly Project.

Sorry for the delay, just back from PTO. I've gotten ASan builds going on this patch, I think most of the major roadblocks have gone away. Will clear needinfo? request only a little later.

Comment 25

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

No big items found after almost a day. Tested with 64-bit opt Linux ASan builds and funfuzz. I'll leave it running a little more.

Let me know if debug ASan builds are also preferred.

Comment 26

[Nicolas B. Pierron [:nbp]]

(In reply to Gary Kwong [:gkw] [:nth10sd] - in-n-out; clearing backlog from comment #25)
> Let me know if debug ASan builds are also preferred.

I think optimized-ASan builds should be enough to catch the issues related to LifoAlloc usage, if any.

Comment 27

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Comment on attachment 8995252 [details] [diff] [review]
Add a red-zone for each LifoAlloc allocation.

Review of attachment 8995252 [details] [diff] [review]:
-----------------------------------------------------------------

After almost 2 days, f+ from me. It's already been a long enough delay from me already.

Comment 28

[Nicolas B. Pierron [:nbp]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d4209d1632d5ac78070c7a094edef0911e2e9e80

Comment 29

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/d4209d1632d5