1699569 - Assertion failure: linebufArg[linebufLengthArg] == '\0', at /builds/worker/checkouts/gecko/js/src/jsapi.cpp:4999

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Kelsey Gilbert [:jgilbert]]

Asserts on this Try push:
https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=DDFI37Q3RRewR1g9VRKP6w.0&revision=7f2638ef6b8e0e3a4e22107643bf21919d510daa

Fixed by this patch:
https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=DDFI37Q3RRewR1g9VRKP6w.0&revision=c7b0263378b620b5de04623e2583cb446d175d95

Not sure what's up in particular, but there's not much difference between the fixed and broken JS files.

Comment 1

[Kelsey Gilbert [:jgilbert]]

Specifically this is an issue for test-linux1804-64/debug-mochitest-browser-chrome-e10s-4
"Linux 18.04 x64 debug" M(bc4)

Comment 2

[Nicolas B. Pierron [:nbp]]

Tom, Matt, this bug is quite strange and can be split in 3 parts:

• Would you know what caused the error when replacing the const by a let?
• How the second patch removed fixed error?
• What is the stack of the error creation which caused a failure under initBorrowedLineBuf, and how to fix this issue?

Comment 3

[Tom S. (please needinfo tschuster)]

The string allocated by https://searchfox.org/mozilla-central/rev/1758450798ae14492ba28b695f48143840ad6c5b/dom/ipc/ClonedErrorHolder.cpp#232 doesn't seem to be null-terminated. Not sure why we assert that though, when we have a length anyway. I am not an expert on the parser however.

Comment 4

[Matthew Gaudet (he/him) [:mgaudet]]

Doing a touch of archeology, and ended up Bug 1212328, which introduced this assert. Bug 1212328 Comment 14 suggests that this has to do with the use of nsDependentString when consumed here.

The function Tom points out has only one caller, so we can easily fix it and rename it to something along the lines of ToJSStringBufferNullTerminated;

Having said all this, the comments in JSErrorReport aren't super clear about expecting a null terminated string, except insofar as the length field ought not include the null terminator.

Comment 5

[Matthew Gaudet (he/him) [:mgaudet]]

Notably as well: This code is all uncovered in code-coverage, which is why we've not seen this before.

https://coverage.moz.tools/#revision=latest&path=dom/ipc/ClonedErrorHolder.cpp&view=file&line=305

Comment 6

[Matthew Gaudet (he/him) [:mgaudet]]

Setting P3, then setting self-ni? to look at fixing this later.

Comment 7

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1699569 - Null terminate copied string under error condition r?evilpie

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/30f92429e81b
Null terminate copied string under error condition r=evilpie,kmag

Comment 9

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/30f92429e81b