Closed Bug 1655455 Opened 4 years ago Closed 4 years ago

Self-hosted JavaScript assertion info: [Latin 1]"./../../checkouts/gecko/js/src/builtin/Module.js:199: Bad module state in GetModuleNamespace"

Categories

(Core :: JavaScript Engine, defect, P2)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
81 Branch
Tracking Status
firefox-esr68 --- wontfix
firefox-esr78 --- wontfix
firefox79 --- wontfix
firefox80 --- wontfix
firefox81 --- fixed

People

(Reporter: decoder, Assigned: jonco)

References

Details

(4 keywords, Whiteboard: [bugmon:update,bisected,confirmed])

Attachments

(2 files)

The following testcase crashes on mozilla-central revision 20200727-56082fc4acfa (debug build, run with --fuzzing-safe --cpu-count=2 --ion-offthread-compile=off --baseline-eager):

var lfLogBuffer = `
assert.type();
if (typeof addIntlExtras === "function") {
  Object.defineProperty(Intl, "DisplayNames", {});
}
//corefuzz-dcd-endofdata
//corefuzz-dcd-endofdata
//corefuzz-dcd-endofdata
export * as await  from './doesnotexist4.js';
//corefuzz-dcd-endofdata
//corefuzz-dcd-endofdata
import { x12 as x12 } from './doesnotexist3.js';
//corefuzz-dcd-endofdata
import * as a86 from './doesnotexist1.js';
import * as b87 from './doesnotexist2.js';
`;
lfLogBuffer = lfLogBuffer.split('\n');
var lfModules = new Array();
setModuleResolveHook(function(module, specifier) {
    return lfModules.shift();
});
let lfCodeBuffer = "";
while (true) {
    let line = lfLogBuffer.shift();
    if (line == null) {
        break;
    } else if (line == "//corefuzz-dcd-endofdata") {
        loadFile(lfCodeBuffer);
        lfCodeBuffer = "";
    } else {
        lfCodeBuffer += line + "\n";
    }
}
if (lfCodeBuffer) loadFile(lfCodeBuffer);
function loadFile(lfVarx, lfForceRunType = 0) {
    try {
        lfMod = parseModule(lfVarx);
        lfModules.push(lfMod);
        lfMod.declarationInstantiation();
        lfMod.evaluation();
    } catch (lfVare) {
        if (lfVare.toString().indexOf("is not defined") >= 0)
            loadFile(lfVarx);
    }
}

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x0000555555d59da6 in intrinsic_AssertionFailed(JSContext*, unsigned int, JS::Value*) ()
#1  0x0000555555948482 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) ()
#2  0x0000555555947d59 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) ()
#3  0x00005555559493ec in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) ()
#4  0x00005555563a36cd in js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) ()
#5  0x0000049b70added3 in ?? ()
[...]
#36 0x0000000000000000 in ?? ()
rax	0x555556ff0f47	93825020137287
rbx	0x3d5e0644aa20	67474041383456
rcx	0x5555583dd840	93825041029184
rdx	0x0	0
rsi	0x7ffff7105770	140737338431344
rdi	0x7ffff7104540	140737338426688
rbp	0x7ffffffa3490	140737487975568
rsp	0x7ffffffa3450	140737487975504
r8	0x7ffff7105770	140737338431344
r9	0x7ffff7f9bd40	140737353727296
r10	0x58	88
r11	0x7ffff6dac7a0	140737334921120
r12	0x7ffff56f4c00	140737311099904
r13	0x7ffffffa3710	140737487976208
r14	0x7ffffffa3450	140737487975504
r15	0x555555d59c10	93825000643600
rip	0x555555d59da6 <intrinsic_AssertionFailed(JSContext*, unsigned int, JS::Value*)+406>
=> 0x555555d59da6 <_ZL25intrinsic_AssertionFailedP9JSContextjPN2JS5ValueE+406>:	movl   $0x1eb,0x0
   0x555555d59db1 <_ZL25intrinsic_AssertionFailedP9JSContextjPN2JS5ValueE+417>:	callq  0x55555584d42e <abort>

This testcase is behaving strangely. First of all, it only reproduces with --baseline-eager although I don't see anything about the crash that would be related to the JITs. Second, the test is very sensitive to the number of lines in lfLogBuffer (could also be due to JITs, but could also be something else). Therefore I didn't manage to process this test further, despite all sorts of efforts to unwind it further for reduction.

Because it's unclear how these properties fit together, I'm filing this s-s until investigated. I should also note that I have seen other self-hosted assertion failures that were never reproducible at all.

Attached file Testcase

Jon, can you investigate what is the issue and who might be the right person to look at this issue?

