Closed Bug 981999 Opened 6 years ago Closed 6 years ago

Conditional jump or move depends on uninitialised value(s) and Use of uninitialised value of size 8 [@ SaveSharedScriptData]

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: bc, Assigned: till)

References

()

Details

(Keywords: valgrind)

Attachments

(2 files)

Attached file valgrind messages
ecma_6/Generators/iteration.js, ecma_6/Generators/objects.js and ecma_6/Generators/runtime.js

you must package the tests first.

 (rm -f ./jstestbrowser.log && /mozilla/builds/nightly/mozilla/firefox-debug/_virtualenv/bin/python _tests/reftest/runreftest.py --extra-profile-file=dist/plugins --symbols-path=dist/crashreporter-symbols --debugger='valgrind' --debugger-args='--tool=memcheck --vex-iropt-register-updates=allregs-at-each-insn --smc-check=all-non-file --soname-synonyms=somalloc=NONE --trace-children=yes --track-origins=yes --read-var-info=yes' --timeout=86400  --total-chunks=40 --this-chunk=9 'dist/test-package-stage/jsreftest/tests//jstests.list' --extra-profile-file=dist/test-package-stage/jsreftest/tests/user.js | tee ./jstestbrowser.log) > valgrind.log 2>&1

allocation sites:

==15765==  Uninitialised value was created by a heap allocation
==15765==    at 0x4A06513: malloc (vg_replace_malloc.c:292)
==15765==    by 0x93E2706: js::MallocProvider<js::ThreadSafeContext>::malloc_(unsigned long) (Utility.h:134)
==15765==    by 0x973FCCE: js::SharedScriptData::new_(js::ExclusiveContext*, unsigned int, unsigned int, unsigned int) (jsscript.cpp:1948)
==15765==    by 0x977B94F: JSScript::fullyInitFromEmitter(js::ExclusiveContext*, JS::Handle<JSScript*>, js::frontend::BytecodeEmitter*) (jsscript.cpp:2389)
==15765==    by 0x94787A2: js::frontend::EmitFunctionScript(js::ExclusiveContext*, js::frontend::BytecodeEmitter*, js::frontend::ParseNode*) (BytecodeEmitter.cpp:2944)
==15765==    by 0x947989B: CompileFunctionBody(JSContext*, JS::MutableHandle<JSFunction*>, JS::ReadOnlyCompileOptions const&, js::AutoNameVector const&, char16_t const*, unsigned long, js::GeneratorKind) (BytecodeCompiler.cpp:614)
==15765==    by 0x9479AE2: js::frontend::CompileStarGeneratorBody(JSContext*, JS::MutableHandle<JSFunction*>, JS::ReadOnlyCompileOptions const&, js::AutoNameVector const&, char16_t const*, unsigned long) (BytecodeCompiler.cpp:646)
==15765==    by 0x970CB6B: FunctionConstructor(JSContext*, unsigned int, JS::Value*, js::GeneratorKind) (jsfun.cpp:1625)
==15765==    by 0x970CD1F: js::Generator(JSContext*, unsigned int, JS::Value*) (jsfun.cpp:1641)
==15765==    by 0x98436DC: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:239)
==15765==    by 0x9890996: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) (Interpreter.cpp:476)
==15765==    by 0x988A5C8: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:2614)

==15765==  Uninitialised value was created by a heap allocation
==15765==    at 0x4A06513: malloc (vg_replace_malloc.c:292)
==15765==    by 0x93E2706: js::MallocProvider<js::ThreadSafeContext>::malloc_(unsigned long) (Utility.h:134)
==15765==    by 0x973FCCE: js::SharedScriptData::new_(js::ExclusiveContext*, unsigned int, unsigned int, unsigned int) (jsscript.cpp:1948)
==15765==    by 0x977B94F: JSScript::fullyInitFromEmitter(js::ExclusiveContext*, JS::Handle<JSScript*>, js::frontend::BytecodeEmitter*) (jsscript.cpp:2389)
==15765==    by 0x94787A2: js::frontend::EmitFunctionScript(js::ExclusiveContext*, js::frontend::BytecodeEmitter*, js::frontend::ParseNode*) (BytecodeEmitter.cpp:2944)
==15765==    by 0x947989B: CompileFunctionBody(JSContext*, JS::MutableHandle<JSFunction*>, JS::ReadOnlyCompileOptions const&, js::AutoNameVector const&, char16_t const*, unsigned long, js::GeneratorKind) (BytecodeCompiler.cpp:614)
==15765==    by 0x9479AE2: js::frontend::CompileStarGeneratorBody(JSContext*, JS::MutableHandle<JSFunction*>, JS::ReadOnlyCompileOptions const&, js::AutoNameVector const&, char16_t const*, unsigned long) (BytecodeCompiler.cpp:646)
==15765==    by 0x970CB6B: FunctionConstructor(JSContext*, unsigned int, JS::Value*, js::GeneratorKind) (jsfun.cpp:1625)
==15765==    by 0x970CD1F: js::Generator(JSContext*, unsigned int, JS::Value*) (jsfun.cpp:1641)
==15765==    by 0x98436DC: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:239)
==15765==    by 0x984B155: js::CallJSNativeConstructor(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:272)
==15765==    by 0x9890FE0: js::InvokeConstructor(JSContext*, JS::CallArgs) (Interpreter.cpp:554)
==15765==
The original log for this is available at <http://sisyphus.bughunter.ateam.phx1.mozilla.com/bughunter/media/logs/2014/03/10/16/16/2014-03-10-16-16-42,firefox,nightly,debug,linux,valgrind4.bughunter.ateam.phx1.mozilla.com,jstestbrowser.log>. You will need MozillaVPN and the right acls to reach it.

