Skip to content

Memory Leak of StringTaints #61

@tmbrbr

Description

@tmbrbr

The StringTaint implementation is leaking memory, for example in SpiderMonkey.

The following JavaScript sample:

s = String.tainted("hello");
s = s.replace("l","o");

t = s;
for (let i = 0; i < 100; i++) {
    t = t + s;
}

console.log(t);

when run with valgrind (using suggestions from here):

G_SLICE=always-malloc valgrind --leak-check=full --smc-check=all-non-file --vex-iropt-register-updates=allregs-at-mem-access --show-mismatched-frees=no --log-file="valgrind.log" --read-inline-info=yes obj-tf-spider/dist/bin/js taint-test.js

gives the following output:

...
==2729935== 1,830,552 (2,136 direct, 1,828,416 indirect) bytes in 89 blocks are definitely lost in loss record 26 of 26
==2729935==    at 0x483BE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==2729935==    by 0xC81EFA: StringTaint::operator=(StringTaint const&) (taint/Taint.cpp:437)
==2729935==    by 0x4D9757: init (js/src/vm/StringType-inl.h:207)
==2729935==    by 0x4D9757: new_<js::CanGC> (js/src/vm/StringType-inl.h:236)
==2729935==    by 0x4D9757: JSString* js::ConcatStringsQuiet<(js::AllowGC)1>(JSContext*, js::MaybeRooted<JSString*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JSString*, (js::AllowGC)1>::HandleType, js::gc::InitialHeap) (js/src/vm/StringType.cpp:1003)
==2729935==    by 0x4DA2CB: JSString* js::ConcatStrings<(js::AllowGC)1>(JSContext*, js::MaybeRooted<JSString*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JSString*, (js::AllowGC)1>::HandleType, js::gc::InitialHeap) (js/src/vm/StringType.cpp:1026)
==2729935==    by 0xA11A53C6225: ???
==2729935==    by 0x7198DC7: ???
==2729935==    by 0xA11A53C454E: ???
==2729935==    by 0x80F081: EnterBaseline (js/src/jit/BaselineJIT.cpp:143)
==2729935==    by 0x80F081: js::jit::EnterBaselineInterpreterAtBranch(JSContext*, js::InterpreterFrame*, unsigned char*) (js/src/jit/BaselineJIT.cpp:212)
==2729935==    by 0x2B52E2: Interpret(JSContext*, js::RunState&) (js/src/vm/Interpreter.cpp:2201)
==2729935==    by 0x2B49B5: js::RunScript(JSContext*, js::RunState&) (js/src/vm/Interpreter.cpp:397)
==2729935==    by 0x2C4F86: js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::Handle<JS::Value>, js::AbstractFramePtr, JS::MutableHandle<JS::Value>) (js/src/vm/Interpreter.cpp:780)
==2729935==    by 0x39BE50: JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>) (js/src/vm/CompilationAndEvaluation.cpp:539)
==2729935== 
==2729935== LEAK SUMMARY:
==2729935==    definitely lost: 2,472 bytes in 103 blocks
==2729935==    indirectly lost: 1,852,160 bytes in 10,211 blocks
==2729935==      possibly lost: 0 bytes in 0 blocks
==2729935==    still reachable: 0 bytes in 0 blocks
==2729935==         suppressed: 0 bytes in 0 blocks
==2729935== 
==2729935== Use --track-origins=yes to see where uninitialised values come from
==2729935== For lists of detected and suppressed errors, rerun with: -s
==2729935== ERROR SUMMARY: 7786 errors from 45 contexts (suppressed: 0 from 0)

