From efe2f45aa02213e2c86dcba46783139cfacc4845 Mon Sep 17 00:00:00 2001 From: Timm Bäder Date: Oct 10 2023 04:15:23 +0000 Subject: Backport fix for RHEL-1650 --- diff --git a/cfg.patch b/cfg.patch new file mode 100644 index 0000000..779558a --- /dev/null +++ b/cfg.patch @@ -0,0 +1,298 @@ +commit ad4a5130277776d8f15f40ac5a6dede6ad3aabfb +Author: Timm Bäder +Date: Tue Aug 8 14:11:33 2023 +0200 + + [clang][CFG] Cleanup functions + + Add declarations declared with attribute(cleanup(...)) to the CFG, + similar to destructors. + + Differential Revision: https://reviews.llvm.org/D157385 + +diff --git a/clang/include/clang/Analysis/CFG.h b/clang/include/clang/Analysis/CFG.h +index cf4fa2da2a35..67383bb316d3 100644 +--- a/clang/include/clang/Analysis/CFG.h ++++ b/clang/include/clang/Analysis/CFG.h +@@ -14,10 +14,11 @@ + #ifndef LLVM_CLANG_ANALYSIS_CFG_H + #define LLVM_CLANG_ANALYSIS_CFG_H + +-#include "clang/Analysis/Support/BumpVector.h" +-#include "clang/Analysis/ConstructionContext.h" ++#include "clang/AST/Attr.h" + #include "clang/AST/ExprCXX.h" + #include "clang/AST/ExprObjC.h" ++#include "clang/Analysis/ConstructionContext.h" ++#include "clang/Analysis/Support/BumpVector.h" + #include "clang/Basic/LLVM.h" + #include "llvm/ADT/DenseMap.h" + #include "llvm/ADT/GraphTraits.h" +@@ -74,7 +75,8 @@ public: + MemberDtor, + TemporaryDtor, + DTOR_BEGIN = AutomaticObjectDtor, +- DTOR_END = TemporaryDtor ++ DTOR_END = TemporaryDtor, ++ CleanupFunction, + }; + + protected: +@@ -383,6 +385,32 @@ private: + } + }; + ++class CFGCleanupFunction final : public CFGElement { ++public: ++ CFGCleanupFunction() = default; ++ CFGCleanupFunction(const VarDecl *VD) ++ : CFGElement(Kind::CleanupFunction, VD) { ++ assert(VD->hasAttr()); ++ } ++ ++ const VarDecl *getVarDecl() const { ++ return static_cast(Data1.getPointer()); ++ } ++ ++ /// Returns the function to be called when cleaning up the var decl. ++ const FunctionDecl *getFunctionDecl() const { ++ const CleanupAttr *A = getVarDecl()->getAttr(); ++ return A->getFunctionDecl(); ++ } ++ ++private: ++ friend class CFGElement; ++ ++ static bool isKind(const CFGElement E) { ++ return E.getKind() == Kind::CleanupFunction; ++ } ++}; ++ + /// Represents C++ object destructor implicitly generated for automatic object + /// or temporary bound to const reference at the point of leaving its local + /// scope. +@@ -1142,6 +1170,10 @@ public: + Elements.push_back(CFGAutomaticObjDtor(VD, S), C); + } + ++ void appendCleanupFunction(const VarDecl *VD, BumpVectorContext &C) { ++ Elements.push_back(CFGCleanupFunction(VD), C); ++ } ++ + void appendLifetimeEnds(VarDecl *VD, Stmt *S, BumpVectorContext &C) { + Elements.push_back(CFGLifetimeEnds(VD, S), C); + } +diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp +index b82f9010a83f..03ab4c6fdf29 100644 +--- a/clang/lib/Analysis/CFG.cpp ++++ b/clang/lib/Analysis/CFG.cpp +@@ -881,6 +881,10 @@ private: + B->appendAutomaticObjDtor(VD, S, cfg->getBumpVectorContext()); + } + ++ void appendCleanupFunction(CFGBlock *B, VarDecl *VD) { ++ B->appendCleanupFunction(VD, cfg->getBumpVectorContext()); ++ } ++ + void appendLifetimeEnds(CFGBlock *B, VarDecl *VD, Stmt *S) { + B->appendLifetimeEnds(VD, S, cfg->getBumpVectorContext()); + } +@@ -1346,7 +1350,8 @@ private: + return {}; + } + +- bool hasTrivialDestructor(VarDecl *VD); ++ bool hasTrivialDestructor(const VarDecl *VD) const; ++ bool needsAutomaticDestruction(const VarDecl *VD) const; + }; + + } // namespace +@@ -1861,14 +1866,14 @@ void CFGBuilder::addAutomaticObjDestruction(LocalScope::const_iterator B, + if (B == E) + return; + +- SmallVector DeclsNonTrivial; +- DeclsNonTrivial.reserve(B.distance(E)); ++ SmallVector DeclsNeedDestruction; ++ DeclsNeedDestruction.reserve(B.distance(E)); + + for (VarDecl* D : llvm::make_range(B, E)) +- if (!hasTrivialDestructor(D)) +- DeclsNonTrivial.push_back(D); ++ if (needsAutomaticDestruction(D)) ++ DeclsNeedDestruction.push_back(D); + +- for (VarDecl *VD : llvm::reverse(DeclsNonTrivial)) { ++ for (VarDecl *VD : llvm::reverse(DeclsNeedDestruction)) { + if (BuildOpts.AddImplicitDtors) { + // If this destructor is marked as a no-return destructor, we need to + // create a new block for the destructor which does not have as a +@@ -1879,7 +1884,8 @@ void CFGBuilder::addAutomaticObjDestruction(LocalScope::const_iterator B, + Ty = getReferenceInitTemporaryType(VD->getInit()); + Ty = Context->getBaseElementType(Ty); + +- if (Ty->getAsCXXRecordDecl()->isAnyDestructorNoReturn()) ++ const CXXRecordDecl *CRD = Ty->getAsCXXRecordDecl(); ++ if (CRD && CRD->isAnyDestructorNoReturn()) + Block = createNoReturnBlock(); + } + +@@ -1890,8 +1896,10 @@ void CFGBuilder::addAutomaticObjDestruction(LocalScope::const_iterator B, + // objects, we end lifetime with scope end. + if (BuildOpts.AddLifetime) + appendLifetimeEnds(Block, VD, S); +- if (BuildOpts.AddImplicitDtors) ++ if (BuildOpts.AddImplicitDtors && !hasTrivialDestructor(VD)) + appendAutomaticObjDtor(Block, VD, S); ++ if (VD->hasAttr()) ++ appendCleanupFunction(Block, VD); + } + } + +@@ -1922,7 +1930,7 @@ void CFGBuilder::addScopeExitHandling(LocalScope::const_iterator B, + // is destroyed, for automatic variables, this happens when the end of the + // scope is added. + for (VarDecl* D : llvm::make_range(B, E)) +- if (hasTrivialDestructor(D)) ++ if (!needsAutomaticDestruction(D)) + DeclsTrivial.push_back(D); + + if (DeclsTrivial.empty()) +@@ -2095,7 +2103,11 @@ LocalScope* CFGBuilder::addLocalScopeForDeclStmt(DeclStmt *DS, + return Scope; + } + +-bool CFGBuilder::hasTrivialDestructor(VarDecl *VD) { ++bool CFGBuilder::needsAutomaticDestruction(const VarDecl *VD) const { ++ return !hasTrivialDestructor(VD) || VD->hasAttr(); ++} ++ ++bool CFGBuilder::hasTrivialDestructor(const VarDecl *VD) const { + // Check for const references bound to temporary. Set type to pointee. + QualType QT = VD->getType(); + if (QT->isReferenceType()) { +@@ -2149,7 +2161,7 @@ LocalScope* CFGBuilder::addLocalScopeForVarDecl(VarDecl *VD, + return Scope; + + if (!BuildOpts.AddLifetime && !BuildOpts.AddScopes && +- hasTrivialDestructor(VD)) { ++ !needsAutomaticDestruction(VD)) { + assert(BuildOpts.AddImplicitDtors); + return Scope; + } +@@ -5287,6 +5299,7 @@ CFGImplicitDtor::getDestructorDecl(ASTContext &astContext) const { + case CFGElement::CXXRecordTypedCall: + case CFGElement::ScopeBegin: + case CFGElement::ScopeEnd: ++ case CFGElement::CleanupFunction: + llvm_unreachable("getDestructorDecl should only be used with " + "ImplicitDtors"); + case CFGElement::AutomaticObjectDtor: { +@@ -5830,6 +5843,11 @@ static void print_elem(raw_ostream &OS, StmtPrinterHelper &Helper, + break; + } + ++ case CFGElement::Kind::CleanupFunction: ++ OS << "CleanupFunction (" ++ << E.castAs().getFunctionDecl()->getName() << ")\n"; ++ break; ++ + case CFGElement::Kind::LifetimeEnds: + Helper.handleDecl(E.castAs().getVarDecl(), OS); + OS << " (Lifetime ends)\n"; +diff --git a/clang/lib/Analysis/PathDiagnostic.cpp b/clang/lib/Analysis/PathDiagnostic.cpp +index 348afc42319e..0cb03943c547 100644 +--- a/clang/lib/Analysis/PathDiagnostic.cpp ++++ b/clang/lib/Analysis/PathDiagnostic.cpp +@@ -567,6 +567,7 @@ getLocationForCaller(const StackFrameContext *SFC, + } + case CFGElement::ScopeBegin: + case CFGElement::ScopeEnd: ++ case CFGElement::CleanupFunction: + llvm_unreachable("not yet implemented!"); + case CFGElement::LifetimeEnds: + case CFGElement::LoopExit: +diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +index 0e2ac78f7089..d7c5bd1d4042 100644 +--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp ++++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +@@ -993,6 +993,7 @@ void ExprEngine::processCFGElement(const CFGElement E, ExplodedNode *Pred, + ProcessLoopExit(E.castAs().getLoopStmt(), Pred); + return; + case CFGElement::LifetimeEnds: ++ case CFGElement::CleanupFunction: + case CFGElement::ScopeBegin: + case CFGElement::ScopeEnd: + return; +diff --git a/clang/test/Analysis/scopes-cfg-output.cpp b/clang/test/Analysis/scopes-cfg-output.cpp +index 6877d124e67a..4eb8967e3735 100644 +--- a/clang/test/Analysis/scopes-cfg-output.cpp ++++ b/clang/test/Analysis/scopes-cfg-output.cpp +@@ -1419,3 +1419,68 @@ label: + } + } + } ++ ++// CHECK: [B1] ++// CHECK-NEXT: 1: CFGScopeBegin(i) ++// CHECK-NEXT: 2: int i __attribute__((cleanup(cleanup_int))); ++// CHECK-NEXT: 3: CleanupFunction (cleanup_int) ++// CHECK-NEXT: 4: CFGScopeEnd(i) ++void cleanup_int(int *i); ++void test_cleanup_functions() { ++ int i __attribute__((cleanup(cleanup_int))); ++} ++ ++// CHECK: [B1] ++// CHECK-NEXT: 1: 10 ++// CHECK-NEXT: 2: i ++// CHECK-NEXT: 3: [B1.2] = [B1.1] ++// CHECK-NEXT: 4: return; ++// CHECK-NEXT: 5: CleanupFunction (cleanup_int) ++// CHECK-NEXT: 6: CFGScopeEnd(i) ++// CHECK-NEXT: Preds (1): B3 ++// CHECK-NEXT: Succs (1): B0 ++// CHECK: [B2] ++// CHECK-NEXT: 1: return; ++// CHECK-NEXT: 2: CleanupFunction (cleanup_int) ++// CHECK-NEXT: 3: CFGScopeEnd(i) ++// CHECK-NEXT: Preds (1): B3 ++// CHECK-NEXT: Succs (1): B0 ++// CHECK: [B3] ++// CHECK-NEXT: 1: CFGScopeBegin(i) ++// CHECK-NEXT: 2: int i __attribute__((cleanup(cleanup_int))); ++// CHECK-NEXT: 3: m ++// CHECK-NEXT: 4: [B3.3] (ImplicitCastExpr, LValueToRValue, int) ++// CHECK-NEXT: 5: 1 ++// CHECK-NEXT: 6: [B3.4] == [B3.5] ++// CHECK-NEXT: T: if [B3.6] ++// CHECK-NEXT: Preds (1): B4 ++// CHECK-NEXT: Succs (2): B2 B1 ++void test_cleanup_functions2(int m) { ++ int i __attribute__((cleanup(cleanup_int))); ++ ++ if (m == 1) { ++ return; ++ } ++ ++ i = 10; ++ return; ++} ++ ++// CHECK: [B1] ++// CHECK-NEXT: 1: CFGScopeBegin(f) ++// CHECK-NEXT: 2: (CXXConstructExpr, [B1.3], F) ++// CHECK-NEXT: 3: F f __attribute__((cleanup(cleanup_F))); ++// CHECK-NEXT: 4: CleanupFunction (cleanup_F) ++// CHECK-NEXT: 5: [B1.3].~F() (Implicit destructor) ++// CHECK-NEXT: 6: CFGScopeEnd(f) ++// CHECK-NEXT: Preds (1): B2 ++// CHECK-NEXT: Succs (1): B0 ++class F { ++public: ++ ~F(); ++}; ++void cleanup_F(F *f); ++ ++void test() { ++ F f __attribute((cleanup(cleanup_F))); ++} diff --git a/clang.spec b/clang.spec index 57962b7..310d740 100644 --- a/clang.spec +++ b/clang.spec @@ -55,7 +55,7 @@ Name: %pkg_name Version: %{clang_version}%{?rc_ver:~rc%{rc_ver}}%{?llvm_snapshot_version_suffix:~%{llvm_snapshot_version_suffix}} -Release: 1%{?dist} +Release: 2%{?dist} Summary: A C language family front-end for LLVM License: Apache-2.0 WITH LLVM-exception OR NCSA @@ -88,6 +88,11 @@ Patch4: 0001-Produce-DWARF4-by-default.patch # Workaround a bug in ORC on ppc64le. # More info is available here: https://reviews.llvm.org/D159115#4641826 Patch5: 0001-Workaround-a-bug-in-ORC-on-ppc64le.patch +# Patches for https://issues.redhat.com/browse/RHEL-1650 +# Remove in clang 18. +Patch6: cfg.patch +Patch7: tsa.patch + # RHEL specific patches # Avoid unwanted dependency on python-recommonmark @@ -641,6 +646,9 @@ false %changelog %{?llvm_snapshot_changelog_entry} +* Mon Oct 09 2023 Timm Bäder - 17.0.2-2 +- Backport upstream fixes for https://issues.redhat.com/browse/RHEL-1650 + * Wed Oct 04 2023 Tulio Magno Quites Machado Filho - 17.0.2-1 - Update to LLVM 17.0.2 diff --git a/tsa.patch b/tsa.patch new file mode 100644 index 0000000..2cf698e --- /dev/null +++ b/tsa.patch @@ -0,0 +1,124 @@ +commit cf8e189a99f988398a48148b9ea7901948665ab0 +Author: Timm Bäder +Date: Wed Sep 6 12:19:20 2023 +0200 + + [clang][TSA] Thread safety cleanup functions + + Consider cleanup functions in thread safety analysis. + + Differential Revision: https://reviews.llvm.org/D152504 + +diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h +index 9d28325c1ea6..13e37ac2b56b 100644 +--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h ++++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h +@@ -361,7 +361,7 @@ public: + unsigned NumArgs = 0; + + // Function arguments +- const Expr *const *FunArgs = nullptr; ++ llvm::PointerUnion FunArgs = nullptr; + + // is Self referred to with -> or .? + bool SelfArrow = false; +diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp +index 3107d035254d..3e6ceb7d54c4 100644 +--- a/clang/lib/Analysis/ThreadSafety.cpp ++++ b/clang/lib/Analysis/ThreadSafety.cpp +@@ -1773,7 +1773,8 @@ void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK, + /// + /// \param Exp The call expression. + /// \param D The callee declaration. +-/// \param Self If \p Exp = nullptr, the implicit this argument. ++/// \param Self If \p Exp = nullptr, the implicit this argument or the argument ++/// of an implicitly called cleanup function. + /// \param Loc If \p Exp = nullptr, the location. + void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D, + til::LiteralPtr *Self, SourceLocation Loc) { +@@ -2417,6 +2418,15 @@ void ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) { + AD.getTriggerStmt()->getEndLoc()); + break; + } ++ ++ case CFGElement::CleanupFunction: { ++ const CFGCleanupFunction &CF = BI.castAs(); ++ LocksetBuilder.handleCall(/*Exp=*/nullptr, CF.getFunctionDecl(), ++ SxBuilder.createVariable(CF.getVarDecl()), ++ CF.getVarDecl()->getLocation()); ++ break; ++ } ++ + case CFGElement::TemporaryDtor: { + auto TD = BI.castAs(); + +diff --git a/clang/lib/Analysis/ThreadSafetyCommon.cpp b/clang/lib/Analysis/ThreadSafetyCommon.cpp +index b8286cef396c..63cc66852a9e 100644 +--- a/clang/lib/Analysis/ThreadSafetyCommon.cpp ++++ b/clang/lib/Analysis/ThreadSafetyCommon.cpp +@@ -110,7 +110,8 @@ static StringRef ClassifyDiagnostic(QualType VDT) { + /// \param D The declaration to which the attribute is attached. + /// \param DeclExp An expression involving the Decl to which the attribute + /// is attached. E.g. the call to a function. +-/// \param Self S-expression to substitute for a \ref CXXThisExpr. ++/// \param Self S-expression to substitute for a \ref CXXThisExpr in a call, ++/// or argument to a cleanup function. + CapabilityExpr SExprBuilder::translateAttrExpr(const Expr *AttrExp, + const NamedDecl *D, + const Expr *DeclExp, +@@ -144,7 +145,11 @@ CapabilityExpr SExprBuilder::translateAttrExpr(const Expr *AttrExp, + + if (Self) { + assert(!Ctx.SelfArg && "Ambiguous self argument"); +- Ctx.SelfArg = Self; ++ assert(isa(D) && "Self argument requires function"); ++ if (isa(D)) ++ Ctx.SelfArg = Self; ++ else ++ Ctx.FunArgs = Self; + + // If the attribute has no arguments, then assume the argument is "this". + if (!AttrExp) +@@ -312,8 +317,14 @@ til::SExpr *SExprBuilder::translateDeclRefExpr(const DeclRefExpr *DRE, + ? (cast(D)->getCanonicalDecl() == Canonical) + : (cast(D)->getCanonicalDecl() == Canonical)) { + // Substitute call arguments for references to function parameters +- assert(I < Ctx->NumArgs); +- return translate(Ctx->FunArgs[I], Ctx->Prev); ++ if (const Expr *const *FunArgs = ++ Ctx->FunArgs.dyn_cast()) { ++ assert(I < Ctx->NumArgs); ++ return translate(FunArgs[I], Ctx->Prev); ++ } ++ ++ assert(I == 0); ++ return Ctx->FunArgs.get(); + } + } + // Map the param back to the param of the original function declaration +diff --git a/clang/test/Sema/warn-thread-safety-analysis.c b/clang/test/Sema/warn-thread-safety-analysis.c +index 355616b73d96..642ea88ec3c9 100644 +--- a/clang/test/Sema/warn-thread-safety-analysis.c ++++ b/clang/test/Sema/warn-thread-safety-analysis.c +@@ -72,6 +72,8 @@ int get_value(int *p) SHARED_LOCKS_REQUIRED(foo_.mu_){ + return *p; + } + ++void unlock_scope(struct Mutex *const *mu) __attribute__((release_capability(**mu))); ++ + int main(void) { + + Foo_fun1(1); // expected-warning{{calling function 'Foo_fun1' requires holding mutex 'mu2'}} \ +@@ -127,6 +129,13 @@ int main(void) { + // expected-note@-1{{mutex released here}} + mutex_shared_unlock(&mu1); // expected-warning {{releasing mutex 'mu1' that was not held}} + ++ /// Cleanup functions ++ { ++ struct Mutex* const __attribute__((cleanup(unlock_scope))) scope = &mu1; ++ mutex_exclusive_lock(scope); // Note that we have to lock through scope, because no alias analysis! ++ // Cleanup happens automatically -> no warning. ++ } ++ + return 0; + } +