Severity: -- → S3
Flags: needinfo?(jcoppeard)
Priority: -- → P2

I don't know why it's only reproducing with baseline, but the problem is probably that the function supplied to setModuleResolveHook() breaks the module system's invariants. That function probably shouldn't be exposed to the fuzzers.

Can this be unhidden then? sec-other?

(Decoder let me know that the setModuleResolveHook() function is necessary for fuzzing modules.)

decoder, would the registerModule() function in the patch above work for your needs instead?

Flags: needinfo?(jcoppeard) → needinfo?(choller)

(In reply to Jon Coppeard (:jonco) from comment #6)

(Decoder let me know that the setModuleResolveHook() function is necessary for fuzzing modules.)

decoder, would the registerModule() function in the patch above work for you needs instead?

I think that's a good start and we can make this work. However, we are usually facing the problem that we don't know the names of the requested modules in advance. E.g. we have code like

import y from 'a'

and we don't know about the name 'a' in advance. With the old function, this didn't matter because we could just hand out a module when it was requested, no matter how the file was called. I think it would be most helpful if we either had that behavior or 1) a way to get the names of all registered modules and 2) a way to get the module for a registered name. This way, we can catch the error, check the requested filename, grab a random pre-registered model and register it under the requested name, then rerun the code.

Flags: needinfo?(choller)

(In reply to Christian Holler (:decoder) from comment #7)

(In reply to Jon Coppeard (:jonco) from comment #6)
However, we are usually facing the problem that we don't know the names of the requested modules in advance

Would it help if there was an API that gives information about which modules a module imports? This is already present internally because it's needed by the browser module loader.

(In reply to Jon Coppeard (:jonco) from comment #8)

(In reply to Christian Holler (:decoder) from comment #7)

(In reply to Jon Coppeard (:jonco) from comment #6)
However, we are usually facing the problem that we don't know the names of the requested modules in advance

Would it help if there was an API that gives information about which modules a module imports? This is already present internally because it's needed by the browser module loader.

Yes, that would help greatly, if we can get this information after the parseModule step.

(In reply to Christian Holler (:decoder) from comment #9)
I started working on this and realised it's already exposed. You can examine the 'requestedModules' property on the module object returned from parseModule(), as used here:

https://searchfox.org/mozilla-central/source/js/src/jit-test/tests/modules/requested-modules.js

(In reply to Jon Coppeard (:jonco) from comment #10)

(In reply to Christian Holler (:decoder) from comment #9)
I started working on this and realised it's already exposed. You can examine the 'requestedModules' property on the module object returned from parseModule(), as used here:

https://searchfox.org/mozilla-central/source/js/src/jit-test/tests/modules/requested-modules.js

This looks great. So I guess the following workflow for fuzzing would work?

  1. parseModule stuff
  2. Check with requestedModules if it has any dependencies
    3a. If no dependencies, register it with registerModule
    3b. If it has dependencies, we need to make sure we satisfy them somehow, by looking at what other modules we already have parsed and adding them to registerModule with the proper name. For this purpose, we globally keep track of modules already passed to registerModule, so we can randomly pick one, and re-register it under the required name.
Keywords: bugmon
Whiteboard: [bugmon:update,bisect] → [bugmon:update,bisected,confirmed]
Bugmon Analysis:
The bug appears to have been fixed in the following build range:
> Start: 6798cb55d9c90bfeceeddb22995768db65f3bb12 (20200730140408)
> End: ac9c811bc427ed15bca6b1b6410cf1f9f1ea8a59 (20200730142504)
> Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=6798cb55d9c90bfeceeddb22995768db65f3bb12&tochange=ac9c811bc427ed15bca6b1b6410cf1f9f1ea8a59
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.

(In reply to Christian Holler (:decoder) from comment #11)
As discussed with decoder, we're going to remove setModuleResolveHook() and the fuzzers are going to use module.requestedModules to get the information they need.

(In reply to Jason Kratzer [:jkratzer] from comment #12)

The bug appears to have been fixed in the following build range:

Those changes don't fix this (although it's possible they make the testcase stop reproducing).

This is a shell-only problem, so it's not security sensitive.

Assignee: nobody → jcoppeard
Group: javascript-core-security
Attachment #9167275 - Attachment description: Bug 1655455 - Add registerModule() test function → Bug 1655455 - Replace setModuleResolveHook() with registerModule() test function r?jandem

SpiderMonkey changes are ready to go. Decoder, please let us know when the fuzzers have been updated.

Flags: needinfo?(choller)
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2f8d79b33e8a
Replace setModuleResolveHook() with registerModule() test function r=jandem
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: