Bug 157047

Summary: Assertion failure in bmalloc::vmRevokePermissions(void*, unsigned long).
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: bmallocAssignee: Geoffrey Garen <ggaren>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, enrica, ggaren, mark.lam, webkit-bug-importer
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch fpizlo: review+

Description Mark Lam 2016-04-26 15:26:45 PDT 
Just built ToT r200102 for ARM64 and ran jsc.  That got me this crash:

Process 285 stopped
* thread #1: tid = 0x1199, 0x00000001011496e8 JavaScriptCore`bmalloc::vmValidate(unsigned long) + 96, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef)
    frame #0: 0x00000001011496e8 JavaScriptCore`bmalloc::vmValidate(unsigned long) + 96
JavaScriptCore`bmalloc::vmValidate:
->  0x1011496e8 <+96>:  str    wzr, [x8]
    0x1011496ec <+100>: b      0x1011496f0               ; <+104>
    0x1011496f0 <+104>: b      0x1011496f4               ; <+108>
    0x1011496f4 <+108>: mov    sp, x29

(lldb) bt
* thread #1: tid = 0x1199, 0x00000001011496e8 JavaScriptCore`bmalloc::vmValidate(unsigned long) + 96, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef)
  * frame #0: 0x00000001011496e8 JavaScriptCore`bmalloc::vmValidate(unsigned long) + 96
    frame #1: 0x000000010114971c JavaScriptCore`bmalloc::vmValidate(void*, unsigned long) + 28
    frame #2: 0x000000010114db1c JavaScriptCore`bmalloc::vmRevokePermissions(void*, unsigned long) + 32
    frame #3: 0x000000010114d914 JavaScriptCore`bmalloc::VMHeap::allocateSmallChunk(std::__1::lock_guard<bmalloc::StaticMutex>&, unsigned long) + 180
    frame #4: 0x0000000101148524 JavaScriptCore`bmalloc::VMHeap::allocateSmallPage(std::__1::lock_guard<bmalloc::StaticMutex>&, unsigned long) + 92
    frame #5: 0x0000000101145d00 JavaScriptCore`bmalloc::Heap::allocateSmallPage(std::__1::lock_guard<bmalloc::StaticMutex>&, unsigned long)::$_1::operator()() const + 212
    frame #6: 0x0000000101145bc8 JavaScriptCore`bmalloc::Heap::allocateSmallPage(std::__1::lock_guard<bmalloc::StaticMutex>&, unsigned long) + 164
    frame #7: 0x0000000101146234 JavaScriptCore`bmalloc::Heap::allocateSmallBumpRangesByMetadata(std::__1::lock_guard<bmalloc::StaticMutex>&, unsigned long, bmalloc::BumpAllocator&, bmalloc::FixedVector<bmalloc::BumpRange, 3ul>&) + 52
    frame #8: 0x0000000101143bc0 JavaScriptCore`bmalloc::Heap::allocateSmallBumpRanges(std::__1::lock_guard<bmalloc::StaticMutex>&, unsigned long, bmalloc::BumpAllocator&, bmalloc::FixedVector<bmalloc::BumpRange, 3ul>&) + 96
    frame #9: 0x0000000101143b38 JavaScriptCore`bmalloc::Allocator::refillAllocatorSlowCase(bmalloc::BumpAllocator&, unsigned long) + 292
    frame #10: 0x0000000101143f2c JavaScriptCore`bmalloc::Allocator::allocateSlowCase(unsigned long) + 220
    frame #11: 0x00000001010f7f70 JavaScriptCore`bmalloc::Allocator::allocate(unsigned long) + 56
    frame #12: 0x000000010114f598 JavaScriptCore`bmalloc::Cache::allocateSlowCaseNullCache(unsigned long) + 32
    frame #13: 0x00000001010f7ef4 JavaScriptCore`bmalloc::Cache::allocate(unsigned long) + 40
    frame #14: 0x00000001010f78f4 JavaScriptCore`bmalloc::api::malloc(unsigned long) + 24
    frame #15: 0x00000001010f7770 JavaScriptCore`WTF::fastMalloc(unsigned long) + 24
    frame #16: 0x0000000100f80588 JavaScriptCore`WTF::AtomicStringTable::operator new(unsigned long) + 24
    frame #17: 0x00000001010de554 JavaScriptCore`WTF::AtomicStringTable::create(WTF::WTFThreadData&) + 44
    frame #18: 0x00000001011429c8 JavaScriptCore`WTF::WTFThreadData::WTFThreadData() + 112
    frame #19: 0x0000000101142a40 JavaScriptCore`WTF::WTFThreadData::WTFThreadData() + 28
    frame #20: 0x0000000101142aec JavaScriptCore`WTF::WTFThreadData::createAndRegisterForGetspecificDirect() + 28
    frame #21: 0x00000001002eb890 JavaScriptCore`WTF::wtfThreadData() + 100
    frame #22: 0x000000010113275c JavaScriptCore`WTF::initializeThreading() + 80
    frame #23: 0x0000000100008f40 jsc`main + 68
    frame #24: 0x0000000184eb586c libdyld.dylib`start + 4

