From 833344cad4b79a18d3e8a4f8d531b5a795a6c25a Mon Sep 17 00:00:00 2001 From: ASDAlexander77 Date: Tue, 20 Oct 2026 00:26:28 +0100 Subject: [PATCH] -mm=own: @lends, and a string parameter the body assigns owns what it holds - `@lends("p")` says a function's result is a borrow of argument p: the caller owns nothing of it and it may not outlive p. It crosses modules through __decls, is checked against the body and family overrides, and names one parameter; it cannot be combined with @borrows. - Under own a string parameter the body assigns, including `p.length = n`, starts a local of its own made from a copy, as rc already did. Left as the parameter's slot, the assigned string leaked and `return this` returned a copy read up to its first NUL: the default library's `"".clone().resize(n)` returned one byte that its caller filled with n, which corrupted the own DefaultLib's regexp results. Co-Authored-By: Claude Opus 5.5 --- .../2026-10-19-own-param-markers-design.md | 32 +++-- tslang/include/TypeScript/Defines.h | 9 +- .../MLIRLogic/MLIRDeclarationPrinter.h | 4 +- .../TypeScript/MLIRLogic/MLIRGenStore.h | 5 +- tslang/lib/TypeScript/DeclarationPrinter.cpp | 12 +- tslang/lib/TypeScript/LowerToLLVM.cpp | 3 +- tslang/lib/TypeScript/MLIRGenFunctions.cpp | 46 +++++-- tslang/lib/TypeScript/MLIRGenImpl.h | 23 +++- .../lib/TypeScript/OwnershipSignaturePass.cpp | 93 +++++++++++--- tslang/test/tester/CMakeLists.txt | 14 ++- .../tester/own/own_err_lends_and_borrows.ts | 18 +++ .../test/tester/own/own_err_lends_mismatch.ts | 17 +++ .../test/tester/own/own_err_lends_missing.ts | 17 +++ .../tester/own/own_err_lends_not_borrow.ts | 18 +++ .../test/tester/own/own_err_lends_outlives.ts | 21 ++++ .../tester/own/own_err_lends_two_params.ts | 17 +++ tslang/test/tester/own/own_lends.ts | 67 ++++++++++ .../tester/own/own_string_param_assigned.ts | 115 ++++++++++++++++++ .../test/tester/tests/export_own_markers.ts | 12 +- .../test/tester/tests/import_own_markers.ts | 8 +- .../tests/import_own_markers_lent_outlives.ts | 10 ++ 21 files changed, 504 insertions(+), 57 deletions(-) create mode 100644 tslang/test/tester/own/own_err_lends_and_borrows.ts create mode 100644 tslang/test/tester/own/own_err_lends_mismatch.ts create mode 100644 tslang/test/tester/own/own_err_lends_missing.ts create mode 100644 tslang/test/tester/own/own_err_lends_not_borrow.ts create mode 100644 tslang/test/tester/own/own_err_lends_outlives.ts create mode 100644 tslang/test/tester/own/own_err_lends_two_params.ts create mode 100644 tslang/test/tester/own/own_lends.ts create mode 100644 tslang/test/tester/own/own_string_param_assigned.ts create mode 100644 tslang/test/tester/tests/import_own_markers_lent_outlives.ts diff --git a/tslang/docs/superpowers/specs/2026-10-19-own-param-markers-design.md b/tslang/docs/superpowers/specs/2026-10-19-own-param-markers-design.md index 1b9164f29..cb911c731 100644 --- a/tslang/docs/superpowers/specs/2026-10-19-own-param-markers-design.md +++ b/tslang/docs/superpowers/specs/2026-10-19-own-param-markers-design.md @@ -25,6 +25,10 @@ Both go on a function, method or constructor, and name its parameters. `"this"` - `@borrows("this", "expr")`: the caller owns the result, but the result may not outlive these arguments. The result is either the state of a generator over them, or what a call with such a bounded result returns (`return this.value.matchAll(expr)`, a "forwarder"). +- `@lends("l")` (added 2026-10-20): the result is a borrow of the one argument `l` + (`__own_result_borrows`). The caller owns nothing of it, and it may not outlive `l`. It names + exactly one parameter, and it cannot be combined with `@borrows` (a result is either the + caller's or lent). It can be combined with `@keeps`. Under gc, rc and none they mean nothing. MLIRGen still records them, and the LLVM lowering drops the attributes. @@ -36,8 +40,9 @@ the attributes. (`ts.own_declared_param_names`) for the errors. A name that is not a parameter is an error. The generator wrapper copies the method's decorators, so a marked generator's maker carries them. - **OwnershipSignaturePass:** - - A marked function's `@keeps` facts stand even when it is open (`meetFacts`). A family does not - erase them. + - A marked function's `@keeps` and `@lends` facts stand even when it is open (`meetFacts`). A + family does not erase them. A call this pass cannot resolve gets them from its callees' + signatures (`computeFacts`). - A call gets what its callees' signatures say, whether or not it is resolved (`declaredByCallees`, over `Callees::named`). The named callees are the direct callee, every member of a virtual slot's family, or a dynamic import's address global. @@ -49,7 +54,11 @@ the attributes. - `findBoundedForwarders` gives a marked non-generator what its returned bounded calls borrow, mapped back to its own parameters (directly, or a field read of one). - `checkDeclared` holds each body to its signature, exactly. It also checks that the overrides in - a marked method's family say the same. + a marked method's family say the same. A function that says `@keeps` or `@lends` must say both + facts its body has: a body that returns a borrow needs `@lends`, one that keeps an argument + needs `@keeps`. + - The error for an open function that returns a borrow or keeps a parameter says that `@lends` + or `@keeps` would put it in its signature. - **OwnershipInferencePass:** a marked function may return a bounded result (`boundedUses`). - **Across modules:** - `__decls` prints the markers with the printed parameter names (`p1`, and `this` for a @@ -116,9 +125,13 @@ With these, `lib.ts` compiles under gc, rc, none and own, release and `--di`. De made from the shape freed the borrowed argument a second time (exit 127). `import_final_class` and `import_own_markers` now run under own too. `own_iterator_next.ts` covers the interface-shaped call in one module, which already worked. -- **An own DefaultLib returns corrupted regex strings.** This is on main, and it is not this - branch's. `let buffer = "".clone().resize(len)` (`MatchResults.get`) borrows a temporary that is - freed after the statement, and own does not reject it. It needs its own fix. +- **An own DefaultLib returned corrupted regex strings.** Fixed 2026-10-20. It was not a borrowed + temporary. `resize` assigns its `this` parameter (`this.length = n`), and under own a string + parameter's slot did not own what was assigned to it: that string leaked, and `return this` + returned a copy of it, read up to its first NUL - one byte, which the caller filled with `len` + characters. Under own a string parameter the body assigns (including `p.length = n`) now starts + a local of its own, made from a copy, as rc already did for every assigned parameter + (`mlirGenFunctionOwnAssignedParams`). Covered by `own_string_param_assigned.ts`. - **A generator local's previous value leaks** under rc and own (on main too): a `const` declared in a loop inside a generator is stored into its state field again each iteration, and nothing gives back what the last iteration stored. Releasing it there fixes rc, but under own it makes `next` @@ -127,9 +140,10 @@ With these, `lib.ts` compiles under gc, rc, none and own, release and `--di`. De - **A generator local's type from a function declared later** is `none` in the state object, and the lowering fails ("failed to legalize operation 'ts.Cast'") under every model. `matchAll` annotates `const more: boolean`. -- **`@borrows` for an ordinary function whose result points into an argument** - (`__own_result_borrows`, where the caller owns nothing) is not expressible yet. Such a function - is still rejected when it is open. +- **An ordinary function whose result points into an argument** (`__own_result_borrows`, where + the caller owns nothing) says so with `@lends`, added 2026-10-20. Covered by `own_lends.ts`, the + `own_err_lends_*` cases, and `import_own_markers` / `import_own_markers_lent_outlives` across + modules. - **A ternary that picks an array literal or `null`** (`return i < 3 ? ["a"] : null`) fails MLIR verification ("operand #0 does not dominate this use") under rc and own. This is unrelated. - **Android** still builds gc, rc and none only. diff --git a/tslang/include/TypeScript/Defines.h b/tslang/include/TypeScript/Defines.h index 6c0aef4dc..e4d933fcc 100644 --- a/tslang/include/TypeScript/Defines.h +++ b/tslang/include/TypeScript/Defines.h @@ -256,13 +256,16 @@ // knows them - one in another module too, which reads them from the declaration it re-parses. // `@keeps` names the parameters the function keeps: its callers move them in. `@borrows` names the // arguments a generator's state object borrows: the caller owns the result, but it may not outlive -// them. -mm=own checks a body against what it says; the other memory models ignore both. Each is a -// FuncOp attribute of argument indices. +// them. `@lends` names the one argument the result is a borrow of (OWN_RESULT_BORROWS_ATTR_NAME): +// the caller owns nothing, and the result may not outlive it. -mm=own checks a body against what +// it says; the other memory models ignore all three. Each is a FuncOp attribute of argument indices. #define KEEPS_DECORATOR "keeps" #define BORROWS_DECORATOR "borrows" +#define LENDS_DECORATOR "lends" #define OWN_DECLARED_KEEPS_ATTR_NAME "ts.own_declared_keeps" #define OWN_DECLARED_BORROWS_ATTR_NAME "ts.own_declared_borrows" -// the names of the arguments of a function that says either, for -mm=own's errors about it +#define OWN_DECLARED_LENDS_ATTR_NAME "ts.own_declared_lends" +// the names of the arguments of a function that says any, for -mm=own's errors about it #define OWN_DECLARED_PARAM_NAMES_ATTR_NAME "ts.own_declared_param_names" // The process-heap allocator of a Windows module (TypeScript/ProcessHeap.h), which ProcessHeapPass diff --git a/tslang/include/TypeScript/MLIRLogic/MLIRDeclarationPrinter.h b/tslang/include/TypeScript/MLIRLogic/MLIRDeclarationPrinter.h index 825d469bc..ae1364bdf 100644 --- a/tslang/include/TypeScript/MLIRLogic/MLIRDeclarationPrinter.h +++ b/tslang/include/TypeScript/MLIRLogic/MLIRDeclarationPrinter.h @@ -24,7 +24,7 @@ namespace typescript void printEnum(StringRef, NamespaceInfo::TypePtr, mlir::DictionaryAttr); void printVariableDeclaration(StringRef, NamespaceInfo::TypePtr, mlir::Type, bool, StringRef = StringRef()); void print(StringRef, NamespaceInfo::TypePtr, mlir_ts::FunctionType, StringRef = StringRef(), - ArrayRef ownKeeps = {}, ArrayRef ownBorrows = {}); + ArrayRef ownKeeps = {}, ArrayRef ownBorrows = {}, ArrayRef ownLends = {}); void print(ClassInfo::TypePtr); void print(InterfaceInfo::TypePtr); void printGenericClass(NamespaceInfo::TypePtr, StringRef); @@ -40,7 +40,7 @@ namespace typescript bool filterName(StringRef); bool filterField(mlir::Attribute); void printParams(ArrayRef, mlir::Type); - void printOwnMarkers(ArrayRef, ArrayRef, bool, unsigned); + void printOwnMarkers(ArrayRef, ArrayRef, ArrayRef, bool, unsigned); void printFunction(StringRef, ArrayRef, mlir::Type); void printMethod(bool, StringRef, ArrayRef, mlir::Type, mlir::Type); void printAccessor(bool, StringRef, StringRef, mlir_ts::AccessLevel, ArrayRef, mlir::Type, mlir::Type, StringRef); diff --git a/tslang/include/TypeScript/MLIRLogic/MLIRGenStore.h b/tslang/include/TypeScript/MLIRLogic/MLIRGenStore.h index 68a421850..d272f1588 100644 --- a/tslang/include/TypeScript/MLIRLogic/MLIRGenStore.h +++ b/tslang/include/TypeScript/MLIRLogic/MLIRGenStore.h @@ -163,10 +163,11 @@ struct MethodInfo int cppSlot = -1; // a slot of a @cpp class's vtable that no method names (ClassInfo::getVirtualTable) bool isCppSlotGap = false; - // `@keeps`, `@borrows` (OWN_DECLARED_KEEPS_ATTR_NAME): argument indices, `this` 0, for the - // declaration an importer re-parses + // `@keeps`, `@borrows`, `@lends` (OWN_DECLARED_KEEPS_ATTR_NAME): argument indices, `this` 0, for + // the declaration an importer re-parses; `@lends` names one, or none (-1) std::vector ownKeeps; std::vector ownBorrows; + int32_t ownLends = -1; }; struct GenericMethodInfo diff --git a/tslang/lib/TypeScript/DeclarationPrinter.cpp b/tslang/lib/TypeScript/DeclarationPrinter.cpp index d6d7dd9a1..eacdda372 100644 --- a/tslang/lib/TypeScript/DeclarationPrinter.cpp +++ b/tslang/lib/TypeScript/DeclarationPrinter.cpp @@ -230,7 +230,8 @@ namespace typescript // `@keeps("p1")`, `@borrows("this", "p1")`: by the names printParams gives the parameters, `this` // the receiver of a method, so the importer reads the same argument indices back - void MLIRDeclarationPrinter::printOwnMarkers(ArrayRef keeps, ArrayRef borrows, bool hasThis, unsigned indent) + void MLIRDeclarationPrinter::printOwnMarkers(ArrayRef keeps, ArrayRef borrows, ArrayRef lends, + bool hasThis, unsigned indent) { auto printMarker = [&](StringRef name, ArrayRef indices) { if (indices.empty()) @@ -261,6 +262,7 @@ namespace typescript printMarker(KEEPS_DECORATOR, keeps); printMarker(BORROWS_DECORATOR, borrows); + printMarker(LENDS_DECORATOR, lends); } void MLIRDeclarationPrinter::printFunction(StringRef name, ArrayRef params, mlir::Type returnType) @@ -438,7 +440,7 @@ namespace typescript } void MLIRDeclarationPrinter::print(StringRef name, NamespaceInfo::TypePtr elementNamespace, mlir_ts::FunctionType funcType, StringRef dllName, - ArrayRef ownKeeps, ArrayRef ownBorrows) + ArrayRef ownKeeps, ArrayRef ownBorrows, ArrayRef ownLends) { printNamespaceBegin(elementNamespace); @@ -450,7 +452,7 @@ namespace typescript newline(); } - printOwnMarkers(ownKeeps, ownBorrows, /*hasThis=*/false, 0); + printOwnMarkers(ownKeeps, ownBorrows, ownLends, /*hasThis=*/false, 0); printFunction( name, @@ -698,7 +700,9 @@ namespace typescript newline(); } - printOwnMarkers(method.ownKeeps, method.ownBorrows, /*hasThis=*/!method.isStatic, 4); + printOwnMarkers(method.ownKeeps, method.ownBorrows, + method.ownLends >= 0 ? ArrayRef(method.ownLends) : ArrayRef(), + /*hasThis=*/!method.isStatic, 4); // the slot of a @cpp class's virtual method: without it the importer takes the method // for a non-virtual one diff --git a/tslang/lib/TypeScript/LowerToLLVM.cpp b/tslang/lib/TypeScript/LowerToLLVM.cpp index 49145b3d6..ca38f6eb0 100644 --- a/tslang/lib/TypeScript/LowerToLLVM.cpp +++ b/tslang/lib/TypeScript/LowerToLLVM.cpp @@ -2051,7 +2051,8 @@ struct FuncOpLowering : public TsLlvmPattern // read by -mm=own only if (name == OWN_DECLARED_NO_DROPS_ATTR_NAME || name == OWN_DECLARED_KEEPS_ATTR_NAME || - name == OWN_DECLARED_BORROWS_ATTR_NAME || name == OWN_DECLARED_PARAM_NAMES_ATTR_NAME) { + name == OWN_DECLARED_BORROWS_ATTR_NAME || name == OWN_DECLARED_LENDS_ATTR_NAME || + name == OWN_DECLARED_PARAM_NAMES_ATTR_NAME) { continue; } diff --git a/tslang/lib/TypeScript/MLIRGenFunctions.cpp b/tslang/lib/TypeScript/MLIRGenFunctions.cpp index 3af7ae0dd..1967c9551 100644 --- a/tslang/lib/TypeScript/MLIRGenFunctions.cpp +++ b/tslang/lib/TypeScript/MLIRGenFunctions.cpp @@ -1287,7 +1287,8 @@ namespace mlirgen auto globalName = isFullNamespaceName ? funcName.str() : getFullNamespaceName(funcName).str(); if (auto globalOp = theModule.lookupSymbol(globalName)) { - for (auto *name : {OWN_DECLARED_KEEPS_ATTR_NAME, OWN_DECLARED_BORROWS_ATTR_NAME, OWN_DECLARED_PARAM_NAMES_ATTR_NAME}) + for (auto *name : {OWN_DECLARED_KEEPS_ATTR_NAME, OWN_DECLARED_BORROWS_ATTR_NAME, OWN_DECLARED_LENDS_ATTR_NAME, + OWN_DECLARED_PARAM_NAMES_ATTR_NAME}) { if (auto attr = declared->getAttr(name)) { @@ -1479,8 +1480,10 @@ namespace mlirgen // The names a function body assigns: `x = ...` and every compound assignment, the targets of a destructuring // assignment, and the variable of `for (x of ...)` / `for (x in ...)`. Nested functions are walked as well, and // a destructuring target counts every name in it: naming one too many costs a retain and a release, missing - // one leaves a parameter pointing at freed memory. - static void collectAssignedNames(ts::Node body, llvm::StringSet<> &names) + // one leaves a parameter pointing at freed memory. `lengthNames`, when given, collects the names whose + // `.length` is assigned (`this` for `this.length = n`): resizing a string makes a new one (spec 22.3), so for a + // string that is an assignment too. + static void collectAssignedNames(ts::Node body, llvm::StringSet<> &names, llvm::StringSet<> *lengthNames = nullptr) { auto addTargets = [&](ts::Node target) { while (target == SyntaxKind::ParenthesizedExpression) @@ -1511,6 +1514,25 @@ namespace mlirgen if (ts::isAssignmentOperator(binaryExpression->operatorToken)) { addTargets(binaryExpression->left); + + auto left = binaryExpression->left; + if (lengthNames && left == SyntaxKind::PropertyAccessExpression) + { + auto propertyAccess = left.as(); + auto object = propertyAccess->expression; + if (propertyAccess->name == SyntaxKind::Identifier && + MLIRHelper::getName(propertyAccess->name.as()) == LENGTH_FIELD_NAME) + { + if (object == SyntaxKind::Identifier) + { + lengthNames->insert(MLIRHelper::getName(object.as())); + } + else if (object == SyntaxKind::ThisKeyword) + { + lengthNames->insert(THIS_NAME); + } + } + } } } else if (node == SyntaxKind::ForOfStatement) @@ -1540,33 +1562,41 @@ namespace mlirgen // copied into a local of the same name, which owns what it holds as any local does: a reference taken on the // caller's value here, handed over by each assignment, given back at the function's exit. // - // -mm=own refuses the assignment itself (OwnershipInferencePass), and a collector needs none of this. + // -mm=own: a string parameter the body assigns - `s = s + t`, or `this.length = n`, which resizes into a new + // string - starts a local of its own the same way, which own makes from a copy of the caller's string (spec + // 22.3) and which then owns each string assigned to it. Left as the parameter's slot, what was assigned to it + // had no owner: it leaked, and `return s` gave back a copy of it - read up to its first NUL, so the default + // library's `"".clone().resize(n)` returned a string of one byte that its caller wrote n into. Any other + // parameter own refuses to assign (OwnershipInferencePass), and a collector needs none of this. mlir::LogicalResult MLIRGenImpl::mlirGenFunctionOwnAssignedParams(mlir::Location location, FunctionLikeDeclarationBase functionLikeDeclarationBaseAST, FunctionPrototypeDOM::TypePtr funcProto, const GenContext &genContext) { - if (!compileOptions.isRefCounted()) + auto own = compileOptions.memoryModel == MemoryModelOwn; + if (!compileOptions.isRefCounted() && !own) { return mlir::success(); } llvm::StringSet<> assignedNames; + llvm::StringSet<> lengthNames; auto scanned = false; for (const auto ¶m : funcProto->getParams()) { - if (param->getBindingPattern() || !mth.ownsHeapMemory(location, param->getType())) + if (param->getBindingPattern() || !mth.ownsHeapMemory(location, param->getType()) || + (own && !mlir::isa(param->getType()))) { continue; } if (!scanned) { - collectAssignedNames(functionLikeDeclarationBaseAST->body, assignedNames); + collectAssignedNames(functionLikeDeclarationBaseAST->body, assignedNames, own ? &lengthNames : nullptr); scanned = true; } - if (!assignedNames.contains(param->getName())) + if (!assignedNames.contains(param->getName()) && !lengthNames.contains(param->getName())) { continue; } diff --git a/tslang/lib/TypeScript/MLIRGenImpl.h b/tslang/lib/TypeScript/MLIRGenImpl.h index 10e30a333..30e4b3399 100644 --- a/tslang/lib/TypeScript/MLIRGenImpl.h +++ b/tslang/lib/TypeScript/MLIRGenImpl.h @@ -3150,7 +3150,7 @@ class MLIRGenImpl } } - // `@keeps("p")`, `@borrows("this", "p")` (OWN_DECLARED_KEEPS_ATTR_NAME): the parameters they + // `@keeps("p")`, `@borrows("this", "p")`, `@lends("p")` (OWN_DECLARED_KEEPS_ATTR_NAME): the parameters they // name, as indices of the function's arguments - after the captured values, when its type has // an input more than it has parameters. mlir::LogicalResult processOwnParamMarkers(mlir::Location location, FunctionLikeDeclarationBase functionLikeDeclarationBaseAST, @@ -3159,12 +3159,14 @@ class MLIRGenImpl { auto result = mlir::success(); iterateDecorators(functionLikeDeclarationBaseAST, genContext, [&](StringRef name, SmallVector args) { - if (name != KEEPS_DECORATOR && name != BORROWS_DECORATOR) + if (name != KEEPS_DECORATOR && name != BORROWS_DECORATOR && name != LENDS_DECORATOR) { return; } - auto attrName = name == KEEPS_DECORATOR ? OWN_DECLARED_KEEPS_ATTR_NAME : OWN_DECLARED_BORROWS_ATTR_NAME; + auto attrName = name == KEEPS_DECORATOR ? OWN_DECLARED_KEEPS_ATTR_NAME + : name == BORROWS_DECORATOR ? OWN_DECLARED_BORROWS_ATTR_NAME + : OWN_DECLARED_LENDS_ATTR_NAME; if (llvm::any_of(attrs, [&](mlir::NamedAttribute &attr) { return attr.getName() == attrName; })) { emitError(location) << "'@" << name << "' is given twice"; @@ -3179,6 +3181,14 @@ class MLIRGenImpl return; } + // a result is a borrow of one argument + if (name == LENDS_DECORATOR && args.size() > 1) + { + emitError(location) << "'@" << name << "' names one parameter, the one the result is a borrow of"; + result = mlir::failure(); + return; + } + auto first = funcType && funcType.getInputs().size() == params.size() + 1 ? 1 : 0; SmallVector indices; for (auto arg : args) @@ -12590,6 +12600,11 @@ class MLIRGenImpl { methodInfo.ownBorrows.assign(borrows.asArrayRef().begin(), borrows.asArrayRef().end()); } + + if (auto lends = funcOp->getAttrOfType(OWN_DECLARED_LENDS_ATTR_NAME)) + { + methodInfo.ownLends = lends.asArrayRef().empty() ? -1 : lends.asArrayRef().front(); + } } if (propertyName.size() > 0) @@ -13960,7 +13975,7 @@ class MLIRGenImpl return attr ? attr.asArrayRef() : ArrayRef(); }; dp.print(funcProto->getNameWithoutNamespace(), elementNamespace, funcProto->getFuncType(), dllName, - marker(OWN_DECLARED_KEEPS_ATTR_NAME), marker(OWN_DECLARED_BORROWS_ATTR_NAME)); + marker(OWN_DECLARED_KEEPS_ATTR_NAME), marker(OWN_DECLARED_BORROWS_ATTR_NAME), marker(OWN_DECLARED_LENDS_ATTR_NAME)); declExports << ss.str().str(); } diff --git a/tslang/lib/TypeScript/OwnershipSignaturePass.cpp b/tslang/lib/TypeScript/OwnershipSignaturePass.cpp index 366c85480..cf2934247 100644 --- a/tslang/lib/TypeScript/OwnershipSignaturePass.cpp +++ b/tslang/lib/TypeScript/OwnershipSignaturePass.cpp @@ -452,7 +452,7 @@ class OwnershipSignaturePass : public mlir::PassWrapper factsLocal; llvm::DenseMap factsEffective; - // ---- Facts a signature says (`@keeps`, `@borrows`) ---- + // ---- Facts a signature says (`@keeps`, `@borrows`, `@lends`) ---- // // A function that says them has them whoever calls it: a caller in another module reads them // from the declaration it re-parses. Its body is checked against them instead (checkDeclared). @@ -469,7 +469,21 @@ class OwnershipSignaturePass : public mlir::PassWrapperhasAttr(OWN_DECLARED_KEEPS_ATTR_NAME) || funcOp->hasAttr(OWN_DECLARED_BORROWS_ATTR_NAME); + return funcOp->hasAttr(OWN_DECLARED_KEEPS_ATTR_NAME) || funcOp->hasAttr(OWN_DECLARED_BORROWS_ATTR_NAME) || + funcOp->hasAttr(OWN_DECLARED_LENDS_ATTR_NAME); + } + + // The facts `@keeps` and `@lends` say (Facts): what callers move in, and what the result is a + // borrow of. + static bool declaresCallerFacts(mlir::Operation *funcOp) + { + return funcOp->hasAttr(OWN_DECLARED_KEEPS_ATTR_NAME) || funcOp->hasAttr(OWN_DECLARED_LENDS_ATTR_NAME); + } + + static int declaredLends(mlir::Operation *funcOp) + { + auto lends = declared(funcOp, OWN_DECLARED_LENDS_ATTR_NAME); + return lends.empty() ? -1 : lends.front(); } // What every function a call names declares under `name`, when they all declare the same; @@ -987,22 +1001,57 @@ class OwnershipSignaturePass : public mlir::PassWrapperhasAttr(OWN_DECLARED_KEEPS_ATTR_NAME)) + if (funcOp->hasAttr(OWN_DECLARED_LENDS_ATTR_NAME) && funcOp->hasAttr(OWN_DECLARED_BORROWS_ATTR_NAME)) + { + funcOp.emitError("'") << funcOp.getSymName() + << "' cannot say both '@" LENDS_DECORATOR "' and '@" BORROWS_DECORATOR + "': its caller owns a result that borrows, and nothing of one it is lent"; + signalPassFailure(); + continue; + } + + if (declaresCallerFacts(funcOp)) { auto local = factsLocal.lookup(funcOp); - auto says = declared(funcOp, OWN_DECLARED_KEEPS_ATTR_NAME); - if (local.borrows >= 0) + auto keeps = declared(funcOp, OWN_DECLARED_KEEPS_ATTR_NAME); + auto lends = declaredLends(funcOp); + if (local.borrows != lends) { - funcOp.emitError("'") << funcOp.getSymName() - << "' returns a borrow of an argument, which '@" KEEPS_DECORATOR - "' cannot say; -mm=own cannot export that yet"; + if (lends < 0) + { + funcOp.emitError("'") << funcOp.getSymName() << "' returns a borrow of " + << describeParams(funcOp, local.borrows) + << ", which it has to say with '@" LENDS_DECORATOR "'"; + } + else if (local.borrows < 0) + { + funcOp.emitError("'@" LENDS_DECORATOR "' of '") + << funcOp.getSymName() << "' names " << describeParams(funcOp, lends) + << ", but its body does not return a borrow of it"; + } + else + { + funcOp.emitError("'@" LENDS_DECORATOR "' of '") + << funcOp.getSymName() << "' names " << describeParams(funcOp, lends) + << ", but its body returns a borrow of " << describeParams(funcOp, local.borrows); + } + signalPassFailure(); } - else if (llvm::ArrayRef(local.owned) != says) + else if (llvm::ArrayRef(local.owned) != keeps) { - funcOp.emitError("'@" KEEPS_DECORATOR "' of '") - << funcOp.getSymName() << "' names " << describeParams(funcOp, says) << ", but its body keeps " - << describeParams(funcOp, local.owned); + if (funcOp->hasAttr(OWN_DECLARED_KEEPS_ATTR_NAME)) + { + funcOp.emitError("'@" KEEPS_DECORATOR "' of '") + << funcOp.getSymName() << "' names " << describeParams(funcOp, keeps) << ", but its body keeps " + << describeParams(funcOp, local.owned); + } + else + { + funcOp.emitError("'") << funcOp.getSymName() << "' keeps " << describeParams(funcOp, local.owned) + << ", which it has to say with '@" KEEPS_DECORATOR "'"; + } + signalPassFailure(); } } @@ -1054,13 +1103,13 @@ class OwnershipSignaturePass : public mlir::PassWrapperhasAttr(name) != declaring->hasAttr(name) || declared(funcOp, name) != declared(declaring, name)) { funcOp.emitError("'") << funcOp.getSymName() << "' overrides '" << declaring.getSymName() - << "', and has to say the same '@" KEEPS_DECORATOR "' and '@" BORROWS_DECORATOR - "'"; + << "', and has to say the same '@" KEEPS_DECORATOR "', '@" BORROWS_DECORATOR + "' and '@" LENDS_DECORATOR "'"; signalPassFailure(); break; } @@ -1433,7 +1482,8 @@ class OwnershipSignaturePass : public mlir::PassWrapperhasAttr(OWN_DECLARED_KEEPS_ATTR_NAME)) + if (declaresCallerFacts(entry.second)) { auto keeps = declared(entry.second, OWN_DECLARED_KEEPS_ATTR_NAME); - factsEffective[entry.second] = Facts{-1, llvm::SmallVector(keeps.begin(), keeps.end())}; + factsEffective[entry.second] = + Facts{declaredLends(entry.second), llvm::SmallVector(keeps.begin(), keeps.end())}; } } @@ -1774,7 +1829,7 @@ class OwnershipSignaturePass : public mlir::PassWrapperhasAttr(OWN_DECLARED_KEEPS_ATTR_NAME) && factsEffective.erase(funcOp)) + funcOp && !declaresCallerFacts(funcOp) && factsEffective.erase(funcOp)) { changed = true; } diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index 6c1afd931..0e6a9659b 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -1527,6 +1527,10 @@ add_test(NAME test-compile-shared-import-own-markers-outlives COMMAND test-runner -shared -mm=own "${PROJECT_SOURCE_DIR}/test/tester/tests/import_own_markers_outlives.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_own_markers.ts") set_tests_properties(test-compile-shared-import-own-markers-outlives PROPERTIES PASS_REGULAR_EXPRESSION "borrows its generator's arguments and cannot be stored, returned or captured" FAIL_REGULAR_EXPRESSION "Stack dump|Assertion failed") +add_test(NAME test-compile-shared-import-own-markers-lent-outlives + COMMAND test-runner -shared -mm=own "${PROJECT_SOURCE_DIR}/test/tester/tests/import_own_markers_lent_outlives.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_own_markers.ts") +set_tests_properties(test-compile-shared-import-own-markers-lent-outlives PROPERTIES PASS_REGULAR_EXPRESSION "borrows a call's result but is used here after it may be released or overwritten" + FAIL_REGULAR_EXPRESSION "Stack dump|Assertion failed") # the markers mean nothing to the other models foreach(markers_mm gc rc) tslang_add_test(NAME test-jit-${markers_mm}-own-generator-locals COMMAND test-runner -jit -mm=${markers_mm} "${PROJECT_SOURCE_DIR}/test/tester/own/own_generator_locals.ts") @@ -3433,7 +3437,7 @@ foreach(own_test own_fresh_string own_fresh_array own_return_new own_try_local o own_string_copy_branch own_array_param_push own_declared_no_drops own_return_paths own_string_concat_element own_final_fresh_result own_final_field_lend own_null_narrowing_di - own_param_markers own_generator_locals) + own_param_markers own_generator_locals own_string_param_assigned own_lends) tslang_add_test(NAME test-jit-own-${own_test} COMMAND test-runner -jit -mm=own "${PROJECT_SOURCE_DIR}/test/tester/own/${own_test}.ts") tslang_add_test(NAME test-compile-own-${own_test} COMMAND test-runner -mm=own "${PROJECT_SOURCE_DIR}/test/tester/own/${own_test}.ts") # The promise: no counting at all in what own emits. @@ -3664,7 +3668,13 @@ set(own_error_cases "own_err_borrows_not_generator|'@borrows' of 'copyOf' names 'w', but it neither makes a generator whose state borrows its arguments nor returns one" "own_err_borrows_mixed_return|'@borrows' of 'pick' names 'w', but it neither makes a generator whose state borrows its arguments nor returns one" "own_err_marker_unknown_param|'@keeps' names 'd', which is not a parameter of this function" - "own_err_marker_override|'Derived.set' overrides 'Base.set', and has to say the same '@keeps' and '@borrows'" + "own_err_marker_override|'Derived.set' overrides 'Base.set', and has to say the same '@keeps', '@borrows' and '@lends'" + "own_err_lends_missing|'firstOf' could return a borrow or keep a parameter, but it can be called from another module. '@lends' or '@keeps' would say it in its signature" + "own_err_lends_mismatch|'@lends' of 'pick' names 'a', but its body returns a borrow of 'b'" + "own_err_lends_not_borrow|'@lends' of 'made' names 'l', but its body does not return a borrow of it" + "own_err_lends_and_borrows|'firstOf' cannot say both '@lends' and '@borrows'" + "own_err_lends_two_params|'@lends' names one parameter, the one the result is a borrow of" + "own_err_lends_outlives|borrows the result of 'firstOf' but is used here after it may be released or overwritten" "own_err_generator_method_this|'this value' is a parameter, kept here, but -mm=own cannot make the callers of 'S.chars' give it up" "own_err_string_call_keeps_twice|borrows a field and cannot be stored, returned or captured" "own_err_string_pushed_twice|is used here after its value was moved" diff --git a/tslang/test/tester/own/own_err_lends_and_borrows.ts b/tslang/test/tester/own/own_err_lends_and_borrows.ts new file mode 100644 index 000000000..574850644 --- /dev/null +++ b/tslang/test/tester/own/own_err_lends_and_borrows.ts @@ -0,0 +1,18 @@ +// -mm=own rejects: a result is either the caller's (`@borrows`) or lent (`@lends`), not both. +export class Node { + constructor(public v: number) {} +} + +export class List { + first: Node = new Node(1); +} + +@lends("l") +@borrows("l") +export function firstOf(l: List): Node { + return l.first; +} + +function main() { + print(firstOf(new List()).v); +} diff --git a/tslang/test/tester/own/own_err_lends_mismatch.ts b/tslang/test/tester/own/own_err_lends_mismatch.ts new file mode 100644 index 000000000..3bf5bafb8 --- /dev/null +++ b/tslang/test/tester/own/own_err_lends_mismatch.ts @@ -0,0 +1,17 @@ +// -mm=own rejects: `@lends` names `a`, but the body returns a borrow of `b`. +export class Node { + constructor(public v: number) {} +} + +export class List { + first: Node = new Node(1); +} + +@lends("a") +export function pick(a: List, b: List): Node { + return b.first; +} + +function main() { + print(pick(new List(), new List()).v); +} diff --git a/tslang/test/tester/own/own_err_lends_missing.ts b/tslang/test/tester/own/own_err_lends_missing.ts new file mode 100644 index 000000000..d8bda4022 --- /dev/null +++ b/tslang/test/tester/own/own_err_lends_missing.ts @@ -0,0 +1,17 @@ +// -mm=own rejects: an exported function returns a borrow of its argument, and a caller in another +// module could not know it; the note says how its signature would. +export class Node { + constructor(public v: number) {} +} + +export class List { + first: Node = new Node(1); +} + +export function firstOf(l: List): Node { + return l.first; +} + +function main() { + print(firstOf(new List()).v); +} diff --git a/tslang/test/tester/own/own_err_lends_not_borrow.ts b/tslang/test/tester/own/own_err_lends_not_borrow.ts new file mode 100644 index 000000000..823235950 --- /dev/null +++ b/tslang/test/tester/own/own_err_lends_not_borrow.ts @@ -0,0 +1,18 @@ +// -mm=own rejects: `@lends` says the result is a borrow of `l`, but the body makes a new one, which +// its callers would then never give back. +export class Node { + constructor(public v: number) {} +} + +export class List { + first: Node = new Node(1); +} + +@lends("l") +export function made(l: List): Node { + return new Node(l.first.v); +} + +function main() { + print(made(new List()).v); +} diff --git a/tslang/test/tester/own/own_err_lends_outlives.ts b/tslang/test/tester/own/own_err_lends_outlives.ts new file mode 100644 index 000000000..961173921 --- /dev/null +++ b/tslang/test/tester/own/own_err_lends_outlives.ts @@ -0,0 +1,21 @@ +// -mm=own rejects: `n` is lent by `firstOf(l)`, a field under `l`; the overwrite of `l.first` +// destroys it before `n.v` reads it. `@lends` is what tells the caller so. +export class Node { + constructor(public v: number) {} +} + +export class List { + first: Node = new Node(1); +} + +@lends("l") +export function firstOf(l: List): Node { + return l.first; +} + +function main() { + const l = new List(); + const n = firstOf(l); + l.first = new Node(9); + print(n.v); +} diff --git a/tslang/test/tester/own/own_err_lends_two_params.ts b/tslang/test/tester/own/own_err_lends_two_params.ts new file mode 100644 index 000000000..c80c13b45 --- /dev/null +++ b/tslang/test/tester/own/own_err_lends_two_params.ts @@ -0,0 +1,17 @@ +// -mm=own rejects: a result is a borrow of one argument. +export class Node { + constructor(public v: number) {} +} + +export class List { + first: Node = new Node(1); +} + +@lends("a", "b") +export function pick(a: List, b: List): Node { + return a.first; +} + +function main() { + print(pick(new List(), new List()).v); +} diff --git a/tslang/test/tester/own/own_lends.ts b/tslang/test/tester/own/own_lends.ts new file mode 100644 index 000000000..b6cfd836e --- /dev/null +++ b/tslang/test/tester/own/own_lends.ts @@ -0,0 +1,67 @@ +// -mm=own: `@lends("p")` says the result is a borrow of the argument `p` - the caller owns nothing +// of it, and it may not outlive `p`. An exported function says it in its signature, since a caller +// in another module cannot see the body; without the marker it is rejected ("it can be called from +// another module"). Under gc and rc the marker means nothing. +export class Node { + constructor(public v: number) {} +} + +@final +export class List { + first: Node = new Node(1); + second: Node = new Node(2); + + @lends("this") + head(): Node { + return this.first; + } +} + +@lends("l") +export function firstOf(l: List): Node { + return l.first; +} + +// a parameter other than the first +@lends("l") +export function secondOf(n: number, l: List): Node { + return n > 0 ? l.second : l.second; +} + +// keeps one argument and lends another +@keeps("n") +@lends("l") +export function swapFirst(n: Node, l: List): Node { + l.second = n; + return l.first; +} + +class Vec { + x: number; + + constructor(x: number) { + this.x = x; + } +} + +// Allocate over whatever has just been freed, so a use-after-free reads something else. +function churn() { + for (let i = 0; i < 64; i++) { + let filler = new Vec(999); + } +} + +function main() { + const l = new List(); + const h = l.head(); + const f = firstOf(l); + const s = secondOf(1, l); + churn(); + assert(h.v == 1 && f.v == 1 && s.v == 2); + + const kept = swapFirst(new Node(3), l); + churn(); + assert(kept.v == 1 && l.second.v == 3 && l.first.v == 1); + + print("done."); +} diff --git a/tslang/test/tester/own/own_string_param_assigned.ts b/tslang/test/tester/own/own_string_param_assigned.ts new file mode 100644 index 000000000..6f194d7aa --- /dev/null +++ b/tslang/test/tester/own/own_string_param_assigned.ts @@ -0,0 +1,115 @@ +// -mm=own: a string parameter the body assigns is a local of its own, made from a copy of the +// caller's string, which owns each string assigned to it and is given back or moved out. Left as +// the parameter's slot, what was assigned had no owner: it leaked, and `return s` gave back a copy +// of it, read up to its first NUL. The default library's `"".clone().resize(n)` (padStart, +// repeat, the regexp results) so returned a one-byte string, which its caller filled with n +// characters: a heap overrun, and the regexp results came back corrupted. +// +// The block has to move for the bug to show, so the strings grow well past their size, and the +// heap is churned before anything is read back. + +namespace __String { + function copyOf(this: string, add = ""): string { + return this + add; + } + + // the default library's `resize`: `this.length = n` assigns `this` a new string + function resized(this: string, newSize: index): string { + this.length = newSize + 1; + this[newSize] = 0; + return this; + } +} + +class Vec { + x: number; + + constructor(x: number) { + this.x = x; + } +} + +// Allocate over whatever has just been freed, so a use-after-free reads something else. +function churn() { + for (let i = 0; i < 64; i++) { + let filler = new Vec(999); + } +} + +function repeated(c: string, n: int): string { + let s = ""; + for (let i = 0; i < n; i++) { + s += c; + } + + return s; +} + +// The default library's shape: the string is the caller's temporary, borrowed by `this`. +function filled(n: int): string { + let s = "".copyOf().resized(n); + for (let i = 0; i < n; i++) { + s[i] = 65; + } + + return s; +} + +// `s.length = n` of a parameter named as one +function grown(s: string, n: int): string { + s.length = n + 1; + s[n] = 0; + for (let i = 0; i < n; i++) { + s[i] = 66; + } + + return s; +} + +function appended(s: string, t: string): string { + s = s + t; + return s; +} + +function appendedInLoop(s: string, n: int): string { + for (let i = 0; i < n; i++) { + s += "x"; + } + + return s; +} + +function appendedOnOnePath(s: string, add: boolean): string { + if (add) { + s = s + "!"; + } + + return s; +} + +function main() { + const a = filled(40); + churn(); + const b = filled(40); + churn(); + assert(a == repeated("A", 40)); + assert(b == a); + + const original = "cd"; + const g = grown(original, 40); + churn(); + assert(g == repeated("B", 40)); + assert(original == "cd"); + + const p = "ab"; + const q = appended(p, repeated("y", 40)); + churn(); + assert(q == "ab" + repeated("y", 40)); + assert(p == "ab"); + + assert(appendedInLoop("z", 40) == "z" + repeated("x", 40)); + assert(appendedOnOnePath("hi", true) == "hi!"); + assert(appendedOnOnePath("hi", false) == "hi"); + + print("done."); +} diff --git a/tslang/test/tester/tests/export_own_markers.ts b/tslang/test/tester/tests/export_own_markers.ts index 140d5a7bb..993fe2aca 100644 --- a/tslang/test/tester/tests/export_own_markers.ts +++ b/tslang/test/tester/tests/export_own_markers.ts @@ -1,4 +1,4 @@ -// The library side of @keeps and @borrows across a module boundary: the importer reads them from +// The library side of @keeps, @borrows and @lends across a module boundary: the importer reads them from // the declarations (__decls) and calls these knowing them. See import_own_markers.ts. export class Tag { constructor(public name: string) {} @@ -12,6 +12,16 @@ export class Holder { constructor(tag: Tag) { this.tag = tag; } + + @lends("this") + peek(): Tag { + return this.tag; + } +} + +@lends("h") +export function tagOf(h: Holder): Tag { + return h.tag; } @final diff --git a/tslang/test/tester/tests/import_own_markers.ts b/tslang/test/tester/tests/import_own_markers.ts index d0f949a33..0e3d51324 100644 --- a/tslang/test/tester/tests/import_own_markers.ts +++ b/tslang/test/tester/tests/import_own_markers.ts @@ -1,10 +1,14 @@ -// @keeps and @borrows from another module (export_own_markers.ts): the constructor keeps the tag the -// importer moves in, and the generators borrow the word the importer still owns. +// @keeps, @borrows and @lends from another module (export_own_markers.ts): the constructor keeps the +// tag the importer moves in, the generators borrow the word the importer still owns, and peek and +// tagOf lend the tag the holder still owns. import './export_own_markers' function main() { const h = new Holder(new Tag("kept")); assert(h.tag.name == "kept"); + assert(h.peek().name == "kept"); + const t = tagOf(h); + assert(t.name == "kept"); const w = new Word("abc"); let s = ""; diff --git a/tslang/test/tester/tests/import_own_markers_lent_outlives.ts b/tslang/test/tester/tests/import_own_markers_lent_outlives.ts new file mode 100644 index 000000000..36bae896a --- /dev/null +++ b/tslang/test/tester/tests/import_own_markers_lent_outlives.ts @@ -0,0 +1,10 @@ +// An error by design under -mm=own: @lends crossed the module boundary, so the tag tagOf lends is +// not read after the holder that owns it is overwritten. +import './export_own_markers' + +function main() { + let h = new Holder(new Tag("kept")); + const t = tagOf(h); + h = new Holder(new Tag("other")); + print(t.name); +}