Closed
Bug 1297360
Opened 5 years ago
Closed 5 years ago
AddressSanitizer: heap-use-after-free [@ void mozilla::PodAssign<unsigned char>] with READ of size 1 with Profiling
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
DUPLICATE
of bug 1316150
mozilla51
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: decoder, Assigned: jonco)
References
Details
(4 keywords, Whiteboard: [jsbugmon:])
Attachments
(1 file)
7.71 KB,
patch
|
djvj
:
review+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision f97a056ae623 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-debug --enable-address-sanitizer --disable-jemalloc --enable-optimize=-O2, run with --fuzzing-safe --no-threads --ion-eager): var lfLogBuffer = ` gczeal(14); enableSPSProfiling(); gczeal(15,3); var s = ""; for (let i = 0; i != 30; i+=2) {} readSPSProfilingStack(s, "c0d1c0d1c0d1c0d1c0d1c0d1c0d1c0"); `; loadFile(lfLogBuffer); function loadFile(lfVarx) { let m = parseModule(lfVarx); m.declarationInstantiation(); m.evaluation(); } Backtrace: ==24850==ERROR: AddressSanitizer: heap-use-after-free on address 0x6020000098b0 at pc 0x0000016dc3d0 bp 0x7ffff8295c90 sp 0x7ffff8295c88 READ of size 1 at 0x6020000098b0 thread T0 #0 0x16dc3cf in void mozilla::PodAssign<unsigned char>(unsigned char*, unsigned char const*) dist/include/mozilla/PodOperations.h:87:3 #1 0x16dc3cf in void mozilla::PodCopy<unsigned char>(unsigned char*, unsigned char const*, unsigned long) dist/include/mozilla/PodOperations.h:107 #2 0x16dc3cf in JSInlineString* js::NewInlineString<(js::AllowGC)1, unsigned char>(js::ExclusiveContext*, mozilla::Range<unsigned char const>) js/src/vm/String-inl.h:60 #3 0x16dc3cf in JSFlatString* js::NewStringCopyNDontDeflate<(js::AllowGC)1, unsigned char>(js::ExclusiveContext*, unsigned char const*, unsigned long) js/src/vm/String.cpp:1241 #4 0x1a195a2 in JSFlatString* js::NewStringCopyN<(js::AllowGC)1>(js::ExclusiveContext*, char const*, unsigned long) js/src/vm/String.h:1190:12 #5 0x1a195a2 in JSFlatString* js::NewStringCopyZ<(js::AllowGC)1>(js::ExclusiveContext*, char const*) js/src/vm/String.h:1210 #6 0x1a195a2 in ReadSPSProfilingStack(JSContext*, unsigned int, JS::Value*) js/src/builtin/TestingFunctions.cpp:1696 [...] 0x6020000098b0 is located 0 bytes inside of 9-byte region [0x6020000098b0,0x6020000098b9) freed by thread T0 here: #0 0x5112e0 in __interceptor_free /srv/repos/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:38 #1 0xb453ea in js_free(void*) dist/include/js/Utility.h:263:5 #2 0xb453ea in js::jit::JitcodeGlobalEntry::IonEntry::destroy() js/src/jit/JitcodeMap.cpp:142 #3 0xb4810f in js::jit::JitcodeGlobalEntry::destroy() js/src/jit/JitcodeMap.h:604:13 #4 0xb4810f in js::jit::JitcodeGlobalTable::removeEntry(js::jit::JitcodeGlobalEntry&, js::jit::JitcodeGlobalEntry**, JSRuntime*) js/src/jit/JitcodeMap.cpp:553 #5 0xb494f3 in js::jit::JitcodeGlobalTable::releaseEntry(js::jit::JitcodeGlobalEntry&, js::jit::JitcodeGlobalEntry**, JSRuntime*) js/src/jit/JitcodeMap.cpp:567:5 #6 0xb494f3 in js::jit::JitcodeGlobalTable::Enum::removeFront() js/src/jit/JitcodeMap.cpp:432 #7 0xb494f3 in js::jit::JitcodeGlobalTable::sweep(JSRuntime*) js/src/jit/JitcodeMap.cpp:826 #8 0x10abd62 in js::gc::GCRuntime::beginSweepingZoneGroup(js::AutoLockForExclusiveAccess&) js/src/jsgc.cpp:5037:13 #9 0x10af22f in js::gc::GCRuntime::beginSweepPhase(bool, js::AutoLockForExclusiveAccess&) js/src/jsgc.cpp:5189:5 #10 0x10b613c in js::gc::GCRuntime::incrementalCollectSlice(js::SliceBudget&, JS::gcreason::Reason, js::AutoLockForExclusiveAccess&) js/src/jsgc.cpp:5888:9 #11 0x10b7ede in js::gc::GCRuntime::gcCycle(bool, js::SliceBudget&, JS::gcreason::Reason) js/src/jsgc.cpp:6142:5 #12 0x10b9560 in js::gc::GCRuntime::collect(bool, js::SliceBudget, JS::gcreason::Reason) js/src/jsgc.cpp:6249:25 #13 0x10c0380 in js::gc::GCRuntime::gc(JSGCInvocationKind, JS::gcreason::Reason) js/src/jsgc.cpp:6314:5 #14 0x10c0380 in js::gc::GCRuntime::runDebugGC() js/src/jsgc.cpp:6770 #15 0x1ae2d0c in js::gc::GCRuntime::gcIfNeededPerAllocation(JSContext*) js/src/gc/Allocator.cpp:224:9 #16 0x1aefef4 in bool js::gc::GCRuntime::checkAllocatorState<(js::AllowGC)1>(JSContext*, js::gc::AllocKind) js/src/gc/Allocator.cpp:189:14 #17 0x1aefef4 in JSString* js::Allocate<JSString, (js::AllowGC)1>(js::ExclusiveContext*) js/src/gc/Allocator.cpp:139 #18 0x16daf5c in JSThinInlineString* JSThinInlineString::new_<(js::AllowGC)1>(js::ExclusiveContext*) js/src/vm/String-inl.h:250:45 #19 0x16daf5c in JSInlineString* js::AllocateInlineString<(js::AllowGC)1, unsigned char>(js::ExclusiveContext*, unsigned long, unsigned char**) js/src/vm/String-inl.h:30 #20 0x16daf5c in JSInlineString* js::NewInlineString<(js::AllowGC)1, unsigned char>(js::ExclusiveContext*, mozilla::Range<unsigned char const>) js/src/vm/String-inl.h:56 #21 0x16daf5c in JSFlatString* js::NewStringCopyNDontDeflate<(js::AllowGC)1, unsigned char>(js::ExclusiveContext*, unsigned char const*, unsigned long) js/src/vm/String.cpp:1241 #22 0x1a195a2 in JSFlatString* js::NewStringCopyN<(js::AllowGC)1>(js::ExclusiveContext*, char const*, unsigned long) js/src/vm/String.h:1190:12 #23 0x1a195a2 in JSFlatString* js::NewStringCopyZ<(js::AllowGC)1>(js::ExclusiveContext*, char const*) js/src/vm/String.h:1210 #24 0x1a195a2 in ReadSPSProfilingStack(JSContext*, unsigned int, JS::Value*) js/src/builtin/TestingFunctions.cpp:1696 [...] previously allocated by thread T0 here: #0 0x511628 in __interceptor_malloc /srv/repos/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:52 #1 0x5a2b6b in js_malloc(unsigned long) dist/include/js/Utility.h:235:12 #2 0x5a2b6b in char* js_pod_malloc<char>(unsigned long) dist/include/js/Utility.h:483 #3 0x5a2b6b in char* js::MallocProvider<js::ExclusiveContext>::maybe_pod_malloc<char>(unsigned long) js/src/vm/MallocProvider.h:57 #4 0x5a2b6b in char* js::MallocProvider<js::ExclusiveContext>::pod_malloc<char>(unsigned long) js/src/vm/MallocProvider.h:90 #5 0xb462fc in js::jit::JitcodeGlobalEntry::createScriptString(JSContext*, JSScript*, unsigned long*) js/src/jit/JitcodeMap.cpp:356:17 #6 0xb4ce12 in js::jit::JitcodeIonTable::makeIonEntry(JSContext*, js::jit::JitCode*, unsigned int, JSScript**, js::jit::JitcodeGlobalEntry::IonEntry&) js/src/jit/JitcodeMap.cpp:1454:21 #7 0x9620ab in js::jit::CodeGenerator::link(JSContext*, js::CompilerConstraintList*) js/src/jit/CodeGenerator.cpp:9341:14 #8 0xabb20c in LinkCodeGen(JSContext*, js::jit::IonBuilder*, js::jit::CodeGenerator*) js/src/jit/Ion.cpp:537:10 #9 0x9fdced in js::jit::IonCompile(JSContext*, JSScript*, js::jit::BaselineFrame*, unsigned char*, bool, bool, js::jit::OptimizationLevel) js/src/jit/Ion.cpp:2311:21 #10 0x9fdced in js::jit::Compile(JSContext*, JS::Handle<JSScript*>, js::jit::BaselineFrame*, unsigned char*, bool, bool) js/src/jit/Ion.cpp:2469 #11 0x9ff048 in BaselineCanEnterAtBranch(JSContext*, JS::Handle<JSScript*>, js::jit::BaselineFrame*, unsigned char*) js/src/jit/Ion.cpp:2656:27 [...] I'm not sure if we have this on file somewhere already, I remember a similar issue a while ago.
This is a recent regression?
Updated•5 years ago
|
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:bisect]
Comment 2•5 years ago
|
||
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
Updated•5 years ago
|
Whiteboard: [jsbugmon:bisect] → [jsbugmon:]
Assignee | ||
Comment 3•5 years ago
|
||
ProfilingFrameIterator::extractStack fills in Frame::label with a malloced string pointer that is owned by the JIT code table. These strings can be freed by GC, so this is not safe.
Assignee | ||
Comment 4•5 years ago
|
||
Patch to copy the label string and make the label field a UniquePtr so that it gets cleaned up properly. I made this abort silently on OOM. I could make it fallible and report OOM to the caller if you think it's worth it.
Assignee: nobody → jcoppeard
Attachment #8788474 -
Flags: review?(kvijayan)
Comment 5•5 years ago
|
||
Comment on attachment 8788474 [details] [diff] [review] bug1297360-frame-label Review of attachment 8788474 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. Just wanted to confirm that with C++ structs, applying |move| to the struct correctly moves all the constituent fields? If you know that, r=me.
Attachment #8788474 -
Flags: review?(kvijayan) → review+
Comment 6•5 years ago
|
||
(In reply to Kannan Vijayan [:djvj] from comment #5) > Just wanted to confirm that with C++ structs, applying |move| > to the struct correctly moves all the constituent fields? Moving doesn't "do" anything at all. It just effectively says, "My contents are available to be taken from me, because I'm going to be destroyed soon." If the *consumer* of the move decides to take advantage of that offer, then things are handled more intelligently than just a plain copy would. If it doesn't take advantage of that, nothing special happens. Put another way, |std::move(foo)| or |mozilla::Move(foo)| does nothing but cast its argument. All the intelligence of moving is provided by typical consumers like |operator=(Foo&& foo)| or |Foo(Foo&& foo)|, or by less-common consumers like |void bar(Foo&& foo)| that take an rvalue reference (the result of a move-cast) and take advantage of the movability of |foo| in that case.
Assignee | ||
Comment 7•5 years ago
|
||
(In reply to Kannan Vijayan [:djvj] from comment #5) And further to what Waldo wrote, in this case Frame will get an implicitly defined move constructor which will move construct all its members. UniquePtr implements a move constructor which will correctly move the label. All other fields will be copied as normal.
Assignee | ||
Comment 8•5 years ago
|
||
The test code causes an assertion failure in compacting GC builds. I don't think this is related to the issue here so I'm going to land the patch without testcode and file a new bug for that.
Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/093a7b8b7285 Copy strings returned by ProfilingFrameIterator API r=djvj
Comment 10•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/093a7b8b7285
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Comment 11•5 years ago
|
||
Backout by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c207eec1dcd8 Backout changeset 093a7b8b7285 for causing bug 1316150
Comment 12•5 years ago
|
||
backoutbugherder |
https://hg.mozilla.org/mozilla-central/rev/c207eec1dcd8
Comment 13•5 years ago
|
||
reopened because of the backout
Status: RESOLVED → REOPENED
Flags: needinfo?(jcoppeard)
Resolution: FIXED → ---
Assignee | ||
Comment 14•5 years ago
|
||
This is fixed by the patch in bug 1316150.
Status: REOPENED → RESOLVED
Closed: 5 years ago → 5 years ago
Flags: needinfo?(jcoppeard)
Resolution: --- → DUPLICATE
Duplicate of bug: 1316150
You need to log in
before you can comment on or make changes to this bug.
Description
•