Closed Bug 1297360 Opened 3 years ago Closed 3 years ago

AddressSanitizer: heap-use-after-free [@ void mozilla::PodAssign<unsigned char>] with READ of size 1 with Profiling

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED DUPLICATE of bug 1316150
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: decoder, Assigned: jonco)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [jsbugmon:])

Attachments

(1 file)

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?
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:bisect]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
Whiteboard: [jsbugmon:bisect] → [jsbugmon:]
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.
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 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+
(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.
(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.
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.
See Also: → 1301377
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/093a7b8b7285
Copy strings returned by ProfilingFrameIterator API r=djvj
https://hg.mozilla.org/mozilla-central/rev/093a7b8b7285
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Depends on: 1316150
reopened because of the backout
Status: RESOLVED → REOPENED
Flags: needinfo?(jcoppeard)
Resolution: FIXED → ---
This is fixed by the patch in bug 1316150.
Status: REOPENED → RESOLVED
Closed: 3 years ago3 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.