Skip to content

Commit b4bf2bb

Browse files
ASDAlexander77claude
ASDAlexander77
and
claude
authored
Narrowing after an early exit; a narrowed field is read from the field (#383)
`if (x === null) return;` now narrows x for the rest of the block, as an else branch does: an if without else whose body always returns, throws, breaks or continues applies its else narrowing after the if. Each block opens its own narrowing scope, so a narrowing made in it ends with it. A narrowed field (`this.head` after `this.head !== null`) was mapped to the value the narrowing cast, and every later read reused that value. An assignment to the field then failed ("saving to constant object"), a read after it returned the old value, and a value used outside its region was the use-after-free #377 fixed for else. safeTypesMap now keeps only the narrowed type: a read loads the field again and casts it the way addSafeCastStatement does (castToNarrowedType), and an assignment writes the field itself. Found in the BrowserLib sources attached to #231 (Event.ts `remove`). Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 5017269 commit b4bf2bb

6 files changed

Lines changed: 218 additions & 13 deletions

File tree

‎tslang/include/TypeScript/MLIRLogic/MLIRDefines.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,6 @@ using SymbolTableScopeT = llvm::ScopedHashTableScope<StringRef, VariablePairT>;
5050
using BoundRefCacheScopeT = llvm::ScopedHashTableScope<mlir::Value, mlir::Value>;
5151

5252
typedef std::pair<mlir::Type, StringRef> SafeTypeKeyType;
53-
using SafeTypesMapScopeT = llvm::ScopedHashTableScope<SafeTypeKeyType, mlir::Value>;
53+
using SafeTypesMapScopeT = llvm::ScopedHashTableScope<SafeTypeKeyType, mlir::Type>;
5454

5555
#endif // MLIR_TYPESCRIPT_MLIRGENLOGIC_MLIRDEFINES_H_

‎tslang/lib/TypeScript/MLIRGenExpressions.cpp‎

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -895,16 +895,38 @@ namespace mlirgen
895895
auto namePtr = MLIRHelper::getName(propertyAccessExpression->name, stringAllocator);
896896
auto propAccessStrRef = mlir::StringRef(print(propertyAccessExpression)).copy(stringAllocator);
897897

898-
// check if we have safe type mapped value
899-
auto safeTypedValue = safeTypesMap.lookup({ expressionValue.getType(), propAccessStrRef });
900-
if (safeTypedValue)
901-
{
902-
LLVM_DEBUG(llvm::dbgs() << "\n\t...safe type fieldname: \t "
903-
<< propAccessStrRef << "." << namePtr << "type: " << expressionValue.getType() << " = " << safeTypedValue;);
904-
return safeTypedValue;
898+
auto fieldResult = mlirGenPropertyAccessExpression(location, expressionValue, namePtr,
899+
!!propertyAccessExpression->questionDotToken, genContext);
900+
901+
// a narrowed field: read as it is now, cast to the type the test narrowed it to
902+
auto safeType = safeTypesMap.lookup({ expressionValue.getType(), propAccessStrRef });
903+
if (safeType && !fieldResult.failed_or_no_value() && V(fieldResult).getType() != safeType)
904+
{
905+
LLVM_DEBUG(llvm::dbgs() << "\n\t...safe type fieldname: \t "
906+
<< propAccessStrRef << "." << namePtr << "type: " << expressionValue.getType() << " = " << safeType;);
907+
auto fieldValue = V(fieldResult);
908+
auto castValue = castToNarrowedType(location, fieldValue, safeType, genContext);
909+
if (!castValue)
910+
{
911+
return mlir::failure();
912+
}
913+
914+
return V(builder.create<mlir_ts::SafeCastOp>(location, safeType, castValue, fieldValue));
905915
}
906916

907-
return mlirGenPropertyAccessExpression(location, expressionValue, namePtr,
917+
return fieldResult;
918+
}
919+
920+
ValueOrLogicalResult MLIRGenImpl::mlirGenAssignedPropertyAccess(PropertyAccessExpression propertyAccessExpression, const GenContext &genContext)
921+
{
922+
// what an assignment writes is the field itself, never the value a narrowing cast from it
923+
auto location = loc(propertyAccessExpression);
924+
925+
auto result = mlirGen(propertyAccessExpression->expression.as<Expression>(), genContext);
926+
EXIT_IF_FAILED_OR_NO_VALUE(result)
927+
928+
auto namePtr = MLIRHelper::getName(propertyAccessExpression->name, stringAllocator);
929+
return mlirGenPropertyAccessExpression(location, V(result), namePtr,
908930
!!propertyAccessExpression->questionDotToken, genContext);
909931
}
910932

‎tslang/lib/TypeScript/MLIRGenImpl.h‎

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3308,7 +3308,7 @@ class MLIRGenImpl
33083308
auto propAccess = expr.as<PropertyAccessExpression>();
33093309
auto objType = evaluate(propAccess->expression, genContext);
33103310
LLVM_DEBUG(llvm::dbgs() << "\n!! Safe Type map for: " << nameStr << " of " << objType << " is [" << safeValue.getType() << "]\n");
3311-
safeTypesMap.insert({ objType, nameStr }, safeValue);
3311+
safeTypesMap.insert({ objType, nameStr }, safeValue.getType());
33123312
}
33133313
}
33143314
}
@@ -3317,6 +3317,34 @@ class MLIRGenImpl
33173317
return result2;
33183318
}
33193319

