Tyson Smith [:tsmith] Reporter
	Description • 7 years ago

This seems to be triggered after a few minutes with regular browsing.

Found in mozilla-central changeset: 402372:3df7961bad2c. Built with -fsanitize=bool

I am guessing this is actually uninitialized memory but I'm not 100%.

/layout/style/MediaQueryList.cpp:78:10: runtime error: load of value 228, which is not a valid value for type 'bool'
    #0 0x7fa22e2ef382 in mozilla::dom::MediaQueryList::Matches() /layout/style/MediaQueryList.cpp:78:10
    #1 0x7fa22aa4686c in mozilla::dom::MediaQueryListBinding::get_matches(JSContext*, JS::Handle<JSObject*>, mozilla::dom::MediaQueryList*, JSJitGetterCallArgs) /objdir-ff-ubsan/dom/bindings/MediaQueryListBinding.cpp:66:21
    #2 0x7fa22bfe64aa in mozilla::dom::GenericBindingGetter(JSContext*, unsigned int, JS::Value*) /dom/bindings/BindingUtils.cpp:2906:13
    #3 0x7fa23446a88e in CallJSNative /js/src/jscntxtinlines.h:291:15
    #4 0x7fa23446a88e in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /js/src/vm/Interpreter.cpp:473
    #5 0x7fa23446b4cf in InternalCall(JSContext*, js::AnyInvokeArgs const&) /js/src/vm/Interpreter.cpp:522:12
    #6 0x7fa23446b6aa in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /js/src/vm/Interpreter.cpp:541:10
    #7 0x7fa23446c898 in js::CallGetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) /js/src/vm/Interpreter.cpp:656:12
    #8 0x7fa2353fc5f5 in CallGetter(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<js::Shape*>, JS::MutableHandle<JS::Value>) /js/src/vm/NativeObject.cpp:2145:16
    #9 0x7fa2353dc4c1 in GetExistingProperty<js::AllowGC::CanGC> /js/src/vm/NativeObject.cpp:2198:12
    #10 0x7fa2353dc4c1 in NativeGetPropertyInline<js::AllowGC::CanGC> /js/src/vm/NativeObject.cpp:2401
    #11 0x7fa2353dc4c1 in js::NativeGetProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) /js/src/vm/NativeObject.cpp:2437
    #12 0x7fa2343d2eb4 in js::GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>) /js/src/vm/NativeObject.h:1630:12
    #13 0x7fa2343d2cc7 in js::GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, js::PropertyName*, JS::MutableHandle<JS::Value>) /js/src/jsobj.h:822:12
    #14 0x7fa234472593 in js::GetProperty(JSContext*, JS::Handle<JS::Value>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>) /js/src/vm/Interpreter.cpp:4405:12
    #15 0x7fa234482647 in GetPropertyOperation(JSContext*, js::InterpreterFrame*, JS::Handle<JSScript*>, unsigned char*, JS::MutableHandle<JS::Value>, JS::MutableHandle<JS::Value>) /js/src/vm/Interpreter.cpp:219:12
    #16 0x7fa23443f114 in Interpret(JSContext*, js::RunState&) /js/src/vm/Interpreter.cpp:2815:10
    #17 0x7fa234433428 in js::RunScript(JSContext*, js::RunState&) /js/src/vm/Interpreter.cpp:423:12
    #18 0x7fa23446a779 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /js/src/vm/Interpreter.cpp:495:15
    #19 0x7fa23446b4cf in InternalCall(JSContext*, js::AnyInvokeArgs const&) /js/src/vm/Interpreter.cpp:522:12
    #20 0x7fa23446b6aa in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /js/src/vm/Interpreter.cpp:541:10
    #21 0x7fa234d5f64e in js::jit::InvokeFunction(JSContext*, JS::Handle<JSObject*>, bool, bool, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) /js/src/jit/VMFunctions.cpp:112:12
    #22 0x7fa234d60125 in js::jit::InvokeFromInterpreterStub(JSContext*, js::jit::InterpreterStubExitFrameLayout*) /js/src/jit/VMFunctions.cpp:141:10
    #23 0x7fa1c8b5df76  (<unknown module>)

	Xidorn Quan [:xidorn] UTC+10
	Comment 2 • 7 years ago
mozreview-review

Comment on attachment 8948878 [details]
Bug 1436240: Make sure to always initialize mMatches to something meaningful.

https://reviewboard.mozilla.org/r/218272/#review224060

May make more sense to have it be `Maybe<bool>`, but keeping them separate is probably fine as well, especially given there is code depending on their being separate.

	Xidorn Quan [:xidorn] UTC+10
	Comment 3 • 7 years ago

But actually, the question is why would this happen at all? The value is protected by mMatchesValid, so we shouldn't be reading mMatches as bool when mMatchesValid is false.

	Emilio Cobos Álvarez (:emilio) Assignee
	Comment 4 • 7 years ago

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #2)
> Comment on attachment 8948878 [details]
> Bug 1436240: Make sure to always initialize mMatches to something meaningful.
> 
> https://reviewboard.mozilla.org/r/218272/#review224060
> 
> May make more sense to have it be `Maybe<bool>`, but keeping them separate
> is probably fine as well, especially given there is code depending on their
> being separate.

Yeah, maybe... I went for the cheap fix here I admit :)

	Emilio Cobos Álvarez (:emilio) Assignee
	Comment 5 • 7 years ago

(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #3)
> But actually, the question is why would this happen at all? The value is
> protected by mMatchesValid, so we shouldn't be reading mMatches as bool when
> mMatchesValid is false.

The key is that RecomputeMatches bails out in a bunch of places without setting mMatches, then don't bother checking, like in ::Matches()

	Pulsebot
	Comment 7 • 7 years ago

Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/d71fadfe8e78
Make sure to always initialize mMatches to something meaningful. r=xidorn

	Xidorn Quan [:xidorn] UTC+10
	Comment 8 • 7 years ago

(In reply to Emilio Cobos Álvarez [:emilio] from comment #5)
> (In reply to Xidorn Quan [:xidorn] UTC+10 from comment #3)
> > But actually, the question is why would this happen at all? The value is
> > protected by mMatchesValid, so we shouldn't be reading mMatches as bool when
> > mMatchesValid is false.
> 
> The key is that RecomputeMatches bails out in a bunch of places without
> setting mMatches, then don't bother checking, like in ::Matches()

Oh, hmmm, that makes sense then.

	Dorel Luca [:dluca]
	Comment 9 • 7 years ago
bugherder

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