1652854 - AddressSanitizer: SEGV or Crash [@ js::SetObject::trace] or Assertion failure: !arr->getDenseElement(i).isMagic(JS_ELEMENTS_HOLE), at vm/NativeObject-inl.h:855

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

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

x = [];
Object.defineProperty(x, 17, {
  configurable: true,
  enumerable: true,
  writable: true,
});
Object.defineProperty(this, "y", {
  get: function () {
    return x.slice();
  }
});
y.shift();

Compiled using GCC 9.3.0 and Clang 9 with:

AR=ar sh ./configure --enable-debug --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

Run with:

--fuzzing-safe --no-threads --no-baseline --no-ion

Tested on m-c rev bca48c382991..

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/f112125cab4d
user:        Jan de Mooij
date:        Tue Jul 14 07:02:11 2020 +0000
summary:     Bug 1651645 part 9 - Add NON_PACKED flag to ObjectElements, use it in IsPackedArray. r=evilpie,anba

Unsure if this is a bad assertion failure, deferring to :jandem. ASan opt builds don't show anything.

Comment 1

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

Attachment: stack

Comment 2

[Christian Holler (:decoder)]

LangFuzz found the same bug, see also bug 1652875.

Comment 3

[Jan de Mooij [:jandem]]

Good find. The TI-based code relied on CopyDenseArrayElements => NewFullyAllocatedArrayTryReuseGroup either reusing the group or using a group with bad type information, so we didn't have to propagate the flag when memcpy'ing the elements.

Comment 4

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

Attachment: ASan stack

x = new Set([, 0].slice())

Run with: --fuzzing-safe --no-threads --no-baseline --no-ion on m-c rev d31bc01c978f.

Here's an even smaller, even better testcase that causes an ASan error on opt builds. It shows the same assertion failure on debug builds too.

With all due respect, I couldn't get my original testcase in comment 0 nor :decoder's testcase in bug 1652875 to cause an ASan error on opt builds, but perhaps the fuzzing infrastructure has already found something.

(This is partially why I believe it is important to consider retaining split bounties on ties found by MoCo internal infra within first 3 days / 72 hours - it inspired me to poke more at this issue)

Comment 5

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

Attachment: Opt stack for testcase in comment 4

Comment 6

[Christian Holler (:decoder)]

(In reply to Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) from comment #4)

Here's an even smaller, even better testcase that causes an ASan error on opt builds.

The ASan error you attached is not an ASan error but hitting a release assertion.

Comment 7

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

Attachment: screenshot.jpg

Nonetheless, the testcase in comment 4 also crashes in latest ASan Windows builds by wrapping in a <script></script> tag.

Downloaded via https://firefox-source-docs.mozilla.org/tools/sanitizer/asan.html#downloading-artifact-builds from https://firefox-ci-tc.services.mozilla.com/api/index/v1/task/gecko.v2.mozilla-central.latest.firefox.win64-asan-opt/artifacts/public/build/target.zip (also shows the crash on Linux). Takes a few seconds.

I don't remember why I currently can't find about:crashes in this ASan build though - perhaps I'm doing something wrong? How would people report ASan bugs by running ASan builds?

Comment 8

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

Attachment: testcase.html

Comment 9

[Jan de Mooij [:jandem]]

Attachment: Bug 1652854 - Mark dense elements non-packed in initDenseElements if necessary. r?anba!

I also added a jit-test for the IsPackedArray intrinsic. Now that this doesn't
depend on TI anymore, the behavior should be more predictable and this way we
can make sure we don't regress/break the packed-array optimization.

Comment 10

[Jan de Mooij [:jandem]]

I did a quick audit and I suspect this will either hit the release-assert in Value::isMagic, crash at a near-null address, or have non-exploitable correctness bugs.

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1651645

Comment 13

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/autoland/rev/b9d91d5c50c5980c44881c986b1f41cdeba74ef6

Comment 15

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

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

Comment 16

[Daniel Veditz [:dveditz]]

Awarding a split bounty with bug 1652875 and bug 1652963 under the rules existing at the time. This cluster of bugs were all filed within a day of the regressing bug landing and in the future that will fall within our 4-day exclusion window.