3320+
// the value a narrowing to `safeType` gives `exprValue`, cast the way addSafeCastStatement casts it
3321+
mlir::Value castToNarrowedType(mlir::Location location, mlir::Value exprValue, mlir::Type safeType, const GenContext &genContext)
3322+
{
3323+
auto exprType = exprValue.getType();
3324+
if (isa<mlir_ts::AnyType>(exprType))
3325+
{
3326+
return builder.create<mlir_ts::UnboxOp>(location, safeType, exprValue);
3327+
}
3328+
3329+
if (auto optType = dyn_cast<mlir_ts::OptionalType>(exprType))
3330+
{
3331+
if (optType.getElementType() == safeType)
3332+
{
3333+
return builder.create<mlir_ts::ValueOp>(location, safeType, exprValue);
3334+
}
3335+
}
3336+
3337+
if (isa<mlir_ts::UnionType>(exprType))
3338+
{
3339+
return isa<mlir_ts::UnionType>(safeType)
3340+
? builder.create<mlir_ts::CastOp>(location, safeType, exprValue).getResult()
3341+
: builder.create<mlir_ts::GetValueFromUnionOp>(location, safeType, exprValue).getResult();
3342+
}
3343+
3344+
auto result = cast(location, safeType, exprValue, genContext);
3345+
return result.failed_or_no_value() ? mlir::Value() : V(result);
3346+
}
3347+
33203348
mlir::LogicalResult addSafeCastStatement(mlir::Location location, StringRef parameterName, mlir::Value exprValue, mlir::Type safeType, bool inverse, ElseSafeCase* elseSafeCase, const GenContext &genContext)
33213349
{
33223350
mlir::Value castedValue;
@@ -5459,7 +5487,9 @@ class MLIRGenImpl
54595487
genContext);
54605488
}
54615489

5462-
auto result = mlirGen(leftExpression, genContext);
5490+
auto result = leftExpression == SyntaxKind::PropertyAccessExpression
5491+
? mlirGenAssignedPropertyAccess(leftExpression.as<PropertyAccessExpression>(), genContext)
5492+
: mlirGen(leftExpression, genContext);
54635493
EXIT_IF_FAILED_OR_NO_VALUE(result)
54645494
auto leftExpressionValue = V(result);
54655495

@@ -6378,6 +6408,8 @@ class MLIRGenImpl
63786408

63796409
ValueOrLogicalResult mlirGen(PropertyAccessExpression propertyAccessExpression, const GenContext &genContext);
63806410

6411+
ValueOrLogicalResult mlirGenAssignedPropertyAccess(PropertyAccessExpression propertyAccessExpression, const GenContext &genContext);
6412+
63816413
ValueOrLogicalResult mlirGenPropertyAccessExpression(mlir::Location location, mlir::Value objectValue,
63826414
mlir::StringRef name, const GenContext &genContext);
63836415

@@ -12792,7 +12824,9 @@ class MLIRGenImpl
1279212824

1279312825
llvm::ScopedHashTable<StringRef, mlir::LLVM::DIScopeAttr> debugScope;
1279412826

12795-
llvm::ScopedHashTable<SafeTypeKeyType, mlir::Value> safeTypesMap;
12827+
// a narrowed field (`this.head` after `this.head !== null`) and the type it is narrowed to; a
12828+
// read of it loads the field again and casts it, so no value outlives the region it is in
12829+
llvm::ScopedHashTable<SafeTypeKeyType, mlir::Type> safeTypesMap;
1279612830

1279712831
// helper to get line number
1279812832
Parser parser;

