Skip to content

Commit

Permalink
Check global uses more strictly (#2632)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: Use of global variables (in the Wasm sense) is now checked more strictly to prevent undesirable execution order. If the compiler detects that it is possible that a variable might not have been initialized when accessed, a diagnostic is produced. It cannot be ruled out that some amount of existing code will be affected, since such checks are performed at runtime in JS but are proven at compile time in AS. If encountered, the fix is to move the variable's declaration up, say before the first invocation of a function (that might call another function) accessing the variable, so it is guaranteed that it is initialized before its first use.
  • Loading branch information
dcodeIO committed Jan 31, 2023
1 parent 941b0e1 commit 5cbbf84
Show file tree
Hide file tree
Showing 12 changed files with 4,267 additions and 137 deletions.
5 changes: 3 additions & 2 deletions src/builtins.ts
Expand Up @@ -787,6 +787,9 @@ export const builtinVariables_onAccess = new Map<string, (ctx: BuiltinVariableCo

// === Static type evaluation =================================================================

// helper global used by checkConstantType
let checkConstantType_expr: ExpressionRef = 0;

// isBoolean<T!>() / isBoolean<T?>(value: T) -> bool
function builtin_isBoolean(ctx: BuiltinFunctionContext): ExpressionRef {
let compiler = ctx.compiler;
Expand Down Expand Up @@ -10620,8 +10623,6 @@ export function compileRTTI(compiler: Compiler): void {

// Helpers

let checkConstantType_expr: ExpressionRef = 0;

/** Checks the constant type of a type argument *or* expression. */
function checkConstantType(ctx: BuiltinFunctionContext): Type | null {
let compiler = ctx.compiler;
Expand Down
39 changes: 30 additions & 9 deletions src/compiler.ts
Expand Up @@ -3,6 +3,10 @@
* @license Apache-2.0
*/

// helper globals used by mangleImportName
let mangleImportName_moduleName: string = "";
let mangleImportName_elementName: string = "";

import {
BuiltinNames,
BuiltinFunctionContext,
Expand Down Expand Up @@ -1097,6 +1101,20 @@ export class Compiler extends DiagnosticEmitter {

// === Globals ==================================================================================

/** Tries to compile a global variable lazily. */
compileGlobalLazy(global: Global, reportNode: Node): bool {
if (global.is(CommonFlags.Compiled)) return !global.is(CommonFlags.Errored);
if (global.hasAnyDecorator(DecoratorFlags.Lazy | DecoratorFlags.Builtin) || global.is(CommonFlags.Ambient)) {
return this.compileGlobal(global); // compile now
}
// Otherwise the global is used before its initializer executes
this.errorRelated(
DiagnosticCode.Variable_0_used_before_its_declaration,
reportNode.range, global.identifierNode.range, global.internalName
);
return false;
}

/** Compiles a global variable. */
compileGlobal(global: Global): bool {
if (global.is(CommonFlags.Compiled)) return !global.is(CommonFlags.Errored);
Expand Down Expand Up @@ -5548,8 +5566,9 @@ export class Compiler extends DiagnosticEmitter {
let targetType: Type;
switch (target.kind) {
case ElementKind.Global: {
// not yet compiled if a static field compiled as a global
if (!this.compileGlobal(<Global>target)) return this.module.unreachable(); // reports
if (!this.compileGlobalLazy(<Global>target, expression)) {
return this.module.unreachable();
}
// fall-through
}
case ElementKind.Local: {
Expand Down Expand Up @@ -5691,7 +5710,9 @@ export class Compiler extends DiagnosticEmitter {
}
case ElementKind.Global: {
let global = <Global>target;
if (!this.compileGlobal(global)) return module.unreachable();
if (!this.compileGlobalLazy(global, valueExpression)) {
return module.unreachable();
}
if (target.isAny(CommonFlags.Const | CommonFlags.Readonly)) {
this.error(
DiagnosticCode.Cannot_assign_to_0_because_it_is_a_constant_or_a_read_only_property,
Expand Down Expand Up @@ -6766,7 +6787,7 @@ export class Compiler extends DiagnosticEmitter {
let resolved = this.resolver.lookupExpression(initializer, instance.flow, parameterTypes[i], ReportMode.Swallow);
if (resolved && resolved.kind == ElementKind.Global) {
let global = <Global>resolved;
if (this.compileGlobal(global) && global.is(CommonFlags.Inlined)) {
if (this.compileGlobalLazy(global, initializer) && global.is(CommonFlags.Inlined)) {
operands.push(
this.compileInlineConstant(global, parameterTypes[i], Constraints.ConvImplicit)
);
Expand Down Expand Up @@ -7329,7 +7350,7 @@ export class Compiler extends DiagnosticEmitter {
}
case ElementKind.Global: {
let global = <Global>target;
if (!this.compileGlobal(global)) { // reports; not yet compiled if a static field
if (!this.compileGlobalLazy(global, expression)) {
return module.unreachable();
}
let globalType = global.type;
Expand Down Expand Up @@ -8895,7 +8916,9 @@ export class Compiler extends DiagnosticEmitter {
switch (target.kind) {
case ElementKind.Global: { // static field
let global = <Global>target;
if (!this.compileGlobal(global)) return module.unreachable(); // reports
if (!this.compileGlobalLazy(global, expression)) {
return module.unreachable();
}
let globalType = global.type;
assert(globalType != Type.void);
if (this.pendingElements.has(global)) {
Expand Down Expand Up @@ -10388,6 +10411,7 @@ export class Compiler extends DiagnosticEmitter {
}

// helpers

function mangleImportName(
element: Element,
declaration: DeclarationStatement
Expand Down Expand Up @@ -10444,6 +10468,3 @@ function mangleImportName(
);
}
}

let mangleImportName_moduleName: string = "";
let mangleImportName_elementName: string = "";
4 changes: 2 additions & 2 deletions src/program.ts
Expand Up @@ -1782,7 +1782,7 @@ export class Program extends DiagnosticEmitter {
let global = new Global(
name,
this.nativeFile,
DecoratorFlags.None,
DecoratorFlags.Lazy,
this.makeNativeVariableDeclaration(name, CommonFlags.Const | CommonFlags.Export)
);
global.setConstantIntegerValue(value, type);
Expand All @@ -1795,7 +1795,7 @@ export class Program extends DiagnosticEmitter {
let global = new Global(
name,
this.nativeFile,
DecoratorFlags.None,
DecoratorFlags.Lazy,
this.makeNativeVariableDeclaration(name, CommonFlags.Const | CommonFlags.Export)
);
global.setConstantFloatValue(value, type);
Expand Down

0 comments on commit 5cbbf84

Please sign in to comment.