1532868 - UBSan: left shift of negative value in include/mozilla/FontPropertyTypes.h:101:2

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect, P3)

Description

[Tyson Smith [:tsmith]]

Found in m-c commit 78601cacfe69

This was triggered while on youtube.com

This was build with undefined behavior sanitizer checks enabled via mozconfig.
ac_add_options --enable-undefined-sanitizer="shift"

src/objdir-ff-ubsan/dist/include/mozilla/FontPropertyTypes.h:101:23: runtime error: left shift of negative value -90
    #0 0x7fb961189808 in FontPropertyValue src/objdir-ff-ubsan/dist/include/mozilla/FontPropertyTypes.h:101:23
    #1 0x7fb961189808 in FontPropertyValue src/objdir-ff-ubsan/dist/include/mozilla/FontPropertyTypes.h:56
    #2 0x7fb961189808 in FontSlantStyle src/objdir-ff-ubsan/dist/include/mozilla/FontPropertyTypes.h:264
    #3 0x7fb961189808 in mozilla::dom::FontFaceSet::FindMatchingFontFaces(nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, nsTArray<mozilla::dom::FontFace*>&, mozilla::ErrorResult&) src/layout/style/FontFaceSet.cpp:263
    #4 0x7fb96118a8f9 in mozilla::dom::FontFaceSet::Load(JSContext*, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, mozilla::ErrorResult&) src/layout/style/FontFaceSet.cpp:337:3
    #5 0x7fb95eb8d050 in load src/objdir-ff-ubsan/dom/bindings/FontFaceSetBinding.cpp:888:45
    #6 0x7fb95eb8d050 in mozilla::dom::FontFaceSet_Binding::load_promiseWrapper(JSContext*, JS::Handle<JSObject*>, mozilla::dom::FontFaceSet*, JSJitMethodCallArgs const&) src/objdir-ff-ubsan/dom/bindings/FontFaceSetBinding.cpp:904
    #7 0x7fb95ef5529d in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ConvertExceptionsToPromises>(JSContext*, unsigned int, JS::Value*) src/dom/bindings/BindingUtils.cpp:3144:13
    #8 0x7fb9649c3c2d in CallJSNative src/js/src/vm/Interpreter.cpp:440:13
    #9 0x7fb9649c3c2d in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) src/js/src/vm/Interpreter.cpp:532
    #10 0x7fb9649c504d in InternalCall(JSContext*, js::AnyInvokeArgs const&) src/js/src/vm/Interpreter.cpp:587:10
    #11 0x7fb96499c1e5 in CallFromStack src/js/src/vm/Interpreter.cpp:591:10
    #12 0x7fb96499c1e5 in Interpret(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:3055
    #13 0x7fb9649855a0 in js::RunScript(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:420:10
    #14 0x7fb9649c71c2 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) src/js/src/vm/Interpreter.cpp:779:13
    #15 0x7fb9649c7659 in js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) src/js/src/vm/Interpreter.cpp:812:10
    #16 0x7fb964bd45af in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::Value*) src/js/src/vm/CompilationAndEvaluation.cpp:438:10
    #17 0x7fb964bd4c99 in ExecuteScript(JSContext*, JS::AutoVector<JSObject*>&, JS::Handle<JSScript*>, JS::Value*) src/js/src/vm/CompilationAndEvaluation.cpp:458:10
    #18 0x7fb95d736ec2 in nsJSUtils::ExecutionContext::ExecScript() src/dom/base/nsJSUtils.cpp:389:8
    #19 0x7fb960bdc39d in mozilla::dom::ExecuteCompiledScript(JSContext*, mozilla::dom::ScriptLoadRequest*, nsJSUtils::ExecutionContext&) src/dom/script/ScriptLoader.cpp:2496:16
    #20 0x7fb960bdae49 in mozilla::dom::ScriptLoader::EvaluateScript(mozilla::dom::ScriptLoadRequest*) src/dom/script/ScriptLoader.cpp:2716:20
    #21 0x7fb960bd65ba in mozilla::dom::ScriptLoader::ProcessRequest(mozilla::dom::ScriptLoadRequest*) src/dom/script/ScriptLoader.cpp:2223:10
    #22 0x7fb960bd4788 in mozilla::dom::ScriptLoader::ProcessInlineScript(nsIScriptElement*, mozilla::dom::ScriptKind) src/dom/script/ScriptLoader.cpp:1804:10
    #23 0x7fb960bbfc4c in mozilla::dom::ScriptLoader::ProcessScriptElement(nsIScriptElement*) src/dom/script/ScriptLoader.cpp:1527:10
    #24 0x7fb960bbf040 in mozilla::dom::ScriptElement::MaybeProcessScript() src/dom/script/ScriptElement.cpp:118:18
    #25 0x7fb95c4f29f4 in nsIScriptElement::AttemptToExecute() src/objdir-ff-ubsan/dist/include/nsIScriptElement.h:224:18
    #26 0x7fb95c4c69fc in nsHtml5TreeOpExecutor::RunScript(nsIContent*) src/parser/html/nsHtml5TreeOpExecutor.cpp:727:22
    #27 0x7fb95c4c3372 in nsHtml5TreeOpExecutor::RunFlushLoop() src/parser/html/nsHtml5TreeOpExecutor.cpp:530:7
    #28 0x7fb95c4f7098 in nsHtml5ExecutorReflusher::Run() src/parser/html/nsHtml5TreeOpExecutor.cpp:68:16
    #29 0x7fb95a5178eb in mozilla::SchedulerGroup::Runnable::Run() src/xpcom/threads/SchedulerGroup.cpp:295:32
    #30 0x7fb95a542fd1 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1166:14
    #31 0x7fb95a5479fd in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:482:10
    #32 0x7fb95b6447a8 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:88:21
    #33 0x7fb95b50c650 in RunHandler src/ipc/chromium/src/base/message_loop.cc:308:3
    #34 0x7fb95b50c650 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290
    #35 0x7fb960e7bb1d in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
    #36 0x7fb964788a60 in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:911:20
    #37 0x7fb95b50c650 in RunHandler src/ipc/chromium/src/base/message_loop.cc:308:3
    #38 0x7fb95b50c650 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290
    #39 0x7fb964787f00 in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:749:34
    #40 0x55946af6420f in content_process_main(mozilla::Bootstrap*, int, char**) src/browser/app/../../ipc/contentproc/plugin-container.cpp:49:28
    #41 0x55946af643ea in main src/browser/app/nsBrowserApp.cpp:265:18

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1532868 - Use multiplication rather than left-shift to avoid potential UB in FontPropertyValue constructor. r?jwatt

Comment 2

[Jonathan Kew [:jfkthame]]

Ah, right -- in the case of FontSlantStyle, the integer value may be negative, in which case left-shifting it is undefined behavior. In practice it's unlikely to be a problem on any twos-complement architecture, but still, let's just fix it.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2db1d305083d
Use multiplication rather than left-shift to avoid potential UB in FontPropertyValue constructor. r=jwatt

Comment 4

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/2db1d305083d

Comment 5

[Julien Cristau [:jcristau]]

Per comment 2 this isn't a huge deal in practice, setting wontfix for 67.