Skip to content

Commit 809a4d4

Browse files
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 be7137d commit 809a4d4

File tree

5 files changed

+33
-19
lines changed

5 files changed

+33
-19
lines changed

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: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1715,8 +1715,9 @@ export class Compiler extends DiagnosticEmitter {
17151715
this.makeFieldInitializationInConstructor(classInstance, allocStmts);
17161716

17171717
// Insert right before the body
1718-
for (let i = stmts.length - 1; i >= bodyStartIndex; --i) {
1719-
stmts[i + 1] = stmts[i];
1718+
let bodyEndIndex = stmts.length++;
1719+
for (let i = bodyEndIndex; i > bodyStartIndex; --i) {
1720+
stmts[i] = stmts[i - 1];
17201721
}
17211722
stmts[bodyStartIndex] = module.flatten(allocStmts, TypeRef.None);
17221723

@@ -2755,7 +2756,7 @@ export class Compiler extends DiagnosticEmitter {
27552756
let outerFlow = this.currentFlow;
27562757
let tempLocal = outerFlow.getTempLocal(Type.u32);
27572758
let tempLocalIndex = tempLocal.index;
2758-
let breaks = new Array<ExpressionRef>(1 + numCases);
2759+
let breaks = new Array<ExpressionRef>(2 + numCases);
27592760
breaks[0] = module.local_set(tempLocalIndex, condExpr, false); // u32
27602761

27612762
// Make one br_if per labeled case and leave it to Binaryen to optimize the
@@ -2784,6 +2785,7 @@ export class Compiler extends DiagnosticEmitter {
27842785
? `case${defaultIndex}|${label}`
27852786
: `break|${label}`
27862787
);
2788+
breaks.length = breakIndex + 1;
27872789

27882790
// Nest the case blocks in order, to be targeted by the br_if sequence
27892791
let currentBlock = module.block(`case0|${label}`, breaks, TypeRef.None);
@@ -6373,7 +6375,7 @@ export class Compiler extends DiagnosticEmitter {
63736375
}
63746376
let numOptional = assert(maxOperands - minOperands);
63756377

6376-
let forwardedOperands = new Array<ExpressionRef>(minOperands);
6378+
let forwardedOperands = new Array<ExpressionRef>(maxOperands);
63776379
let operandIndex = 0;
63786380
let stmts = new Array<ExpressionRef>();
63796381

@@ -6597,7 +6599,7 @@ export class Compiler extends DiagnosticEmitter {
65976599
let body: ExpressionRef;
65986600
let instanceClass = instance.getBoundClassOrInterface();
65996601
if (!instance.is(CommonFlags.Abstract) && !(instanceClass && instanceClass.kind == ElementKind.Interface)) {
6600-
let paramExprs = new Array<ExpressionRef>(numParameters);
6602+
let paramExprs = new Array<ExpressionRef>(numParameters + 1);
66016603
paramExprs[0] = module.local_get(0, sizeTypeRef); // this
66026604
for (let i = 0, k = parameterTypes.length; i < k; ++i) {
66036605
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
@@ -524,15 +524,25 @@ export class Flow {
524524
setLocalFlag(index: i32, flag: LocalFlags): void {
525525
if (index < 0) return;
526526
let localFlags = this.localFlags;
527-
let flags = index < localFlags.length ? unchecked(localFlags[index]) : 0;
527+
let flags = 0;
528+
if (index < localFlags.length) {
529+
flags = unchecked(localFlags[index]);
530+
} else {
531+
localFlags.length = index + 1;
532+
}
528533
localFlags[index] = flags | flag;
529534
}
530535

531536
/** Unsets the specified flag or flags on the local at the specified index. */
532537
unsetLocalFlag(index: i32, flag: LocalFlags): void {
533538
if (index < 0) return;
534539
let localFlags = this.localFlags;
535-
let flags = index < localFlags.length ? unchecked(localFlags[index]) : 0;
540+
let flags = 0;
541+
if (index < localFlags.length) {
542+
flags = unchecked(localFlags[index]);
543+
} else {
544+
localFlags.length = index + 1;
545+
}
536546
localFlags[index] = flags & ~flag;
537547
}
538548

@@ -717,7 +727,7 @@ export class Flow {
717727
let numThisLocalFlags = thisLocalFlags.length;
718728
let otherLocalFlags = other.localFlags;
719729
let numOtherLocalFlags = otherLocalFlags.length;
720-
let maxLocalFlags = max(numThisLocalFlags, numOtherLocalFlags);
730+
let maxLocalFlags = thisLocalFlags.length = max(numThisLocalFlags, numOtherLocalFlags);
721731
for (let i = 0; i < maxLocalFlags; ++i) {
722732
let thisFlags = i < numThisLocalFlags ? thisLocalFlags[i] : 0;
723733
let otherFlags = i < numOtherLocalFlags ? otherLocalFlags[i] : 0;
@@ -829,21 +839,23 @@ export class Flow {
829839
if (leftFlags & FlowFlags.Terminates) {
830840
if (!(rightFlags & FlowFlags.Terminates)) {
831841
let rightLocalFlags = right.localFlags;
832-
for (let i = 0, k = rightLocalFlags.length; i < k; ++i) {
842+
const numRightLocalFlags = thisLocalFlags.length = rightLocalFlags.length;
843+
for (let i = 0, k = numRightLocalFlags; i < k; ++i) {
833844
thisLocalFlags[i] = rightLocalFlags[i];
834845
}
835846
}
836847
} else if (rightFlags & FlowFlags.Terminates) {
837848
let leftLocalFlags = left.localFlags;
838-
for (let i = 0, k = leftLocalFlags.length; i < k; ++i) {
849+
const numLeftLocalFlags = thisLocalFlags.length = leftLocalFlags.length;
850+
for (let i = 0, k = numLeftLocalFlags; i < k; ++i) {
839851
thisLocalFlags[i] = leftLocalFlags[i];
840852
}
841853
} else {
842854
let leftLocalFlags = left.localFlags;
843855
let numLeftLocalFlags = leftLocalFlags.length;
844856
let rightLocalFlags = right.localFlags;
845857
let numRightLocalFlags = rightLocalFlags.length;
846-
let maxLocalFlags = max(numLeftLocalFlags, numRightLocalFlags);
858+
let maxLocalFlags = thisLocalFlags.length = max(numLeftLocalFlags, numRightLocalFlags);
847859
for (let i = 0; i < maxLocalFlags; ++i) {
848860
let leftFlags = i < numLeftLocalFlags ? leftLocalFlags[i] : 0;
849861
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
@@ -3783,7 +3783,7 @@ export class Function extends TypedElement {
37833783
let scopedLocals = this.flow.scopedLocals;
37843784
if (!scopedLocals) this.flow.scopedLocals = scopedLocals = new Map();
37853785
scopedLocals.set(CommonNames.this_, local);
3786-
this.localsByIndex[local.index] = local;
3786+
this.localsByIndex.push(local);
37873787
flow.setLocalFlag(local.index, LocalFlags.Initialized);
37883788
}
37893789
let parameterTypes = signature.parameterTypes;
@@ -3799,7 +3799,7 @@ export class Function extends TypedElement {
37993799
let scopedLocals = this.flow.scopedLocals;
38003800
if (!scopedLocals) this.flow.scopedLocals = scopedLocals = new Map();
38013801
scopedLocals.set(parameterName, local);
3802-
this.localsByIndex[local.index] = local;
3802+
this.localsByIndex.push(local);
38033803
flow.setLocalFlag(local.index, LocalFlags.Initialized);
38043804
}
38053805
}
@@ -3858,7 +3858,7 @@ export class Function extends TypedElement {
38583858
if (scopedLocals.has(name)) throw new Error("duplicate local name");
38593859
scopedLocals.set(name, local);
38603860
}
3861-
localsByIndex[localIndex] = local;
3861+
localsByIndex.push(local);
38623862
return local;
38633863
}
38643864

0 commit comments

Comments
 (0)