1205130 - Assertion failure: base[size - 1] == 0, at vm/Printer.cpp

Status: RESOLVED DUPLICATE of bug 1221747

(Core :: JavaScript Engine: JIT, defect)

Description

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

x = Array()
x.push(`oomAfterAllocations(5)`)
var y = x.shift()
function z(a) {
    eval(a)
}
z(y)

asserts js debug shell on m-c changeset c69e31de9aec with --fuzzing-safe --ion-eager -D at Assertion failure: base[size - 1] == 0, at vm/Printer.cpp

Configure options:

MAKE=mozmake AR=ar sh c://Users//mozillaadmin//trees//mozilla-central//js//src//configure --host=x86_64-pc-mingw32 --target=x86_64-pc-mingw32 --enable-debug --disable-threadsafe --enable-more-deterministic --enable-gczeal --enable-debug-symbols --disable-tests

python -u ~/funfuzz/js/compileShell.py -b "--enable-debug --enable-more-deterministic" -r c69e31de9aec

This is an intermittent assertion - setting needinfo? from Hannes to see if he can reproduce this

Comment 1

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

Attachment: stack

# Child-SP          RetAddr           Call Site
00 00000000`0457f500 00000001`3fc73d2d js!js::Sprinter::checkInvariants+0x118 [c:\users\mozillaadmin\trees\mozilla-central\js\src\vm\printer.cpp @ 131]
01 00000000`0457f540 00000001`3fc5b52e js!js::Sprinter::vprintf+0x12d [c:\users\mozillaadmin\trees\mozilla-central\js\src\vm\printer.cpp @ 218]
02 00000000`0457f590 00000001`3f9832e2 js!js::GenericPrinter::printf+0x1e [c:\users\mozillaadmin\trees\mozilla-central\js\src\vm\printer.cpp @ 54]
03 00000000`0457f5c0 00000001`3f98335d js!js::jit::GenericAssembler::spew+0x52 [c:\users\mozillaadmin\trees\mozilla-central\js\src\jit\x86-shared\assemblerbuffer-x86-shared.cpp @ 22]
04 00000000`0457f6d0 00000001`3f9828d8 js!js::jit::GenericAssembler::spew+0x3d [c:\users\mozillaadmin\trees\mozilla-central\js\src\jit\x86-shared\assemblerbuffer-x86-shared.h @ 204]
05 00000000`0457f700 00000001`3f9827d4 js!js::jit::X86Encoding::BaseAssembler::shrq_ir+0xf8 [c:\users\mozillaadmin\trees\mozilla-central\js\src\jit\x86-shared\baseassembler-x86-shared.h @ 1578]

Comment 2

[Hannes Verschore [:h4writer]]

Couldn't reproduce but reading code:

Seems like an issue with Sprinter. We got an allocation that fails: 
https://dxr.mozilla.org/mozilla-central/source/js/src/vm/Printer.cpp#218
As a result the checkInvariant will fail. (
https://dxr.mozilla.org/mozilla-central/source/js/src/vm/Printer.cpp#127
I think the solution is to not check that variant if we failed through OOM. Or maybe better, make the base[size] zero at the end.

Second observation:
why do we keep arr[size] = 0. While Sprinter only has content between arr[0 - offset]. The data between offset and size is garbage IIUC. We should probably make "arr[offset] = 0" !

Comment 3

[Nicolas B. Pierron [:nbp | PTO until 11/11]]

(In reply to Hannes Verschore [:h4writer] from comment #2)
> Second observation:
> why do we keep arr[size] = 0. While Sprinter only has content between arr[0
> - offset]. The data between offset and size is garbage IIUC. We should
> probably make "arr[offset] = 0" !

I think keeping invariants is better.

The problem seems to be that vsnprintf will overwrite the '\0' which is at the end of the string.  I guess resetting the last character when the realloc fails should be fine.

I think we keep arr[size - 1] = 0, to avoid doing it too frequently, but I see no good reason to not do it with offset, as all string copies ensure that we end with a zero as last character (except in the previous case, when vsnprintf does not have enough room for storing the string)

Comment 4

[Hannes Verschore [:h4writer]]

I looked into the code again and actually I don't see the issue I reported earlier.
I falsely thought that vsnprintf could fail with setting only part of the data, which isn't true.
I tried to reproduce, but failed.

I'm wondering if you still can reproduce this? If you can, it would be helpful to have the values for Sprinter::base, Sprinter::size, Sprinter::offset and the contents of Sprinter::base[0:Sprinter::size]

Comment 5

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

Attachment: screenshot.jpg

I can still reproduce on m-c rev 031db40e2b55, but I think I had to apply the patch in bug 1206987 comment 2 to get it to stop in windbg.

Sprinter::base is: 0x00000000`00000000 ""
Sprinter::size is: 0
Sprinter::offset is: 0n0

Let me know how else I can help.

Comment 6

[Hannes Verschore [:h4writer]]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #5)
> Created attachment 8666987 [details]
> screenshot.jpg
> 
> I can still reproduce on m-c rev 031db40e2b55, but I think I had to apply
> the patch in bug 1206987 comment 2 to get it to stop in windbg.
> 
> Sprinter::base is: 0x00000000`00000000 ""
> Sprinter::size is: 0
> Sprinter::offset is: 0n0
> 
> Let me know how else I can help.

Are you sure size was 0??? I see no way that that can be, except if "init()" wasn't called. But in that case we should assert earlier on "MOZ_ASSERT(initialized);"

Comment 7

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

See the screenshot.

Also, I still have it in windbg, so if you let me know what commands to enter in the debugger, I can get you other info if you want.

Comment 8

[Hannes Verschore [:h4writer]]

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #7)
> See the screenshot.
> 
> Also, I still have it in windbg, so if you let me know what commands to
> enter in the debugger, I can get you other info if you want.

I still don't see a potential issue, skimming over the code.
Can you give me the value of:
mozilla::Maybe<ScriptCountBlockState> blockCounts;
in
0a 00000000`0457f920 00000001`4032e468 js!js::jit::CodeGenerator::generateBody+0x396 [c:\users\mozillaadmin\trees\mozilla-central\js\src\jit\codegenerator.cpp @ 4151]

?

Comment 9

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

Attachment: screenshot2.jpg

See the screenshot.

Comment 10

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

0:012> .frame 7
07 00000000`0472f5b0 00000001`3fab9f37 js!js::jit::CodeGenerator::generateBody+0x3b4 [c:\users\fuzz1win\trees\mozilla-central\js\src\jit\codegenerator.cpp @ 4152]
0:012> dt blockCounts
Local var @ 0x472f600 Type mozilla::Maybe<js::jit::ScriptCountBlockState>
   +0x000 mIsSome          : 1
   +0x008 mStorage         : mozilla::AlignedStorage2<js::jit::ScriptCountBlockState>

Comment 11

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

This issue was also discovered in bug 1221747 comment 9. (and later fixed)