Skip to content

Commit b750406

Browse files
svaaralasvaarala
authored
Merge pull request svaarala#1132 from svaarala/perf-explicit-lexenv-varenv
Add explicit lexenv/varenv fields to duk_hcompfunc
2 parents 57af66a + b77a85c commit b750406

30 files changed

Lines changed: 355 additions & 327 deletions

‎RELEASES.rst‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2008,6 +2008,11 @@ Planned
20082008

20092009
* Add an fmod() self test (GH-1108)
20102010

2011+
* Fix duk_hcompfunc 'data' field != NULL assumptions which might lead to
2012+
memory unsafe behavior if Duktape ran out of memory when creating a
2013+
duk_hcompfunc during compilation or function instantiation (GH-1144,
2014+
GH-1132)
2015+
20112016
* Fix a few bugs in object property handling (delete property and
20122017
Object.defineProperty()) where an object property table resize triggered
20132018
by a finalizer of a previous value could cause memory unsafe behavior
@@ -2117,15 +2122,16 @@ Planned
21172122
GH-973, GH-1042); minor RegExp compile/execute optimizations (GH-974,
21182123
GH-1033); minor IEEE double handling optimizations (GH-1051); precomputed
21192124
duk_hstring array index (GH-1056); duk_get_{type,type_mask}() optimization
2120-
(GH-1077)
2125+
(GH-1077); explicit lexenv/varenv fields in duk_hcompfunc struct (GH-1132)
21212126

21222127
* Miscellaneous footprint improvements: RegExp compiler/executor (GH-977);
21232128
internal duk_dup() variants (GH-990); allow stripping of (almost) all
21242129
built-ins for low memory builds (GH-989); remove internal accessor setup
21252130
helper and use duk_def_prop() instead (GH-1010); minor IEEE double handling
21262131
optimizations (GH-1051); precomputed duk_hstring array index (GH-1056);
21272132
internal value stack access improvements (GH-1058); shared bitpacked string
2128-
format for heap and thread initialization data (GH-1119)
2133+
format for heap and thread initialization data (GH-1119); explicit
2134+
lexenv/varenv fields in duk_hcompfunc struct (GH-1132)
21292135

21302136
* Internal change: rework shared internal string handling so that shared
21312137
strings are plain string constants used in macro values, rather than

‎debugger/duk_debug_proxy.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -937,7 +937,7 @@ TargetConnHandler.prototype.trialParseDvalue = function trialParseDvalue() {
937937
if (avail >= 2 + len) {
938938
v = new Uint8Array(len);
939939
v.set(buf.subarray(2, 2 + len));
940-
v = { type: 'heapptr', pointer: Duktape.enc('hex', plainof(v)) };
940+
v = { type: 'heapptr', pointer: Duktape.enc('hex', plainOf(v)) };
941941
consume(2 + len);
942942
}
943943
}

‎doc/debugger.rst‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2433,6 +2433,10 @@ The following list describes artificial keys included in Duktape 1.5.0, see
24332433
+---------------------------------+---------------------------+---------------------------------------------------------+
24342434
| (not present yet) | ``duk_hcompfunc`` | Ecmascript function data area, including bytecode. |
24352435
+---------------------------------+---------------------------+---------------------------------------------------------+
2436+
| ``lex_env`` | ``duk_hcompfunc`` | Function lexical environment. |
2437+
+---------------------------------+---------------------------+---------------------------------------------------------+
2438+
| ``var_env`` | ``duk_hcompfunc`` | Function variable environment. |
2439+
+---------------------------------+---------------------------+---------------------------------------------------------+
24362440
| ``nregs`` | ``duk_hcompfunc`` | Number of bytecode executor registers. |
24372441
+---------------------------------+---------------------------+---------------------------------------------------------+
24382442
| ``nargs`` | ``duk_hcompfunc`` | Number of stack arguments. |