Disassembly of the crash site says:

lldb) disass
JavaScriptCore`bmalloc::vmValidate:
    0x101149688 <+0>:   stp    x29, x30, [sp, #-16]!
    0x10114968c <+4>:   mov    x29, sp
    0x101149690 <+8>:   sub    sp, sp, #16
    0x101149694 <+12>:  str    x0, [sp, #8]
    0x101149698 <+16>:  ldr    x8, [sp, #8]
    0x10114969c <+20>:  cbnz   x8, 0x1011496b4           ; <+44>
    0x1011496a0 <+24>:  b      0x1011496a4               ; <+28>
    0x1011496a4 <+28>:  movz   x8, #0xbbad, lsl #16
    0x1011496a8 <+32>:  movk   x8, #0xbeef
    0x1011496ac <+36>:  str    wzr, [x8]
    0x1011496b0 <+40>:  b      0x1011496b4               ; <+44>
    0x1011496b4 <+44>:  b      0x1011496b8               ; <+48>
    0x1011496b8 <+48>:  b      0x1011496bc               ; <+52>
    0x1011496bc <+52>:  ldr    x8, [sp, #8]
    0x1011496c0 <+56>:  str    x8, [sp]
    0x1011496c4 <+60>:  bl     0x101144f50               ; bmalloc::vmPageSize()
    0x1011496c8 <+64>:  ldr    x1, [sp, #8]
    0x1011496cc <+68>:  bl     0x101143408               ; unsigned long bmalloc::roundUpToMultipleOf<unsigned long>(unsigned long, unsigned long)
    0x1011496d0 <+72>:  ldr    x8, [sp]
    0x1011496d4 <+76>:  cmp    x8, x0
    0x1011496d8 <+80>:  b.eq   0x1011496f0               ; <+104>
    0x1011496dc <+84>:  b      0x1011496e0               ; <+88>
    0x1011496e0 <+88>:  movz   x8, #0xbbad, lsl #16
    0x1011496e4 <+92>:  movk   x8, #0xbeef
->  0x1011496e8 <+96>:  str    wzr, [x8]
    0x1011496ec <+100>: b      0x1011496f0               ; <+104>
    0x1011496f0 <+104>: b      0x1011496f4               ; <+108>
    0x1011496f4 <+108>: mov    sp, x29
    0x1011496f8 <+112>: ldp    x29, x30, [sp], #16
    0x1011496fc <+116>: ret    

Registers at crash point are:

(lldb) reg read
General Purpose Registers:
        x0 = 0x0000000000004000
        x1 = 0x0000000000001000
        x2 = 0x0000000000200000
        x3 = 0x0000000000001002
        x4 = 0x0000000035000000
        x5 = 0x0000000000000000
        x6 = 0x000000016fdff5c0
        x7 = 0x0000000000000f70
        x8 = 0x00000000bbadbeef
        x9 = 0x0000000000004fff
       x10 = 0x0000000000003fff
       x11 = 0x00000001ab17f124  
       x12 = 0x00000001ab17f124  
       x13 = 0x000000000000003d
       x14 = 0x0000000000000001
       x15 = 0x0000000000000881
       x16 = 0x0000000000000049
       x17 = 0x0000000000000080
       x18 = 0x0000000000000000
       x19 = 0x0000000000000000
       x20 = 0x0000000000000000
       x21 = 0x0000000000000000
       x22 = 0x0000000000000000
       x23 = 0x0000000000000000
       x24 = 0x0000000000000000
       x25 = 0x0000000000000000
       x26 = 0x0000000000000000
       x27 = 0x0000000000000000
       x28 = 0x000000016fdffcd0
        fp = 0x000000016fdff450
        lr = 0x00000001011496d0  JavaScriptCore`bmalloc::vmValidate(unsigned long) + 72
        sp = 0x000000016fdff440
        pc = 0x00000001011496e8  JavaScriptCore`bmalloc::vmValidate(unsigned long) + 96
      cpsr = 0x80000000

The crash came from this comparison:
    0x1011496d0 <+72>:  ldr    x8, [sp]
    0x1011496d4 <+76>:  cmp    x8, x0

x8 already got trashed in the setting up of 0xbbadbeef for the crash.  So, let's peek at it on the stack:

(lldb) x/2x $sp
0x16fdff440: 0x00001000 0x00000000

The value compared against is:
        x0 = 0x0000000000004000

So, the issue here is that we're failing this assertion:
    BASSERT(vmSize == roundUpToMultipleOf(vmPageSize(), vmSize));

with ...
    vmSize = 0x1000 => 4096
    roundUpToMultipleOf(vmPageSize(), vmSize) = 0x4000 ==> 16384
Comment 1 Geoffrey Garen 2016-04-26 15:49:02 PDT 
The bug here is that pageSize is a multiple of the physical page size, but we need to use a multiple of the virtual page size.
Comment 2 Geoffrey Garen 2016-04-27 17:36:05 PDT 
Created attachment 277556 [details]
Patch
Comment 3 Geoffrey Garen 2016-04-27 17:37:02 PDT 
Created attachment 277557 [details]
Patch
Comment 4 Geoffrey Garen 2016-04-27 17:37:30 PDT 
Comment on attachment 277557 [details]
Patch

Darin reviewed this.
Comment 5 Geoffrey Garen 2016-04-27 18:20:31 PDT 
Created attachment 277565 [details]
Patch
Comment 6 Geoffrey Garen 2016-04-27 18:21:28 PDT 
Comment on attachment 277565 [details]
Patch

Changed max to round because max(a, b) does not produce multiples of b when a is larger than b.
Comment 7 WebKit Commit Bot 2016-04-27 19:10:50 PDT 
Comment on attachment 277565 [details]
Patch

Clearing flags on attachment: 277565

Committed r200167: <http://trac.webkit.org/changeset/200167>
Comment 8 WebKit Commit Bot 2016-04-27 19:10:54 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 9 Geoffrey Garen 2016-05-03 12:30:31 PDT 
Enrica reports that this is not fully fixed.
Comment 10 Geoffrey Garen 2016-05-03 12:33:00 PDT 
Created attachment 278012 [details]
Patch
Comment 11 Geoffrey Garen 2016-05-03 12:43:17 PDT 
Committed r200385: <http://trac.webkit.org/changeset/200385>
Comment 12 Filip Pizlo 2016-05-03 12:43:58 PDT 
Comment on attachment 278012 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=278012&action=review

> Source/bmalloc/bmalloc/Algorithm.h:104
> +template<typename T> inline T roundUpToMultipleOfSloppy(size_t divisor, T x)

I would have called this roundUpToMultipleOfNonPower.  It's not a great name, but it conveys a bit more information.
Comment 13 Geoffrey Garen 2016-05-03 13:54:34 PDT 
Renamed in <http://trac.webkit.org/changeset/200386>.