1557664 - LeakSanitizer: [@ ShellPrincipals] with newGlobal

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

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

The following testcase crashes on mozilla-central revision b8d6494caa57 (build with --enable-address-sanitizer, run with --fuzzing-safe --no-threads --no-baseline --no-ion) with ASAN_OPTIONS=detect_leaks=1 in the environment variable:

// Adapted from randomly chosen test: js/src/jit-test/tests/realms/basic.js
newGlobal({
    sameCompartmentAs: newGlobal({
        newCompartment: true,
        systemPrincipal: true
    }),
    principal: true
});

Backtrace:

Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x56308a7f0397 in __interceptor_malloc (/home/ubuntu/shell-cache/js-64-asan-linux-x86_64-b8d6494caa57/js-64-asan-linux-x86_64-b8d6494caa57+0x1457397)
#1 0x56308a894bfe in js_arena_malloc(unsigned long, unsigned long) /home/ubuntu/shell-cache/js-64-asan-linux-x86_64-b8d6494caa57/objdir-js/dist/include/js/Utility.h:367:10
#2 0x56308a894bfe in unsigned char* js_pod_arena_malloc<unsigned char>(unsigned long, unsigned long) /home/ubuntu/shell-cache/js-64-asan-linux-x86_64-b8d6494caa57/objdir-js/dist/include/js/Utility.h:572
#3 0x56308a894bfe in unsigned char* js::MallocProvider<JSContext>::maybe_pod_malloc<unsigned char>(unsigned long, unsigned long) js/src/vm/MallocProvider.h:53
#4 0x56308a894bfe in unsigned char* js::MallocProvider<JSContext>::pod_malloc<unsigned char>(unsigned long, unsigned long) js/src/vm/MallocProvider.h:90
#5 0x56308a894bfe in ShellPrincipals* js::MallocProvider<JSContext>::new_<ShellPrincipals, unsigned int&>(unsigned int&) js/src/vm/MallocProvider.h:196
#6 0x56308a894bfe in NewGlobal(JSContext*, unsigned int, JS::Value*) js/src/shell/js.cpp:6271
/snip

For detailed crash information, see attachment.

Comment 1

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

Attachment: Detailed Crash Information

Comment 2

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

autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/9f73af1f3b2c
user: Jan de Mooij
date: Sat Aug 11 13:12:49 2018 +0200
summary: Bug 1482423 part 1 - Assert compartments don't contain both system/non-system realms. r=luke

Jan, is bug 1482423 a likely regressor?

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1557664 - Add AutoHoldPrincipals RAII class and use it to fix a leak in the JS shell. r?jwalden!

Unfortunately JS_DropPrincipals wants the cx so it's hard to use RefPtr<>.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cb05bf1bcc54
Add AutoHoldPrincipals RAII class and use it to fix a leak in the JS shell. r=jwalden

Comment 5

[Raul Gurzau (:RaulG)]

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

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Gary, is this something that'll help you with fuzzing if we backport to 68?

Comment 7

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

Sure, redirecting to Jan.

Comment 8

[Jan de Mooij [:jandem]]

LeakSanitizer is great to have, but I think we should just let it ride the trains and not bother with using it on beta for now. There are a number of trivial shell-only fixes like this one and uplifting all of them just takes time for various people and I'm not convinced it's worth uplifting every patch.