From ae721f96201116d598334615e35058e2c32fc911 Mon Sep 17 00:00:00 2001 From: EdwardWang Date: Tue, 11 Feb 2025 19:10:36 +0800 Subject: [PATCH 1/3] [cfg] small refactor to make it more stable - Do not build cfg when dataflow analysis is not required. - Report error when cfg is not structured. --- .../clang/Analysis/Analyses/BSCOwnership.h | 6 +++- clang/include/clang/Analysis/CFG.h | 2 +- clang/lib/Analysis/CFG.cpp | 33 ++++++++++++++----- clang/lib/Sema/BSC/SemaDeclBSC.cpp | 5 +-- clang/lib/Sema/Sema.cpp | 4 +-- .../Ownership/RuleCheck/bad-cfg/bad-cfg.cbs | 22 +++++++++++++ 6 files changed, 58 insertions(+), 14 deletions(-) create mode 100644 clang/test/BSC/Negative/Ownership/RuleCheck/bad-cfg/bad-cfg.cbs diff --git a/clang/include/clang/Analysis/Analyses/BSCOwnership.h b/clang/include/clang/Analysis/Analyses/BSCOwnership.h index b42484560591..a2c673e6f5ed 100644 --- a/clang/include/clang/Analysis/Analyses/BSCOwnership.h +++ b/clang/include/clang/Analysis/Analyses/BSCOwnership.h @@ -48,7 +48,11 @@ public: class OwnershipStatus { public: - enum Source { OPS, S, BOP }; + enum Source { + OPS, // Owned Pointer to Struct, e.g. struct S* owned s + S, // Struct, e.g. struct S s + BOP // Owned Pointer to Basic Types, e.g. int* owned p + }; using OwnershipSet = llvm::BitVector; // owned pointer struct status, e.g. struct S * owned s diff --git a/clang/include/clang/Analysis/CFG.h b/clang/include/clang/Analysis/CFG.h index a15fc7a14397..2a27a377c54c 100644 --- a/clang/include/clang/Analysis/CFG.h +++ b/clang/include/clang/Analysis/CFG.h @@ -1246,7 +1246,7 @@ public: bool AddTemporaryDtors = false; bool AddScopes = false; #if ENABLE_BSC - bool AddAllScopes = false; + bool BSCMode = false; #endif bool AddStaticInitBranches = false; bool AddCXXNewAllocator = false; diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index 2ae5a41136bc..15afb0df89f9 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -537,6 +537,11 @@ public: bool alwaysAdd(const Stmt *stmt); + // Only used by BSC, report error when the CFG is not structured. + DiagnosticsEngine& getDiagnostic() { + return Context->getDiagnostics(); + } + private: // Visitors to walk an AST and construct the CFG. CFGBlock *VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc); @@ -624,12 +629,12 @@ private: const Stmt *S) { if (ScopePos && (VD == ScopePos.getFirstVarInScope()) #if ENABLE_BSC - && !BuildOpts.AddAllScopes + && !BuildOpts.BSCMode #endif ) appendScopeBegin(B, VD, S); #if ENABLE_BSC - else if (BuildOpts.AddAllScopes && ScopePos) + else if (BuildOpts.BSCMode && ScopePos) appendScopeBegin(B, VD, S); #endif } @@ -1846,10 +1851,10 @@ void CFGBuilder::addAutomaticObjDtors(LocalScope::const_iterator B, if (B == E) return; #if ENABLE_BSC - if (BuildOpts.AddAllScopes && ScopePos == LocalScope::const_iterator()) + if (BuildOpts.BSCMode && ScopePos == LocalScope::const_iterator()) return; int dist = B.distance(E); - if (BuildOpts.AddAllScopes && dist <= 0) + if (BuildOpts.BSCMode && dist <= 0) return; #endif // We need to append the destructors in reverse order, but any one of them @@ -1867,14 +1872,14 @@ void CFGBuilder::addAutomaticObjDtors(LocalScope::const_iterator B, // ScopeEnd marker in a Block. if (BuildOpts.AddScopes && DeclsWithEndedScope.count(VD) #if ENABLE_BSC - && !BuildOpts.AddAllScopes + && !BuildOpts.BSCMode #endif ) { autoCreateBlock(); appendScopeEnd(Block, VD, S); } #if ENABLE_BSC - else if (BuildOpts.AddAllScopes && BuildOpts.AddScopes) { + else if (BuildOpts.BSCMode && BuildOpts.AddScopes) { autoCreateBlock(); appendScopeEnd(Block, VD, S); } @@ -3419,8 +3424,20 @@ CFGBlock *CFGBuilder::VisitGotoStmt(GotoStmt *G) { LabelMapTy::iterator I = LabelMap.find(G->getLabel()); if (I == LabelMap.end()) - // We will need to backpatch this block later. - BackpatchBlocks.push_back(JumpSource(Block, ScopePos)); + // Label appears first, then goto stmt + // This is not structured cfg, bishengc should report error + if (BuildOpts.BSCMode) { + badCFG = true; + DiagnosticsEngine &Diag = getDiagnostic(); + unsigned ID = Diag.getCustomDiagID( + DiagnosticsEngine::Error, + "The CFG is not structured" + ); + Diag.Report(G->getBeginLoc(), ID); + } else { + // We will need to backpatch this block later. + BackpatchBlocks.push_back(JumpSource(Block, ScopePos)); + } else { JumpTarget JT = I->second; addAutomaticObjHandling(ScopePos, JT.scopePosition, G); diff --git a/clang/lib/Sema/BSC/SemaDeclBSC.cpp b/clang/lib/Sema/BSC/SemaDeclBSC.cpp index 28cb56b03c89..d21738804eba 100644 --- a/clang/lib/Sema/BSC/SemaDeclBSC.cpp +++ b/clang/lib/Sema/BSC/SemaDeclBSC.cpp @@ -161,10 +161,11 @@ void Sema::BSCDataflowAnalysis(const Decl *D, bool EnableOwnershipCheck, AC.getCFGBuildOptions().AddImplicitDtors = true; AC.getCFGBuildOptions().AddTemporaryDtors = true; AC.getCFGBuildOptions().AddScopes = true; - AC.getCFGBuildOptions().AddAllScopes = true; + AC.getCFGBuildOptions().BSCMode = true; AC.getCFGBuildOptions().setAllAlwaysAdd(); - if (AC.getCFG()) { + bool RequireCFGAnalysis = LangOpts.BSC ? FindSafeFeatures(dyn_cast_or_null(D)) : false; + if (RequireCFGAnalysis && AC.getCFG()) { const FunctionDecl *FD = cast(D); unsigned NumNullabilityCheckErrorsInCurrFD = 0; LangOptions::NullCheckZone CheckZone = getLangOpts().getNullabilityCheck(); diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index a5a00c76f649..f31227ebaec2 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -2214,8 +2214,8 @@ Sema::PopFunctionScopeInfo(const AnalysisBasedWarnings::Policy *WP, #if ENABLE_BSC // If D does not use memory safety features like "owned, borrow, &mut, &const", // we do not do borrow checking. - bool useSafeFeature = LangOpts.BSC ? FindSafeFeatures(dyn_cast_or_null(D)) : false; - bool EnableOwnershipCheck = useSafeFeature && !(getLangOpts().DisableOwnershipCheck); + // bool useSafeFeature = LangOpts.BSC ? FindSafeFeatures(dyn_cast_or_null(D)) : false; + bool EnableOwnershipCheck = LangOpts.BSC ? (!(getLangOpts().DisableOwnershipCheck)) : false; bool EnableNullabilityCheck = LangOpts.BSC ? (getLangOpts().getNullabilityCheck() != LangOptions::NC_NONE) : false; if (EnableOwnershipCheck || EnableNullabilityCheck) { if (!isBSCCoroutine && diff --git a/clang/test/BSC/Negative/Ownership/RuleCheck/bad-cfg/bad-cfg.cbs b/clang/test/BSC/Negative/Ownership/RuleCheck/bad-cfg/bad-cfg.cbs new file mode 100644 index 000000000000..a2de69fc467c --- /dev/null +++ b/clang/test/BSC/Negative/Ownership/RuleCheck/bad-cfg/bad-cfg.cbs @@ -0,0 +1,22 @@ +// RUN: %clang_cc1 -verify %s + +// bad cfg +void test1(int* owned p, int x) { +L1: + x = x + 1; + if (x < 10) goto L1; // expected-error {{The CFG is not structured}} + void* temp = (void*)(void* owned)p; +} + +// good cfg +void test2(int* owned p, int x) { + + if (x > 100) goto L1; + + for (int i=0; i<10; i++) { + x = x + i; + } + +L1: + void* temp = (void*)(void* owned)p; +} -- Gitee From ef7d6e453821d9003319667ad566f328bed9e555 Mon Sep 17 00:00:00 2001 From: EdwardWang Date: Mon, 17 Feb 2025 21:12:59 +0800 Subject: [PATCH 2/3] [SafeFeatureFinder] find if owned or borrow is used This patch avoids borrow checking when not necessary, a typical case is that no 'owned' or 'borrow' is used in a Function. A SafeFeatureFinder is implemented as follows: - Reuse the CRTP-based visitor in Clang. For StmtVisitor,it offers the basic traversing logic if VisitStmt is implemented properly in SafeFeatureFinder. - The VisitStmt in SafeFeatureFinder traverse the 'Children', which is a common interface for all XStmt. - If XStmt does not implement VisitXStmt, it will fallback to VisitStmt. - In the SafeFeatureFinder, only leaf nodes which use owned or borrow, need to implement the VisitXSmt methods. Others will fallback to VisitStmt method. - DeclVisitor is different than StmtVisitor, there has no good fallback mechanism to traverse the nodes. We have to override every VisitXDecl if for traversing purpose. - TypeVisitor is much the same as DeclVisitor. --- clang/include/clang/AST/BSC/WalkerBSC.h | 133 ++++++++++++++---- clang/lib/Sema/BSC/SemaDeclBSC.cpp | 35 ++++- clang/lib/Sema/Sema.cpp | 5 +- .../Ownership/RuleCheck/Maple/if/if.cbs | 7 +- 4 files changed, 141 insertions(+), 39 deletions(-) diff --git a/clang/include/clang/AST/BSC/WalkerBSC.h b/clang/include/clang/AST/BSC/WalkerBSC.h index bc18f1206cb1..0694c0f77afb 100644 --- a/clang/include/clang/AST/BSC/WalkerBSC.h +++ b/clang/include/clang/AST/BSC/WalkerBSC.h @@ -202,14 +202,10 @@ public: bool VisitStaticAssertDecl(StaticAssertDecl *D) { return Visit(D->getAssertExpr()); } - - bool VisitTypeAliasDecl(TypeAliasDecl *D) { - return true; - } - bool VisitTypeAliasTemplateDecl(TypeAliasTemplateDecl *D) { - return true; - } + bool VisitTypeAliasDecl(TypeAliasDecl *D) { return true; } + + bool VisitTypeAliasTemplateDecl(TypeAliasTemplateDecl *D) { return true; } bool VisitStmt(Stmt *S) { for (auto *C : S->children()) { @@ -337,26 +333,6 @@ public: return false; } - bool VisitParenExpr(ParenExpr *PE) { return Visit(PE->getSubExpr()); } - - bool VisitCallExpr(CallExpr *CE) { - for (auto *Arg : CE->arguments()) { - if (Visit(Arg)) { - return true; - } - } - return Visit(CE->getCallee()); - } - - bool VisitImplicitCastExpr(ImplicitCastExpr *ICE) { - return Visit(ICE->getSubExpr()); - } - - bool VisitBinaryOperator(BinaryOperator *BO) { - return Visit(BO->getLHS()) || Visit(BO->getRHS()); - } - - bool VisitUnaryOperator(UnaryOperator *UO) { return Visit(UO->getSubExpr()); } protected: ASTContext &Context; @@ -377,6 +353,109 @@ private: } }; +/// Whether a FunctionDecl has any "safe" features in it. +/// TODO: add Unit Test +class SafeFeatureFinder : public StmtVisitor, + public DeclVisitor, + public TypeVisitor { +public: + using TypeVisitor::Visit; + using DeclVisitor::Visit; + using StmtVisitor::Visit; + + // TODO: Is this enough? Do we need VisitType API? + bool VisitQualType(QualType QT) { + if (QT.isOwnedQualified() || QT.isBorrowQualified() || + QT->hasBorrowFields() || QT->hasOwnedFields()) { + return true; + } + return false; + } + + bool VisitFunctionDecl(FunctionDecl *FD) { + // Return Type has owned, borrow + if (VisitQualType(FD->getType())) { + return true; + } + + // Function Parameters has owned, borrow + for (auto *Param : FD->parameters()) { + if (VisitQualType(Param->getType())) { + return true; + } + } + + // FuncBody has owned, borrow + if (FD->doesThisDeclarationHaveABody()) { + // Dispatch to VisitCompoundStmt + if (Visit(FD->getBody())) { + return true; + } + } + + return false; + } + + bool VisitVarDecl(VarDecl *D) { + if (VisitQualType(D->getType())) { + return true; + } + + if (D->hasInit()) { + if (Visit((D->getInit()))) { + return true; + } + if (VisitQualType(D->getInit()->getType())) { + return true; + } + } + return false; + } + + // TODO: Not sure other Decls should be override or not. + // RecordDecl, StaticAssertDecl, TypeAliasDecl, BSCMethodDecl etc. + + // If VisitXXXStmt is not implemented, fallback to VisitStmt + bool VisitStmt(Stmt *S) { + // Children() is the common interface implemented by XStmt + for (auto *C : S->children()) { + if (C) { + if (Visit(C)) { + return true; + } + } + } + return false; + } + + bool VisitDeclStmt(DeclStmt *DS) { + for (auto *D : DS->decls()) { + if (Visit(D)) + return true; + } + return false; + } + + bool VisitCStyleCastExpr(CStyleCastExpr *E) { + if (VisitQualType(E->getType())) { + return true; + } + if (VisitExplicitCastExpr(E)) { + return true; + } + return false; + } + + bool VisitUnaryOperator(UnaryOperator *UO) { + constexpr std::array BSCOps = { + UO_AddrMut, UO_AddrMutDeref, UO_AddrConst, UO_AddrConstDeref}; + return std::find(BSCOps.begin(), BSCOps.end(), UO->getOpcode()) != + BSCOps.end(); + } + + bool FindOwnedOrBorrow(FunctionDecl *FD) { return VisitFunctionDecl(FD); } +}; + } // namespace clang #endif // ENABLE_BSC diff --git a/clang/lib/Sema/BSC/SemaDeclBSC.cpp b/clang/lib/Sema/BSC/SemaDeclBSC.cpp index d21738804eba..27824d29e0e2 100644 --- a/clang/lib/Sema/BSC/SemaDeclBSC.cpp +++ b/clang/lib/Sema/BSC/SemaDeclBSC.cpp @@ -111,10 +111,12 @@ bool Sema::HasDiffBorrowOrOwnedParamsTypeAtBothFunction(QualType LHS, } // Return true if any memory safe features are found in a FunctionDecl. +// Qualifiers: owned, borrow, fat +// AddrOp: &mut, &const bool Sema::FindSafeFeatures(const FunctionDecl* FnDecl) { if (!FnDecl) return false; - BSCFeatureFinder finder = BSCFeatureFinder(Context); - if (finder.Visit(const_cast(FnDecl))) { + SafeFeatureFinder finder = SafeFeatureFinder(); + if (finder.FindOwnedOrBorrow(const_cast(FnDecl))) { return true; } return false; @@ -151,6 +153,17 @@ bool Sema::HasSafeZoneInFunction(const FunctionDecl* FnDecl) { return HasSafeZoneInCompoundStmt(FuncBody); } +/// BSC's dataflow analysis process is as follows: +/// ==================================================================== +/// __________________ ___________________ +/// | | | | +/// FuncDecl--> CFG --> | NullabilityCheck | --> | OwnershipAnalysis | --> +/// |__________________| |___________________| +/// __________________ +/// | | +/// --> | BorrowCheck | --> FuncDecl --> CodeGen +/// |__________________| +/// ==================================================================== void Sema::BSCDataflowAnalysis(const Decl *D, bool EnableOwnershipCheck, bool EnableNullabilityCheck) { AnalysisDeclContext AC(/* AnalysisDeclContextManager */ nullptr, D); @@ -164,8 +177,19 @@ void Sema::BSCDataflowAnalysis(const Decl *D, bool EnableOwnershipCheck, AC.getCFGBuildOptions().BSCMode = true; AC.getCFGBuildOptions().setAllAlwaysAdd(); - bool RequireCFGAnalysis = LangOpts.BSC ? FindSafeFeatures(dyn_cast_or_null(D)) : false; - if (RequireCFGAnalysis && AC.getCFG()) { + const FunctionDecl *FD = nullptr; + if (D->getKind() == Decl::BSCMethod) { + const BSCMethodDecl* BSCMethod = dyn_cast_or_null(D); + FD = BSCMethod; + } else { + FD = dyn_cast_or_null(D); + } + + // If D does not use memory safety features like "owned, borrow, &mut, &const", + // we should not do borrow checking. + bool RequireBorrowCheck = LangOpts.BSC ? FindSafeFeatures(FD) : false; + bool RequireCFGAnalysis = EnableNullabilityCheck || RequireBorrowCheck; + if (RequireCFGAnalysis && AC.getCFG()) { // Only build the CFG when needed. const FunctionDecl *FD = cast(D); unsigned NumNullabilityCheckErrorsInCurrFD = 0; LangOptions::NullCheckZone CheckZone = getLangOpts().getNullabilityCheck(); @@ -179,7 +203,8 @@ void Sema::BSCDataflowAnalysis(const Decl *D, bool EnableOwnershipCheck, } // Run ownership analysis when there is no nullability errors in current // function. - if (EnableOwnershipCheck && !NumNullabilityCheckErrorsInCurrFD) { + bool DoBorrowCheck = EnableOwnershipCheck & RequireBorrowCheck; + if (DoBorrowCheck && !NumNullabilityCheckErrorsInCurrFD) { OwnershipDiagReporter OwnershipReporter(*this); runOwnershipAnalysis(*FD, *AC.getCFG(), AC, OwnershipReporter, Context); OwnershipReporter.flushDiagnostics(); diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index f31227ebaec2..7f55f3a83696 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -2212,10 +2212,9 @@ Sema::PopFunctionScopeInfo(const AnalysisBasedWarnings::Policy *WP, popOpenMPFunctionRegion(Scope.get()); #if ENABLE_BSC - // If D does not use memory safety features like "owned, borrow, &mut, &const", - // we do not do borrow checking. - // bool useSafeFeature = LangOpts.BSC ? FindSafeFeatures(dyn_cast_or_null(D)) : false; + // FIXME: Rename EnableOwnershipCheck, maybe EnableBorrowCheck is a better name. bool EnableOwnershipCheck = LangOpts.BSC ? (!(getLangOpts().DisableOwnershipCheck)) : false; + // If NullabilityCheck is in Mode: {SafeOnly, All}, we should build CFG. bool EnableNullabilityCheck = LangOpts.BSC ? (getLangOpts().getNullabilityCheck() != LangOptions::NC_NONE) : false; if (EnableOwnershipCheck || EnableNullabilityCheck) { if (!isBSCCoroutine && diff --git a/clang/test/BSC/Negative/Ownership/RuleCheck/Maple/if/if.cbs b/clang/test/BSC/Negative/Ownership/RuleCheck/Maple/if/if.cbs index f0586ae7ca07..188ba79a4b50 100644 --- a/clang/test/BSC/Negative/Ownership/RuleCheck/Maple/if/if.cbs +++ b/clang/test/BSC/Negative/Ownership/RuleCheck/Maple/if/if.cbs @@ -72,9 +72,8 @@ void func6(int * owned argA, int * owned argB) { int * owned b; label1: if (rand()) { - a = argA; // expected-error {{assign to owned value: `a`}} expected-error {{use of moved value: `argA`}} - goto label1; + a = argA; + goto label1; // expected-error {{The CFG is not structured}} } -} // expected-error {{memory leak of value: `a`}} expected-error {{memory leak of value: `argA`}} expected-error {{memory leak of value: `argB`}} - +} int main() { return 0; } \ No newline at end of file -- Gitee From b782dd7304ca3dc4a25bd1632123384294e0d6b1 Mon Sep 17 00:00:00 2001 From: EdwardWang Date: Tue, 25 Feb 2025 14:25:39 +0800 Subject: [PATCH 3/3] [CFG] fix review comments --- clang/include/clang/AST/BSC/WalkerBSC.h | 9 +--- clang/lib/Analysis/CFG.cpp | 10 ++-- clang/lib/Parse/ParseExpr.cpp | 1 + clang/lib/Parse/ParseTemplate.cpp | 3 +- clang/lib/Sema/BSC/SemaBSCOverload.cpp | 3 +- clang/lib/Sema/BSC/SemaDeclBSC.cpp | 39 +++++++++------- clang/lib/Sema/SemaExpr.cpp | 1 + .../Ownership/RuleCheck/Maple/if/if.cbs | 2 +- .../Ownership/RuleCheck/bad-cfg/bad-cfg.cbs | 2 +- .../Positive/OperatorOverload/deref/deref.cbs | 46 ++++++++----------- .../member_ref/member_ref.cbs | 32 ++++++------- 11 files changed, 71 insertions(+), 77 deletions(-) diff --git a/clang/include/clang/AST/BSC/WalkerBSC.h b/clang/include/clang/AST/BSC/WalkerBSC.h index 0694c0f77afb..9def7ca98e82 100644 --- a/clang/include/clang/AST/BSC/WalkerBSC.h +++ b/clang/include/clang/AST/BSC/WalkerBSC.h @@ -356,10 +356,8 @@ private: /// Whether a FunctionDecl has any "safe" features in it. /// TODO: add Unit Test class SafeFeatureFinder : public StmtVisitor, - public DeclVisitor, - public TypeVisitor { + public DeclVisitor { public: - using TypeVisitor::Visit; using DeclVisitor::Visit; using StmtVisitor::Visit; @@ -447,10 +445,7 @@ public: } bool VisitUnaryOperator(UnaryOperator *UO) { - constexpr std::array BSCOps = { - UO_AddrMut, UO_AddrMutDeref, UO_AddrConst, UO_AddrConstDeref}; - return std::find(BSCOps.begin(), BSCOps.end(), UO->getOpcode()) != - BSCOps.end(); + return UO->getOpcode() >= UO_AddrMut && UO->getOpcode() <= UO_AddrConstDeref; } bool FindOwnedOrBorrow(FunctionDecl *FD) { return VisitFunctionDecl(FD); } diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index 15afb0df89f9..10d20507ef50 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -3423,7 +3423,8 @@ CFGBlock *CFGBuilder::VisitGotoStmt(GotoStmt *G) { // If we already know the mapping to the label block add the successor now. LabelMapTy::iterator I = LabelMap.find(G->getLabel()); - if (I == LabelMap.end()) + if (I == LabelMap.end()) { + #if ENABLE_BSC // Label appears first, then goto stmt // This is not structured cfg, bishengc should report error if (BuildOpts.BSCMode) { @@ -3431,13 +3432,16 @@ CFGBlock *CFGBuilder::VisitGotoStmt(GotoStmt *G) { DiagnosticsEngine &Diag = getDiagnostic(); unsigned ID = Diag.getCustomDiagID( DiagnosticsEngine::Error, - "The CFG is not structured" + "Invalid code structure. Jumping back to previous labels is not allowed." ); Diag.Report(G->getBeginLoc(), ID); } else { - // We will need to backpatch this block later. + #endif // We will need to backpatch this block later. BackpatchBlocks.push_back(JumpSource(Block, ScopePos)); + #if ENABLE_BSC } + #endif + } else { JumpTarget JT = I->second; addAutomaticObjHandling(ScopePos, JT.scopePosition, G); diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp index 0a6fd4e9e241..e711ea022f1b 100644 --- a/clang/lib/Parse/ParseExpr.cpp +++ b/clang/lib/Parse/ParseExpr.cpp @@ -2294,6 +2294,7 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) { ArgsSizeOK = ArgExprs.size() == 1 || ArgExprs.size() - 2 == CommaLocs.size(); } + // TODO: change this ASSERT to to a Diagnostic. assert(ArgsSizeOK && "Unexpected number of commas!"); #else assert( diff --git a/clang/lib/Parse/ParseTemplate.cpp b/clang/lib/Parse/ParseTemplate.cpp index 31dfc0662809..8f8feed4fd80 100644 --- a/clang/lib/Parse/ParseTemplate.cpp +++ b/clang/lib/Parse/ParseTemplate.cpp @@ -1069,9 +1069,8 @@ NamedDecl *Parser::ParseTypeParameter(unsigned Depth, unsigned Position) { NamedDecl *Parser::ParseBSCTypeParameter(unsigned Depth, unsigned Position) { // Check Tok location Token PeekTok = PP.LookAhead(BSCGenericLookAhead); - bool isBSCTemplateTypeParameter = getLangOpts().BSC; // TODO: fix the cond assert( - isBSCTemplateTypeParameter && + getLangOpts().BSC && "A type-parameter starts with 'class', 'typename' or a type-constraint"); if (PeekTok.is(tok::kw_struct)) { diff --git a/clang/lib/Sema/BSC/SemaBSCOverload.cpp b/clang/lib/Sema/BSC/SemaBSCOverload.cpp index 1c3d1acb6aa7..b459407b60a2 100644 --- a/clang/lib/Sema/BSC/SemaBSCOverload.cpp +++ b/clang/lib/Sema/BSC/SemaBSCOverload.cpp @@ -29,8 +29,7 @@ bool Sema::IsDesugarNeededOperatorKind(OverloadedOperatorKind Op) { // overloaded function. Expr *Sema::DesugarOperatorFirstArg(FunctionDecl *FD, ArrayRef Args) { if (const auto *FPT = FD->getType()->castAs()) { - unsigned num = FPT->getNumParams(); - assert(num != 0 && "The function must have at least one parameter."); + assert(FPT->getNumParams() != 0 && "The function must have at least one parameter."); QualType FirstParamTy = FPT->getParamType(0); if (FirstParamTy->isPointerType() && !Args[0]->getType()->isPointerType()) { QualType QT = Args[0]->getType(); diff --git a/clang/lib/Sema/BSC/SemaDeclBSC.cpp b/clang/lib/Sema/BSC/SemaDeclBSC.cpp index 27824d29e0e2..3dc080a812ee 100644 --- a/clang/lib/Sema/BSC/SemaDeclBSC.cpp +++ b/clang/lib/Sema/BSC/SemaDeclBSC.cpp @@ -115,7 +115,7 @@ bool Sema::HasDiffBorrowOrOwnedParamsTypeAtBothFunction(QualType LHS, // AddrOp: &mut, &const bool Sema::FindSafeFeatures(const FunctionDecl* FnDecl) { if (!FnDecl) return false; - SafeFeatureFinder finder = SafeFeatureFinder(); + SafeFeatureFinder finder; if (finder.FindOwnedOrBorrow(const_cast(FnDecl))) { return true; } @@ -168,6 +168,7 @@ void Sema::BSCDataflowAnalysis(const Decl *D, bool EnableOwnershipCheck, bool EnableNullabilityCheck) { AnalysisDeclContext AC(/* AnalysisDeclContextManager */ nullptr, D); + // TODO: understand how these parameters affect the CFG. AC.getCFGBuildOptions().PruneTriviallyFalseEdges = true; AC.getCFGBuildOptions().AddEHEdges = false; AC.getCFGBuildOptions().AddInitializers = true; @@ -177,39 +178,43 @@ void Sema::BSCDataflowAnalysis(const Decl *D, bool EnableOwnershipCheck, AC.getCFGBuildOptions().BSCMode = true; AC.getCFGBuildOptions().setAllAlwaysAdd(); - const FunctionDecl *FD = nullptr; - if (D->getKind() == Decl::BSCMethod) { - const BSCMethodDecl* BSCMethod = dyn_cast_or_null(D); - FD = BSCMethod; - } else { - FD = dyn_cast_or_null(D); - } + const FunctionDecl *FD = dyn_cast_or_null(D); // If D does not use memory safety features like "owned, borrow, &mut, &const", // we should not do borrow checking. bool RequireBorrowCheck = LangOpts.BSC ? FindSafeFeatures(FD) : false; - bool RequireCFGAnalysis = EnableNullabilityCheck || RequireBorrowCheck; - if (RequireCFGAnalysis && AC.getCFG()) { // Only build the CFG when needed. - const FunctionDecl *FD = cast(D); + // nullability-check happens in mode: {SafeOnly, All}. + // For SafeOnly, do not build cfg when there is no SafeZone in Function. + bool RequireNullabilityCheck = EnableNullabilityCheck; + if (getLangOpts().getNullabilityCheck() == LangOptions::NC_SAFE) { + if(HasSafeZoneInFunction(FD)) { + RequireNullabilityCheck = EnableNullabilityCheck; + } else { + RequireNullabilityCheck = false; + } + } + bool RequireCFGAnalysis = RequireNullabilityCheck || RequireBorrowCheck; + if (RequireCFGAnalysis && FD && AC.getCFG()) { // Only build the CFG when needed. + // Step one: Run NullabilityCheck unsigned NumNullabilityCheckErrorsInCurrFD = 0; - LangOptions::NullCheckZone CheckZone = getLangOpts().getNullabilityCheck(); - if (EnableNullabilityCheck && (CheckZone == LangOptions::NC_ALL || HasSafeZoneInFunction(FD))) { + if (RequireNullabilityCheck) { NullabilityCheckDiagReporter NullabilityCheckReporter(*this); runNullabilityCheck(*FD, *AC.getCFG(), AC, NullabilityCheckReporter, Context); NullabilityCheckReporter.flushDiagnostics(); NumNullabilityCheckErrorsInCurrFD = NullabilityCheckReporter.getNumErrors(); + } - // Run ownership analysis when there is no nullability errors in current - // function. + // Step two: Run ownership analysis when there is no nullability errors in + // current function. bool DoBorrowCheck = EnableOwnershipCheck & RequireBorrowCheck; if (DoBorrowCheck && !NumNullabilityCheckErrorsInCurrFD) { OwnershipDiagReporter OwnershipReporter(*this); runOwnershipAnalysis(*FD, *AC.getCFG(), AC, OwnershipReporter, Context); OwnershipReporter.flushDiagnostics(); - // Run borrow check when there is no other ownership errors in current - // function. + // Step three: Run borrow check when there is no other ownership errors in + // current function. if (!OwnershipReporter.getNumErrors()) { BorrowCheckDiagReporter BorrowCheckReporter(*this); runBorrowCheck(*FD, *AC.getCFG(), BorrowCheckReporter, Context); diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 8fc00e78e831..246582275fef 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -14032,6 +14032,7 @@ enum NonConstCaptureKind { NCCK_None, NCCK_Block, NCCK_Lambda }; static NonConstCaptureKind isReferenceToNonConstCapture(Sema &S, Expr *E) { #if ENABLE_BSC if (S.getLangOpts().BSC) { + // FIXME: Change these ASSERT to Diagostic if (const UnaryOperator *UO = dyn_cast(E)) assert(E->isLValue() && (E->getType().isConstQualified() || UO->getSubExpr()->getType().isConstBorrow())); diff --git a/clang/test/BSC/Negative/Ownership/RuleCheck/Maple/if/if.cbs b/clang/test/BSC/Negative/Ownership/RuleCheck/Maple/if/if.cbs index 188ba79a4b50..d43c7256ce6f 100644 --- a/clang/test/BSC/Negative/Ownership/RuleCheck/Maple/if/if.cbs +++ b/clang/test/BSC/Negative/Ownership/RuleCheck/Maple/if/if.cbs @@ -73,7 +73,7 @@ void func6(int * owned argA, int * owned argB) { label1: if (rand()) { a = argA; - goto label1; // expected-error {{The CFG is not structured}} + goto label1; // expected-error {{Invalid code structure. Jumping back to previous labels is not allowed.}} } } int main() { return 0; } \ No newline at end of file diff --git a/clang/test/BSC/Negative/Ownership/RuleCheck/bad-cfg/bad-cfg.cbs b/clang/test/BSC/Negative/Ownership/RuleCheck/bad-cfg/bad-cfg.cbs index a2de69fc467c..fc99af5f6b2d 100644 --- a/clang/test/BSC/Negative/Ownership/RuleCheck/bad-cfg/bad-cfg.cbs +++ b/clang/test/BSC/Negative/Ownership/RuleCheck/bad-cfg/bad-cfg.cbs @@ -4,7 +4,7 @@ void test1(int* owned p, int x) { L1: x = x + 1; - if (x < 10) goto L1; // expected-error {{The CFG is not structured}} + if (x < 10) goto L1; // expected-error {{Invalid code structure. Jumping back to previous labels is not allowed.}} void* temp = (void*)(void* owned)p; } diff --git a/clang/test/BSC/Positive/OperatorOverload/deref/deref.cbs b/clang/test/BSC/Positive/OperatorOverload/deref/deref.cbs index 80c0585677e7..04f9e4652105 100644 --- a/clang/test/BSC/Positive/OperatorOverload/deref/deref.cbs +++ b/clang/test/BSC/Positive/OperatorOverload/deref/deref.cbs @@ -167,18 +167,15 @@ int main() { // CHECK-NEXT: struct RcData_int; // CHECK-NEXT: struct Rc_int; // CHECK-NEXT: struct RcRecord_int; -// CHECK-NEXT: struct E { -// CHECK-NEXT: int a; -// CHECK-NEXT: }; -// CHECK-EMPTY: -// CHECK: struct G { -// CHECK-NEXT: int a; -// CHECK-NEXT: }; -// CHECK-EMPTY: -// CHECK: struct I { -// CHECK-NEXT: int a; -// CHECK-NEXT: }; -// CHECK-EMPTY: +// CHECK-DAG: struct E { +// CHECK-DAG-NEXT: int a; +// CHECK-DAG-NEXT: }; +// CHECK-DAG: struct G { +// CHECK-DAG-NEXT: int a; +// CHECK-DAG-NEXT: }; +// CHECK-DAG: struct I { +// CHECK-DAG-NEXT: int a; +// CHECK-DAG-NEXT: }; // CHECK: struct Rc_int { // CHECK-NEXT: struct RcData_int *ptr; // CHECK-NEXT: }; @@ -187,20 +184,17 @@ int main() { // CHECK-NEXT: int data; // CHECK-NEXT: }; // CHECK-EMPTY: -// CHECK: struct F { -// CHECK-NEXT: struct E e; -// CHECK-NEXT: }; -// CHECK-EMPTY: -// CHECK: struct H { -// CHECK-NEXT: struct G g; -// CHECK-NEXT: int b; -// CHECK-NEXT: }; -// CHECK-EMPTY: -// CHECK: struct J { -// CHECK-NEXT: struct I i; -// CHECK-NEXT: int b; -// CHECK-NEXT: }; -// CHECK-EMPTY: +// CHECK-DAG: struct F { +// CHECK-DAG-NEXT: struct E e; +// CHECK-DAG-NEXT: }; +// CHECK-DAG: struct H { +// CHECK-DAG-NEXT: struct G g; +// CHECK-DAG-NEXT: int b; +// CHECK-DAG-NEXT: }; +// CHECK-DAG: struct J { +// CHECK-DAG-NEXT: struct I i; +// CHECK-DAG-NEXT: int b; +// CHECK-DAG-NEXT: }; // CHECK: struct RcRecord_int { // CHECK-NEXT: struct Rc_int rc; // CHECK-NEXT: }; diff --git a/clang/test/BSC/Positive/OperatorOverload/member_ref/member_ref.cbs b/clang/test/BSC/Positive/OperatorOverload/member_ref/member_ref.cbs index 11b0c00f1148..e5aea663a858 100644 --- a/clang/test/BSC/Positive/OperatorOverload/member_ref/member_ref.cbs +++ b/clang/test/BSC/Positive/OperatorOverload/member_ref/member_ref.cbs @@ -224,24 +224,20 @@ int main(){ // CHECK-NEXT: struct RcData_int; // CHECK-NEXT: struct Rc_int; // CHECK-NEXT: struct RcRecord_int; -// CHECK-NEXT: struct A { -// CHECK-NEXT: int m; -// CHECK-NEXT: }; -// CHECK-EMPTY: -// CHECK: struct AP { -// CHECK-NEXT: struct A *a; -// CHECK-NEXT: }; -// CHECK-EMPTY: -// CHECK: struct B { -// CHECK-NEXT: int x; -// CHECK-NEXT: int y; -// CHECK-NEXT: }; -// CHECK-EMPTY: -// CHECK: struct BP { -// CHECK-NEXT: struct A *a; -// CHECK-NEXT: struct B *b; -// CHECK-NEXT: }; -// CHECK-EMPTY: +// CHECK-DAG: struct A { +// CHECK-DAG-NEXT: int m; +// CHECK-DAG-NEXT: }; +// CHECK-DAG: struct AP { +// CHECK-DAG-NEXT: struct A *a; +// CHECK-DAG-NEXT: }; +// CHECK-DAG: struct B { +// CHECK-DAG-NEXT: int x; +// CHECK-DAG-NEXT: int y; +// CHECK-DAG-NEXT: }; +// CHECK-DAG: struct BP { +// CHECK-DAG-NEXT: struct A *a; +// CHECK-DAG-NEXT: struct B *b; +// CHECK-DAG-NEXT: }; // CHECK: struct CP { // CHECK-NEXT: struct A * a; // CHECK-NEXT: }; -- Gitee