From b115b2f836a1ddbe5b5c1d865aa32fa03457f985 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 24 Oct 2024 09:09:40 +0200 Subject: [PATCH 1/3] Fix UndefinedBehaviorSanitizer error UBSan is right to complain that `s->ptr_last == NULL` when tracing is disabled. --- quickjs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/quickjs.c b/quickjs.c index 85735785c..892a05cc4 100644 --- a/quickjs.c +++ b/quickjs.c @@ -35019,7 +35019,8 @@ static JSValue JS_ReadFunctionTag(BCReaderState *s) goto fail; if (b->source_len) { bc_read_trace(s, "source: %d bytes\n", b->source_len); - s->ptr_last += b->source_len; // omit source code hex dump + if (s->ptr_last) + s->ptr_last += b->source_len; // omit source code hex dump /* b->source is a UTF-8 encoded null terminated C string */ b->source = js_mallocz(ctx, b->source_len + 1); if (!b->source) From 8b2adcdf8ea599fa884042625bf1cd8e46dc558c Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 24 Oct 2024 09:09:40 +0200 Subject: [PATCH 2/3] Improve deserializer error message for bytecode Don't raise a "invalid tag 12" exception when encountering bytecode and JS_READ_OBJ_BYTECODE is not set, because no one knows what "tag 12" means without looking it up, not even quickjs maintainers. --- quickjs-libc.c | 6 ++++++ quickjs.c | 8 +++++--- tests/test_bjson.js | 21 +++++++++++++++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/quickjs-libc.c b/quickjs-libc.c index e930ff8b5..3dcb4ebcf 100644 --- a/quickjs-libc.c +++ b/quickjs-libc.c @@ -837,6 +837,7 @@ static JSValue js_evalScript(JSContext *ctx, JSValue this_val, JSValue ret; JSValue options_obj; BOOL backtrace_barrier = FALSE; + BOOL compile_only = FALSE; BOOL is_async = FALSE; int flags; @@ -845,6 +846,9 @@ static JSValue js_evalScript(JSContext *ctx, JSValue this_val, if (get_bool_option(ctx, &backtrace_barrier, options_obj, "backtrace_barrier")) return JS_EXCEPTION; + if (get_bool_option(ctx, &compile_only, options_obj, + "compile_only")) + return JS_EXCEPTION; if (get_bool_option(ctx, &is_async, options_obj, "async")) return JS_EXCEPTION; @@ -860,6 +864,8 @@ static JSValue js_evalScript(JSContext *ctx, JSValue this_val, flags = JS_EVAL_TYPE_GLOBAL; if (backtrace_barrier) flags |= JS_EVAL_FLAG_BACKTRACE_BARRIER; + if (compile_only) + flags |= JS_EVAL_FLAG_COMPILE_ONLY; if (is_async) flags |= JS_EVAL_FLAG_ASYNC; ret = JS_Eval(ctx, str, len, "", flags); diff --git a/quickjs.c b/quickjs.c index 892a05cc4..a998afec9 100644 --- a/quickjs.c +++ b/quickjs.c @@ -35473,12 +35473,14 @@ static JSValue JS_ReadObjectRec(BCReaderState *s) break; case BC_TAG_FUNCTION_BYTECODE: if (!s->allow_bytecode) - goto invalid_tag; + goto no_allow_bytecode; obj = JS_ReadFunctionTag(s); break; case BC_TAG_MODULE: - if (!s->allow_bytecode) - goto invalid_tag; + if (!s->allow_bytecode) { + no_allow_bytecode: + return JS_ThrowSyntaxError(ctx, "no bytecode allowed"); + } obj = JS_ReadModule(s); break; case BC_TAG_OBJECT: diff --git a/tests/test_bjson.js b/tests/test_bjson.js index 6a2ee2cbf..989310d28 100644 --- a/tests/test_bjson.js +++ b/tests/test_bjson.js @@ -1,3 +1,4 @@ +import * as std from "std"; import * as bjson from "bjson"; import { assert } from "./assert.js"; @@ -227,6 +228,25 @@ function bjson_test_symbol() assert(o, r); } +function bjson_test_bytecode() +{ + var buf, o, r, e; + + o = std.evalScript(";(function f(o){ return o.i })", {compile_only: true}); + buf = bjson.write(o, /*JS_WRITE_OBJ_BYTECODE*/(1 << 0)); + try { + bjson.read(buf, 0, buf.byteLength); + } catch (_e) { + e = _e; + } + assert(String(e), "SyntaxError: no bytecode allowed"); + + // can't really do anything with |o| at the moment, + // no way to pass it to JS_EvalFunction + o = bjson.read(buf, 0, buf.byteLength, /*JS_READ_OBJ_BYTECODE*/(1 << 0)); + assert(String(o), "[function bytecode]"); +} + function bjson_test_fuzz() { var corpus = [ @@ -277,6 +297,7 @@ function bjson_test_all() bjson_test_map(); bjson_test_set(); bjson_test_symbol(); + bjson_test_bytecode(); bjson_test_fuzz(); } From 553dee301a2bb6663bfea7c4630793cde5dc74f5 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 24 Oct 2024 09:09:40 +0200 Subject: [PATCH 3/3] Handle bytecode without IC state Deserialized bytecode does not have IC state, i.e., `bc->ic == NULL`. That may or may not be bug (IMO, it is and we should rebuild the IC state during deserialization) but, either way, don't segfault. DRY add_ic_slot() and its call sites in a hopefully NFC manner. --- quickjs-libc.c | 23 +++++++++++++++++------ quickjs.c | 32 +++++++++++++++++--------------- tests/test_bjson.js | 6 +++--- 3 files changed, 37 insertions(+), 24 deletions(-) diff --git a/quickjs-libc.c b/quickjs-libc.c index 3dcb4ebcf..ae82f3abc 100644 --- a/quickjs-libc.c +++ b/quickjs-libc.c @@ -832,11 +832,12 @@ static JSValue js_evalScript(JSContext *ctx, JSValue this_val, { JSRuntime *rt = JS_GetRuntime(ctx); JSThreadState *ts = JS_GetRuntimeOpaque(rt); - const char *str; + const char *str = NULL; size_t len; - JSValue ret; + JSValue ret, obj; JSValue options_obj; BOOL backtrace_barrier = FALSE; + BOOL eval_function = FALSE; BOOL compile_only = FALSE; BOOL is_async = FALSE; int flags; @@ -846,6 +847,9 @@ static JSValue js_evalScript(JSContext *ctx, JSValue this_val, if (get_bool_option(ctx, &backtrace_barrier, options_obj, "backtrace_barrier")) return JS_EXCEPTION; + if (get_bool_option(ctx, &eval_function, options_obj, + "eval_function")) + return JS_EXCEPTION; if (get_bool_option(ctx, &compile_only, options_obj, "compile_only")) return JS_EXCEPTION; @@ -854,9 +858,11 @@ static JSValue js_evalScript(JSContext *ctx, JSValue this_val, return JS_EXCEPTION; } - str = JS_ToCStringLen(ctx, &len, argv[0]); - if (!str) - return JS_EXCEPTION; + if (!eval_function) { + str = JS_ToCStringLen(ctx, &len, argv[0]); + if (!str) + return JS_EXCEPTION; + } if (!ts->recv_pipe && ++ts->eval_script_recurse == 1) { /* install the interrupt handler */ JS_SetInterruptHandler(JS_GetRuntime(ctx), interrupt_handler, NULL); @@ -868,7 +874,12 @@ static JSValue js_evalScript(JSContext *ctx, JSValue this_val, flags |= JS_EVAL_FLAG_COMPILE_ONLY; if (is_async) flags |= JS_EVAL_FLAG_ASYNC; - ret = JS_Eval(ctx, str, len, "", flags); + if (eval_function) { + obj = JS_DupValue(ctx, argv[0]); + ret = JS_EvalFunction(ctx, obj); // takes ownership of |obj| + } else { + ret = JS_Eval(ctx, str, len, "", flags); + } JS_FreeCString(ctx, str); if (!ts->recv_pipe && --ts->eval_script_recurse == 0) { /* remove the interrupt handler */ diff --git a/quickjs.c b/quickjs.c index a998afec9..ce6d85a35 100644 --- a/quickjs.c +++ b/quickjs.c @@ -7362,9 +7362,8 @@ static JSValue JS_GetPropertyInternal2(JSContext *ctx, JSValue obj, continue; } } else { - if (icu && proto_depth == 0 && p->shape->is_hashed) { + if (proto_depth == 0) add_ic_slot(ctx, icu, prop, p, offset); - } return js_dup(pr->u.value); } } @@ -8658,9 +8657,7 @@ static int JS_SetPropertyInternal2(JSContext *ctx, JSValue obj, JSAtom prop, if (likely((prs->flags & (JS_PROP_TMASK | JS_PROP_WRITABLE | JS_PROP_LENGTH)) == JS_PROP_WRITABLE)) { /* fast case */ - if (icu && p->shape->is_hashed) { - add_ic_slot(ctx, icu, prop, p, offset); - } + add_ic_slot(ctx, icu, prop, p, offset); set_value(ctx, &pr->u.value, val); return TRUE; } else if (prs->flags & JS_PROP_LENGTH) { @@ -54516,10 +54513,19 @@ static void add_ic_slot(JSContext *ctx, JSInlineCacheUpdate *icu, { int32_t i; uint32_t h; - JSInlineCache *ic = icu->ic; JSInlineCacheHashSlot *ch; JSInlineCacheRingSlot *cr; + JSInlineCache *ic; JSShape *sh; + + if (!icu) + return; + ic = icu->ic; + if (!ic) + return; + sh = object->shape; + if (!sh->is_hashed) + return; cr = NULL; h = get_index_hash(atom, ic->hash_bits); for (ch = ic->hash[h]; ch != NULL; ch = ch->next) { @@ -54528,21 +54534,17 @@ static void add_ic_slot(JSContext *ctx, JSInlineCacheUpdate *icu, break; } } - assert(cr != NULL); i = cr->index; - for (;;) { - if (object->shape == cr->shape[i]) { + do { + if (sh == cr->shape[i]) { cr->prop_offset[i] = prop_offset; goto end; } i = (i + 1) % countof(cr->shape); - if (unlikely(i == cr->index)) - break; - } - sh = cr->shape[i]; - cr->shape[i] = js_dup_shape(object->shape); - js_free_shape_null(ctx->rt, sh); + } while (i != cr->index); + js_free_shape_null(ctx->rt, cr->shape[i]); + cr->shape[i] = js_dup_shape(sh); cr->prop_offset[i] = prop_offset; end: icu->offset = ch->index; diff --git a/tests/test_bjson.js b/tests/test_bjson.js index 989310d28..a957ceeff 100644 --- a/tests/test_bjson.js +++ b/tests/test_bjson.js @@ -230,7 +230,7 @@ function bjson_test_symbol() function bjson_test_bytecode() { - var buf, o, r, e; + var buf, o, r, e, i; o = std.evalScript(";(function f(o){ return o.i })", {compile_only: true}); buf = bjson.write(o, /*JS_WRITE_OBJ_BYTECODE*/(1 << 0)); @@ -241,10 +241,10 @@ function bjson_test_bytecode() } assert(String(e), "SyntaxError: no bytecode allowed"); - // can't really do anything with |o| at the moment, - // no way to pass it to JS_EvalFunction o = bjson.read(buf, 0, buf.byteLength, /*JS_READ_OBJ_BYTECODE*/(1 << 0)); assert(String(o), "[function bytecode]"); + o = std.evalScript(o, {eval_function: true}); + for (i = 0; i < 42; i++) o({i}); // exercise o.i IC } function bjson_test_fuzz()