‎doc/function-objects.rst‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -251,21 +251,6 @@ user documentation for the exposed parts):
251251
+---------------+---------------------------------------------------------+
252252
| ``fileName`` | See function templates. |
253253
+---------------+---------------------------------------------------------+
254-
| ``_Lexenv`` | Together with DUK_HOBJECT_FLAG_NEWENV, controls the |
255-
| | initialization of variable/lexical environment when a |
256-
| | function call occurs. |
257-
| | |
258-
| | The DUK_HOBJECT_FLAG_NEWENV is set for ordinary |
259-
| | functions, which always get a new environment record |
260-
| | when they are called. The flag is cleared for global |
261-
| | code and eval code, which "share" an existing |
262-
| | environment record. |
263-
| | |
264-
| | If _Varenv is missing, it defaults to _Lexenv (which is |
265-
| | very often the case). |
266-
+---------------+---------------------------------------------------------+
267-
| ``_Varenv`` | See ``_Lexenv``. |
268-
+---------------+---------------------------------------------------------+
269254
| ``_Varmap`` | See function templates. |
270255
+---------------+---------------------------------------------------------+
271256
| ``_Formals`` | See function templates. |

‎doc/hobject-design.rst‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1643,9 +1643,10 @@ double brackets are omitted from the specification property names
16431643
| HasInstance | Not stored, implicit in algorithms. |
16441644
| | |
16451645
+-------------------+------------------------------------------------------+
1646-
| Scope | Internal properties ``_Lexenv`` and ``_Varenv``. |
1647-
| | (Unlike E5, global and eval code are also compiled |
1648-
| | into functions, hence two scope fields are needed.) |
1646+
| Scope | Internal ``duk_hcompfunc`` fields ``lex_env`` and |
1647+
| | ``var_env``. Unlike E5, global and eval code are |
1648+
| | also compiled into functions, hence two scope fields |
1649+
| | are needed.) |
16491650
+-------------------+------------------------------------------------------+
16501651
| FormalParameters | Internal property ``_Formals``. |
16511652
| | |

‎doc/identifier-handling.rst‎

Lines changed: 2 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -590,8 +590,8 @@ detailed properties vary a bit between the two.
590590

591591
More concretely:
592592

593-
* The ``DUK_HOBJECT_FLAG_NEWENV`` object level flag, and the internal
594-
properties ``_Lexenv`` and ``_Varenv`` control activation record
593+
* The ``DUK_HOBJECT_FLAG_NEWENV`` object level flag, and the ``lex_env``
594+
and ``var_env`` fields of ``duk_hcompfunc`` control activation record
595595
lexical and variable environment initialization as described below.
596596

597597
* The internal property ``_Varmap`` contains a mapping from an
@@ -607,35 +607,6 @@ More concretely:
607607
name (stored in ``name``) needs to be bound in an environment record just
608608
outside a function activation's environment record.
609609

610-
To minimize book-keeping in common cases, the following short cuts
611-
are supported:
612-
613-
* If both scope references are missing:
614-
615-
+ Assume that the function has an empty declarative environment record,
616-
whose parent is the global environment record.
617-
618-
+ For variable lookups this means that we proceed directly to the global
619-
environment record.
620-
621-
+ For variable declarations this means that a declarative environment
622-
record needs to be created on demand.
623-
624-
* If ``_Varenv`` is missing:
625-
626-
+ Assume that ``_Varenv`` has the same value as ``_Lexenv``.
627-
628-
+ This is very common, and saves one (unnecessary) reference.
629-
630-
+ Note: it would be more logical to allow ``_Lexenv`` to be missing
631-
and default it to ``_Varenv``; however, dynamic variable
632-
declarations are comparatively rare so the defaulting is more
633-
useful this way around
634-
635-
* If ``_Varmap`` is missing:
636-
637-
+ Assume that the function has no register-mapped variables.
638-
639610
* The compiler attempts to drop any fields not required from compiled
640611
objects. In many common cases (even when dynamic variables accesses
641612
cannot be ruled out) no control fields are required.
@@ -652,11 +623,6 @@ Notes:
652623
needed (e.g. for a function declaration). It is not created
653624
unnecessarily when a function is called.
654625

655-
* The default behavior for ``_Lexenv`` and ``_Varenv`` allows them to
656-
be omitted in a large number of cases (for instance, many functions
657-
are declared in the global scope, and for many compiled eval
658-
functions the values are the same).
659-
660626
* The ``DUK_HOBJECT_FLAG_NEWENV`` is set for ordinary functions, which
661627
always get a new environment record for variable declaration and
662628
lookup. The flag is cleared for global code and eval code; or rather

‎src-input/duk_api_bytecode.c‎

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,8 @@ static duk_uint8_t *duk__dump_func(duk_context *ctx, duk_hcompfunc *func, duk_bu
335335
fn++;
336336
}
337337

