Skip to content

Commit 77e4e73

Browse files
CountBleckCountBleck
committed
Refrain from setting array indices outside the array length
The code `array[i] = foo` where `i >= array.length` does, for obvious reasons, not work inside unchecked contexts. Code like this was the cause of crashes when compiling with `--uncheckedBehavior always`. In my opinion, this practice is sloppy, although my workarounds can be considered equally sloppy.
1 parent c41d12e commit 77e4e73

5 files changed

Lines changed: 32 additions & 19 deletions

File tree

‎src/builtins.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10329,17 +10329,17 @@ export function compileVisitMembers(compiler: Compiler): void {
1032910329
let instanceId = _keys[i];
1033010330
assert(instanceId == nextId++);
1033110331
let instance = assert(managedClasses.get(instanceId));
10332-
names[i] = instance.internalName;
10332+
names.push(instance.internalName);
1033310333
if (instance.isPointerfree) {
10334-
cases[i] = module.return();
10334+
cases.push(module.return());
1033510335
} else {
10336-
cases[i] = module.block(null, [
10336+
cases.push(module.block(null, [
1033710337
module.call(`${instance.internalName}~visit`, [
1033810338
module.local_get(0, sizeTypeRef), // this
1033910339
module.local_get(1, TypeRef.I32) // cookie
1034010340
], TypeRef.None),
1034110341
module.return()
10342-
], TypeRef.None);
10342+
], TypeRef.None));
1034310343
ensureVisitMembersOf(compiler, instance);
1034410344
}
1034510345
}

