Assertion `!atom_is_free(p)' with -DDUMP_FREE=1

[bnoordhuis]

Moved over from bnoordhuis/quickjit#1. Only seems to happen with string-tagcloud.js from sunspider.

a.out: quickjs.c:3050: JS_AtomGetStrRT: Assertion `!atom_is_free(p)' failed.

Backtrace looks like this:

quickjs-ng/quickjs#1  0x00007ffff7c33b66 in __assert_fail (assertion=0x555555606aa5 "!atom_is_free(p)", file=0x555555606814 "quickjs.c", line=3050, 
    function=0x55555560c500 <__PRETTY_FUNCTION__.52> "JS_AtomGetStrRT") at ./assert/assert.c:101
quickjs-ng/quickjs-tmp#7  0x00005555555636b9 in JS_AtomGetStrRT (rt=0x55555662b480, buf=0x7fffffffd9e0 "\300\036dVUU", buf_size=64, atom=479) at quickjs.c:3050
quickjs-ng/quickjs-tmp#8  0x000055555557e74d in JS_DumpValueShort (rt=0x55555662b480, val=...) at quickjs.c:11925
#9  0x000055555556a43b in __JS_FreeValueRT (rt=0x55555662b480, v=...) at quickjs.c:5485
#10 0x000055555555f299 in JS_FreeValueRT (rt=0x55555662b480, v=...) at /home/bnoordhuis/src/quickjit/quickjs.h:658
#11 0x0000555555569d5d in js_bytecode_function_finalizer (rt=0x55555662b480, val=...) at quickjs.c:5322
#12 0x000055555556a23c in free_object (rt=0x55555662b480, p=0x555556648550) at quickjs.c:5427
#13 0x000055555556a30f in free_gc_object (rt=0x55555662b480, gp=0x555556648550) at quickjs.c:5447
#14 0x000055555556aedf in gc_free_cycles (rt=0x55555662b480) at quickjs.c:5777
#15 0x000055555556b014 in JS_RunGC (rt=0x55555662b480) at quickjs.c:5807
#16 0x00005555555608b6 in JS_FreeRuntime (rt=0x55555662b480) at quickjs.c:1942

And also:

(gdb) f 7
quickjs-ng/quickjs-tmp#7  0x00005555555636b9 in JS_AtomGetStrRT (rt=0x55555662b480, buf=0x7fffffffd9e0 "\300\036dVUU", buf_size=64, atom=479) at quickjs.c:3050
3050                assert(!atom_is_free(p));
(gdb) p *p
Cannot access memory at address 0x1c5

Note to self: atom=479 with the free flag removed is 479 >> 1 == 239 but rt->atom_array[239] doesn't contain a valid entry either.

Interestingly:

(gdb) p rt->atom_free_index 
$1 = 479

---

It's a use-after-free in the runtime teardown code, i=479:

JS_FreeAtomStruct (rt=0x55555662b480, p=0x555556641ec0) at quickjs.c:2919
2919        rt->atom_array[i] = atom_set_free(rt->atom_free_index);
(gdb) bt
#0  JS_FreeAtomStruct (rt=0x55555662b480, p=0x555556641ec0) at quickjs.c:2919
quickjs-ng/quickjs-tmp#1  0x000055555556a4ce in __JS_FreeValueRT (rt=0x55555662b480, v=...) at quickjs.c:5498
quickjs-ng/quickjs-tmp#2  0x000055555555f299 in JS_FreeValueRT (rt=0x55555662b480, v=...) at /home/bnoordhuis/src/quickjit/quickjs.h:658
quickjs-ng/quickjs-tmp#3  0x000055555556992d in free_property (rt=0x55555662b480, pr=0x555556638510, prop_flags=1) at quickjs.c:5183
quickjs-ng/quickjs-tmp#4  0x000055555556a193 in free_object (rt=0x55555662b480, p=0x555556648550) at quickjs.c:5411
quickjs-ng/quickjs#2  0x000055555556a340 in free_gc_object (rt=0x55555662b480, gp=0x555556648550) at quickjs.c:5449
quickjs-ng/quickjs#1  0x000055555556af10 in gc_free_cycles (rt=0x55555662b480) at quickjs.c:5779
quickjs-ng/quickjs-tmp#7  0x000055555556b045 in JS_RunGC (rt=0x55555662b480) at quickjs.c:5809
quickjs-ng/quickjs-tmp#8  0x00005555555608b6 in JS_FreeRuntime (rt=0x55555662b480) at quickjs.c:1942
#9  0x000055555555eb78 in main (argc=2, argv=0x7fffffffde58) at t.c:62

A short while later it tries to print the function name of the bytecode object but that's the atom that was freed.

[bnoordhuis]

It's not just string-tagcloud.js, I also hit it with make qjs:

qjsc: quickjs.c:2852: JS_AtomGetStrRT: Assertion `!atom_is_free(p)' failed.

[bnoordhuis]

Interestingly, I can no longer reproduce with string-tagcloud.js (or sunspider in general) but build/qjsc -o build/repl.c repl.js triggers the assert reliably.

[bnoordhuis]

I observe that b->vardefs and b->cpool contain elements that point to the same atom, and that free_function_bytecode() first frees b->vardefs, then b->cpool. The second one hits the asserts.

Specifically, it's the symcmp atom of the get_completions function. There's only one element in its cpool.

My hunch is that something somewhere forgets to call JS_DupAtom() but hard to pin down where.

[saghul]

There is a mode to debug refcounts, maybe that helps? It's commented in quickjs.h

[saghul]

This one: CONFIG_CHECK_JSVALUE -- I've never tried it though...

[saghul]

Fixed in #350

[RReverser]

I'm still running into the same assertion failure. I only did a quick search through the code, but this conditional assert does seems clearly wrong?

        if (atom_is_free(p)) {
            assert(!atom_is_free(p));

https://github.com/quickjs-ng/quickjs/blob/0191aea691e0ae37c35bc6737e73760506876148/quickjs.c#L3217-L3218

Looks like it was added by the PR #350 linked above (cc @chqrlie).

[bnoordhuis]

That does look blatantly wrong. Fix in #1117.

[RReverser]

@bnoordhuis Thanks!