338+
/* Lexenv and varenv are not dumped. */
339+
338340
/* Object extra properties.
339341
*
340342
* There are some difference between function templates and functions.
@@ -382,6 +384,8 @@ static duk_uint8_t *duk__load_func(duk_context *ctx, duk_uint8_t *p, duk_uint8_t
382384
duk_idx_t idx_base;
383385
duk_tval *tv1;
384386
duk_uarridx_t arr_idx;
387+
duk_hobject *func_env;
388+
duk_bool_t need_pop;
385389

386390
/* XXX: There's some overlap with duk_js_closure() here, but
387391
* seems difficult to share code. Ensure that the final function
@@ -416,8 +420,8 @@ static duk_uint8_t *duk__load_func(duk_context *ctx, duk_uint8_t *p, duk_uint8_t
416420
/* Push function object, init flags etc. This must match
417421
* duk_js_push_closure() quite carefully.
418422
*/
419-
duk_push_compiledfunction(ctx);
420-
h_fun = duk_known_hcompfunc(ctx, -1);
423+
h_fun = duk_push_compiledfunction(ctx);
424+
DUK_ASSERT(h_fun != NULL);
421425
DUK_ASSERT(DUK_HOBJECT_IS_COMPFUNC((duk_hobject *) h_fun));
422426
DUK_ASSERT(DUK_HCOMPFUNC_GET_DATA(thr->heap, h_fun) == NULL);
423427
DUK_ASSERT(DUK_HCOMPFUNC_GET_FUNCS(thr->heap, h_fun) == NULL);
@@ -560,26 +564,37 @@ static duk_uint8_t *duk__load_func(duk_context *ctx, duk_uint8_t *p, duk_uint8_t
560564
duk_push_u32(ctx, tmp32);
561565
duk_xdef_prop_stridx(ctx, -2, DUK_STRIDX_LENGTH, DUK_PROPDESC_FLAGS_NONE);
562566

563-
p = duk__load_string_raw(ctx, p);
567+
p = duk__load_string_raw(ctx, p); /* -> [ func funcname ] */
568+
func_env = thr->builtins[DUK_BIDX_GLOBAL_ENV];
569+
DUK_ASSERT(func_env != NULL);
570+
need_pop = 0;
564571
if (DUK_HOBJECT_HAS_NAMEBINDING((duk_hobject *) h_fun)) {
565572
/* Original function instance/template had NAMEBINDING.
566573
* Must create a lexical environment on loading to allow
567574
* recursive functions like 'function foo() { foo(); }'.
568575
*/
569-
duk_hobject *proto;
576+
duk_hobject *new_env;
577+
578+
new_env = duk_push_object_helper_proto(ctx,
579+
DUK_HOBJECT_FLAG_EXTENSIBLE |
580+
DUK_HOBJECT_CLASS_AS_FLAGS(DUK_HOBJECT_CLASS_DECENV),
581+
func_env);
582+
DUK_ASSERT(new_env != NULL);
583+
func_env = new_env;
570584

571-
proto = thr->builtins[DUK_BIDX_GLOBAL_ENV];
572-
(void) duk_push_object_helper_proto(ctx,
573-
DUK_HOBJECT_FLAG_EXTENSIBLE |
574-
DUK_HOBJECT_CLASS_AS_FLAGS(DUK_HOBJECT_CLASS_DECENV),
575-
proto);
576585
duk_dup_m2(ctx); /* -> [ func funcname env funcname ] */
577586
duk_dup(ctx, idx_base); /* -> [ func funcname env funcname func ] */
578587
duk_xdef_prop(ctx, -3, DUK_PROPDESC_FLAGS_NONE); /* -> [ func funcname env ] */
579-
duk_xdef_prop_stridx(ctx, idx_base, DUK_STRIDX_INT_LEXENV, DUK_PROPDESC_FLAGS_WC);
580-
/* since closure has NEWENV, never define DUK_STRIDX_INT_VARENV, as it
581-
* will be ignored anyway
582-
*/
588+
589+
need_pop = 1; /* Need to pop env, but -after- updating h_fun and increfs. */
590+
}
591+
DUK_ASSERT(func_env != NULL);
592+
DUK_HCOMPFUNC_SET_LEXENV(thr->heap, h_fun, func_env);
593+
DUK_HCOMPFUNC_SET_VARENV(thr->heap, h_fun, func_env);
594+
DUK_HOBJECT_INCREF(thr, func_env);
595+
DUK_HOBJECT_INCREF(thr, func_env);
596+
if (need_pop) {
597+
duk_pop(ctx);
583598
}
584599
duk_xdef_prop_stridx(ctx, -2, DUK_STRIDX_NAME, DUK_PROPDESC_FLAGS_NONE);
585600

‎src-input/duk_api_heap.c‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -161,23 +161,19 @@ DUK_EXTERNAL void duk_set_global_object(duk_context *ctx) {
161161
* same (initial) built-ins.
162162
*/
163163

164-
(void) duk_push_object_helper(ctx,
165-
DUK_HOBJECT_FLAG_EXTENSIBLE |
166-
DUK_HOBJECT_CLASS_AS_FLAGS(DUK_HOBJECT_CLASS_OBJENV),
167-
-1); /* no prototype, updated below */
164+
h_env = duk_push_object_helper(ctx,
165+
DUK_HOBJECT_FLAG_EXTENSIBLE |
166+
DUK_HOBJECT_CLASS_AS_FLAGS(DUK_HOBJECT_CLASS_OBJENV),
167+
-1); /* no prototype, updated below */
168+
DUK_ASSERT(h_env != NULL);
168169

169170
duk_dup_m2(ctx);
170171
duk_dup_m3(ctx);
171-
172-
/* [ ... new_glob new_env new_glob new_glob ] */
173-
174172
duk_xdef_prop_stridx(thr, -3, DUK_STRIDX_INT_TARGET, DUK_PROPDESC_FLAGS_NONE);
175173
duk_xdef_prop_stridx(thr, -2, DUK_STRIDX_INT_THIS, DUK_PROPDESC_FLAGS_NONE);
176174

177175
/* [ ... new_glob new_env ] */
178176

179-
h_env = duk_known_hobject(ctx, -1);
180-
181177
h_prev_env = thr->builtins[DUK_BIDX_GLOBAL_ENV];
182178
thr->builtins[DUK_BIDX_GLOBAL_ENV] = h_env;
183179
DUK_HOBJECT_INCREF(thr, h_env);

‎src-input/duk_api_internal.h‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,9 @@ DUK_INTERNAL_DECL void duk_push_hbuffer(duk_context *ctx, duk_hbuffer *h);
152152
#define duk_push_hnatfunc(ctx,h) \
153153
duk_push_hobject((ctx), (duk_hobject *) (h))
154154
DUK_INTERNAL_DECL void duk_push_hobject_bidx(duk_context *ctx, duk_small_int_t builtin_idx);
155-
DUK_INTERNAL_DECL duk_idx_t duk_push_object_helper(duk_context *ctx, duk_uint_t hobject_flags_and_class, duk_small_int_t prototype_bidx);
156-
DUK_INTERNAL_DECL duk_idx_t duk_push_object_helper_proto(duk_context *ctx, duk_uint_t hobject_flags_and_class, duk_hobject *proto);
157-
DUK_INTERNAL_DECL duk_idx_t duk_push_compiledfunction(duk_context *ctx);
155+
DUK_INTERNAL_DECL duk_hobject *duk_push_object_helper(duk_context *ctx, duk_uint_t hobject_flags_and_class, duk_small_int_t prototype_bidx);
156+
DUK_INTERNAL_DECL duk_hobject *duk_push_object_helper_proto(duk_context *ctx, duk_uint_t hobject_flags_and_class, duk_hobject *proto);
157+
DUK_INTERNAL_DECL duk_hcompfunc *duk_push_compiledfunction(duk_context *ctx);
158158
DUK_INTERNAL_DECL void duk_push_c_function_noexotic(duk_context *ctx, duk_c_function func, duk_int_t nargs);
159159
DUK_INTERNAL_DECL void duk_push_c_function_noconstruct_noexotic(duk_context *ctx, duk_c_function func, duk_int_t nargs);
160160

@@ -211,6 +211,8 @@ DUK_INTERNAL_DECL void duk_require_constructor_call(duk_context *ctx);
211211

212212
DUK_INTERNAL_DECL void duk_require_constructable(duk_context *ctx, duk_idx_t idx);
213213

214+
DUK_INTERNAL_DECL duk_idx_t duk_get_top_index_unsafe(duk_context *ctx);
215+
214216
/* Raw internal valstack access macros: access is unsafe so call site
215217
* must have a guarantee that the index is valid. When that is the case,
216218
* using these macro results in faster and smaller code than duk_get_tval().

0 commit comments

Comments
 (0)