diff --git a/changelog/dmd.fastdfa.borrowchecker.dd b/changelog/dmd.fastdfa.borrowchecker.dd new file mode 100644 index 000000000000..68dfcf636b87 --- /dev/null +++ b/changelog/dmd.fastdfa.borrowchecker.dd @@ -0,0 +1,38 @@ +The fast DFA engine has gained a borrow checker + +The fast DFA engine's escape analysis can now track borrows, protecting against +mutation of, and outliving, the borrowed-from object. + +A function signals that its return value borrows from a parameter (or from +`this`, when placed on the function) by annotating it with the +`__fastdfa_returnborrow` attribute: + +```d +enum __fastdfa_returnborrow; // declared in core.attributes + +int* getData(@__fastdfa_returnborrow ref int x) => &x; + +struct Buffer +{ + int* data; + + // return borrows from `this` + int* get() @__fastdfa_returnborrow => data; +} +``` + +The borrow checker enforces the following protections at the call site: + +* The owner of an active borrow cannot have a reference-typed value reassigned + while the borrow is alive. +* A borrow variable cannot be changed (reassigned, set to null, etc.) unless it + is declared inside the loop it is changed in. +* A borrow cannot outlive the variable it borrows from (including being returned + from a function when the borrow is of a stack local). +* The owner of an active borrow may not be passed to a function whose parameter + could mutate it; the parameter must be `const`/`immutable` where it reaches + the cell. +* Storing a borrow through a dereference is rejected in `@safe` code. + +This is an experimental feature, enabled with the `-preview=fastdfa` switch. It +is subject to change and may be removed at a later date. diff --git a/compiler/src/dmd/dfa/fast/analysis.d b/compiler/src/dmd/dfa/fast/analysis.d index c6f22c0e36eb..5b927cc39e3d 100644 --- a/compiler/src/dmd/dfa/fast/analysis.d +++ b/compiler/src/dmd/dfa/fast/analysis.d @@ -29,6 +29,7 @@ import dmd.func; import dmd.declaration; import dmd.astenums; import dmd.mtype; +import dmd.typesem; import dmd.root.array; import dmd.common.outbuffer; import core.stdc.stdio; @@ -179,6 +180,12 @@ struct DFAAnalyzer cctx.obj.derivedFrom = dfaCommon.makeInCellObject(source); return; case ParameterDFAInfo.EscapedRelationship.Borrows: + // The return value borrows from the source object, one level deep. + // Registration of the borrow against the ultimate cells happens + // when the return value is assigned to a variable. + cctx.obj = dfaCommon.makeObject(cctx.obj); + cctx.obj.isBorrow = true; + cctx.obj.borrowsFrom = source; return; } } @@ -200,6 +207,42 @@ struct DFAAnalyzer ParameterDFAInfo* paramInfo = list.each[i].paramInfo; DFAObject* sourceObject = list.each[i].lr.getContextObject; + // A by-ref argument of a value type has no object of its own, + // but the callee receives the cell of the variable itself. + if (sourceObject is null && paramInfo.isByRef) + { + if (DFAVar* ctxVar = list.each[i].lr.getContextVar) + sourceObject = dfaCommon.makeObject(ctxVar); + } + + // The argument object is only reliable immediately after the argument + // walk; the later convergence drops it from the lattice. The borrow + // checker needs it to dispatch the borrow-source relationship. + ParameterDFAInfo.Inferrable tempUserForSource = paramInfo.userSupplied; + const isBorrow = tempUserForSource.willEscape(-3) + == ParameterDFAInfo.EscapedRelationship.Borrows; + + if (sourceObject is null && isBorrow) + sourceObject = list.each[i].argObject; + + // A borrow from an object that is not a stack variable's cell (e.g. a + // class instance, or a by-ref argument with no object of its own) must + // be anchored to the variable holding the object, so that the owner + // object is guaranteed to outlive the borrow; the variable's lifetime + // bounds the object's lifetime. A dereference has created a child + // deref-var, so walk back (indirection aware) to the root variable to + // find the DFAObject of the `this` pointer. + if (isBorrow && (sourceObject is null || sourceObject.storageFor is null)) + { + if (DFAVar* ctxVar = list.each[i].lr.getContextVar) + { + ctxVar.visitIfReferenceToAnotherVar((DFAVar* root) { + if (sourceObject is null) + sourceObject = dfaCommon.makeObject(root); + }); + } + } + // I.e. could be because of meet due to unknown resolution of branches if (sourceObject is null) continue; @@ -215,6 +258,13 @@ struct DFAAnalyzer ulong escapesInto = paramInfo.inferred.escapesInto != 0 ? paramInfo.inferred.escapesInto : paramInfo.userSupplied.escapesInto; + + // Explicitly declared borrows win over inference: the body analysis can + // only infer ByValue/PointerTo relationships, which would silently + // downgrade a user declared `Borrows` (via the __fastdfa_returnborrow UDA). + ParameterDFAInfo.Inferrable tempUser = paramInfo.userSupplied; + if (tempUser.willEscape(-3) == ParameterDFAInfo.EscapedRelationship.Borrows) + escapesInto = paramInfo.userSupplied.escapesInto; int outputParamId = -3; if (escapesInto != 0) @@ -1081,6 +1131,8 @@ struct DFAAnalyzer // A function call argument, may initialize the parameter if its by-ref or if its the this pointer. this.onRead(lr, loc, isByRef, isByRef); + this.checkBorrowArgument(argListItem, loc); + DFAVar* ctx; DFAConsequence* cctx = lr.getContext(ctx); @@ -1119,7 +1171,7 @@ struct DFAAnalyzer newCctx.obj = dfaCommon.makeObject(rootCctx !is null ? rootCctx.obj : null); } - seeWrite(root, temp); + seeWrite(root, temp, loc); this.convergeExpression(temp, true); // now its all set to unknown @@ -1144,7 +1196,7 @@ struct DFAAnalyzer { DFAScope* sideEffectScope = dfaCommon.getSideEffectScope(); DFAScopeVar* scv = sideEffectScope.getScopeVar(root.storageFor); - seeWrite(root.storageFor, scv.lr); + seeWrite(root.storageFor, scv.lr, loc); } }); } @@ -1173,6 +1225,60 @@ struct DFAAnalyzer argListItem.lr = lr; } + /*********************************************************** + * Borrow checker: an owner with an active borrow may only be + * passed to a function whose parameter cannot mutate it. + * + * The parameter must be const/immutable where it reaches the cell. + * The borrow source parameter itself is exempt, allowing multiple + * borrows of one owner. + */ + void checkBorrowArgument(DFAArgumentList.Each* argListItem, ref Loc loc) + { + ParameterDFAInfo* paramInfo = argListItem.paramInfo; + if (paramInfo is null || argListItem.paramType is null) + return; + + // The borrow source parameter is exempt: it exists to create borrows, + // so multiple borrows of one owner are allowed. + ParameterDFAInfo.Inferrable tempUser = paramInfo.userSupplied; + if (tempUser.willEscape(-3) == ParameterDFAInfo.EscapedRelationship.Borrows) + return; + + bool canMutate; + if (paramInfo.isByRef) + canMutate = !(argListItem.paramType.isConst || argListItem.paramType.isImmutable); + else + { + // A by-value reference type parameter can still reach the cell + // through the pointee; by-value value types are copies. + if (!(argListItem.paramType.isTypePointer || argListItem.paramType.isTypeDArray + || argListItem.paramType.isTypeAArray || argListItem.paramType.isTypeClass)) + return; + + // nextOf() may be null (e.g. a class with no base class). + auto next = argListItem.paramType.nextOf(); + canMutate = next is null || !(next.isConst || next.isImmutable); + } + + if (!canMutate) + return; + + DFABorrowEntry* entry; + + if (DFAObject* argObj = argListItem.argObject) + { + dfaCommon.resolveBorrowCells(argObj, (cellVar, cellObj) { + if (entry is null) + entry = dfaCommon.findBorrowEntry(cellObj); + }); + } + + if (entry !is null) + reporter.onBorrowOwnerPassedToMutatingFunction(entry, + argListItem.paramIdent !is null ? argListItem.paramIdent.toChars : null, loc); + } + void transferAssert(DFALatticeRef lr, ref Loc loc, bool ignoreWriteCount, bool dueToConditional = false) { @@ -1468,6 +1574,35 @@ struct DFAAnalyzer if (!construct && noLR) assignToCtx.markUnmodellable(); } + + // Borrow checker: handle borrows entering or leaving variables. + if (!wasDereferenced) + { + DFAConsequence* lhsCctx = assignTo.getContext; + + if (lhsCctx !is null && lhsCctx.obj !is null && lhsCctx.obj.isBorrow + && !construct) + { + // The variable currently holds a borrow; changing it is only + // allowed for borrow variables declared inside the loop. + const loopDepth = dfaCommon.lastLoopyLabel.depth; + + if (loopDepth == 1 + || assignToCtx.youngestLifeTimeAllowedDepth <= loopDepth) + reporter.onBorrowVariableReassignment(assignToCtx, loc); + else + dfaCommon.removeBorrowEntries(assignToCtx); + } + + if (lrCctx !is null && lrCctx.obj !is null && lrCctx.obj.isBorrow) + this.registerBorrows(assignToCtx, lrCctx.obj, loc); + } + else if (lrCctx !is null && lrCctx.obj !is null && lrCctx.obj.isBorrow) + { + // *p = borrow(...); + // The borrow lives in memory, not a tracked variable. + reporter.onBorrowStoredThroughDereference(loc); + } } else { @@ -1566,7 +1701,7 @@ struct DFAAnalyzer if (rootCount == 1) { exactlyOneRoot = true; - this.seeWrite(firstRoot, ret); + this.seeWrite(firstRoot, ret, loc); DFAConsequence* c2 = ret.addConsequence(firstRoot); c2.truthiness = retCctx.truthiness; @@ -1602,7 +1737,7 @@ struct DFAAnalyzer } } - seeWrite(assignToCtx, ret); + seeWrite(assignToCtx, ret, loc); ret.setContext(assignToCtx); DFAScopeVar* scv = this.convergeExpression(ret.copy, true); @@ -2621,7 +2756,7 @@ struct DFAAnalyzer DFAVar* ctx = lr.getContextVar; if (ctx !is null) - this.seeWrite(ctx, lr); + this.seeWrite(ctx, lr, loc); } private: @@ -2929,7 +3064,63 @@ private: couldBeUnknown = true; } - void seeWrite(DFAVar* assignTo, ref DFALatticeRef from) + void registerBorrows(DFAVar* borrower, DFAObject* borrowObj, ref Loc loc) + { + borrowObj.walkBorrowSources((DFAObject* node) { + DFAObject* source = node.borrowsFrom; + assert(source !is null); + + // Direct owner, one level deep: when the source is itself a borrow, + // the owner is the variable holding the borrowed value. + DFAVar* owner; + if (source.isBorrow) + { + owner = source.holderVar; + if (owner is null) + { + dfaCommon.resolveBorrowCells(source, (cellVar, cellObj) { + if (owner is null) + owner = cellVar; + }); + } + } + + bool reportedOutlives; + dfaCommon.resolveBorrowCells(source, (cellVar, cellObj) { + if (!cellObj.onTheStack) + return; + + if (owner is null) + owner = cellVar; + + if (!reportedOutlives && owner !is null) + { + // Returning a borrow of a stack local escapes the function + // with a dangling pointer. The depth comparison alone cannot + // catch this, since the return variable shares the function + // scope with the local. + if (borrower is dfaCommon.getReturnVariable && owner.var !is null + && !owner.var.isParameter()) + { + reportedOutlives = true; + reporter.onBorrowOutlivesOwner(borrower, owner, loc); + } + else if (borrower.youngestLifeTimeAllowedDepth < owner.youngestLifeTimeAllowedDepth) + { + reportedOutlives = true; + reporter.onBorrowOutlivesOwner(borrower, owner, loc); + } + } + + if (!reportedOutlives) + dfaCommon.registerBorrow(borrower, cellObj, loc); + }); + + node.holderVar = borrower; + }); + } + + void seeWrite(DFAVar* assignTo, ref DFALatticeRef from, ref Loc loc) { version (none) { @@ -2953,6 +3144,17 @@ private: DFAConsequence* c = from.addConsequence(root); c.writeOnVarAtThisPoint = root.writeCount; + + // Borrow checker: reassigning a reference-type owner of an active + // borrow could invalidate the borrow. Mutating a basic type's value + // (e.g. `int x; x = 5;`) never does, so only reference-type owners + // are checked here. + if (root.storageFor !is null && root.var !is null + && isTypeNullable(root.var.type)) + { + if (DFABorrowEntry* entry = dfaCommon.findBorrowEntry(root.storageFor)) + reporter.onBorrowOwnerMutation(root, entry, loc); + } }); version (none) diff --git a/compiler/src/dmd/dfa/fast/expression.d b/compiler/src/dmd/dfa/fast/expression.d index 5c54a0d376c6..696871fc78da 100644 --- a/compiler/src/dmd/dfa/fast/expression.d +++ b/compiler/src/dmd/dfa/fast/expression.d @@ -2635,6 +2635,16 @@ struct ExpressionWalker }); DFALatticeRef argExp = this.walk(arg); + list.each[i].argObject = argExp.getContextObject; + + if (toCallFunctionType !is null && toCallFunctionType.parameterList.parameters !is null + && argOffset < toCallFunctionType.parameterList.parameters.length) + { + auto param = (*toCallFunctionType.parameterList.parameters)[argOffset]; + list.each[i].paramType = param.type; + list.each[i].paramIdent = param.ident; + } + this.seeFunctionCallArgument(argExp, &list.each[i], toCallFunction, loc); if (dfaCommon.currentDFAScope.controlFlowJumped) diff --git a/compiler/src/dmd/dfa/fast/report.d b/compiler/src/dmd/dfa/fast/report.d index e6a3f35590ef..8cd3f9b8e5d7 100644 --- a/compiler/src/dmd/dfa/fast/report.d +++ b/compiler/src/dmd/dfa/fast/report.d @@ -397,6 +397,89 @@ struct DFAReporter } } + /*********************************************************** + * Reports an error when the owner of an active borrow is mutated. + * + * Params: + * owner = The variable being mutated (the owner of the borrow). + * entry = The borrow that protects the owner. + * loc = location mutation + */ + void onBorrowOwnerMutation(DFAVar* owner, DFABorrowEntry* entry, ref const Loc loc) + { + errorSink.error(loc, "Cannot mutate the owner of an active borrow"); + + if (owner !is null && owner.var !is null) + errorSink.errorSupplemental(owner.var.loc, "For variable `%s`", owner.var.ident.toChars); + + if (entry !is null) + errorSink.errorSupplemental(entry.loc, "Borrowed here"); + } + + /*********************************************************** + * Reports an error when a borrow would outlive the variable it + * borrows from (the direct source, one level deep). + */ + void onBorrowOutlivesOwner(DFAVar* borrower, DFAVar* owner, ref const Loc loc) + { + errorSink.error(loc, "A borrow cannot outlive the variable it borrows from"); + + if (owner !is null && owner.var !is null) + errorSink.errorSupplemental(owner.var.loc, "For variable `%s`", owner.var.ident.toChars); + + if (borrower !is null && borrower.var !is null) + errorSink.errorSupplemental(borrower.var.loc, + "The borrow is stored in variable `%s`", borrower.var.ident.toChars); + } + + /*********************************************************** + * Reports an error when a borrow variable is changed outside of a + * loop, or is declared outside the loop it is changed in. + */ + void onBorrowVariableReassignment(DFAVar* borrower, ref const Loc loc) + { + errorSink.error(loc, "Cannot change a borrow variable declared outside of a loop"); + + if (borrower !is null && borrower.var !is null) + errorSink.errorSupplemental(borrower.var.loc, + "For variable `%s`", borrower.var.ident.toChars); + } + + /*********************************************************** + * Reports an error when a borrow is stored through a dereference. + * + * This is allowed in @system code (no borrow checker protections + * apply there), but not in @safe code. + */ + void onBorrowStoredThroughDereference(ref const Loc loc) + { + auto tf = dfaCommon.currentFunction !is null + ? dfaCommon.currentFunction.type.isTypeFunction : null; + if (tf is null || tf.trust != TRUST.safe) + return; + + errorSink.error(loc, "Cannot store a borrow through a dereference in @safe code"); + } + + /*********************************************************** + * Reports an error when the owner of an active borrow is passed + * to a function whose parameter may mutate it. + */ + void onBorrowOwnerPassedToMutatingFunction(DFABorrowEntry* entry, + const(char)* paramName, ref const Loc loc) + { + errorSink.error(loc, + "Cannot pass the owner of an active borrow to a function that may mutate it"); + + if (paramName !is null) + errorSink.errorSupplemental(loc, "Parameter `%s` must be const or immutable", paramName); + else + errorSink.errorSupplemental(loc, "The parameter must be const or immutable"); + + if (entry !is null) + errorSink.errorSupplemental(entry.loc, "Borrowed here"); + } + void onThrowEscape(DFAObject* obj, ref const Loc loc) { if (obj is null) diff --git a/compiler/src/dmd/dfa/fast/structure.d b/compiler/src/dmd/dfa/fast/structure.d index 84d8da8aa33a..7fb20c691d45 100644 --- a/compiler/src/dmd/dfa/fast/structure.d +++ b/compiler/src/dmd/dfa/fast/structure.d @@ -724,6 +724,8 @@ struct DFACommon ret.onTheStack = (base1 !is null && base1.onTheStack) || (base2 !is null && base2 .onTheStack); + ret.isBorrow = (base1 !is null && base1.isBorrow) || (base2 !is null && base2.isBorrow); + ret.minimumDeclaredAtDepth = -1; if (base1 !is null && base1.minimumDeclaredAtDepth > -1) ret.minimumDeclaredAtDepth = base1.minimumDeclaredAtDepth; @@ -752,6 +754,181 @@ struct DFACommon return ret; } + /*********************************************************** + * Finds the scope that a variable was declared in. + * + * Depths strictly decrease up the scope chain, and the variable + * must be in scope at the current position, so walking up until + * the depth matches the variable's youngest allowed lifetime is + * unambiguous. + * + * Returns: + * The declaring scope of the variable. + */ + DFAScope* findDeclaringScope(DFAVar* var) + { + assert(var !is null); + + DFAScope* sc = this.currentDFAScope; + assert(sc !is null); + + while (sc.depth > var.youngestLifeTimeAllowedDepth) + { + sc = sc.parent; + assert(sc !is null); + } + + return sc; + } + + /*********************************************************** + * Registers that `borrower` holds a borrow of the cell `cell`. + * + * The entry is attached to the declaring scope of the borrower and + * deduplicated per (borrower, cell) pair. It lives until that scope + * is popped, at which point it is freed with the scope. + */ + void registerBorrow(DFAVar* borrower, DFAObject* cell, ref const Loc loc) + { + DFAScope* sc = findDeclaringScope(borrower); + + DFABorrowEntry* entry = sc.borrowEntries; + while (entry !is null) + { + if (entry.borrower is borrower && entry.borrowedFrom is cell) + return; + + entry = entry.next; + } + + entry = allocator.makeBorrowEntry(borrower, cell, loc); + entry.next = sc.borrowEntries; + sc.borrowEntries = entry; + } + + /*********************************************************** + * Removes all borrow entries of the given borrower from its + * declaring scope. + * + * Callers must ensure the borrower currently holds a borrow on + * this path (the lattice is path-sensitive), so entries registered + * from sibling branches are never removed. + */ + void removeBorrowEntries(DFAVar* borrower) + { + DFAScope* sc = findDeclaringScope(borrower); + + DFABorrowEntry** bucket = &sc.borrowEntries; + while (*bucket !is null) + { + if ((*bucket).borrower is borrower) + { + DFABorrowEntry* toFree = *bucket; + *bucket = toFree.next; + allocator.free(toFree); + } + else + bucket = &(*bucket).next; + } + } + + /*********************************************************** + * Finds a borrow entry for the given cell on the current scope chain. + * + * Only scopes that are still live (ancestors of the current scope) + * are consulted, so a borrow that has ended no longer protects its + * owner. + * + * Returns: + * The first matching entry, or null. + */ + DFABorrowEntry* findBorrowEntry(DFAObject* cell) + { + if (cell is null) + return null; + + for (DFAScope* sc = this.currentDFAScope; sc !is null; sc = sc.parent) + { + DFABorrowEntry* entry = sc.borrowEntries; + while (entry !is null) + { + if (entry.borrowedFrom is cell) + return entry; + + entry = entry.next; + } + } + + return null; + } + + /*********************************************************** + * Is the cell currently borrowed from on this path? + */ + bool isCellBorrowed(DFAObject* cell) + { + return findBorrowEntry(cell) !is null; + } + + /*********************************************************** + * Resolves an object graph to the ultimate cell objects it is + * derived from. + * + * Only used by the borrow checker, this is additive and does not + * alter the behaviour of the existing walks (`walkIndirection`, + * `walkRoots`). Traverses `base1`, `base2`, `derivedFrom`, `inCell` + * and `borrowsFrom` chains, delivering each root that is a cell of + * a variable (i.e. `storageFor` is set on a variable without a base). + */ + void resolveBorrowCells(DFAObject* obj, + scope void delegate(DFAVar* cellVar, DFAObject* cellObj) del) + { + void resolve(DFAObject* current) + { + while (current !is null + && (current.base1 !is null || current.derivedFrom !is null + || current.inCell !is null || current.borrowsFrom !is null)) + { + if (current.derivedFrom !is null) + { + if (current.base1 !is null) + resolve(current.derivedFrom); + else + { + current = current.derivedFrom; + continue; + } + } + + if (current.borrowsFrom !is null) + { + if (current.base1 !is null) + resolve(current.borrowsFrom); + else + { + current = current.borrowsFrom; + continue; + } + } + + if (current.inCell !is null) + resolve(current.inCell); + + if (current.base2 !is null) + resolve(current.base2); + + current = current.base1; + } + + if (current !is null && current.storageFor !is null + && current.storageFor.var !is null && !current.storageFor.haveBase) + del(current.storageFor, current); + } + + if (obj !is null) + resolve(obj); + } + DFAArgumentListRef makeArgumentListRef(size_t countArgs) { assert(countArgs >= 0); @@ -1078,6 +1255,7 @@ struct DFAAllocator DFAArgumentList* freelistarglist; DFALattice* freelistlattice; DFAConsequence* freelistconsequence; + DFABorrowEntry* freelistborrow; Region* currentRegion; size_t regionUsed; @@ -1267,6 +1445,26 @@ struct DFAAllocator return ret; } + DFABorrowEntry* makeBorrowEntry(DFAVar* borrower, DFAObject* borrowedFrom, ref const Loc loc) + { + DFABorrowEntry* ret = allocInternal!DFABorrowEntry(freelistborrow); + ret.borrower = borrower; + ret.borrowedFrom = borrowedFrom; + ret.loc = loc; + return ret; + } + + void free(DFABorrowEntry* s) + { + s.borrower = null; + s.borrowedFrom = null; + s.next = null; + s.loc = Loc.init; + + s.listnext = freelistborrow; + freelistborrow = s; + } + DFACaseState* makeCaseState(Statement caseStatement) { DFACaseState* ret = allocInternal!DFACaseState(freelistcase); @@ -1311,6 +1509,15 @@ struct DFAAllocator s.beforeScopeState = DFAScopeRef.init; s.afterScopeState = DFAScopeRef.init; + DFABorrowEntry* borrowEntry = s.borrowEntries; + while (borrowEntry !is null) + { + DFABorrowEntry* next = borrowEntry.next; + this.free(borrowEntry); + borrowEntry = next; + } + s.borrowEntries = null; + foreach (ref bucket; s.buckets) { DFAScopeVar* next; @@ -1333,6 +1540,9 @@ struct DFAAllocator { list.each[i].lr = DFALatticeRef.init; list.each[i].paramInfo = null; + list.each[i].argObject = null; + list.each[i].paramType = null; + list.each[i].paramIdent = null; } list.listnext = freelistarglist; @@ -1986,6 +2196,15 @@ struct DFAObject // This is in the following cell i.e. pointer to array member or pointer to variable DFAObject* inCell; + // This object is a borrow of the following object (the direct source, one level deep) + DFAObject* borrowsFrom; + + // This object is the result of a borrow call, or combines one + bool isBorrow; + + // The variable currently holding this borrow value (most recent holder wins) + DFAVar* holderVar; + // Having a base means it could be one of these. // We do not know which one object specifically this will have at runtime. DFAObject* base1; @@ -2129,6 +2348,36 @@ struct DFAObject if (findConstraintsDel !is null) findConstraints(&this, 0, -1); } + + /*********************************************************** + * Visits every borrow node in this object's graph. + * + * A borrow node is an object that has `borrowsFrom` set (it is the + * result of a borrow call, or a combine base of one). Only used by + * the borrow checker, this is additive and does not alter the + * behaviour of the existing walks. + * + * Params: + * del = Receives each borrow node. + */ + void walkBorrowSources(scope void delegate(DFAObject* borrowNode) del) + { + void walk(DFAObject* current) + { + while (current !is null) + { + if (current.base2 !is null) + walk(current.base2); + + if (current.borrowsFrom !is null) + del(current); + + current = current.base1; + } + } + + walk(&this); + } } struct DFAScopeRef @@ -2308,6 +2557,34 @@ struct DFAScopeRef } } +/*********************************************************** + * Represents a borrow of a cell object by a variable. + * + * Entries live on the declaring scope of the borrower variable and + * are only consulted while that scope is on the current scope chain, + * so a borrow stops protecting its owner once its scope is popped. + * Only used by the borrow checker. + */ +struct DFABorrowEntry +{ + private + { + DFABorrowEntry* listnext; + } + + /// The variable holding the borrow. + DFAVar* borrower; + + /// The cell object that is borrowed from. + DFAObject* borrowedFrom; + + /// Where the borrow was created, for error supplements. + Loc loc; + + /// Next entry in the scope's list. + DFABorrowEntry* next; +} + /*********************************************************** * Represents a specific region of code execution (a scope). * @@ -2332,6 +2609,9 @@ struct DFAScope DFAScopeVar*[16] buckets; int depth; + /// Borrow entries created by variables declared in this scope. + DFABorrowEntry* borrowEntries; + uint controlFlow; bool isLoopyLabel; // Is a loop or label bool isLoopyLabelKnownToHaveRun; // was the loopy label guaranteed to have at least one iteration? @@ -2766,6 +3046,16 @@ struct DFAArgumentList { DFALatticeRef lr; ParameterDFAInfo* paramInfo; + + /// The object of the argument as computed immediately after its walk, + /// which later convergence drops from the lattice. Used by the borrow + /// checker for the borrow-source dispatch. + DFAObject* argObject; + + /// The parameter type and identifier, captured at the call site for the + /// borrow checker's const/immutable parameter check. + Type paramType; + Identifier paramIdent; } } diff --git a/compiler/src/dmd/dfa/utils.d b/compiler/src/dmd/dfa/utils.d index 60f1d46d5560..48d62dd49e50 100644 --- a/compiler/src/dmd/dfa/utils.d +++ b/compiler/src/dmd/dfa/utils.d @@ -15,10 +15,75 @@ import dmd.mtype; import dmd.visitor; import dmd.identifier; import dmd.expression; -import dmd.typesem : isFloating; +import dmd.typesem : isFloating, toBasetype; import dmd.func; +import dmd.attrib; import core.stdc.stdio; +/*********************************************************** + * Is the given UDA expression the `__fastdfa_returnborrow` marker? + * + * The marker is used to tell the borrow checker that the return value + * of a function borrows from a parameter (or `this`, when the UDA is + * placed on the function). It is written as `enum __fastdfa_returnborrow;` + * and applied as `@__fastdfa_returnborrow`. + */ +private bool isBorrowUDA(const(Expression) e) +{ + import core.stdc.string : strcmp; + + if (e is null) + return false; + + const(char)* name; + + if (auto ie = e.isIdentifierExp) + name = ie.ident.toChars; + else if (auto te = e.isTypeExp) + { + auto bt = (cast() te.type).toBasetype(); + if (auto ts = bt.isTypeStruct) + { + if (ts.sym !is null && ts.sym.ident !is null) + name = ts.sym.ident.toChars; + } + else if (auto ts = bt.isTypeEnum) + { + if (ts.sym !is null && ts.sym.ident !is null) + name = ts.sym.ident.toChars; + } + else if (auto ti = bt.isTypeIdentifier) + { + if (ti.ident !is null) + name = ti.ident.toChars; + } + } + else if (auto sl = e.isStructLiteralExp) + { + if (sl.sd !is null && sl.sd.ident !is null) + name = sl.sd.ident.toChars; + } + + return name !is null && strcmp(name, "__fastdfa_returnborrow") == 0; +} + +/*********************************************************** + * Does the symbol carry the `__fastdfa_returnborrow` UDA? + */ +private bool hasBorrowUDA(const(UserAttributeDeclaration) uad) +{ + if (uad is null || uad.atts is null) + return false; + + foreach (const(Expression) e; *uad.atts) + { + if (isBorrowUDA(e)) + return true; + } + + return false; +} + /// Ensure that a function declaration is properly attributed for the fast DFA engine. ParametersDFAInfo* ensureDFAParameters(FuncDeclaration fd) { @@ -55,6 +120,11 @@ ParametersDFAInfo* ensureDFAParameters(FuncDeclaration fd) fd.parametersDFAInfo.thisPointer.userSupplied.escapeIntoNothing = true; } + // Function-level __fastdfa_returnborrow means the return borrows from `this`. + if (hasBorrowUDA(fd.userAttribDecl)) + fd.parametersDFAInfo.thisPointer.userSupplied.willEscape(-3, + ParameterDFAInfo.EscapedRelationship.Borrows); + { // Getting the actual number of parameters is all over the place, depending on the stage of compilation. @@ -88,6 +158,13 @@ ParametersDFAInfo* ensureDFAParameters(FuncDeclaration fd) : ParameterDFAInfo.EscapedRelationship.ByValue); if ((stc & STC.scope_) && (stc & (STC.scopeinferred | STC.returnScope | STC.returnRef)) == 0) paramDFAInfo.userSupplied.escapeIntoNothing = true; + + // Parameter-level __fastdfa_returnborrow means the return borrows + // from this parameter. + if (hasBorrowUDA(param.userAttribDecl) + || hasBorrowUDA(vd !is null ? vd.userAttribDecl : null)) + paramDFAInfo.userSupplied.willEscape(-3, + ParameterDFAInfo.EscapedRelationship.Borrows); } } @@ -127,6 +204,12 @@ void ensureDFAParameter(int id, FuncDeclaration fd, TypeFunction tf, : ParameterDFAInfo.EscapedRelationship.ByValue); if ((stc & STC.scope_) && (stc & (STC.scopeinferred | STC.returnScope | STC.returnRef)) == 0) paramDFAInfo.userSupplied.escapeIntoNothing = true; + + // Parameter-level __fastdfa_returnborrow means the return borrows + // from this parameter. + if (hasBorrowUDA((*tf.parameterList.parameters)[id].userAttribDecl)) + paramDFAInfo.userSupplied.willEscape(-3, + ParameterDFAInfo.EscapedRelationship.Borrows); } } diff --git a/compiler/test/compilable/fastdfa.d b/compiler/test/compilable/fastdfa.d index 3ccc31b38312..fcc5ebdb17d8 100644 --- a/compiler/test/compilable/fastdfa.d +++ b/compiler/test/compilable/fastdfa.d @@ -1274,3 +1274,160 @@ void checkCtfeOnly() @__ctfe int* ptr; int val = *ptr; } + +/****************** Borrow checker (ok) ******************/ + +@system: + +enum __fastdfa_returnborrow; + +int* borrowFn(@__fastdfa_returnborrow int* x) @trusted { return x; } +int** borrowFn2(@__fastdfa_returnborrow int** x) @trusted { return x; } + +struct BorrowS { int field; } +struct BorrowDtor { ~this() {} int field; } + +struct BorrowStruct +{ + int* p; + + int** get() @__fastdfa_returnborrow @trusted { return &this.p; } +} + +class BorrowClass +{ + int* p; + + int** get() @__fastdfa_returnborrow @trusted { return &this.p; } +} + +void borrowTakeConst(const(int)* p) @safe {} +void methodTakeConst(const(int**) p) @safe {} + +void borrowOk1() +{ + int x; + { + int* b = borrowFn(&x); + int v = *b; + } + x = 5; // ok, borrow scope ended +} + +void borrowOk2() +{ + int x; + { + int* b = borrowFn(&x); + } + x = 5; // ok +} + +void borrowOk3() +{ + int x; + int* b1 = borrowFn(&x); + int* b2 = borrowFn(&x); // ok, multiple borrows +} + +void borrowOk4() +{ + int x; + { + int* b = borrowFn(&x); + int* c = b; // ok, propagation + int v = *c; + } + x = 5; +} + +void borrowOk5() +{ + int x; + int* b = borrowFn(&x); + borrowTakeConst(&x); // ok +} + +void borrowOk6() +{ + BorrowS s; + { + int* b = borrowFn(&s.field); + } + s.field = 5; // ok +} + +void borrowOk7() +{ + int x; + int* b = borrowFn(&x); + x = 5; // ok: mutating a basic type value doesn't invalidate the borrow +} + +void borrowOk8() +{ + int x; + int* b = borrowFn(&x); + int* c = borrowFn(&x); + x = 5; // ok: basic type mutation +} + +void borrowOk9() +{ + BorrowS s; + int* b = borrowFn(&s.field); + s.field = 5; // ok: basic type field mutation +} + +void borrowSysOk(int** p) @system +{ + int x; + *p = borrowFn(&x); // ok +} + +void borrowLoopOk1() +{ + int x; + for (int i = 0; i < 2; ++i) + { + int* b = borrowFn(&x); // loop-local borrow + b = null; // ok, loop-local change allowed + } + x = 5; // ok +} + +void borrowLoopOk2() +{ + int x; + int* b; + for (int i = 0; i < 2; ++i) + { + b = borrowFn(&x); // first assignment, b declared outside loop + } + // b holds a borrow of x here; no mutation, so ok +} + +void methodPassOk() +{ + BorrowStruct s; + int** b = s.get(); + methodTakeConst(&s.p); // ok: const parameter +} + +void methodOk() +{ + BorrowStruct s; + { + int** b = s.get(); + int v = **b; + } + s.p = null; // ok: borrow scope ended +} + +void classOk() +{ + BorrowClass c = new BorrowClass; + int** b = c.get(); // ok: heap owner, no lifetime constraint +} + +/****************** End borrow checker (ok) ******************/ diff --git a/compiler/test/fail_compilation/fastdfa.d b/compiler/test/fail_compilation/fastdfa.d index 6f76f9346247..5484f353f544 100644 --- a/compiler/test/fail_compilation/fastdfa.d +++ b/compiler/test/fail_compilation/fastdfa.d @@ -48,6 +48,34 @@ fail_compilation/fastdfa.d(1350): Error: Expression reads from an uninitialized fail_compilation/fastdfa.d(1349): For variable `foo` fail_compilation/fastdfa.d(1361): Error: Dereference on null variable `foo` fail_compilation/fastdfa.d(1368): Error: Dereference on null object +fail_compilation/fastdfa.d(1399): Error: Cannot mutate the owner of an active borrow +fail_compilation/fastdfa.d(1397): For variable `p` +fail_compilation/fastdfa.d(1398): Borrowed here +fail_compilation/fastdfa.d(1406): Error: Cannot change a borrow variable declared outside of a loop +fail_compilation/fastdfa.d(1405): For variable `b` +fail_compilation/fastdfa.d(1411): Error: Cannot store a borrow through a dereference in @safe code +fail_compilation/fastdfa.d(1418): Error: Cannot pass the owner of an active borrow to a function that may mutate it +fail_compilation/fastdfa.d(1418): Parameter `p` must be const or immutable +fail_compilation/fastdfa.d(1417): Borrowed here +fail_compilation/fastdfa.d(1425): Error: Cannot pass the owner of an active borrow to a function that may mutate it +fail_compilation/fastdfa.d(1425): Parameter `p` must be const or immutable +fail_compilation/fastdfa.d(1424): Borrowed here +fail_compilation/fastdfa.d(1433): Error: A borrow cannot outlive the variable it borrows from +fail_compilation/fastdfa.d(1432): For variable `s` +fail_compilation/fastdfa.d(1430): The borrow is stored in variable `b` +fail_compilation/fastdfa.d(1441): Error: Cannot pass the owner of an active borrow to a function that may mutate it +fail_compilation/fastdfa.d(1441): Parameter `p` must be const or immutable +fail_compilation/fastdfa.d(1440): Borrowed here +fail_compilation/fastdfa.d(1449): Error: A borrow cannot outlive the variable it borrows from +fail_compilation/fastdfa.d(1448): For variable `x` +fail_compilation/fastdfa.d(1446): The borrow is stored in variable `b` +fail_compilation/fastdfa.d(1456): Error: A borrow cannot outlive the variable it borrows from +fail_compilation/fastdfa.d(1455): For variable `x` +fail_compilation/fastdfa.d(1465): Error: Cannot change a borrow variable declared outside of a loop +fail_compilation/fastdfa.d(1462): For variable `b` +fail_compilation/fastdfa.d(1481): Error: A borrow cannot outlive the variable it borrows from +fail_compilation/fastdfa.d(1480): For variable `c` +fail_compilation/fastdfa.d(1478): The borrow is stored in variable `b` --- */ @@ -422,3 +450,119 @@ void checkViaObjNullDeref(bool cond, int** ptrArg) @system int** ptr = cond ? &var : ptrArg; **ptr = 2; // error } + +/****************** Borrow checker (errors) ******************/ + +@system: + +enum __fastdfa_returnborrow; + +int* borrowFn(@__fastdfa_returnborrow int* x) @trusted { return x; } +int** borrowFn2(@__fastdfa_returnborrow int** x) @trusted { return x; } + +void borrowTake(int* p) @safe {} +void borrowTakeConst(const(int)* p) @safe {} + +struct BorrowS { int field; } +struct BorrowDtor { ~this() {} int field; } + +struct BorrowStruct +{ + int* p; + + int** get() @__fastdfa_returnborrow @trusted { return &this.p; } +} + +void methodTake(int** p) @safe {} + +void borrowErr1() +{ + int* p; + int** b = borrowFn2(&p); + p = null; // error: reassigning a reference-type owner of an active borrow +} + +void borrowErr2() +{ + int x; + int* b = borrowFn(&x); + b = null; // error +} + +void borrowSafeErr(int** p, int* src) @safe +{ + *p = borrowFn(src); // error +} + +void borrowErr4() +{ + int x; + int* b = borrowFn(&x); + borrowTake(&x); // error +} + +void borrowErr5() +{ + int x; + int* b = borrowFn(&x); + borrowTake(b); // error, borrow passed to mutating function +} + +void methodOutliveErr() +{ + int** b; + { + BorrowStruct s; + b = s.get(); // error: borrow of this outlives the owner + } +} + +void methodPassErr() +{ + BorrowStruct s; + int** b = s.get(); // error: passing the borrowed owner to a mutating function + methodTake(&s.p); +} + +void borrowOutliveErr1() +{ + int* b; + { + BorrowDtor x; + b = borrowFn(&x.field); // error: borrow outlives owner + } +} + +int* borrowOutliveErr2() +{ + int x; + return borrowFn(&x); // error: returning a borrow of a local +} + +void borrowLoopErr1() +{ + int x; + int* b = borrowFn(&x); + for (int i = 0; i < 2; ++i) + { + b = null; // error: changing a borrow declared outside the loop + } +} + +class BorrowClass +{ + int* p; + + int** get() @__fastdfa_returnborrow @trusted { return &this.p; } +} + +void classOutliveErr() +{ + int** b; + { + BorrowClass c = new BorrowClass; + b = c.get(); // error: borrow of this outlives the owner object + } +} + +/****************** End borrow checker (errors) ******************/ diff --git a/druntime/src/core/attribute.d b/druntime/src/core/attribute.d index 5e9a97ff9876..c8fb65753b39 100644 --- a/druntime/src/core/attribute.d +++ b/druntime/src/core/attribute.d @@ -346,3 +346,14 @@ enum mustuse; * This is only allowed on `shared` static constructors, not thread-local module constructors. */ enum standalone; + +/** + * Use this attribute on a function parameter or on a function to tell the + * fast DFA engine's borrow checker that the function's return value borrows + * from that parameter (or from `this`, when placed on the function). + * + * This is an experimental feature used by the fast DFA engine (enabled with + * the `-preview=fastdfa` switch). It is subject to change and may be removed + * at a later date without notice; do not rely on it in production code. + */ +enum __fastdfa_returnborrow; diff --git a/plans/fastdfa_borrowchecker/1786383311457-borrow-checker-fast-dfa.md b/plans/fastdfa_borrowchecker/1786383311457-borrow-checker-fast-dfa.md new file mode 100644 index 000000000000..6d41c26842ac --- /dev/null +++ b/plans/fastdfa_borrowchecker/1786383311457-borrow-checker-fast-dfa.md @@ -0,0 +1,325 @@ +# Borrow Checker for the Fast DFA Engine + +## Goal + +Add borrow checking to the fast DFA engine (`compiler/src/dmd/dfa/fast/`): when a function call returns a borrow (triggered by the UDA +`__fastdfa_returnborrow`), the engine must detect, efficiently, whether an object has been borrowed from or is a borrow, and enforce: + +1. The owner cannot be mutated while the borrow is alive. +2. The owner must outlive the borrow. +3. A borrow variable cannot be changed (reassigned, set to null, etc.) unless it is loop-local (minor restriction). + +The DFAVar*/DFAObject* relationship is many-to-many; we need a reverse index for the owner-mutation check. + +## Current State (verified) + +- Escape relationship strength already exists: `ParameterDFAInfo.EscapedRelationship` (func.d:210) — + `Unknown=0b00, ByValue=0b01, PointerTo=0b10, Borrows=0b11`, stored 2 bits per target in `Inferrable.escapesInto` (return = bits 0-1, + `this` = bits 2-3, params = bits 6+2*i). +- `convergeFunctionCall` (analysis.d:117-302) dispatches on the relationship at call sites; the `Borrows` case (analysis.d:181-182) + currently does nothing. +- The write funnel is `seeWrite` (analysis.d:2932), called from `transferAssign` (analysis.d:1569, 1605); all writes (direct, `*p=`, + `x[i]=`, `x.f=`, `x[]=`) resolve to root `DFAVar*`s there. +- `DFAObject` has no borrow state; `DFACommon` uses fixed-size hash buckets with pointer-sorted chains (`vars[16]` pattern, structure.d: + 77-80). +- UDAs are parsed with `foreachUda(sym, sc, dg)` (attribsem.d:64); precedent: `__FastDFAEscapeTest` in entry.d:327-441. +- Lifetimes: `DFAVar.youngestLifeTimeAllowedDepth` = declaring scope depth (expression.d:644), `oldestLifeTimeAllowedDepth` = codegen + storage end (loop boundary, expression.d:643). Params/this/return: depth 1 (statement.d:323-414). Scope depths strictly decrease up the + `parent` chain; the walk never revisits a popped depth. +- `return exp` assigns to the return var via `seeAssign(dfaCommon.getReturnVariable, ...)` (statement.d:580) — the borrow registration + funnel covers returns too. +- `&x` yields the cell object `x.storageFor` via `transferAddressOf` (analysis.d:2387+). `makeObject(storageForVar)` is cached on the var ( + structure.d:692-710). +- Engine runs under `version (FastDFA)` + `global.params.useFastDFA` (`-preview=fastdfa`), per function in `fastDFA` (entry.d:78), called + from semantic3.d:1465. `DFACommon` is a fresh local per function run. + +## Design Decisions (resolved with user) + +1. **UDA semantics**: `@__fastdfa_returnborrow` on a *function* means the return borrows from `this`; on a *parameter* it means the return + borrows from that parameter. Encoded as `willEscape(-3, EscapedRelationship.Borrows)` on the source's `userSupplied` Inferrable. Both on + the same function → error. Test syntax is parameter-placed: `int* f(@__fastdfa_returnborrow ref int x) { return &x; }`. +2. **Registry (liveness)**: flow-insensitive entries attached to the *borrower's declaring DFAScope* (found by walking `currentDFAScope` up + to `depth == borrower.youngestLifeTimeAllowedDepth` — depths are unique on the chain). Entries die with the scope; the mutation check + only consults the current scope chain, so a dead borrow no longer protects its owner, and a new borrow registers its own entry. No + unregistration needed (scope freed → entry leaked into the run allocator, reclaimed with the region). +3. **Changing borrows ban**: a borrow variable cannot be *changed* (reassigned, set to null, etc.) unless the change happens inside a loop + AND the variable is declared inside that loop. Concretely, at an assignment to var `b` whose current LHS lattice object is a borrow ( + `obj.isBorrow`), report an error iff `lastLoopyLabel.depth == 1` (no enclosing loop) OR + `b.youngestLifeTimeAllowedDepth <= lastLoopyLabel.depth` (b declared at the loop level or outside it). Loop-local borrows ( + `int* b = borrow(&x); b = 0;` inside one loop iteration) may change freely. Branch-exclusive first assignments (if/else) are not flagged + because each branch's lattice starts from the pre-if state. +4. **Registry semantics — path-sensitive replace/remove, no union**: entries on a scope are the *current* borrows of each var on the current + path. When the lattice check (decision 3) says `b` currently holds a borrow and the change is allowed (loop-local), `b`'s existing + entries are removed first, then the new borrow (if any) is registered. Branch-exclusive writes never remove each other's entries (the + lattice is path-sensitive: a sibling branch's `b = 0` sees the pre-if state, not a borrow, so no removal). Conditional borrows still end + up with both cells registered (each branch registers its own) — that is correct union behavior emerging naturally, not explicit union + semantics. +5. **Outlive check — one level deep**: at registration, compare the borrower against the *direct* source's owner: if the direct source + object is a cell → owner = `cell.storageFor`; if the direct source is itself a borrow object (`isBorrow`) → owner = that object's + `holderVar` (the variable holding the borrowed value). Require + `borrower.youngestLifeTimeAllowedDepth >= owner.youngestLifeTimeAllowedDepth`, stack owners only. A borrow-of-a-borrow may not outlive + the variable holding the borrowed value; deeper chains compose transitively (each level enforces its own constraint). If `holderVar` is + null (nested call result never held by a var), fall back to the ultimate cell's owner. +6. **Registry keying — transitive**: the mutation check is keyed by cell objects, so registration resolves the borrowsFrom chain + *transitively* through borrow objects to the ultimate cells and keys the entry there. The extra indirection is handled automatically: + `c = borrow(b)` keys `c`'s entry on the same cells as `b`'s, so mutating the owner is caught as long as either is alive — and since `c` + may not outlive `b` (decision 5), `b`'s entry is always alive while `c` is. +7. **Cell resolution is additive**: the borrow checker uses its own resolution walk (base1/base2/derivedFrom/inCell/borrowsFrom → root cells + with `storageFor`). Field and pointer-indirection sources (`&s.f`, `*p`) DO register against the root cell. Existing escape-analysis + rules (`gotACell`/`walkIndirection` behavior at analysis.d:249-297) are not changed — additive rules only. +8. **Callee-side**: no body validation against the UDA in v1 (a callee that doesn't actually return a borrow causes call-site false + positives — documented limitation). +9. **Call-site precedence**: when `userSupplied` declares `Borrows` into the return, `userSupplied.escapesInto` must win over + `inferred.escapesInto` at analysis.d:216 (the body analysis can only infer ByValue/PointerTo, which would silently downgrade the UDA). +10. **Owner passed to a call**: an owner with an active borrow may only be passed to a function whose parameter cannot mutate it — the + parameter must be const/immutable where it reaches the cell. Concretely, at each call-site argument whose object graph resolves to a + borrowed cell: error unless (a) the parameter is by-ref and its type is const/immutable (`ref const(int)` ok, `ref int` error), or (b) + the parameter is a pointer/array/class type whose pointee is const/immutable (`const(int)*` ok, `int*` error), or (c) the parameter is + by-value non-reference (a copy — no reach to the cell, always ok). This covers `foo(&x)`, `foo(x)` by ref, and `foo(p)` where `p` points + to the owner (the callee could mutate `*p`). **The borrow-source parameter is exempt**: an argument whose parameter is the designated + borrow source (`userSupplied.willEscape(-3) == Borrows`, i.e. the `__fastdfa_returnborrow` parameter) is never flagged — multiple + borrows from one owner are allowed (`b1 = borrow(&x); b2 = borrow(&x);` compiles). Reported unconditionally (not gated on `@safe`), like + the owner-mutation check. + +## Implementation Tasks + +### 1. UDA parsing — `entry.d` (fastDFA, entry.d:78) + +New private helper `applyBorrowUDA(FuncDeclaration fd, Scope* sc)` called after the walker setup, before `stmtWalker.start(fd)`: + +- `ensureDFAParameters(fd);` (idempotent, utils.d:23) +- `foreachUda(fd, sc, ...)`: if `StructLiteralExp` with `sd.ident.toString == "__fastdfa_returnborrow"` → + `fd.parametersDFAInfo.thisPointer.userSupplied.willEscape(-3, EscapedRelationship.Borrows)`. +- For each `vd` in `*fd.parameters`: `foreachUda(vd, sc, ...)` → `parameters[i].userSupplied.willEscape(-3, Borrows)`. +- Both function-level and parameter-level present → error via `errorSink` (use `global.errorSink`; entry.d already imports what's needed — + mirror the `checkEscapes` style, entry.d:327-441). + +### 2. Call-site Borrows handling — `analysis.d` + +**a. Precedence fix** (analysis.d:216-217): copy `paramInfo.userSupplied` to a temp first (avoid the `willEscape` read-side shift mutation, +func.d:256 — same trick as report.d:273), and: + +```d +ulong escapesInto = paramInfo.inferred.escapesInto != 0 +? paramInfo.inferred.escapesInto : paramInfo.userSupplied.escapesInto; +if (tempUser.willEscape(-3) == ParameterDFAInfo.EscapedRelationship.Borrows) +escapesInto = paramInfo.userSupplied.escapesInto; +``` + +**b. `handleRelationshipConsequence`** `case Borrows` (analysis.d:181-182), mirroring the ByValue pattern: + +```d +case ParameterDFAInfo.EscapedRelationship.Borrows: +cctx.obj = dfaCommon.makeObject(cctx.obj); +cctx.obj.isBorrow = true; +cctx.obj.borrowsFrom = source; +return; +``` + +The output-param loop (analysis.d:232-300) needs no change (UDA only sets the return slot). + +**c. Call-site owner-passing check — `callFunction` (expression.d:2497-2682)**: in the explicit-argument branch (expression.d:2613-2642), +right after `argExp = this.walk(arg)` (expression.d:2637) and before `seeFunctionCallArgument`, call a new helper +`checkBorrowArgument(argExp, argOffset, loc)`: + +- `DFAObject* argObj = argExp.getContextObject;` — if null, nothing to check (by-value non-reference argument, or unknown). +- +`Parameter* param = toCallFunctionType !is null && toCallFunctionType.parameterList.parameters !is null && argOffset < length ? (*toCallFunctionType.parameterList.parameters)[argOffset] : null;` — +if null (C varargs, function pointers without parameters), skip (conservative). +- `paramCanMutate(ParameterDFAInfo* paramInfo, Parameter* param)`: + - `paramInfo.isByRef` (ref/out/autoref) → return `!(param.type.hasConst || param.type.isImmutable)`. + - `param.type` is Tpointer/Tarray/Taarray/Tclass/Tdelegate/Tsarray → return + `!(param.type.nextOf().hasConst || param.type.nextOf().isImmutable)`. + - Otherwise (by-value value type) → false (a copy, no reach to the cell). +- If `paramCanMutate` is true: + `dfaCommon.resolveBorrowCells(argObj, (cellVar, cellObj) { if (dfaCommon.isCellBorrowed(cellObj)) reporter.onBorrowOwnerPassedToMutatingFunction(cellObj, param, loc); });` + +**Borrow-source exemption**: at the top of `checkBorrowArgument`, copy `list.each[i].paramInfo.userSupplied` to a temp and skip the whole +check when `temp.willEscape(-3) == ParameterDFAInfo.EscapedRelationship.Borrows` (the parameter is the designated borrow source via the +UDA — same temp-copy trick as task 2a). This is what allows multiple borrows of one owner: `b1 = borrow(&x); b2 = borrow(&x);` both pass +`&x` to a mutable `ref` parameter that is the borrow source, and neither is flagged; any *other* function receiving the borrowed owner still +needs const/immutable parameters. + +The check runs against the *current* registry state. The `this`-argument branch (expression.d:2573-2603) is not covered in v1 (needs the +called method's this-mutability; extension point with the identical mechanism). + +### 3. Structure additions — `structure.d` + +- `DFAObject` (structure.d:1970): add fields `DFAObject* borrowsFrom;` (the *direct* source — one level deep, never collapsed at creation), + `bool isBorrow;`, and `DFAVar* holderVar;` (the variable currently holding this borrow value; set at registration, most recent wins) near + `derivedFrom`/`inCell`. +- `makeObject(DFAObject* base1)` and `makeObject(DFAObject* base1, DFAObject* base2)` (structure.d:712-735): propagate + `isBorrow = base1.isBorrow || base2.isBorrow`. Do NOT propagate `borrowsFrom`/`holderVar` (conditional combine bases are walked at + registration instead). +- New `struct DFABorrowEntry { DFAVar* borrower; DFAObject* borrowedFrom; Loc loc; DFABorrowEntry* next; }` — flat per-scope list. +- `DFAScope` (structure.d:2321): add `DFABorrowEntry* borrowEntries;` (public, like other fields). +- `DFAAllocator`: add `DFABorrowEntry* freelistborrow;` + `makeBorrowEntry(DFAVar*, DFAObject*, Loc)` via `allocInternal!DFABorrowEntry` ( + structure.d:1394), + `free(DFABorrowEntry*)`; free the list inside `free(DFAScope)` (structure.d:1277) — or simply leak (region reclaims + at function end); prefer the freelist for consistency. +- `DFACommon` helpers: + - `DFAScope* findDeclaringScope(DFAVar* var)`: walk `currentDFAScope` up while `sc.depth > var.youngestLifeTimeAllowedDepth`; return + that scope (assert non-null: the var is in scope at the assignment). + - `void registerBorrow(DFAVar* borrower, DFAObject* cell, ref Loc loc)`: scan `findDeclaringScope(borrower).borrowEntries` for an + existing `(borrower, cell)` pair; prepend if absent. + - `void removeBorrowEntries(DFAVar* borrower)`: scan `findDeclaringScope(borrower).borrowEntries` and unlink every entry with that + borrower (free via `DFAAllocator.free(DFABorrowEntry*)`). Only ever called when the var's current lattice is a borrow (path-sensitive + guard), so sibling-branch entries survive. + - `bool isCellBorrowed(DFAObject* cell)`: walk `currentDFAScope` up the parent chain, scanning each scope's `borrowEntries` for + `borrowedFrom is cell`; return true on first match. Shared by the owner-mutation check and the call-site argument check. + - `void resolveBorrowCells(DFAObject* obj, scope void delegate(DFAVar* cellVar, DFAObject* cellObj) del)`: **additive** walk used only + by the borrow checker — traverses `base1`, `base2`, `derivedFrom`, `inCell`, and `borrowsFrom` chains to the root objects, delivering + each root whose `storageFor` is a non-null, non-base variable. Handles `&x`, `&s.field`, `*p`, conditional combines, and + borrow-of-borrow chains transitively. Does not modify `walkIndirection`/`gotACell`. + +### 4. Registration + ban + outlive checks — `transferAssign` (analysis.d:1380) + +After `wasDereferenced` is computed (analysis.d:1457), inside `if (assignToCtx !is null)`, guarded by +`lrCctx !is null && lrCctx.obj !is null`: + +1. **Dereference store** (`wasDereferenced`, e.g. `*p = borrow(&x)`): the borrow lives in memory, not a tracked variable. + `reporter.onBorrowStoredThroughDereference(loc)` — that function gates the error on the analyzed function being `@safe` ( + `dfaCommon.currentFunction.type.isTypeFunction.trust == TRUST.safe`); `@system` code accepts it silently with no registration. No other + checks apply. +2. **Change/ban check** (`!wasDereferenced`): `DFAConsequence* lhsCctx = assignTo.getContext;` — if + `lhsCctx.obj !is null && lhsCctx.obj.isBorrow && !construct`: + - `const loopDepth = dfaCommon.lastLoopyLabel.depth;` + - If `loopDepth == 1 || assignToCtx.youngestLifeTimeAllowedDepth <= loopDepth` → + `reporter.onBorrowVariableReassignment(assignToCtx, loc)` (b is a borrow declared outside the nearest loop, or there is no loop — + matching `int* b = borrow(&x); for(;;) b = null;` being an error). + - Else (loop-local borrow): `dfaCommon.removeBorrowEntries(assignToCtx)` — drop the var's existing entries before the new value is + registered below. +3. **Registration + outlive** (guarded additionally by `!wasDereferenced`): add + `DFAObject.walkBorrowSources(scope void delegate(DFAObject* borrowsFrom) del)`: recursive over `base1`, `base2`, and `borrowsFrom`. For + each direct `borrowsFrom` source `S` of `lrCctx.obj`: + - **Direct owner (one level deep)**: if `S.isBorrow` → `DFAVar* owner = S.holderVar;` (fallback if null: resolve `S` via + `resolveBorrowCells` and use the first cell's `storageFor`). Else → resolve `S` via `resolveBorrowCells`; owner = `cell.storageFor` of + the resolved cell. + - **Outlive check**: if owner non-null and the cell is a stack cell (`cell.onTheStack` or `owner.isStackVar`): if + `assignToCtx.youngestLifeTimeAllowedDepth < owner.youngestLifeTimeAllowedDepth` → + `reporter.onBorrowOutlivesOwner(assignToCtx, owner, loc)`. Params (depth 1) always pass; heap/global owners skipped. + - **Transitive keying**: resolve `S` via `resolveBorrowCells` (which follows borrow objects transitively) and + `dfaCommon.registerBorrow(assignToCtx, cell, loc)` for each cell — the entry keys the mutation check on the ultimate owner cell. + - Record the holder: for each direct borrow node `S` processed, set `S.holderVar = assignToCtx` (most recent holder wins) so deeper + borrows check against it. + - Skip registration entirely for a non-stack owner (no lifetime constraint, nothing to protect). + +`removeBorrowEntries(DFAVar* borrower)` (DFACommon helper): scan the borrower's declaring scope's `borrowEntries` and unlink all entries +with `borrower` — called only when the lattice says the var currently holds a borrow, so sibling-branch entries are never removed. + +This single funnel covers: direct `b = borrow(&x)`, construct `int* b = borrow(&x);`, propagation `b2 = b1` (registers `b2` too, keyed on +the same cells), conditional borrows (combine objects walk both bases), field/pointer sources (`borrow(&s.f)`, `borrow(*p)` via the additive +resolution), borrow-of-borrow (`c = borrow(b)` — outlive vs `b`, keyed transitively on `x`'s cell), loop-local changes (remove + +re-register), and `return borrow(...)` (return var is the assignTo). + +### 5. Owner-mutation check — `seeWrite` (analysis.d:2932) + +- Add `ref Loc loc` parameter to `seeWrite`; update both callers (analysis.d:1569, 1605; `constructVariable` → `seeAssign` chain already + passes loc). +- In the `walkRoots` delegate (analysis.d:2943), after `root.writeCount++`: + ```d + if (root.storageFor !is null) + checkBorrowMutation(root.storageFor, loc); + ``` + (`storageFor` is only materialized when the address was taken — zero churn for plain vars.) +- New `void checkBorrowMutation(DFAObject* cell, ref Loc loc)`: `dfaCommon.isCellBorrowed` gives the first matching entry; on a match → + `reporter.onBorrowOwnerMutation(entry, loc)` (report once per write). O(depth × borrows-per-scope); depths are small and borrows are rare. + +### 6. Reporting — `report.d` + +Add to `DFAReporter` (follow existing `errorSink.error` + `errorSupplemental` style): + +- `onBorrowOwnerMutation(DFABorrowEntry* entry, ref const Loc loc)`: "Cannot mutate the owner of an active borrow" + supplement at + `entry.loc` "Borrowed here" + borrower declaration. +- `onBorrowOutlivesOwner(DFAVar* borrower, DFAVar* owner, ref const Loc loc)`: "A borrow cannot outlive the variable it borrows from" + + owner/borrower declaration supplements. +- `onBorrowVariableReassignment(DFAVar* borrower, ref const Loc loc)`: "Cannot change a borrow variable declared outside of a loop" + + borrower declaration. +- `onBorrowStoredThroughDereference(ref const Loc loc)`: "Cannot store a borrow through a dereference in @safe code" — gated internally on + `dfaCommon.currentFunction.type.isTypeFunction.trust == TRUST.safe` (the `TRUST` import already exists in report.d, used at report.d:286); + `@system` functions accept the pattern silently. +- `onBorrowOwnerPassedToMutatingFunction(DFAObject* cell, Parameter* param, ref const Loc loc)`: "Cannot pass the owner of an active borrow + to a function that may mutate it" + supplement naming the parameter (`param.ident` when present) + "Parameter must be const or + immutable" + the borrow site supplement (find the entry via `isCellBorrowed`). +- `report.d` needs `DFABorrowEntry` import — it already imports `dmd.dfa.fast.structure`. + +### 7. Tests + +No new test files. Extend the existing `__fastdfa_escape_test` files (they already carry `REQUIRED_ARGS: -preview=fastdfa` and `#line 1000`; +add `struct __fastdfa_returnborrow {}` to the test module). The fail_compilation file's `TEST_OUTPUT` block pins exact line numbers — append +new cases and update the block (all following line numbers shift). + +`compiler/test/compilable/__fastdfa_escape_test.d` (must compile, no errors): + +- Callee: `int* f(@__fastdfa_returnborrow ref int x) { return &x; }`; caller borrow + read; owner mutation *after* the borrow's block ends ( + no error). +- Borrow of a parameter; param mutation after the borrow's block ends (no error). +- Struct method with function-level UDA: `int* get() @__fastdfa_returnborrow` (borrows from `this`); mutating the struct instance after the + borrow's block ends (no error). +- Multiple concurrent borrowers of one owner: `b1 = borrow(&x); b2 = borrow(&x);` while `b1` is still alive (the second borrow call must + compile — the borrow-source parameter is exempt from the const/immutable check; both borrows block owner mutation); borrow of a field ( + `borrow(&s.field)`); borrow through a pointer (`borrow(p)` where `p = &x`). +- **Struct owner**: `struct S { int field; }` — `int* b = borrow(&s);` (the struct has no DFAObject* of its own; the source resolves through + the *cell of the variable* `s`); read via the borrow; mutate `s.field` after the borrow's block ends (no error); pass `&s` to `const(S)*` + and `ref const(S)` parameters while borrowed (no error). +- `b2 = b1` propagation; conditional borrow (`cond ? borrow(&x) : borrow(&y)`); borrow-of-borrow `c = borrow(b)` where `c` dies before `b`; + `if (c) b = borrow(&x); else b = 0;` (branch-exclusive, no error). +- Loop-local borrow changes: `for (;;) { int* b = borrow(&x); b = 0; }` (allowed — b is loop-local; x is not protected after `b = 0` within + the iteration). +- Borrow created inside a loop assigned to an outer var: `int* b; for (;;) { b = borrow(&x); }` (allowed — first assignment; owner still + protected after the loop via the entry on b's declaring scope). +- `@system` dereference store: `void sys(int* p) { *p = borrow(&x); }` (no error, no registration). +- Passing the borrowed owner to const-accepting calls: `foo(const(int)* p)` and `foo(ref const(int) x)` while the owner is borrowed (no + error); passing the borrow var itself to a `const(int)*` parameter (no error). +- Borrow of heap (`borrow(new int)`) — no checks. + +`compiler/test/fail_compilation/__fastdfa_escape_test.d` (errors, regenerate `TEST_OUTPUT`): + +- Owner mutation while borrow alive: direct straight-line (`x = 5` after `b = borrow(&x)`), inside a nested scope while the borrow is alive, + through the borrow (`*b = 5`), and through an index (`b[i] = 5`). +- **Struct owner mutation while borrowed**: `s.field = 5;` and `s = S(0);` while the borrow of `&s` is alive (both write the root var `s`; + the cell check must fire). +- Borrow outlives owner: block-scoped owner (struct with destructor) assigned to an outer var; `return borrow(&local);`. +- Borrow-of-borrow outlives its direct source: `c = borrow(b)` with `c` declared outside `b`'s block. +- Changing a borrow declared outside a loop: `int* b = borrow(&x); for (;;) b = null; // error`; also straight-line + `b = borrow(&x); b = null;` and `b = borrow(&y);`. +- `@safe` dereference store: `void safe1(int* p) @safe { *p = borrow(&x); }` (error). +- Passing the borrowed owner to a mutating parameter: `foo(int* p)` with `foo(&x)` while x is borrowed; `foo(ref int x)`; `foo(S* p)` with + `foo(&s)` while s (struct) is borrowed; and passing the borrow var itself (`foo(b)` where the parameter is `int*`). +- UDA on both function and parameter (conflicting sources). + +### 8. Verification + +- Build: `cd compiler/src && rdmd build.d` (per AGENTS.md). +- Run the DFA tests with the user's harness: `C:\Program Files\Git\bin\bash.exe testdfa/run.sh` (note: `testdfa/run.sh` is a local script + outside the repo; not present in `P:\dmd`). +- Regression: the existing `__fastdfa_escape_test` cases must still pass, confirming the precedence change (task 2a) does not alter + inferred-escape behavior — the flip only applies when `userSupplied` declares `Borrows`, which no existing case uses. + +## Known Limitations (v1 — acceptable, document in code comments) + +1. Same-scope declaration order is NOT a limitation: `int* b; int x; b = borrow(&x);` in one scope is fine, and destructor-pinned owners are + already handled by the existing variable-lifetime and compiler scoping rules (a struct with a destructor pins + `oldestLifeTimeAllowedDepth` to its exact scope, expression.d:646-663). +2. The `this` argument of method calls on a borrowed owner is not checked in v1 (needs the called method's this-mutability; the mechanism is + identical to the explicit-argument check and is the natural extension point). +3. The callee body is not validated against `__fastdfa_returnborrow` (a lying callee causes call-site false positives). +4. A borrow-of-a-borrow whose direct source object has no `holderVar` (nested call result never held by a variable) falls back to the + ultimate cell's owner for the outlive check — slightly looser than one-level-deep. +5. `@system` code that stores a borrow through a dereference (`*p = borrow(&x)`) gets no borrow protections at all (deliberate — the error + is gated on `@safe`). +6. Goto-label regions count as "loops" for the change-ban allowance (`lastLoopyLabel` is also set for labels) — a borrow declared inside a + label region may be changed there. +7. The call-site argument check skips arguments when the parameter type is unavailable (C varargs, parameterless function pointers) — + conservative skip, no error. + +## Out of Scope + +- `@escape(return^)` syntax parsing (UDA is the v1 trigger). +- `@live`/DIP1021 `dmd.ob` integration. +- Borrowing into output parameters (only the return slot is handled). +- Cross-function borrow tracking (no separate compilation support). + +## Audit Reminder (per AGENTS.md) + +When this work is intended to be contributed back to dmd upstream, audit the changes before contributing. Confirm understanding of each +change (particularly the `userSupplied`/`inferred` precedence change in task 2a, which affects call-site escape dispatch). diff --git a/plans/fastdfa_borrowchecker/prompts-borrowchecker.md b/plans/fastdfa_borrowchecker/prompts-borrowchecker.md new file mode 100644 index 000000000000..4634954d9d41 --- /dev/null +++ b/plans/fastdfa_borrowchecker/prompts-borrowchecker.md @@ -0,0 +1,95 @@ +## DeepSeek V4 flash + +> > You are researching the future implementation of a borrow checker in the fast dfa engine. +> There is some support as part of escape analysis, specifically the ability to represent the borrow-from-owner relationship strength. +> However the relationships between DFAVar* and DFAObject* is a many to many one. +> We need an efficient way of detecting if the object has been borrowed from, or is a borrow. +> So we can enable the borrow checkers protections. +> We may place some minor restrictions on borrows, i.e. no assignments for borrows that live outside a loop. +> The primary restrictions when a borrow has occured is that the owner cannot be mutated and it must outlive the borrow. +> It is triggered typically from a function call I.e. (the syntax isn't part of scope right now): +> int* borrow() @escape(return^) +> For now we'll use a UDA __fastdfa_returnborrow + +> When placed on the function it refers to the this parameter, otherwise it'll be placed on a parameter. + +> Depth based may work?, BUT after a borrow has ended, that owner must be seen as no longer having borrows. +> If a new borrow takes place, that one must be seen instead. + +> Reassignment of a borrow variable (b = something else while b is an active borrow) is banned outside of loops; inside loops it is allowed. The same-var replace/remove semantics I proposed for the registry then only kicks in for loop-scoped borrows. + +> No new files are required for this, existing tests are sufficient +> To run the tests use: C:\Program Files\Git\bin\bash.exe testdfa/run.sh +> wrong test: int* f(ref int x) @__fastdfa_returnborrow { return &x; } +> should be: int* f(@__fastdfa_returnborrow ref int x) { return &x; } +> Same-scope declaration order is ok as long as it isn't a struct with a destructor. This should already be handled in the variable declaration lifetime and general compiler scoping rules for variables. +> Field/pointer-indirection borrow sources needs to register, do not change existing rules, add additive rules that cover just the borrow checker. +> Borrows of borrowed values are treated as borrows of the original cell, wrong it should be from the borrowed value one level deep. The extra indirection should be handled automatically transitively + +> Borrows stored through a dereference is fine for @system, but not @safe code, the report.d error function for this scenario will need to check for safety +> Union semantics inside loops what? No. +> For loops we only need to prevent changing borrows that are in variables declared outside of the loop. +> int* borrow = ...; for(;;) borrow = null; // error + +> Mutation of the owner through a function call, nope, needs to be checked to make sure parameter is const/immutable + +> Test needs to include a check to make sure that the owner can be a struct, it won't have a DFAObject* associated with it, only the cell of the variable. + +> multiple borrows from an owner is ok + +> Just use the test script I told you to. +> To run the tests use: C:\Program Files\Git\bin\bash.exe testdfa/run.sh + +> __fastdfa_returnborrow should be an enum not a struct: enum __fastdfa_returnborrow; +> The logic of detecting it will happen in two places in utils.d ensureDFAParameters and ensureDFAParameter. + +> Note: you can use testdfa/start.d instead of the other test files and then enable the exit 0 +> this will give you a mini test file that won't have other stuff in it. + +> Note: there is a way to turn on the built in logging, with debugStructure + +> Oh hold on, start.d is wrong its startd.d + +> ya know this debugging would go a lot faster if you turned on debugStructure and debugIt with a restriction in entry.d (commented out line 103) to only analyze a single function. + +> ok for the most part the borrow checker is working, except one specific case. +> The pattern of borrowErr1 should be ok. +> The mutation is not on the cell storage for the int, which the taking of reference is to. +> Mutating the integer value which is a basic type, won't invalidate the borrow b. + +> null this pointer for isConst expression.d line 2539 + +> Your new function checkBorrowArgument should be handled as part of argument convergance in analysis.d + +> src\dmd\dfa\fast\analysis.d(1242): Error: no property `nextOf` for `(*argListItem).paramType` of type `dmd.mtype.Type` + +> This error message isn't correct. +> testdfa\startd.d(393): Error: Cannot mutate the owner of an active borrow +> p = null; // error: reassigning a reference-type owner of an active borrow +> testdfa\startd.d(392): Borrowed here +> int** b = borrowFn2(&p); +> testdfa\startd.d(392): For variable `b` +> int** b = borrowFn2(&p); +> At no point is p referenced. + +> Verify if you have a struct or a class, with a method that the borrow checker will fire. Free-functions are not the primary use case. + +> Modify test/compilable/fastdfa.d and test/fails_compilation/fastdfa.d with their respective tests from start.d + +> Add the enum __fastdfa_returnborrow to druntime/src/core/attribute.d make sure to add a comment that makes it clear that it is an experimental feature that may be removed at a later date. +> Write the changelog entry in changelog (.dd) + +> Verify: when you borrow from a class, the owner object must outlive the borrow. + +> Add test case to startd.d and run the run.sh script + +> Nevermind it fires now, I undid a change I made + +> No. Enable logging, add entry point blocking then run run.sh +> If you call a method, the this pointer should become non-null with a DFAObject allocated for it. + +> The way you went about solving this isn't right, when you dereference you create a child of the DFAVar, that is a derefence var, you need to go peeking by walking the DFAVar's to find which DFAObject* the this pointer is + +> walkRoots doesn't sound right either, there are walks that are indirection aware + +> Add the test case.