‎src/compiler.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1695,8 +1695,8 @@ export class Compiler extends DiagnosticEmitter {
16951695
this.makeFieldInitializationInConstructor(classInstance, allocStmts);
16961696

16971697
// Insert right before the body
1698-
for (let i = stmts.length - 1; i >= bodyStartIndex; --i) {
1699-
stmts[i + 1] = stmts[i];
1698+
for (let i = stmts.length++; i > bodyStartIndex; --i) {
1699+
stmts[i] = stmts[i - 1];
17001700
}
17011701
stmts[bodyStartIndex] = module.flatten(allocStmts, TypeRef.None);
17021702

@@ -2800,7 +2800,7 @@ export class Compiler extends DiagnosticEmitter {
28002800
let tempLocalIndex = tempLocal.index;
28012801

28022802
// Prepend initializer to inner block. Does not initiate a new branch, yet.
2803-
let breaks = new Array<ExpressionRef>(1 + numCases);
2803+
let breaks = new Array<ExpressionRef>(2 + numCases);
28042804
breaks[0] = module.local_set( // initializer
28052805
tempLocalIndex,
28062806
this.compileExpression(statement.condition, Type.u32,
@@ -2834,6 +2834,7 @@ export class Compiler extends DiagnosticEmitter {
28342834
? `case${defaultIndex}|${context}`
28352835
: `break|${context}`
28362836
);
2837+
breaks.length = breakIndex + 1;
28372838

28382839
// nest blocks in order
28392840
let currentBlock = module.block(`case0|${context}`, breaks, TypeRef.None);
@@ -6422,7 +6423,7 @@ export class Compiler extends DiagnosticEmitter {
64226423
}
64236424
let numOptional = assert(maxOperands - minOperands);
64246425

6425-
let forwardedOperands = new Array<ExpressionRef>(minOperands);
6426+
let forwardedOperands = new Array<ExpressionRef>(maxOperands);
64266427
let operandIndex = 0;
64276428
let stmts = new Array<ExpressionRef>();
64286429

@@ -6646,7 +6647,7 @@ export class Compiler extends DiagnosticEmitter {
66466647
let body: ExpressionRef;
66476648
let instanceClass = instance.getBoundClassOrInterface();
66486649
if (!instance.is(CommonFlags.Abstract) && !(instanceClass && instanceClass.kind == ElementKind.Interface)) {
6649-
let paramExprs = new Array<ExpressionRef>(numParameters);
6650+
let paramExprs = new Array<ExpressionRef>(numParameters + 1);
66506651
paramExprs[0] = module.local_get(0, sizeTypeRef); // this
66516652
for (let i = 0, k = parameterTypes.length; i < k; ++i) {
66526653
paramExprs[1 + i] = module.local_get(1 + i, parameterTypes[i].toRef());

‎src/flow.ts‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -464,15 +464,25 @@ export class Flow {
464464
setLocalFlag(index: i32, flag: LocalFlags): void {
465465
if (index < 0) return;
466466
let localFlags = this.localFlags;
467-
let flags = index < localFlags.length ? unchecked(localFlags[index]) : 0;
467+
let flags = 0;
468+
if (index < localFlags.length) {
469+
flags = unchecked(localFlags[index]);
470+
} else {
471+
localFlags.length = index + 1;
472+
}
468473
localFlags[index] = flags | flag;
469474
}
470475

471476
/** Unsets the specified flag or flags on the local at the specified index. */
472477
unsetLocalFlag(index: i32, flag: LocalFlags): void {
473478
if (index < 0) return;
474479
let localFlags = this.localFlags;
475-
let flags = index < localFlags.length ? unchecked(localFlags[index]) : 0;
480+
let flags = 0;
481+
if (index < localFlags.length) {
482+
flags = unchecked(localFlags[index]);
483+
} else {
484+
localFlags.length = index + 1;
485+
}
476486
localFlags[index] = flags & ~flag;
477487
}
478488

@@ -669,7 +679,7 @@ export class Flow {
669679
let numThisLocalFlags = thisLocalFlags.length;
670680
let otherLocalFlags = other.localFlags;
671681
let numOtherLocalFlags = otherLocalFlags.length;
672-
let maxLocalFlags = max(numThisLocalFlags, numOtherLocalFlags);
682+
let maxLocalFlags = thisLocalFlags.length = max(numThisLocalFlags, numOtherLocalFlags);
673683
for (let i = 0; i < maxLocalFlags; ++i) {
674684
let thisFlags = i < numThisLocalFlags ? thisLocalFlags[i] : 0;
675685
let otherFlags = i < numOtherLocalFlags ? otherLocalFlags[i] : 0;
@@ -781,21 +791,23 @@ export class Flow {
781791
if (leftFlags & FlowFlags.Terminates) {
782792
if (!(rightFlags & FlowFlags.Terminates)) {
783793
let rightLocalFlags = right.localFlags;
784-
for (let i = 0, k = rightLocalFlags.length; i < k; ++i) {
794+
const numRightLocalFlags = thisLocalFlags.length = rightLocalFlags.length;
795+
for (let i = 0, k = numRightLocalFlags; i < k; ++i) {
785796
thisLocalFlags[i] = rightLocalFlags[i];
786797
}
787798
}
788799
} else if (rightFlags & FlowFlags.Terminates) {
789800
let leftLocalFlags = left.localFlags;
790-
for (let i = 0, k = leftLocalFlags.length; i < k; ++i) {
801+
const numLeftLocalFlags = thisLocalFlags.length = leftLocalFlags.length;
802+
for (let i = 0, k = numLeftLocalFlags; i < k; ++i) {
791803
thisLocalFlags[i] = leftLocalFlags[i];
792804
}
793805
} else {
794806
let leftLocalFlags = left.localFlags;
795807
let numLeftLocalFlags = leftLocalFlags.length;
796808
let rightLocalFlags = right.localFlags;
797809
let numRightLocalFlags = rightLocalFlags.length;
798-
let maxLocalFlags = max(numLeftLocalFlags, numRightLocalFlags);
810+
let maxLocalFlags = thisLocalFlags.length = max(numLeftLocalFlags, numRightLocalFlags);
799811
for (let i = 0; i < maxLocalFlags; ++i) {
800812
let leftFlags = i < numLeftLocalFlags ? leftLocalFlags[i] : 0;
801813
let rightFlags = i < numRightLocalFlags ? rightLocalFlags[i] : 0;

‎src/passes/shadowstack.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ export class ShadowStackPass extends Pass {
467467
let results = _BinaryenFunctionGetResults(funcRef);
468468
let body = assert(_BinaryenFunctionGetBody(funcRef));
469469
let numVars = _BinaryenFunctionGetNumVars(funcRef);
470-
let vars = new Array<TypeRef>();
470+
let vars = new Array<TypeRef>(numVars);
471471
for (let i: Index = 0; i < numVars; ++i) {
472472
vars[i] = _BinaryenFunctionGetVar(funcRef, i);
473473
}

‎src/program.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3781,7 +3781,7 @@ export class Function extends TypedElement {
37813781
let scopedLocals = this.flow.scopedLocals;
37823782
if (!scopedLocals) this.flow.scopedLocals = scopedLocals = new Map();
37833783
scopedLocals.set(CommonNames.this_, local);
3784-
this.localsByIndex[local.index] = local;
3784+
this.localsByIndex.push(local);
37853785
}
37863786
let parameterTypes = signature.parameterTypes;
37873787
for (let i = 0, k = parameterTypes.length; i < k; ++i) {
@@ -3796,7 +3796,7 @@ export class Function extends TypedElement {
37963796
let scopedLocals = this.flow.scopedLocals;
37973797
if (!scopedLocals) this.flow.scopedLocals = scopedLocals = new Map();
37983798
scopedLocals.set(parameterName, local);
3799-
this.localsByIndex[local.index] = local;
3799+
this.localsByIndex.push(local);
38003800
}
38013801
}
38023802
registerConcreteElement(program, this);
@@ -3854,7 +3854,7 @@ export class Function extends TypedElement {
38543854
if (scopedLocals.has(name)) throw new Error("duplicate local name");
38553855
scopedLocals.set(name, local);
38563856
}
3857-
localsByIndex[localIndex] = local;
3857+
localsByIndex.push(local);
38583858
return local;
38593859
}
38603860

0 commit comments

Comments
 (0)