Since the build used for the test was created on 2014/03/10 the line numbers may not completely agree with current trunk. Note that I use a pseudo signature to group valgrind messages which is created by taking the first 2 frames of the message stack and replacing all non alphanumeric characters with spaces. This log contains the following pseudo signatures for these conditional jump messages:

js UnmarkScriptData JSRuntime HashTable h by IncrementalCollectSlice JSRuntime long JS gcreason Reason js JSGCInvocationKind jsgc cpp

js SweepScriptData JSRuntime HashTable h by IncrementalCollectSlice JSRuntime long JS gcreason Reason js JSGCInvocationKind jsgc cpp

js detail HashTable js SharedScriptData const js HashSet js SharedScriptData js ScriptBytecodeHasher js SystemAllocPolicy SetOps js SystemAllocPolicy Range popFront HashTable h by js SweepScriptData JSRuntime jsscript cpp

js detail HashTable js SharedScriptData const js HashSet js SharedScriptData js ScriptBytecodeHasher js SystemAllocPolicy SetOps js SystemAllocPolicy Range front const HashTable h by js UnmarkScriptData JSRuntime jsscript cpp

js detail HashTable js SharedScriptData const js HashSet js SharedScriptData js ScriptBytecodeHasher js SystemAllocPolicy SetOps js SystemAllocPolicy Range front const HashTable h by js SweepScriptData JSRuntime jsscript cpp
Maybe Till or bhackett can look at this?
Turns out I can get rid of one of the ways we count source notes: we can split up finalization of source notes and copying them to their final destination. That in turn allows us to use the count we do in FinalizeSourceNotes to allocate space for them.
Attachment #8397282 - Flags: review?(luke)
Assignee: nobody → till
Status: NEW → ASSIGNED
Oh, and to explain what's going on here: we currently have two different ways of counting the number of source notes a function's script has. We're using one to allocate space for the source note pointers, and the other to actually initialize these pointers. Unfortunately, the former sometimes yields a larger count than the latter. The attached patch solves this by getting rid of it.
Comment on attachment 8397282 [details] [diff] [review]
Remove redundant (and not always agreeing) ways of counting sourcenotes.

Review of attachment 8397282 [details] [diff] [review]:
-----------------------------------------------------------------

Nice job hunting this down.  Sorry for taking so long to get to it.

::: js/src/frontend/BytecodeEmitter.cpp
@@ +6760,5 @@
>      return true;
>  }
>  
>  /*
> + * Finish taking source notes in cx's notePool.

Could you also add a comment explaining the return value?

@@ +6804,5 @@
>          }
>      }
>  
> +    // The prolog count might have changed, so we can't reuse prologCount.
> +    // The + 1 is to account for the final SRC_NULL note.

Can you say "the final SN_MAKE_TERMINATOR" (so the reader doesn't have to know what SN_MAKE_TERMINATOR does)?

@@ +6809,5 @@
> +    return bce->prolog.notes.length() + bce->main.notes.length() + 1;
> +}
> +
> +void
> +frontend::CopySrcNotes(BytecodeEmitter *bce, jssrcnote *destination)

Could you have CopySrcNotes take in nsrcnotes (the return value of FinishTakingSrcNotes) which is asserted to be totalCount + 1?
Attachment #8397282 - Flags: review?(luke) → review+
Depends on: 993075
https://hg.mozilla.org/mozilla-central/rev/4a3477a52c6f
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.