llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-temporal-safety Author: NeKon69 <details> <summary>Changes</summary> This PR introduces C language support for LifetimeSafety. There are a few constraints that make supporting C a bit cumbersome: * C assignment expressions are rvalues, unlike C++ assignment expressions. The analysis has to account for the different origin shape of the assignment result by stripping an origin from `LHSExpr`. * Function addresses in C do not need lifetime tracking. Taking `&f` should not create origins because functions do not have local object lifetime (unlike in C++). * GNU C permits `void*` subscripting/pointer arithmetic. Expressions like `bytes[0]` (where `bytes` is `void*`) have type `void` and do not produce an addressable object with origins, even though `void*` itself can carry pointer origins. * Some C subscripts, such as vector subscripts, are not GLValues, so they do not have storage origins to track. * `va_arg(ap, array_type)` is undefined behavior, so we skip it instead of trying to model origins for it. * C does not have a spelling for `[[gsl::Owner]]` / `[[gsl::Pointer]]`, so C support is limited to pointer/object cases that can be represented in C and `__attribute__((lifetimebound))`. What validation I performed: * Ran a crash-smoke test over C files in the LLVM tree with `-Wlifetime-safety`. It finished without crashes after the fixes. * Generated a C-only subset of `warn-lifetime-safety.cpp` and compiled it with `-Wlifetime-safety`, to reuse existing pointer-flow coverage in C mode. * Built a Linux kernel `allmodconfig` with `-Wlifetime-safety-all`, `-flifetime-safety-inference`, and `-fexperimental-lifetime-safety-tu-analysis`. No lifetime-safety crashes or warnings were seen. Assisted-by: GPT-5.5 for writing bash scripts to perform validation. --- Full diff: https://github.com/llvm/llvm-project/pull/203270.diff 4 Files Affected: - (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h (+1) - (modified) clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp (+23-1) - (modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+2-1) - (added) clang/test/Sema/warn-lifetime-safety-c.c (+152) ``````````diff diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h index 2c961bd305fac..6e7e199a61b80 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h @@ -160,6 +160,7 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> { // exempting it from the check. llvm::DenseMap<const Expr *, UseFact *> UseFacts; const CFGBlock *CurrentBlock; + bool IsCMode = false; }; } // namespace clang::lifetimes::internal diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 9fbfaf8ae606b..c9fa4abedd518 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -106,6 +106,8 @@ void FactsGenerator::run() { llvm::TimeTraceScope TimeProfile("FactGenerator"); const CFG &Cfg = *AC.getCFG(); llvm::SmallVector<Fact *> PlaceholderLoanFacts = issuePlaceholderLoans(); + IsCMode = !AC.getASTContext().getLangOpts().CPlusPlus && + !AC.getASTContext().getLangOpts().ObjC; // Iterate through the CFG blocks in reverse post-order to ensure that // initializations and destructions are processed in the correct sequence. for (const CFGBlock *Block : *AC.getAnalysis<PostOrderCFGView>()) { @@ -324,6 +326,10 @@ void FactsGenerator::VisitCastExpr(const CastExpr *CE) { flow(Dest, Src, /*Kill=*/true); return; case CK_ArrayToPointerDecay: + // va_arg(ap, array_type) is UB and does not provide addressable array + // storage to model. + if (isa<VAArgExpr>(SubExpr)) + return; assert(Src && "Array expression should have origins as it is GL value"); CurrentBlockFacts.push_back(FactMgr.createFact<OriginFlowFact>( Dest->getOuterOriginID(), Src->getOuterOriginID(), /*Kill=*/true)); @@ -347,6 +353,12 @@ void FactsGenerator::VisitUnaryOperator(const UnaryOperator *UO) { switch (UO->getOpcode()) { case UO_AddrOf: { const Expr *SubExpr = UO->getSubExpr(); + // In C, function addresses do not need lifetime tracking. Also skip + // address-of on void expressions: GNU C permits them, but void itself has + // no origins to track. + if (IsCMode && (SubExpr->getType()->isFunctionType() || + SubExpr->getType()->isVoidType())) + return; // The origin of an address-of expression (e.g., &x) is the origin of // its sub-expression (x). This fact will cause the dataflow analysis // to propagate any loans held by the sub-expression's origin to the @@ -392,6 +404,7 @@ void FactsGenerator::handleAssignment(const Expr *TargetExpr, } if (!LHSList) return; + OriginList *RHSList = getOriginsList(*RHSExpr); // For operator= with reference parameters (e.g., // `View& operator=(const View&)`), the RHS argument stays an lvalue, @@ -435,7 +448,12 @@ void FactsGenerator::handleAssignment(const Expr *TargetExpr, // Kill the old loans of the destination origin and flow the new loans // from the source origin. flow(LHSList->peelOuterOrigin(), RHSList, /*Kill=*/true); - killAndFlowOrigin(*TargetExpr, *LHSExpr); + + // In C, assignment expressions are not GLValues, so the assignment result has + // the assigned value origins, not the LHS storage origin. + if (IsCMode) + LHSList = getRValueOrigins(LHSExpr, LHSList); + flow(getOriginsList(*TargetExpr), LHSList, /*Kill=*/true); } void FactsGenerator::handlePointerArithmetic(const BinaryOperator *BO) { @@ -622,6 +640,10 @@ void FactsGenerator::VisitLambdaExpr(const LambdaExpr *LE) { } void FactsGenerator::VisitArraySubscriptExpr(const ArraySubscriptExpr *ASE) { + // Some C subscripts do not refer to addressable storage with origins, such as + // GNU void-pointer subscripts and vector element extraction from rvalues. + if (IsCMode && !ASE->isGLValue()) + return; assert(ASE->isGLValue() && "Array subscript should be a GL value"); OriginList *Dst = getOriginsList(*ASE); assert(Dst && "Array subscript should have origins as it is a GL value"); diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 207ce373339f3..1a3319f3e8726 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -3164,7 +3164,8 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings( // TODO: Enable lifetime safety analysis for other languages once it is // stable. - if (EnableLifetimeSafetyAnalysis && S.getLangOpts().CPlusPlus) { + if (EnableLifetimeSafetyAnalysis && + (S.getLangOpts().CPlusPlus || !S.getLangOpts().ObjC)) { if (AC.getCFG()) { lifetimes::LifetimeSafetySemaHelperImpl LifetimeSafetySemaHelper(S); lifetimes::runLifetimeSafetyAnalysis(AC, &LifetimeSafetySemaHelper, diff --git a/clang/test/Sema/warn-lifetime-safety-c.c b/clang/test/Sema/warn-lifetime-safety-c.c new file mode 100644 index 0000000000000..9c809d4351bb5 --- /dev/null +++ b/clang/test/Sema/warn-lifetime-safety-c.c @@ -0,0 +1,152 @@ +// RUN: %clang_cc1 -fsyntax-only -Wlifetime-safety -Wno-dangling -verify %s + +int *identity(int *p __attribute__((lifetimebound))) { return p; } + +struct PointerField { + int *ptr; +}; + +struct IntField { + int field; +}; + +void simple_case(void) { + int *p; + { + int i; + p = &i; // expected-warning {{local variable 'i' does not live long enough}} + } // expected-note {{destroyed here}} + (void)*p; // expected-note {{later used here}} +} + +void chained_assignment(void) { + int *p, *q, *r; + { + int i; + p = q = r = &i; // expected-warning {{local variable 'i' does not live long enough}} + } // expected-note {{destroyed here}} + (void)*p; // expected-note {{later used here}} +} + +void conditional_branch(int cond) { + int safe; + int *p = &safe; + if (cond) { + int i; + p = &i; // expected-warning {{local variable 'i' does not live long enough}} + } // expected-note {{destroyed here}} + (void)*p; // expected-note {{later used here}} +} + +void loop_with_break(int cond) { + int safe; + int *p = &safe; + for (int n = 0; n != 10; ++n) { + if (cond) { + int i; + p = &i; // expected-warning {{local variable 'i' does not live long enough}} + break; // expected-note {{destroyed here}} + } + } + (void)*p; // expected-note {{later used here}} +} + +int *return_stack_address(void) { + int i; + int *p = &i; // expected-warning {{stack memory associated with local variable 'i' is returned}} + return p; // expected-note {{returned here}} +} + +void lifetimebound_call(void) { + int *p; + { + int i; + p = identity(&i); // expected-warning {{local variable 'i' does not live long enough}} + } // expected-note {{destroyed here}} + (void)*p; // expected-note {{later used here}} +} + +void struct_pointer_field(void) { + int *p; + { + int i; + struct PointerField holder; + // FIXME: Track origins stored in struct pointer fields. + holder.ptr = &i; + p = holder.ptr; + } + (void)*p; +} + +void struct_address_of_field(void) { + int *p; + { + struct IntField holder; + p = &holder.field; // expected-warning {{local variable 'holder' does not live long enough}} + } // expected-note {{destroyed here}} + (void)*p; // expected-note {{later used here}} +} + +void conditional_operator_lifetimebound(int cond) { + int *p; + { + int a, b; + p = identity(cond ? &a // expected-warning {{local variable 'a' does not live long enough}} + : &b); // expected-warning {{local variable 'b' does not live long enough}} + } // expected-note 2 {{destroyed here}} + (void)*p; // expected-note 2 {{later used here}} +} + +union IntOrPtr { + int i; + int *p; +}; + +void union_member(void) { + int *p; + { + union IntOrPtr u; + p = &u.i; // expected-warning {{local variable 'u' does not live long enough}} + } // expected-note {{destroyed here}} + (void)*p; // expected-note {{later used here}} +} + +struct AnonymousUnion { + union { + int i; + float f; + }; +}; + +void anonymous_union_member(void) { + int *p; + { + struct AnonymousUnion u; + p = &u.i; // expected-warning {{local variable 'u' does not live long enough}} + } // expected-note {{destroyed here}} + (void)*p; // expected-note {{later used here}} +} + +void function_address_regression(void) { + extern void function_address_target(void); + char *p = (char *)&function_address_target; + (void)p; +} + +int void_pointer_subscript_regression(void *bytes) { + return &bytes[0] == &bytes[1]; +} + +typedef __attribute__((vector_size(16))) int v4i32; +v4i32 (*vector_factory)(int); + +int vector_subscript_regression(void) { + return (*vector_factory)(0)[0]; +} + +void va_arg_array_regression(int n, ...) { + __builtin_va_list ap; + __builtin_va_start(ap, n); + int *p = __builtin_va_arg(ap, int[4]); // expected-warning {{second argument to 'va_arg' is of array type 'int[4]'}} + (void)p; +} `````````` </details> https://github.com/llvm/llvm-project/pull/203270 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