The reason for the leak is that StringTaint stores a vector of TaintRanges in a pointer which is managed by StringTaint. Actually, the caller has to ensure that clear() is called before the StringTaint is destroyed. In most cases this happens when a StringType is destroyed by the SpiderMonkey VM (via the finalize method, but is not happening in all instances.

Attaching a debugged and watching the location of one of the created (but not finalized) Strings, shows that the JS Cell is only altered when the program terminates. Here's the Stack:

Thread 1 "js" hit Hardware watchpoint 2: *0xb0c076018c0

Old value = 560
New value = 0
__memset_avx2_erms () at ../sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S:151
151	../sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S: No such file or directory.
(gdb) where
#0  __memset_avx2_erms () at ../sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S:151
#1  0x0000555555b95e51 in mozilla::detail::AtomicBase<unsigned long, (mozilla::MemoryOrdering)0>::AtomicBase (this=<optimized out>)
    at /mnt/workspace/project-foxhound/obj-tf-spider/dist/include/mozilla/Atomics.h:291
#2  mozilla::detail::AtomicBaseIncDec<unsigned long, (mozilla::MemoryOrdering)0>::AtomicBaseIncDec (this=<optimized out>) at /mnt/workspace/project-foxhound/obj-tf-spider/dist/include/mozilla/Atomics.h:335
#3  mozilla::Atomic<unsigned long, (mozilla::MemoryOrdering)0, void>::Atomic (this=<optimized out>) at /mnt/workspace/project-foxhound/obj-tf-spider/dist/include/mozilla/Atomics.h:389
#4  js::gc::MarkBitmap::MarkBitmap (this=0xb0c076018e8) at /mnt/workspace/project-foxhound/obj-tf-spider/dist/include/js/HeapAPI.h:202
#5  js::gc::TenuredChunkBase::TenuredChunkBase (this=0xb0c07600000, runtime=0x0) at /mnt/workspace/project-foxhound/obj-tf-spider/dist/include/js/HeapAPI.h:240
#6  js::gc::TenuredChunk::TenuredChunk (this=0xb0c07600000, runtime=0x0) at /mnt/workspace/project-foxhound/js/src/gc/Heap.h:624
#7  js::gc::TenuredChunk::init (this=0xb0c07600000, gc=0x5555574686c8, allMemoryCommitted=false) at /mnt/workspace/project-foxhound/js/src/gc/Allocator.cpp:834
#8  0x0000555555bc2b0f in js::NurseryDecommitTask::run (this=0x55555746ad48, lock=...) at /mnt/workspace/project-foxhound/js/src/gc/Nursery.cpp:178
#9  0x0000555555bb39df in js::GCParallelTask::runTask (this=0x55555746ad48, lock=...) at /mnt/workspace/project-foxhound/js/src/gc/GCParallelTask.cpp:175
#10 js::GCParallelTask::runFromMainThread (this=0x55555746ad48) at /mnt/workspace/project-foxhound/js/src/gc/GCParallelTask.cpp:150
#11 js::Nursery::disable (this=0x55555746a9a0) at /mnt/workspace/project-foxhound/js/src/gc/Nursery.cpp:354
#12 0x0000555555ba031a in js::gc::GCRuntime::finish (this=0x5555574686c8) at /mnt/workspace/project-foxhound/js/src/gc/GC.cpp:841
#13 0x00005555558e6e36 in JSRuntime::destroyRuntime (this=0x555557468190) at /mnt/workspace/project-foxhound/js/src/vm/Runtime.cpp:307
#14 0x000055555583673d in js::DestroyContext (cx=0x5555574731e0) at /mnt/workspace/project-foxhound/js/src/vm/JSContext.cpp:238
#15 0x0000555555670efa in main::$_3::operator() (this=<optimized out>) at /mnt/workspace/project-foxhound/js/src/shell/js.cpp:12628
#16 mozilla::ScopeExit<main::$_3>::~ScopeExit (this=<optimized out>) at /mnt/workspace/project-foxhound/obj-tf-spider/dist/include/mozilla/ScopeExit.h:106
#17 main (argc=<optimized out>, argv=<optimized out>) at /mnt/workspace/project-foxhound/js/src/shell/js.cpp:12833
(gdb) 

Where it looks like instead of clearing up objects individually, the memory is simply overwritten (I guess this is quicker). For regular Strings this is not a problem as any char buffers associated to the object will also be stored in the SpiderMonkey Heap somewhere and also be deleted.

In the case of the TaintRange vector (which lives outside the JS Heap), the pointer is overwritten and therefore memory lost.

Note: The motivation for using regular memory management rather than the SpiderMonkey one is to make the Taint code portable between SpiderMonkey and the other String representations in Firefox, such as DOMStrings and nsStrings.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions