From 31142121e69df69b9bc1546e22fac62a20b47850 Mon Sep 17 00:00:00 2001 From: huangbowen <1374839799@qq.com> Date: Wed, 18 Jun 2025 16:52:24 +0800 Subject: [PATCH 1/2] [BSC] Add recursive null checks for nested struct initializers Previously only the top-level InitListExpr was checked, so nested cases like: struct out a = { {nullptr}, {{nullptr}} }; were not diagnosed. This patch: - Introduces HandleFieldStructInit, a recursive helper that takes a RecordDecl and its InitListExpr. - Recurses on nested record fields to any depth. - Normalizes initializers by stripping wrappers. - Adds tests verifying that any `_Nonnull` pointer in nested structs triggers an error when initialized to nullptr. Test cases: struct Inner { int* _Nonnull p; }; struct Outer { struct Inner in; }; struct Outer o1 = { {nullptr} }; // catches in.p struct Outer o2 = (struct Outer){ {nullptr} }; // catches in.p --- clang/.gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/.gitignore b/clang/.gitignore index 3004923c22af..713dbfef2cf0 100644 --- a/clang/.gitignore +++ b/clang/.gitignore @@ -34,3 +34,4 @@ docs/analyzer/_build .vscode .vs +./test/ -- Gitee From 9184e6f65e593481a63a64bef4b16612885f6b6a Mon Sep 17 00:00:00 2001 From: huangbowen <1374839799@qq.com> Date: Wed, 18 Jun 2025 19:38:12 +0800 Subject: [PATCH 2/2] [nullability] Add recursive null checks for nested struct initializers Previously only the top-level InitListExpr was checked, so nested cases like: struct out a = { {nullptr}, {{nullptr}} }; were not diagnosed. This patch: - Introduces HandleFieldStructInit, a recursive helper that takes a RecordDecl and its InitListExpr. - Recurses on nested record fields to any depth. - Normalizes initializers by stripping wrappers. - Adds tests verifying that any `_Nonnull` pointer in nested structs triggers an error when initialized to nullptr. Test cases: struct Inner { int* _Nonnull p; }; struct Outer { struct Inner in; }; struct Outer o1 = { {nullptr} }; // catches in.p struct Outer o2 = (struct Outer){ {nullptr} }; // catches in.p --- .../lib/Analysis/BSC/BSCNullabilityCheck.cpp | 143 ++++++++++++++---- 1 file changed, 112 insertions(+), 31 deletions(-) diff --git a/clang/lib/Analysis/BSC/BSCNullabilityCheck.cpp b/clang/lib/Analysis/BSC/BSCNullabilityCheck.cpp index ebe781adcee2..1afd46474aaa 100644 --- a/clang/lib/Analysis/BSC/BSCNullabilityCheck.cpp +++ b/clang/lib/Analysis/BSC/BSCNullabilityCheck.cpp @@ -13,12 +13,12 @@ #if ENABLE_BSC #include "clang/Analysis/Analyses/BSC/BSCNullabilityCheck.h" +#include "clang/AST/ParentMap.h" #include "clang/AST/StmtVisitor.h" #include "clang/Analysis/AnalysisDeclContext.h" #include "clang/Analysis/CFG.h" #include "clang/Analysis/FlowSensitive/DataflowWorklist.h" #include "llvm/ADT/DenseMap.h" -#include "clang/AST/ParentMap.h" using namespace clang; using namespace std; @@ -116,6 +116,10 @@ public: void HandlePointerInit(DeclStmt *DS, VarDecl *VD); void HandleStructInit(DeclStmt *DS, VarDecl *VD); void HandleFieldInit(DeclStmt *DS, VarDecl *VD, FieldDecl *FD, Expr *FieldInit); + void HandleFieldStructInit(DeclStmt *DS, VarDecl *VD, RecordDecl *RD, + InitListExpr *ILE); + void HandleFieldInit(DeclStmt *DS, VarDecl *VD, FieldDecl *FD, + Expr *FieldInit); void VisitBinaryOperator(BinaryOperator *BO); void VisitUnaryOperator(UnaryOperator *UO); void VisitMemberExpr(MemberExpr *ME); @@ -269,7 +273,8 @@ bool TransferFunctions::IsStmtInSafeZone(Stmt *S) { } bool TransferFunctions::ShouldReportNullPtrError(Stmt *S) { - LangOptions::NullCheckZone CheckZone = Ctx.getLangOpts().getNullabilityCheck(); + LangOptions::NullCheckZone CheckZone = + Ctx.getLangOpts().getNullabilityCheck(); if (CheckZone == LangOptions::NC_ALL) { return true; } @@ -299,8 +304,7 @@ void TransferFunctions::HandlePointerInit(DeclStmt *DS, VarDecl *VD) { // NonNull pointer cannot be assigned by expr // whose PathNullability is nullable. if (RHSKind == NullabilityKind::Nullable && ShouldReportNullPtrError(DS)) { - NullabilityCheckDiagInfo DI(VD->getLocation(), - NonnullAssignedByNullable); + NullabilityCheckDiagInfo DI(VD->getLocation(), NonnullAssignedByNullable); Reporter.addDiagInfo(DI); } } else if (CurrStatusVD.count(VD)) @@ -310,24 +314,95 @@ void TransferFunctions::HandlePointerInit(DeclStmt *DS, VarDecl *VD) { // Init a struct variable with pointer fields. void TransferFunctions::HandleStructInit(DeclStmt *DS, VarDecl *VD) { - auto RecTy = dyn_cast(VD->getType().getCanonicalType()); - if (RecordDecl *RD = RecTy->getDecl()) { - if (auto InitListE = dyn_cast(VD->getInit())) { - // Init by InitListExpr, such as `struct S s = { .a = &local };` - Expr **Inits = InitListE->getInits(); - for (FieldDecl *FD : RD->fields()) - if (FD->getType()->isPointerType()) - HandleFieldInit(DS, VD, FD, Inits[FD->getFieldIndex()]); + Expr *Init = VD->getInit(); + if (!Init) + return; + + // Skip all ParenExpr/ImplicitCastExpr + Init = Init->IgnoreParenImpCasts(); + // Strip all CStyleCastExpr (casts) and CompoundLiteralExpr (compound + // literals) + while (true) { + if (auto *CSE = dyn_cast(Init)) { + Init = CSE->getSubExpr()->IgnoreParenImpCasts(); + continue; } + if (auto *CLE = dyn_cast(Init)) { + if (Expr *Sub = CLE->getInitializer()) { + Init = Sub->IgnoreParenImpCasts(); + continue; + } + } + break; + } + + // Finally attempt InitListExpr + auto *InitListE = dyn_cast(Init); + if (!InitListE) + return; + + // Get the RecordDecl of variable VD + QualType QT = VD->getType().getCanonicalType(); + if (auto *RT = dyn_cast(QT)) { + RecordDecl *RD = RT->getDecl(); + // Use ParentVD = VD as the recursive context; all subfields belong to the + // same top-level variable + HandleFieldStructInit(DS, VD, RD, InitListE); } } -void TransferFunctions::HandleFieldInit(DeclStmt *DS, VarDecl *VD, FieldDecl *FD, Expr *FieldInit) { +// Modified by Huang Bowen +void TransferFunctions::HandleFieldStructInit(DeclStmt *DS, VarDecl *ParentVD, + RecordDecl *RD, + InitListExpr *ILE) { + Expr **Inits = ILE->getInits(); + // Iterate over all fields in the record + for (FieldDecl *FD : RD->fields()) { + Expr *FieldInit = Inits[FD->getFieldIndex()]->IgnoreParenImpCasts(); + + // Skip all ParenExpr/ImplicitCastExpr + FieldInit = FieldInit->IgnoreParenImpCasts(); + // Process C99 compound literals (CompoundLiteralExpr) and strip all + // CStyleCastExpr (casts) and CompoundLiteralExpr (compound literals) + while (true) { + if (auto *CSE = dyn_cast(FieldInit)) { + FieldInit = CSE->getSubExpr()->IgnoreParenImpCasts(); + continue; + } + if (auto *CLE = dyn_cast(FieldInit)) { + if (Expr *Sub = CLE->getInitializer()) { + FieldInit = Sub->IgnoreParenImpCasts(); + continue; + } + } + break; + } + + // (A) If the field is a pointer, perform nullability check + if (FD->getType()->isPointerType()) { + HandleFieldInit(DS, ParentVD, FD, Inits[FD->getFieldIndex()]); + } + // (B) If the field is also a struct/class, recurse into it + else if (auto *NestedRT = + dyn_cast(FD->getType().getCanonicalType())) { + if (RecordDecl *NestedRD = NestedRT->getDecl()) { + // The inner FieldInit should be an InitListExpr + if (auto *NestedILE = dyn_cast(FieldInit)) { + HandleFieldStructInit(DS, ParentVD, NestedRD, NestedILE); + } + } + } + } +} + +void TransferFunctions::HandleFieldInit(DeclStmt *DS, VarDecl *VD, + FieldDecl *FD, Expr *FieldInit) { NullabilityKind LHSKind = getDefNullability(FD->getType(), Ctx); NullabilityKind RHSKind = getExprPathNullability(FieldInit); if (LHSKind == NullabilityKind::NonNull) { if (RHSKind == NullabilityKind::Nullable && ShouldReportNullPtrError(DS)) { - NullabilityCheckDiagInfo DI(FieldInit->getBeginLoc(), NonnullAssignedByNullable); + NullabilityCheckDiagInfo DI(FieldInit->getBeginLoc(), + NonnullAssignedByNullable); Reporter.addDiagInfo(DI); } } else { @@ -348,7 +423,8 @@ void TransferFunctions::VisitBinaryOperator(BinaryOperator *BO) { if (LHSKind == NullabilityKind::NonNull) { // NonNull pointer cannot be assigned by expr // whose PathNullability is nullable. - if (RHSKind == NullabilityKind::Nullable && ShouldReportNullPtrError(BO)) { + if (RHSKind == NullabilityKind::Nullable && + ShouldReportNullPtrError(BO)) { NullabilityCheckDiagInfo DI(BO->getBeginLoc(), NonnullAssignedByNullable); Reporter.addDiagInfo(DI); @@ -361,7 +437,8 @@ void TransferFunctions::VisitBinaryOperator(BinaryOperator *BO) { if (FieldDecl *FD = dyn_cast(ME->getMemberDecl())) { NullabilityKind LHSKind = getDefNullability(FD->getType(), Ctx); if (LHSKind == NullabilityKind::NonNull) { - if (RHSKind == NullabilityKind::Nullable && ShouldReportNullPtrError(BO)) { + if (RHSKind == NullabilityKind::Nullable && + ShouldReportNullPtrError(BO)) { NullabilityCheckDiagInfo DI(ME->getBeginLoc(), NonnullAssignedByNullable); Reporter.addDiagInfo(DI); @@ -385,7 +462,8 @@ void TransferFunctions::VisitCallExpr(CallExpr *CE) { ParmVarDecl *PVD = FD->getParamDecl(i); if (getDefNullability(PVD->getType(), Ctx) == NullabilityKind::NonNull) { Expr *ArgE = CE->getArg(i); - if (getExprPathNullability(ArgE) == NullabilityKind::Nullable && ShouldReportNullPtrError(CE)) { + if (getExprPathNullability(ArgE) == NullabilityKind::Nullable && + ShouldReportNullPtrError(CE)) { NullabilityCheckDiagInfo DI(ArgE->getBeginLoc(), PassNullableArgument); Reporter.addDiagInfo(DI); @@ -399,7 +477,8 @@ void TransferFunctions::VisitCallExpr(CallExpr *CE) { void TransferFunctions::VisitUnaryOperator(UnaryOperator *UO) { UnaryOperator::Opcode Op = UO->getOpcode(); if (Op == UO_Deref || Op == UO_AddrMutDeref || Op == UO_AddrConstDeref) { - if (getExprPathNullability(UO->getSubExpr()) == NullabilityKind::Nullable && ShouldReportNullPtrError(UO)) { + if (getExprPathNullability(UO->getSubExpr()) == NullabilityKind::Nullable && + ShouldReportNullPtrError(UO)) { NullabilityCheckDiagInfo DI(UO->getBeginLoc(), NullablePointerDereference); Reporter.addDiagInfo(DI); @@ -410,7 +489,8 @@ void TransferFunctions::VisitUnaryOperator(UnaryOperator *UO) { // p->a is not allowed when p has nullable PathNullability. void TransferFunctions::VisitMemberExpr(MemberExpr *ME) { if (ME->isArrow()) { - if (getExprPathNullability(ME->getBase()) == NullabilityKind::Nullable && ShouldReportNullPtrError(ME)) { + if (getExprPathNullability(ME->getBase()) == NullabilityKind::Nullable && + ShouldReportNullPtrError(ME)) { NullabilityCheckDiagInfo DI(ME->getBeginLoc(), NullablePointerAccessMember); Reporter.addDiagInfo(DI); @@ -421,11 +501,12 @@ void TransferFunctions::VisitMemberExpr(MemberExpr *ME) { // (int *_Nonnull)p, (int *borrow)p, (int *owned)p is not allowed // when p has nullable PathNullability. void TransferFunctions::VisitCStyleCastExpr(CStyleCastExpr *CSCE) { - if (getDefNullability(CSCE->getTypeAsWritten(), Ctx) == NullabilityKind::NonNull) { - if (getExprPathNullability(CSCE->getSubExpr()) == NullabilityKind::Nullable && + if (getDefNullability(CSCE->getTypeAsWritten(), Ctx) == + NullabilityKind::NonNull) { + if (getExprPathNullability(CSCE->getSubExpr()) == + NullabilityKind::Nullable && ShouldReportNullPtrError(CSCE)) { - NullabilityCheckDiagInfo DI(CSCE->getBeginLoc(), - NullableCastNonnull); + NullabilityCheckDiagInfo DI(CSCE->getBeginLoc(), NullableCastNonnull); Reporter.addDiagInfo(DI); } } @@ -438,7 +519,8 @@ void TransferFunctions::VisitReturnStmt(ReturnStmt *RS) { if (!RV) return; if (getDefNullability(Fd.getReturnType(), Ctx) == NullabilityKind::NonNull) { - if (getExprPathNullability(RV) == NullabilityKind::Nullable && ShouldReportNullPtrError(RS)) { + if (getExprPathNullability(RV) == NullabilityKind::Nullable && + ShouldReportNullPtrError(RS)) { NullabilityCheckDiagInfo DI(RV->getBeginLoc(), ReturnNullable); Reporter.addDiagInfo(DI); } @@ -628,11 +710,10 @@ StatusFP NullabilityCheckImpl::mergeFP(StatusFP statusA, StatusFP statusB) { return statusA; } -std::pair -NullabilityCheckImpl::runOnBlock(const CFGBlock *block, StatusVD statusVD, - StatusFP statusFP, - NullabilityCheckDiagReporter &reporter, - ASTContext &ctx, const FunctionDecl &fd, ParentMap &PM) { +std::pair NullabilityCheckImpl::runOnBlock( + const CFGBlock *block, StatusVD statusVD, StatusFP statusFP, + NullabilityCheckDiagReporter &reporter, ASTContext &ctx, + const FunctionDecl &fd, ParentMap &PM) { TransferFunctions TF(*this, block, statusVD, statusFP, reporter, ctx, fd, PM); for (CFGBlock::const_iterator it = block->begin(), ei = block->end(); @@ -709,8 +790,8 @@ void clang::runNullabilityCheck(const FunctionDecl &fd, const CFG &cfg, } } - std::pair val = - NCI.runOnBlock(block, valVD, valFP, reporter, ctx, fd, ac.getParentMap()); + std::pair val = NCI.runOnBlock( + block, valVD, valFP, reporter, ctx, fd, ac.getParentMap()); NCI.BlocksEndStatusVD[block] = val.first; NCI.BlocksEndStatusFP[block] = val.second; if (preValVD == val.first && preValFP == val.second) -- Gitee