‎tslang/lib/TypeScript/MLIRGenStatements.cpp‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,8 @@ namespace mlirgen
173173
auto location = loc(blockAST);
174174

175175
SymbolTableScopeT varScope(symbolTable);
176+
// a narrowing made in the block (after an early exit) ends with it
177+
SafeTypesMapScopeT safeTypesMapScope(safeTypesMap);
176178
GenContext genContextUsing(genContext);
177179
genContextUsing.parentBlockContext = &genContext;
178180

@@ -245,6 +247,7 @@ namespace mlirgen
245247
builder.setInsertionPointToStart(&tryOp.getBody().front());
246248

247249
SymbolTableScopeT varScope(symbolTable);
250+
SafeTypesMapScopeT safeTypesMapScope(safeTypesMap);
248251
GenContext tryBodyGenContext(tryGenContext);
249252
tryBodyGenContext.parentBlockContext = &tryGenContext;
250253

@@ -489,6 +492,38 @@ namespace mlirgen
489492
return mlir::success();
490493
}
491494

495+
// whether control never runs past the statement: it returns, throws, breaks or continues on
496+
// every path. Conservative - a statement it cannot tell about does not count.
497+
static bool statementAlwaysExits(Statement statement)
498+
{
499+
switch ((SyntaxKind)statement)
500+
{
501+
case SyntaxKind::ReturnStatement:
502+
case SyntaxKind::ThrowStatement:
503+
case SyntaxKind::BreakStatement:
504+
case SyntaxKind::ContinueStatement:
505+
return true;
506+
case SyntaxKind::Block:
507+
for (auto blockStatement : statement.as<ts::Block>()->statements)
508+
{
509+
if (statementAlwaysExits(blockStatement))
510+
{
511+
return true;
512+
}
513+
}
514+
515+
return false;
516+
case SyntaxKind::IfStatement:
517+
{
518+
auto ifStatement = statement.as<IfStatement>();
519+
return ifStatement->elseStatement && statementAlwaysExits(ifStatement->thenStatement)
520+
&& statementAlwaysExits(ifStatement->elseStatement);
521+
}
522+
default:
523+
return false;
524+
}
525+
}
526+
492527
mlir::LogicalResult MLIRGenImpl::mlirGen(IfStatement ifStatementAST, const GenContext &genContext)
493528
{
494529
auto location = loc(ifStatementAST);
@@ -518,6 +553,9 @@ namespace mlirgen
518553
// narrowed to `string` by `typeof x === "string"`. Under --di the narrowed variable's debug record
519554
// keeps such a cast alive into LLVM lowering even though the branch body was skipped.
520555
ElseSafeCase elseSafeCase{};
556+
// `if (x === null) return;` narrows x for the rest of the block, as an else branch would
557+
auto thenExits = !hasElse && !literalValue.has_value() && genContext.funcOp
558+
&& statementAlwaysExits(ifStatementAST->thenStatement);
521559
{
522560
SymbolTableScopeT varScope(symbolTable);
523561
SafeTypesMapScopeT safeTypesMapScope(safeTypesMap);
@@ -526,7 +564,7 @@ namespace mlirgen
526564
if (processIf)
527565
{
528566
// check if we do safe-cast here
529-
checkSafeCast(ifStatementAST->expression, V(result), hasElse ? &elseSafeCase : nullptr, genContext);
567+
checkSafeCast(ifStatementAST->expression, V(result), hasElse || thenExits ? &elseSafeCase : nullptr, genContext);
530568

531569
auto result = mlirGen(ifStatementAST->thenStatement, genContext);
532570
EXIT_IF_FAILED(result)
@@ -556,6 +594,11 @@ namespace mlirgen
556594

557595
builder.setInsertionPointAfter(ifOp);
558596

597+
if (thenExits && elseSafeCase.safeType)
598+
{
599+
addSafeCastStatement(elseSafeCase.expr, elseSafeCase.safeType, false, nullptr, genContext);
600+
}
601+
559602
return mlir::success();
560603
}
561604

‎tslang/test/tester/CMakeLists.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,7 @@ tslang_add_test(NAME test-compile-00-safe-cast-typeof COMMAND test-runner "${PRO
531531
tslang_add_test(NAME test-compile-00-safe-cast-while COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00safe_cast_while.ts")
532532
tslang_add_test(NAME test-compile-01-safe-cast-while COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/01safe_cast_while.ts")
533533
tslang_add_test(NAME test-compile-00-safe-cast-field-access COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00safe_cast_field_access.ts")
534+
tslang_add_test(NAME test-compile-00-safe-cast-early-exit COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00safe_cast_early_exit.ts")
534535
tslang_add_test(NAME test-compile-00-safe-cast-null-field COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00safe_cast_null_field.ts")
535536
tslang_add_test(NAME test-compile-00-safe-cast-else-scope COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00safe_cast_else_scope.ts")
536537
tslang_add_test(NAME test-compile-00-safe-cast-bug COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00safe_cast_bug.ts")
@@ -977,6 +978,7 @@ tslang_add_test(NAME test-jit-00-safe-cast-typeof COMMAND test-runner -jit "${PR
977978
tslang_add_test(NAME test-jit-00-safe-cast-while COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00safe_cast_while.ts")
978979
tslang_add_test(NAME test-jit-01-safe-cast-while COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/01safe_cast_while.ts")
979980
tslang_add_test(NAME test-jit-00-safe-cast-field-access COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00safe_cast_field_access.ts")
981+
tslang_add_test(NAME test-jit-00-safe-cast-early-exit COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00safe_cast_early_exit.ts")
980982
tslang_add_test(NAME test-jit-00-safe-cast-null-field COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00safe_cast_null_field.ts")
981983
tslang_add_test(NAME test-jit-00-safe-cast-else-scope COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00safe_cast_else_scope.ts")
982984
tslang_add_test(NAME test-jit-00-safe-cast-bug COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00safe_cast_bug.ts")
@@ -1764,6 +1766,7 @@ set(TSLANG_CORPUS
17641766
00reference_ref_deref.ts
17651767
00safe_cast_bug.ts
17661768
00safe_cast_field_access.ts
1769+
00safe_cast_early_exit.ts
17671770
00safe_cast_null_field.ts
17681771
00safe_cast_else_scope.ts
17691772
00safe_cast_typeof.ts

‎tslang/test/tester/tests/00safe_cast_early_exit.ts‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
// `if (x === null) return;` narrows x for the rest of the block, as an else branch would. A narrowed
2+
// field is read from the field every time, so an assignment to it writes the field (it was "saving
3+
// to constant object") and a read after the assignment sees the new value (#231).
4+
5+
class N {
6+
v: number;
7+
next: N | null = null;
8+
9+
constructor(v: number) {
10+
this.v = v;
11+
}
12+
}
13+
14+
class List {
15+
head: N | null = null;
16+
count = 0;
17+
18+
first(): number {
19+
if (this.head === null) return -1;
20+
return this.head.v;
21+
}
22+
23+
add(v: number) {
24+
const n = new N(v);
25+
n.next = this.head;
26+
this.head = n;
27+
this.count++;
28+
}
29+
30+
remove(v: number): void {
31+
if (this.head === null) {
32+
return;
33+
}
34+
35+
if (this.head.v === v) {
36+
this.head = this.head.next;
37+
this.count--;
38+
return;
39+
}
40+
41+
let cur = this.head;
42+
while (cur.next !== null) {
43+
if (cur.next.v === v) {
44+
cur.next = cur.next.next;
45+
this.count--;
46+
return;
47+
}
48+
49+
cur = cur.next;
50+
}
51+
}
52+
53+
replaceHead(v: number): number {
54+
if (this.head !== null) {
55+
this.head = new N(v);
56+
// the field as it is now, not the value the test narrowed
57+
return this.head.v;
58+
}
59+
60+
return -1;
61+
}
62+
}
63+
64+
function valueOf(h: N | null): number {
65+
if (h === null) {
66+
throw "no value";
67+
}
68+
69+
return h.v;
70+
}
71+
72+
function sum(items: (N | null)[]): number {
73+
let s = 0;
74+
for (const i of items) {
75+
if (i === null) continue;
76+
s += i.v;
77+
}
78+
79+
return s;
80+
}
81+
82+
function main() {
83+
const l = new List();
84+
assert(l.first() == -1);
85+
l.remove(3);
86+
87+
l.add(1);
88+
l.add(2);
89+
l.add(3);
90+
assert(l.first() == 3);
91+
92+
l.remove(3);
93+
assert(l.first() == 2 && l.count == 2);
94+
l.remove(1);
95+
assert(l.first() == 2 && l.count == 1);
96+
97+
assert(l.replaceHead(9) == 9 && l.first() == 9);
98+
99+
assert(valueOf(new N(4)) == 4);
100+
assert(sum([new N(1), null, new N(5)]) == 6);
101+
102+
print("done.");
103+
}

0 commit comments

Comments
 (0)