r340452 - [NFC] Test commit
Author: aaronpuchert
Date: Wed Aug 22 14:06:04 2018
New Revision: 340452
URL: http://llvm.org/viewvc/llvm-project?rev=340452&view=rev
Log:
[NFC] Test commit
Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=340452&r1=340451&r2=340452&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Wed Aug 22 14:06:04 2018
@@ -86,8 +86,8 @@ static void warnInvalidLock(ThreadSafety
namespace {
-/// A set of CapabilityInfo objects, which are compiled from the
-/// requires attributes on a function.
+/// A set of CapabilityExpr objects, which are compiled from thread safety
+/// attributes on a function.
class CapExprSet : public SmallVector {
public:
/// Push M onto list, but discard duplicates.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r340459 - Thread safety analysis: Allow relockable scopes
Author: aaronpuchert
Date: Wed Aug 22 15:14:53 2018
New Revision: 340459
URL: http://llvm.org/viewvc/llvm-project?rev=340459&view=rev
Log:
Thread safety analysis: Allow relockable scopes
Summary:
It's already allowed to prematurely release a scoped lock, now we also
allow relocking it again, possibly even in another mode.
This is the second attempt, the first had been merged as r339456 and
reverted in r339558 because it caused a crash.
Reviewers: delesley, aaron.ballman
Reviewed By: delesley, aaron.ballman
Subscribers: hokein, cfe-commits
Differential Revision: https://reviews.llvm.org/D49885
Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=340459&r1=340458&r2=340459&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Wed Aug 22 15:14:53 2018
@@ -142,6 +142,9 @@ public:
handleRemovalFromIntersection(const FactSet &FSet, FactManager &FactMan,
SourceLocation JoinLoc, LockErrorKind LEK,
ThreadSafetyHandler &Handler) const = 0;
+ virtual void handleLock(FactSet &FSet, FactManager &FactMan,
+ const FactEntry &entry, ThreadSafetyHandler &Handler,
+ StringRef DiagKind) const = 0;
virtual void handleUnlock(FactSet &FSet, FactManager &FactMan,
const CapabilityExpr &Cp, SourceLocation UnlockLoc,
bool FullyRemove, ThreadSafetyHandler &Handler,
@@ -873,6 +876,12 @@ public:
}
}
+ void handleLock(FactSet &FSet, FactManager &FactMan, const FactEntry &entry,
+ ThreadSafetyHandler &Handler,
+ StringRef DiagKind) const override {
+Handler.handleDoubleLock(DiagKind, entry.toString(), entry.loc());
+ }
+
void handleUnlock(FactSet &FSet, FactManager &FactMan,
const CapabilityExpr &Cp, SourceLocation UnlockLoc,
bool FullyRemove, ThreadSafetyHandler &Handler,
@@ -913,6 +922,23 @@ public:
}
}
+ void handleLock(FactSet &FSet, FactManager &FactMan, const FactEntry &entry,
+ ThreadSafetyHandler &Handler,
+ StringRef DiagKind) const override {
+for (const auto *UnderlyingMutex : UnderlyingMutexes) {
+ CapabilityExpr UnderCp(UnderlyingMutex, false);
+
+ // We're relocking the underlying mutexes. Warn on double locking.
+ if (FSet.findLock(FactMan, UnderCp))
+Handler.handleDoubleLock(DiagKind, UnderCp.toString(), entry.loc());
+ else {
+FSet.removeLock(FactMan, !UnderCp);
+FSet.addLock(FactMan, llvm::make_unique(
+ UnderCp, entry.kind(), entry.loc()));
+ }
+}
+ }
+
void handleUnlock(FactSet &FSet, FactManager &FactMan,
const CapabilityExpr &Cp, SourceLocation UnlockLoc,
bool FullyRemove, ThreadSafetyHandler &Handler,
@@ -1251,9 +1277,9 @@ void ThreadSafetyAnalyzer::addLock(FactS
}
// FIXME: Don't always warn when we have support for reentrant locks.
- if (FSet.findLock(FactMan, *Entry)) {
+ if (FactEntry *Cp = FSet.findLock(FactMan, *Entry)) {
if (!Entry->asserted())
- Handler.handleDoubleLock(DiagKind, Entry->toString(), Entry->loc());
+ Cp->handleLock(FSet, FactMan, *Entry, Handler, DiagKind);
} else {
FSet.addLock(FactMan, std::move(Entry));
}
Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp?rev=340459&r1=340458&r2=340459&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Wed Aug 22 15:14:53
2018
@@ -2621,6 +2621,170 @@ void Foo::test5() {
} // end namespace ReleasableScopedLock
+namespace RelockableScopedLock {
+
+class SCOPED_LOCKABLE RelockableExclusiveMutexLock {
+public:
+ RelockableExclusiveMutexLock(Mutex *mu) EXCLUSIVE_LOCK_FUNCTION(mu);
+ ~RelockableExclusiveMutexLock() EXCLUSIVE_UNLOCK_FUNCTION();
+
+ void Lock() EXCLUSIVE_LOCK_FUNCTION();
+ void Unlock() UNLOCK_FUNCTION();
+};
+
+struct SharedTraits {};
+struct ExclusiveTraits {};
+
+class SCOPED_LOCKABLE RelockableMutexLock {
+public:
+ RelockableMutexLock(Mutex *mu, SharedTraits) SHARED_LOCK_FUNCTION(mu);
+ RelockableMutexLock(Mutex *mu, ExclusiveTraits) EXCLUSIVE_LOCK_FUNCTION(mu);
+ ~RelockableMutexLock() UNLOCK_FUNCTION();
+
+ void Lock() EXCLUSIVE_LOCK_FUNCTION();
+ void Unlock() UNLOCK_FUNCTION();
+
+ void ReaderLock() SHARED_LOCK_FUNCTION();
+ void R
r340575 - Remove unnecessary const_cast [NFC]
Author: aaronpuchert
Date: Thu Aug 23 14:13:32 2018
New Revision: 340575
URL: http://llvm.org/viewvc/llvm-project?rev=340575&view=rev
Log:
Remove unnecessary const_cast [NFC]
This required adding a few const specifiers on functions.
Also a minor formatting fix suggested in D49885.
Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=340575&r1=340574&r2=340575&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Thu Aug 23 14:13:32 2018
@@ -929,9 +929,9 @@ public:
CapabilityExpr UnderCp(UnderlyingMutex, false);
// We're relocking the underlying mutexes. Warn on double locking.
- if (FSet.findLock(FactMan, UnderCp))
+ if (FSet.findLock(FactMan, UnderCp)) {
Handler.handleDoubleLock(DiagKind, UnderCp.toString(), entry.loc());
- else {
+ } else {
FSet.removeLock(FactMan, !UnderCp);
FSet.addLock(FactMan, llvm::make_unique(
UnderCp, entry.kind(), entry.loc()));
@@ -1002,11 +1002,11 @@ public:
StringRef DiagKind);
template
- void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, Expr *Exp,
+ void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, const Expr *Exp,
const NamedDecl *D, VarDecl *SelfDecl = nullptr);
template
- void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, Expr *Exp,
+ void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, const Expr *Exp,
const NamedDecl *D,
const CFGBlock *PredBlock, const CFGBlock *CurrBlock,
Expr *BrE, bool Neg);
@@ -1315,7 +1315,7 @@ void ThreadSafetyAnalyzer::removeLock(Fa
/// and push them onto Mtxs, discarding any duplicates.
template
void ThreadSafetyAnalyzer::getMutexIDs(CapExprSet &Mtxs, AttrType *Attr,
- Expr *Exp, const NamedDecl *D,
+ const Expr *Exp, const NamedDecl *D,
VarDecl *SelfDecl) {
if (Attr->args_size() == 0) {
// The mutex held is the "this" object.
@@ -1347,7 +1347,7 @@ void ThreadSafetyAnalyzer::getMutexIDs(C
/// any duplicates.
template
void ThreadSafetyAnalyzer::getMutexIDs(CapExprSet &Mtxs, AttrType *Attr,
- Expr *Exp, const NamedDecl *D,
+ const Expr *Exp, const NamedDecl *D,
const CFGBlock *PredBlock,
const CFGBlock *CurrBlock,
Expr *BrE, bool Neg) {
@@ -1460,7 +1460,7 @@ void ThreadSafetyAnalyzer::getEdgeLockse
const LocalVarContext &LVarCtx = PredBlockInfo->ExitContext;
StringRef CapDiagKind = "mutex";
- auto *Exp = const_cast(getTrylockCallExpr(Cond, LVarCtx,
Negate));
+ const auto *Exp = getTrylockCallExpr(Cond, LVarCtx, Negate);
if (!Exp)
return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r340580 - Remove more const_casts by using ConstStmtVisitor [NFC]
Author: aaronpuchert
Date: Thu Aug 23 14:53:04 2018
New Revision: 340580
URL: http://llvm.org/viewvc/llvm-project?rev=340580&view=rev
Log:
Remove more const_casts by using ConstStmtVisitor [NFC]
Again, this required adding some const specifiers.
Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=340580&r1=340579&r2=340580&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Thu Aug 23 14:53:04 2018
@@ -422,7 +422,7 @@ private:
Context::Factory ContextFactory;
std::vector VarDefinitions;
std::vector CtxIndices;
- std::vector> SavedContexts;
+ std::vector> SavedContexts;
public:
LocalVariableMap() {
@@ -463,7 +463,7 @@ public:
/// Return the next context after processing S. This function is used by
/// clients of the class to get the appropriate context when traversing the
/// CFG. It must be called for every assignment or DeclStmt.
- Context getNextContext(unsigned &CtxIndex, Stmt *S, Context C) {
+ Context getNextContext(unsigned &CtxIndex, const Stmt *S, Context C) {
if (SavedContexts[CtxIndex+1].first == S) {
CtxIndex++;
Context Result = SavedContexts[CtxIndex].second;
@@ -525,7 +525,7 @@ protected:
unsigned getContextIndex() { return SavedContexts.size()-1; }
// Save the current context for later replay
- void saveContext(Stmt *S, Context C) {
+ void saveContext(const Stmt *S, Context C) {
SavedContexts.push_back(std::make_pair(S, C));
}
@@ -595,7 +595,7 @@ CFGBlockInfo CFGBlockInfo::getEmptyBlock
namespace {
/// Visitor which builds a LocalVariableMap
-class VarMapBuilder : public StmtVisitor {
+class VarMapBuilder : public ConstStmtVisitor {
public:
LocalVariableMap* VMap;
LocalVariableMap::Context Ctx;
@@ -603,16 +603,16 @@ public:
VarMapBuilder(LocalVariableMap *VM, LocalVariableMap::Context C)
: VMap(VM), Ctx(C) {}
- void VisitDeclStmt(DeclStmt *S);
- void VisitBinaryOperator(BinaryOperator *BO);
+ void VisitDeclStmt(const DeclStmt *S);
+ void VisitBinaryOperator(const BinaryOperator *BO);
};
} // namespace
// Add new local variables to the variable map
-void VarMapBuilder::VisitDeclStmt(DeclStmt *S) {
+void VarMapBuilder::VisitDeclStmt(const DeclStmt *S) {
bool modifiedCtx = false;
- DeclGroupRef DGrp = S->getDeclGroup();
+ const DeclGroupRef DGrp = S->getDeclGroup();
for (const auto *D : DGrp) {
if (const auto *VD = dyn_cast_or_null(D)) {
const Expr *E = VD->getInit();
@@ -630,7 +630,7 @@ void VarMapBuilder::VisitDeclStmt(DeclSt
}
// Update local variable definitions in variable map
-void VarMapBuilder::VisitBinaryOperator(BinaryOperator *BO) {
+void VarMapBuilder::VisitBinaryOperator(const BinaryOperator *BO) {
if (!BO->isAssignmentOp())
return;
@@ -783,7 +783,7 @@ void LocalVariableMap::traverseCFG(CFG *
switch (BI.getKind()) {
case CFGElement::Statement: {
CFGStmt CS = BI.castAs();
- VMapBuilder.Visit(const_cast(CS.getStmt()));
+ VMapBuilder.Visit(CS.getStmt());
break;
}
default:
@@ -1520,7 +1520,7 @@ namespace {
/// An expression may cause us to add or remove locks from the lockset, or else
/// output error messages related to missing locks.
/// FIXME: In future, we may be able to not inherit from a visitor.
-class BuildLockset : public StmtVisitor {
+class BuildLockset : public ConstStmtVisitor {
friend class ThreadSafetyAnalyzer;
ThreadSafetyAnalyzer *Analyzer;
@@ -1540,19 +1540,19 @@ class BuildLockset : public StmtVisitor<
void checkPtAccess(const Expr *Exp, AccessKind AK,
ProtectedOperationKind POK = POK_VarAccess);
- void handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD = nullptr);
+ void handleCall(const Expr *Exp, const NamedDecl *D, VarDecl *VD = nullptr);
public:
BuildLockset(ThreadSafetyAnalyzer *Anlzr, CFGBlockInfo &Info)
- : StmtVisitor(), Analyzer(Anlzr), FSet(Info.EntrySet),
+ : ConstStmtVisitor(), Analyzer(Anlzr), FSet(Info.EntrySet),
LVarCtx(Info.EntryContext), CtxIndex(Info.EntryIndex) {}
- void VisitUnaryOperator(UnaryOperator *UO);
- void VisitBinaryOperator(BinaryOperator *BO);
- void VisitCastExpr(CastExpr *CE);
- void VisitCallExpr(CallExpr *Exp);
- void VisitCXXConstructExpr(CXXConstructExpr *Exp);
- void VisitDeclStmt(DeclStmt *S);
+ void VisitUnaryOperator(const UnaryOperator *UO);
+ void VisitBinaryOperator(const BinaryOperator *BO);
+ void VisitCastExpr(const CastExpr *CE);
+ void VisitCallExpr(const CallExpr *Exp);
+ void VisitCXXConstructExpr(const CXXConstructExpr *Exp);
+ void VisitDeclStmt(const DeclStmt *S);
};
} // namespace
@@ -1744,7 +1744,8 @@ void BuildLockset::checkPtAccess(const E
///
r352549 - Thread safety analysis: Improve diagnostics for double locking
Author: aaronpuchert
Date: Tue Jan 29 14:11:42 2019
New Revision: 352549
URL: http://llvm.org/viewvc/llvm-project?rev=352549&view=rev
Log:
Thread safety analysis: Improve diagnostics for double locking
Summary:
We use the existing diag::note_locked_here to tell the user where we saw
the first locking.
Reviewers: aaron.ballman, delesley
Reviewed By: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D56967
Modified:
cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h?rev=352549&r1=352548&r2=352549&view=diff
==
--- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h Tue Jan 29
14:11:42 2019
@@ -128,9 +128,10 @@ public:
/// \param Kind -- the capability's name parameter (role, mutex, etc).
/// \param LockName -- A StringRef name for the lock expression, to be
printed
/// in the error message.
+ /// \param LocLocked -- The location of the first lock expression.
/// \param Loc -- The location of the second lock expression.
virtual void handleDoubleLock(StringRef Kind, Name LockName,
-SourceLocation Loc) {}
+SourceLocation LocLocked, SourceLocation Loc)
{}
/// Warn about situations where a mutex is sometimes held and sometimes not.
/// The three situations are:
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=352549&r1=352548&r2=352549&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Tue Jan 29 14:11:42 2019
@@ -873,7 +873,7 @@ public:
void handleLock(FactSet &FSet, FactManager &FactMan, const FactEntry &entry,
ThreadSafetyHandler &Handler,
StringRef DiagKind) const override {
-Handler.handleDoubleLock(DiagKind, entry.toString(), entry.loc());
+Handler.handleDoubleLock(DiagKind, entry.toString(), loc(), entry.loc());
}
void handleUnlock(FactSet &FSet, FactManager &FactMan,
@@ -981,12 +981,13 @@ private:
void lock(FactSet &FSet, FactManager &FactMan, const CapabilityExpr &Cp,
LockKind kind, SourceLocation loc, ThreadSafetyHandler *Handler,
StringRef DiagKind) const {
-if (!FSet.findLock(FactMan, Cp)) {
+if (const FactEntry *Fact = FSet.findLock(FactMan, Cp)) {
+ if (Handler)
+Handler->handleDoubleLock(DiagKind, Cp.toString(), Fact->loc(), loc);
+} else {
FSet.removeLock(FactMan, !Cp);
FSet.addLock(FactMan,
llvm::make_unique(Cp, kind, loc));
-} else if (Handler) {
- Handler->handleDoubleLock(DiagKind, Cp.toString(), loc);
}
}
Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=352549&r1=352548&r2=352549&view=diff
==
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Tue Jan 29 14:11:42 2019
@@ -1638,17 +1638,6 @@ class ThreadSafetyReporter : public clan
return ONS;
}
- // Helper functions
- void warnLockMismatch(unsigned DiagID, StringRef Kind, Name LockName,
-SourceLocation Loc) {
-// Gracefully handle rare cases when the analysis can't get a more
-// precise source location.
-if (!Loc.isValid())
- Loc = FunLocation;
-PartialDiagnosticAt Warning(Loc, S.PDiag(DiagID) << Kind << LockName);
-Warnings.emplace_back(std::move(Warning), getNotes());
- }
-
public:
ThreadSafetyReporter(Sema &S, SourceLocation FL, SourceLocation FEL)
: S(S), FunLocation(FL), FunEndLocation(FEL),
@@ -1677,7 +1666,11 @@ class ThreadSafetyReporter : public clan
void handleUnmatchedUnlock(StringRef Kind, Name LockName,
SourceLocation Loc) override {
-warnLockMismatch(diag::warn_unlock_but_no_lock, Kind, LockName, Loc);
+if (Loc.isInvalid())
+ Loc = FunLocation;
+PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_unlock_but_no_lock)
+ << Kind << LockName);
+Warnings.emplace_back(std::move(Warning), getNotes());
}
void handleIncorrectUnlockKind(StringRef Kind, Name LockName,
@@ -1691,8 +1684,18 @@ class ThreadSafetyReporter : public clan
Warnings.emplace_b
r352574 - Fix thread safety tests after r352549
Author: aaronpuchert
Date: Tue Jan 29 16:18:24 2019
New Revision: 352574
URL: http://llvm.org/viewvc/llvm-project?rev=352574&view=rev
Log:
Fix thread safety tests after r352549
Modified:
cfe/trunk/test/PCH/thread-safety-attrs.cpp
cfe/trunk/test/Sema/warn-thread-safety-analysis.c
Modified: cfe/trunk/test/PCH/thread-safety-attrs.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/thread-safety-attrs.cpp?rev=352574&r1=352573&r2=352574&view=diff
==
--- cfe/trunk/test/PCH/thread-safety-attrs.cpp (original)
+++ cfe/trunk/test/PCH/thread-safety-attrs.cpp Tue Jan 29 16:18:24 2019
@@ -213,7 +213,7 @@ void sls_fun_bad_1() {
}
void sls_fun_bad_2() {
- sls_mu.Lock();
+ sls_mu.Lock(); // expected-note{{mutex acquired here}}
sls_mu.Lock(); // \
// expected-warning{{acquiring mutex 'sls_mu' that is already held}}
sls_mu.Unlock();
Modified: cfe/trunk/test/Sema/warn-thread-safety-analysis.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-thread-safety-analysis.c?rev=352574&r1=352573&r2=352574&view=diff
==
--- cfe/trunk/test/Sema/warn-thread-safety-analysis.c (original)
+++ cfe/trunk/test/Sema/warn-thread-safety-analysis.c Tue Jan 29 16:18:24 2019
@@ -77,7 +77,7 @@ int main() {
Foo_fun1(1); // expected-warning{{calling function 'Foo_fun1' requires
holding mutex 'mu2'}} \
expected-warning{{calling function 'Foo_fun1' requires
holding mutex 'mu1' exclusively}}
- mutex_exclusive_lock(&mu1);
+ mutex_exclusive_lock(&mu1); // expected-note{{mutex acquired here}}
mutex_shared_lock(&mu2);
Foo_fun1(1);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r356228 - Add missing override specifier [NFC]
Author: aaronpuchert
Date: Thu Mar 14 19:30:07 2019
New Revision: 356228
URL: http://llvm.org/viewvc/llvm-project?rev=356228&view=rev
Log:
Add missing override specifier [NFC]
This should fix a -Winconsistent-missing-override warning that is only
visible when Z3 is enabled.
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp?rev=356228&r1=356227&r2=356228&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp Thu Mar 14
19:30:07 2019
@@ -102,7 +102,7 @@ public:
Z3_dec_ref(Context.Context, reinterpret_cast(Sort));
}
- void Profile(llvm::FoldingSetNodeID &ID) const {
+ void Profile(llvm::FoldingSetNodeID &ID) const override {
ID.AddInteger(
Z3_get_ast_id(Context.Context, reinterpret_cast(Sort)));
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r356427 - Thread safety analysis: Add note for unlock kind mismatch
Author: aaronpuchert
Date: Mon Mar 18 16:26:54 2019
New Revision: 356427
URL: http://llvm.org/viewvc/llvm-project?rev=356427&view=rev
Log:
Thread safety analysis: Add note for unlock kind mismatch
Summary:
Similar to D56967, we add the existing diag::note_locked_here to tell
the user where we saw the locking that isn't matched correctly.
Reviewers: aaron.ballman, delesley
Reviewed By: aaron.ballman
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D59455
Modified:
cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
cfe/trunk/test/Sema/warn-thread-safety-analysis.c
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h?rev=356427&r1=356426&r2=356427&view=diff
==
--- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h Mon Mar 18
16:26:54 2019
@@ -119,9 +119,11 @@ public:
/// \param Kind -- the capability's name parameter (role, mutex, etc).
/// \param Expected -- the kind of lock expected.
/// \param Received -- the kind of lock received.
+ /// \param LocLocked -- The SourceLocation of the Lock.
/// \param Loc -- The SourceLocation of the Unlock.
virtual void handleIncorrectUnlockKind(StringRef Kind, Name LockName,
LockKind Expected, LockKind Received,
+ SourceLocation LocLocked,
SourceLocation Loc) {}
/// Warn about lock function calls for locks which are already held.
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=356427&r1=356426&r2=356427&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Mon Mar 18 16:26:54 2019
@@ -1335,8 +1335,8 @@ void ThreadSafetyAnalyzer::removeLock(Fa
// Generic lock removal doesn't care about lock kind mismatches, but
// otherwise diagnose when the lock kinds are mismatched.
if (ReceivedKind != LK_Generic && LDat->kind() != ReceivedKind) {
-Handler.handleIncorrectUnlockKind(DiagKind, Cp.toString(),
- LDat->kind(), ReceivedKind, UnlockLoc);
+Handler.handleIncorrectUnlockKind(DiagKind, Cp.toString(), LDat->kind(),
+ ReceivedKind, LDat->loc(), UnlockLoc);
}
LDat->handleUnlock(FSet, FactMan, Cp, UnlockLoc, FullyRemove, Handler,
Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=356427&r1=356426&r2=356427&view=diff
==
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Mon Mar 18 16:26:54 2019
@@ -1642,6 +1642,13 @@ class ThreadSafetyReporter : public clan
return ONS;
}
+ OptionalNotes makeLockedHereNote(SourceLocation LocLocked, StringRef Kind) {
+return LocLocked.isValid()
+ ? getNotes(PartialDiagnosticAt(
+ LocLocked, S.PDiag(diag::note_locked_here) << Kind))
+ : getNotes();
+ }
+
public:
ThreadSafetyReporter(Sema &S, SourceLocation FL, SourceLocation FEL)
: S(S), FunLocation(FL), FunEndLocation(FEL),
@@ -1679,13 +1686,15 @@ class ThreadSafetyReporter : public clan
void handleIncorrectUnlockKind(StringRef Kind, Name LockName,
LockKind Expected, LockKind Received,
+ SourceLocation LocLocked,
SourceLocation Loc) override {
if (Loc.isInvalid())
Loc = FunLocation;
PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_unlock_kind_mismatch)
<< Kind << LockName << Received
<< Expected);
-Warnings.emplace_back(std::move(Warning), getNotes());
+Warnings.emplace_back(std::move(Warning),
+ makeLockedHereNote(LocLocked, Kind));
}
void handleDoubleLock(StringRef Kind, Name LockName, SourceLocation
LocLocked,
@@ -1694,12 +1703,8 @@ class ThreadSafetyReporter : public clan
Loc = FunLocation;
PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_double_lock)
<< Kind << LockName);
-OptionalNotes Notes =
-LocLocked.isValid()
-? getNotes(PartialDiagnosticAt(
-
r356430 - Minor renaming as suggested in review [NFC]
Author: aaronpuchert
Date: Mon Mar 18 17:14:46 2019
New Revision: 356430
URL: http://llvm.org/viewvc/llvm-project?rev=356430&view=rev
Log:
Minor renaming as suggested in review [NFC]
See D59455.
Modified:
cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h
cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h?rev=356430&r1=356429&r2=356430&view=diff
==
--- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafety.h Mon Mar 18
17:14:46 2019
@@ -120,20 +120,21 @@ public:
/// \param Expected -- the kind of lock expected.
/// \param Received -- the kind of lock received.
/// \param LocLocked -- The SourceLocation of the Lock.
- /// \param Loc -- The SourceLocation of the Unlock.
+ /// \param LocUnlock -- The SourceLocation of the Unlock.
virtual void handleIncorrectUnlockKind(StringRef Kind, Name LockName,
LockKind Expected, LockKind Received,
SourceLocation LocLocked,
- SourceLocation Loc) {}
+ SourceLocation LocUnlock) {}
/// Warn about lock function calls for locks which are already held.
/// \param Kind -- the capability's name parameter (role, mutex, etc).
/// \param LockName -- A StringRef name for the lock expression, to be
printed
/// in the error message.
/// \param LocLocked -- The location of the first lock expression.
- /// \param Loc -- The location of the second lock expression.
+ /// \param LocDoubleLock -- The location of the second lock expression.
virtual void handleDoubleLock(StringRef Kind, Name LockName,
-SourceLocation LocLocked, SourceLocation Loc)
{}
+SourceLocation LocLocked,
+SourceLocation LocDoubleLock) {}
/// Warn about situations where a mutex is sometimes held and sometimes not.
/// The three situations are:
Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=356430&r1=356429&r2=356430&view=diff
==
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Mon Mar 18 17:14:46 2019
@@ -1687,22 +1687,22 @@ class ThreadSafetyReporter : public clan
void handleIncorrectUnlockKind(StringRef Kind, Name LockName,
LockKind Expected, LockKind Received,
SourceLocation LocLocked,
- SourceLocation Loc) override {
-if (Loc.isInvalid())
- Loc = FunLocation;
-PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_unlock_kind_mismatch)
- << Kind << LockName << Received
- << Expected);
+ SourceLocation LocUnlock) override {
+if (LocUnlock.isInvalid())
+ LocUnlock = FunLocation;
+PartialDiagnosticAt Warning(
+LocUnlock, S.PDiag(diag::warn_unlock_kind_mismatch)
+ << Kind << LockName << Received << Expected);
Warnings.emplace_back(std::move(Warning),
makeLockedHereNote(LocLocked, Kind));
}
void handleDoubleLock(StringRef Kind, Name LockName, SourceLocation
LocLocked,
-SourceLocation Loc) override {
-if (Loc.isInvalid())
- Loc = FunLocation;
-PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_double_lock)
- << Kind << LockName);
+SourceLocation LocDoubleLock) override {
+if (LocDoubleLock.isInvalid())
+ LocDoubleLock = FunLocation;
+PartialDiagnosticAt Warning(LocDoubleLock, S.PDiag(diag::warn_double_lock)
+ << Kind << LockName);
Warnings.emplace_back(std::move(Warning),
makeLockedHereNote(LocLocked, Kind));
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342418 - Thread safety analysis: Run more tests with capability attributes [NFC]
Author: aaronpuchert
Date: Mon Sep 17 14:37:22 2018
New Revision: 342418
URL: http://llvm.org/viewvc/llvm-project?rev=342418&view=rev
Log:
Thread safety analysis: Run more tests with capability attributes [NFC]
Summary:
We run the tests for -Wthread-safety-{negative,verbose} with the new
attributes as well as the old ones. Also put the macros in a header so
that we don't have to copy them all around.
The warn-thread-safety-parsing.cpp test checks for warnings depending on
the actual attribute name, so it can't undergo the same treatment.
Together with D49275 this should fix PR33754.
Reviewers: aaron.ballman, delesley, grooverdan
Reviewed By: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D52141
Added:
cfe/trunk/test/SemaCXX/thread-safety-annotations.h
Modified:
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-negative.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-verbose.cpp
Added: cfe/trunk/test/SemaCXX/thread-safety-annotations.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/thread-safety-annotations.h?rev=342418&view=auto
==
--- cfe/trunk/test/SemaCXX/thread-safety-annotations.h (added)
+++ cfe/trunk/test/SemaCXX/thread-safety-annotations.h Mon Sep 17 14:37:22 2018
@@ -0,0 +1,42 @@
+// Macros to enable testing of both variants of the thread safety analysis.
+
+#if USE_CAPABILITY
+#define LOCKABLE__attribute__((capability("mutex")))
+#define ASSERT_EXCLUSIVE_LOCK(...)
__attribute__((assert_capability(__VA_ARGS__)))
+#define ASSERT_SHARED_LOCK(...)
__attribute__((assert_shared_capability(__VA_ARGS__)))
+#define EXCLUSIVE_LOCK_FUNCTION(...)
__attribute__((acquire_capability(__VA_ARGS__)))
+#define SHARED_LOCK_FUNCTION(...)
__attribute__((acquire_shared_capability(__VA_ARGS__)))
+#define EXCLUSIVE_TRYLOCK_FUNCTION(...)
__attribute__((try_acquire_capability(__VA_ARGS__)))
+#define SHARED_TRYLOCK_FUNCTION(...)
__attribute__((try_acquire_shared_capability(__VA_ARGS__)))
+#define EXCLUSIVE_LOCKS_REQUIRED(...)
__attribute__((requires_capability(__VA_ARGS__)))
+#define SHARED_LOCKS_REQUIRED(...)
__attribute__((requires_shared_capability(__VA_ARGS__)))
+#else
+#define LOCKABLE__attribute__((lockable))
+#define ASSERT_EXCLUSIVE_LOCK(...)
__attribute__((assert_exclusive_lock(__VA_ARGS__)))
+#define ASSERT_SHARED_LOCK(...)
__attribute__((assert_shared_lock(__VA_ARGS__)))
+#define EXCLUSIVE_LOCK_FUNCTION(...)
__attribute__((exclusive_lock_function(__VA_ARGS__)))
+#define SHARED_LOCK_FUNCTION(...)
__attribute__((shared_lock_function(__VA_ARGS__)))
+#define EXCLUSIVE_TRYLOCK_FUNCTION(...)
__attribute__((exclusive_trylock_function(__VA_ARGS__)))
+#define SHARED_TRYLOCK_FUNCTION(...)
__attribute__((shared_trylock_function(__VA_ARGS__)))
+#define EXCLUSIVE_LOCKS_REQUIRED(...)
__attribute__((exclusive_locks_required(__VA_ARGS__)))
+#define SHARED_LOCKS_REQUIRED(...)
__attribute__((shared_locks_required(__VA_ARGS__)))
+#endif
+
+// Lock semantics only
+#define UNLOCK_FUNCTION(...)
__attribute__((unlock_function(__VA_ARGS__)))
+#define GUARDED_VAR __attribute__((guarded_var))
+#define PT_GUARDED_VAR __attribute__((pt_guarded_var))
+
+// Capabilities only
+#define EXCLUSIVE_UNLOCK_FUNCTION(...)
__attribute__((release_capability(__VA_ARGS__)))
+#define SHARED_UNLOCK_FUNCTION(...)
__attribute__((release_shared_capability(__VA_ARGS__)))
+#define GUARDED_BY(x) __attribute__((guarded_by(x)))
+#define PT_GUARDED_BY(x)__attribute__((pt_guarded_by(x)))
+
+// Common
+#define SCOPED_LOCKABLE __attribute__((scoped_lockable))
+#define ACQUIRED_AFTER(...)
__attribute__((acquired_after(__VA_ARGS__)))
+#define ACQUIRED_BEFORE(...)
__attribute__((acquired_before(__VA_ARGS__)))
+#define LOCK_RETURNED(x)__attribute__((lock_returned(x)))
+#define LOCKS_EXCLUDED(...)
__attribute__((locks_excluded(__VA_ARGS__)))
+#define NO_THREAD_SAFETY_ANALYSIS
__attribute__((no_thread_safety_analysis))
Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp?rev=342418&r1=342417&r2=342418&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Mon Sep 17 14:37:22
2018
@@ -6,42 +6,7 @@
// FIXME: should also run %clang_cc1 -fsyntax-only -verify -Wthread-safety
-std=c++11 -Wc++98-compat %s
// FIXME: should also run %clang_cc1 -fsyntax-only -verify -Wthread-safety %s
-#define SCOPED_LOCKABLE __a
r342519 - Thread safety analysis: Fix crash for function pointers
Author: aaronpuchert
Date: Tue Sep 18 17:19:38 2018
New Revision: 342519
URL: http://llvm.org/viewvc/llvm-project?rev=342519&view=rev
Log:
Thread safety analysis: Fix crash for function pointers
For function pointers, the FunctionDecl of the callee is unknown, so
getDirectCallee will return nullptr. We have to catch that case to avoid
crashing. We assume there is no attribute then.
Modified:
cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
Modified: cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp?rev=342519&r1=342518&r2=342519&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp Tue Sep 18 17:19:38 2018
@@ -354,15 +354,17 @@ til::SExpr *SExprBuilder::translateCallE
const Expr *SelfE) {
if (CapabilityExprMode) {
// Handle LOCK_RETURNED
-const FunctionDecl *FD = CE->getDirectCallee()->getMostRecentDecl();
-if (LockReturnedAttr* At = FD->getAttr()) {
- CallingContext LRCallCtx(Ctx);
- LRCallCtx.AttrDecl = CE->getDirectCallee();
- LRCallCtx.SelfArg = SelfE;
- LRCallCtx.NumArgs = CE->getNumArgs();
- LRCallCtx.FunArgs = CE->getArgs();
- return const_cast(
- translateAttrExpr(At->getArg(), &LRCallCtx).sexpr());
+if (const FunctionDecl *FD = CE->getDirectCallee()) {
+ FD = FD->getMostRecentDecl();
+ if (LockReturnedAttr *At = FD->getAttr()) {
+CallingContext LRCallCtx(Ctx);
+LRCallCtx.AttrDecl = CE->getDirectCallee();
+LRCallCtx.SelfArg = SelfE;
+LRCallCtx.NumArgs = CE->getNumArgs();
+LRCallCtx.FunArgs = CE->getArgs();
+return const_cast(
+translateAttrExpr(At->getArg(), &LRCallCtx).sexpr());
+ }
}
}
Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp?rev=342519&r1=342518&r2=342519&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Tue Sep 18 17:19:38
2018
@@ -2323,6 +2323,7 @@ Foo& getBarFoo(Bar &bar, int c) { return
void test() {
Foo foo;
Foo *fooArray;
+ Foo &(*fooFuncPtr)();
Bar bar;
int a;
int b;
@@ -2359,6 +2360,10 @@ void test() {
(a > 0 ? fooArray[1] : fooArray[b]).mu_.Lock();
(a > 0 ? fooArray[1] : fooArray[b]).a = 0;
(a > 0 ? fooArray[1] : fooArray[b]).mu_.Unlock();
+
+ fooFuncPtr().mu_.Lock();
+ fooFuncPtr().a = 0;
+ fooFuncPtr().mu_.Unlock();
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342600 - Thread safety analysis: Handle ObjCIvarRefExpr in SExprBuilder::translate
Author: aaronpuchert
Date: Wed Sep 19 16:57:38 2018
New Revision: 342600
URL: http://llvm.org/viewvc/llvm-project?rev=342600&view=rev
Log:
Thread safety analysis: Handle ObjCIvarRefExpr in SExprBuilder::translate
Summary:
This imitates the code for MemberExpr.
Fixes PR38896.
Reviewers: aaron.ballman, delesley, lukasza, rjmccall
Reviewed By: delesley
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D52200
Added:
cfe/trunk/test/SemaObjCXX/warn-thread-safety-analysis.mm
Modified:
cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h?rev=342600&r1=342599&r2=342600&view=diff
==
--- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyCommon.h Wed Sep 19
16:57:38 2018
@@ -397,6 +397,8 @@ private:
CallingContext *Ctx) ;
til::SExpr *translateCXXThisExpr(const CXXThisExpr *TE, CallingContext *Ctx);
til::SExpr *translateMemberExpr(const MemberExpr *ME, CallingContext *Ctx);
+ til::SExpr *translateObjCIVarRefExpr(const ObjCIvarRefExpr *IVRE,
+ CallingContext *Ctx);
til::SExpr *translateCallExpr(const CallExpr *CE, CallingContext *Ctx,
const Expr *SelfE = nullptr);
til::SExpr *translateCXXMemberCallExpr(const CXXMemberCallExpr *ME,
Modified: cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp?rev=342600&r1=342599&r2=342600&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp Wed Sep 19 16:57:38 2018
@@ -211,6 +211,8 @@ til::SExpr *SExprBuilder::translate(cons
return translateCXXThisExpr(cast(S), Ctx);
case Stmt::MemberExprClass:
return translateMemberExpr(cast(S), Ctx);
+ case Stmt::ObjCIvarRefExprClass:
+return translateObjCIVarRefExpr(cast(S), Ctx);
case Stmt::CallExprClass:
return translateCallExpr(cast(S), Ctx);
case Stmt::CXXMemberCallExprClass:
@@ -311,9 +313,9 @@ static const ValueDecl *getValueDeclFrom
return nullptr;
}
-static bool hasCppPointerType(const til::SExpr *E) {
+static bool hasAnyPointerType(const til::SExpr *E) {
auto *VD = getValueDeclFromSExpr(E);
- if (VD && VD->getType()->isPointerType())
+ if (VD && VD->getType()->isAnyPointerType())
return true;
if (const auto *C = dyn_cast(E))
return C->castOpcode() == til::CAST_objToPtr;
@@ -344,7 +346,20 @@ til::SExpr *SExprBuilder::translateMembe
D = getFirstVirtualDecl(VD);
til::Project *P = new (Arena) til::Project(E, D);
- if (hasCppPointerType(BE))
+ if (hasAnyPointerType(BE))
+P->setArrow(true);
+ return P;
+}
+
+til::SExpr *SExprBuilder::translateObjCIVarRefExpr(const ObjCIvarRefExpr *IVRE,
+ CallingContext *Ctx) {
+ til::SExpr *BE = translate(IVRE->getBase(), Ctx);
+ til::SExpr *E = new (Arena) til::SApply(BE);
+
+ const auto *D = cast(IVRE->getDecl()->getCanonicalDecl());
+
+ til::Project *P = new (Arena) til::Project(E, D);
+ if (hasAnyPointerType(BE))
P->setArrow(true);
return P;
}
Added: cfe/trunk/test/SemaObjCXX/warn-thread-safety-analysis.mm
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/warn-thread-safety-analysis.mm?rev=342600&view=auto
==
--- cfe/trunk/test/SemaObjCXX/warn-thread-safety-analysis.mm (added)
+++ cfe/trunk/test/SemaObjCXX/warn-thread-safety-analysis.mm Wed Sep 19
16:57:38 2018
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -Wthread-safety-beta
-Wno-objc-root-class %s
+
+class __attribute__((lockable)) Lock {
+public:
+ void Acquire() __attribute__((exclusive_lock_function())) {}
+ void Release() __attribute__((unlock_function())) {}
+};
+
+class __attribute__((scoped_lockable)) AutoLock {
+public:
+ AutoLock(Lock &lock) __attribute__((exclusive_lock_function(lock)))
+ : lock_(lock) {
+lock.Acquire();
+ }
+ ~AutoLock() __attribute__((unlock_function())) { lock_.Release(); }
+
+private:
+ Lock &lock_;
+};
+
+@interface MyInterface {
+@private
+ Lock lock_;
+ int value_;
+}
+
+- (void)incrementValue;
+- (void)decrementValue;
+
+@end
+
+@implementation MyInterface
+
+- (void)incrementValue {
+ AutoLock lock(lock_);
+ value_ += 1;
+}
+
+- (void)decrementValue {
+ lock_.Acquire(); // expected-note{{mutex acquired here}}
+ value_ -= 1;
+} // expected-warning{{mutex 'self->lock_' is still h
r342605 - Thread Safety Analysis: warnings for attributes without arguments
Author: aaronpuchert
Date: Wed Sep 19 17:39:27 2018
New Revision: 342605
URL: http://llvm.org/viewvc/llvm-project?rev=342605&view=rev
Log:
Thread Safety Analysis: warnings for attributes without arguments
Summary:
When thread safety annotations are used without capability arguments,
they are assumed to apply to `this` instead. So we warn when either
`this` doesn't exist, or the class is not a capability type.
This is based on earlier work by Josh Gao that was committed in r310403,
but reverted in r310698 because it didn't properly work in template
classes. See also D36237.
The solution is not to go via the QualType of `this`, which is then a
template type, hence the attributes are not known because it could be
specialized. Instead we look directly at the class in which we are
contained.
Additionally I grouped two of the warnings together. There are two
issues here: the existence of `this`, which requires us to be a
non-static member function, and the appropriate annotation on the class
we are contained in. So we don't distinguish between not being in a
class and being static, because in both cases we don't have `this`.
Fixes PR38399.
Reviewers: aaron.ballman, delesley, jmgao, rtrieu
Reviewed By: delesley
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D51901
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/Sema/attr-capabilities.c
cfe/trunk/test/SemaCXX/warn-thread-safety-parsing.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=342605&r1=342604&r2=342605&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Sep 19 17:39:27
2018
@@ -3008,6 +3008,14 @@ def warn_invalid_capability_name : Warni
def warn_thread_attribute_ignored : Warning<
"ignoring %0 attribute because its argument is invalid">,
InGroup, DefaultIgnore;
+def warn_thread_attribute_not_on_non_static_member : Warning<
+ "%0 attribute without capability arguments can only be applied to non-static
"
+ "methods of a class">,
+ InGroup, DefaultIgnore;
+def warn_thread_attribute_not_on_capability_member : Warning<
+ "%0 attribute without capability arguments refers to 'this', but %1 isn't "
+ "annotated with 'capability' or 'scoped_lockable' attribute">,
+ InGroup, DefaultIgnore;
def warn_thread_attribute_argument_not_lockable : Warning<
"%0 attribute requires arguments whose type is annotated "
"with 'capability' attribute; type here is %1">,
Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=342605&r1=342604&r2=342605&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed Sep 19 17:39:27 2018
@@ -476,6 +476,29 @@ static const RecordType *getRecordType(Q
return nullptr;
}
+template
+static bool checkRecordDeclForAttr(const RecordDecl *RD) {
+ // Check if the record itself has the attribute.
+ if (RD->hasAttr())
+return true;
+
+ // Else check if any base classes have the attribute.
+ if (const auto *CRD = dyn_cast(RD)) {
+CXXBasePaths BPaths(false, false);
+if (CRD->lookupInBases(
+[](const CXXBaseSpecifier *BS, CXXBasePath &) {
+ const auto &Ty = *BS->getType();
+ // If it's type-dependent, we assume it could have the attribute.
+ if (Ty.isDependentType())
+return true;
+ return Ty.getAs()->getDecl()->hasAttr();
+},
+BPaths, true))
+ return true;
+ }
+ return false;
+}
+
static bool checkRecordTypeForCapability(Sema &S, QualType Ty) {
const RecordType *RT = getRecordType(Ty);
@@ -491,21 +514,7 @@ static bool checkRecordTypeForCapability
if (threadSafetyCheckIsSmartPointer(S, RT))
return true;
- // Check if the record itself has a capability.
- RecordDecl *RD = RT->getDecl();
- if (RD->hasAttr())
-return true;
-
- // Else check if any base classes have a capability.
- if (const auto *CRD = dyn_cast(RD)) {
-CXXBasePaths BPaths(false, false);
-if (CRD->lookupInBases([](const CXXBaseSpecifier *BS, CXXBasePath &) {
- const auto *Type = BS->getType()->getAs();
- return Type->getDecl()->hasAttr();
-}, BPaths))
- return true;
- }
- return false;
+ return checkRecordDeclForAttr(RT->getDecl());
}
static bool checkTypedefTypeForCapability(QualType Ty) {
@@ -563,8 +572,27 @@ static bool isCapabilityExpr(Sema &S, co
static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D,
const ParsedAttr &AL,
r342787 - Thread safety analysis: Make sure FactEntrys stored in FactManager are immutable [NFC]
Author: aaronpuchert
Date: Fri Sep 21 16:08:30 2018
New Revision: 342787
URL: http://llvm.org/viewvc/llvm-project?rev=342787&view=rev
Log:
Thread safety analysis: Make sure FactEntrys stored in FactManager are
immutable [NFC]
Since FactEntrys are stored in the FactManager, we can't manipulate them
anymore when they are stored there.
Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=342787&r1=342786&r2=342787&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Fri Sep 21 16:08:30 2018
@@ -151,7 +151,7 @@ public:
StringRef DiagKind) const = 0;
// Return true if LKind >= LK, where exclusive > shared
- bool isAtLeast(LockKind LK) {
+ bool isAtLeast(LockKind LK) const {
return (LKind == LK_Exclusive) || (LK == LK_Shared);
}
};
@@ -162,7 +162,7 @@ using FactID = unsigned short;
/// the analysis of a single routine.
class FactManager {
private:
- std::vector> Facts;
+ std::vector> Facts;
public:
FactID newFact(std::unique_ptr Entry) {
@@ -171,7 +171,6 @@ public:
}
const FactEntry &operator[](FactID F) const { return *Facts[F]; }
- FactEntry &operator[](FactID F) { return *Facts[F]; }
};
/// A FactSet is the set of facts that are known to be true at a
@@ -241,22 +240,23 @@ public:
});
}
- FactEntry *findLock(FactManager &FM, const CapabilityExpr &CapE) const {
+ const FactEntry *findLock(FactManager &FM, const CapabilityExpr &CapE) const
{
auto I = std::find_if(begin(), end(), [&](FactID ID) {
return FM[ID].matches(CapE);
});
return I != end() ? &FM[*I] : nullptr;
}
- FactEntry *findLockUniv(FactManager &FM, const CapabilityExpr &CapE) const {
+ const FactEntry *findLockUniv(FactManager &FM,
+const CapabilityExpr &CapE) const {
auto I = std::find_if(begin(), end(), [&](FactID ID) -> bool {
return FM[ID].matchesUniv(CapE);
});
return I != end() ? &FM[*I] : nullptr;
}
- FactEntry *findPartialMatch(FactManager &FM,
- const CapabilityExpr &CapE) const {
+ const FactEntry *findPartialMatch(FactManager &FM,
+const CapabilityExpr &CapE) const {
auto I = std::find_if(begin(), end(), [&](FactID ID) -> bool {
return FM[ID].partiallyMatches(CapE);
});
@@ -1258,7 +1258,7 @@ void ThreadSafetyAnalyzer::addLock(FactS
if (!ReqAttr && !Entry->negative()) {
// look for the negative capability, and remove it from the fact set.
CapabilityExpr NegC = !*Entry;
-FactEntry *Nen = FSet.findLock(FactMan, NegC);
+const FactEntry *Nen = FSet.findLock(FactMan, NegC);
if (Nen) {
FSet.removeLock(FactMan, NegC);
}
@@ -1277,7 +1277,7 @@ void ThreadSafetyAnalyzer::addLock(FactS
}
// FIXME: Don't always warn when we have support for reentrant locks.
- if (FactEntry *Cp = FSet.findLock(FactMan, *Entry)) {
+ if (const FactEntry *Cp = FSet.findLock(FactMan, *Entry)) {
if (!Entry->asserted())
Cp->handleLock(FSet, FactMan, *Entry, Handler, DiagKind);
} else {
@@ -1575,7 +1575,7 @@ void BuildLockset::warnIfMutexNotHeld(co
if (Cp.negative()) {
// Negative capabilities act like locks excluded
-FactEntry *LDat = FSet.findLock(Analyzer->FactMan, !Cp);
+const FactEntry *LDat = FSet.findLock(Analyzer->FactMan, !Cp);
if (LDat) {
Analyzer->Handler.handleFunExcludesLock(
DiagKind, D->getNameAsString(), (!Cp).toString(), Loc);
@@ -1596,7 +1596,7 @@ void BuildLockset::warnIfMutexNotHeld(co
return;
}
- FactEntry* LDat = FSet.findLockUniv(Analyzer->FactMan, Cp);
+ const FactEntry *LDat = FSet.findLockUniv(Analyzer->FactMan, Cp);
bool NoError = true;
if (!LDat) {
// No exact match found. Look for a partial match.
@@ -1632,7 +1632,7 @@ void BuildLockset::warnIfMutexHeld(const
return;
}
- FactEntry* LDat = FSet.findLock(Analyzer->FactMan, Cp);
+ const FactEntry *LDat = FSet.findLock(Analyzer->FactMan, Cp);
if (LDat) {
Analyzer->Handler.handleFunExcludesLock(
DiagKind, D->getNameAsString(), Cp.toString(), Exp->getExprLoc());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342790 - Thread safety analysis: Make printSCFG compile again [NFC]
Author: aaronpuchert
Date: Fri Sep 21 16:46:35 2018
New Revision: 342790
URL: http://llvm.org/viewvc/llvm-project?rev=342790&view=rev
Log:
Thread safety analysis: Make printSCFG compile again [NFC]
Not used productively, so no observable functional change.
Note that printSCFG doesn't yet work reliably, it seems to crash
sometimes.
Modified:
cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h?rev=342790&r1=342789&r2=342790&view=diff
==
--- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h Fri Sep 21
16:46:35 2018
@@ -785,7 +785,26 @@ protected:
void printCast(const Cast *E, StreamType &SS) {
if (!CStyle) {
SS << "cast[";
- SS << E->castOpcode();
+ switch (E->castOpcode()) {
+ case CAST_none:
+SS << "none";
+break;
+ case CAST_extendNum:
+SS << "extendNum";
+break;
+ case CAST_truncNum:
+SS << "truncNum";
+break;
+ case CAST_toFloat:
+SS << "toFloat";
+break;
+ case CAST_toInt:
+SS << "toInt";
+break;
+ case CAST_objToPtr:
+SS << "objToPtr";
+break;
+ }
SS << "](";
self()->printSExpr(E->expr(), SS, Prec_Unary);
SS << ")";
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=342790&r1=342789&r2=342790&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Fri Sep 21 16:46:35 2018
@@ -64,13 +64,6 @@ using namespace threadSafety;
// Key method definition
ThreadSafetyHandler::~ThreadSafetyHandler() = default;
-namespace {
-
-class TILPrinter :
-public til::PrettyPrinter {};
-
-} // namespace
-
/// Issue a warning about an invalid lock expression
static void warnInvalidLock(ThreadSafetyHandler &Handler,
const Expr *MutexExp, const NamedDecl *D,
Modified: cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp?rev=342790&r1=342789&r2=342790&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp Fri Sep 21 16:46:35 2018
@@ -944,6 +944,16 @@ void SExprBuilder::exitCFG(const CFGBloc
}
/*
+namespace {
+
+class TILPrinter :
+public til::PrettyPrinter {};
+
+} // namespace
+
+namespace clang {
+namespace threadSafety {
+
void printSCFG(CFGWalker &Walker) {
llvm::BumpPtrAllocator Bpa;
til::MemRegionRef Arena(&Bpa);
@@ -951,4 +961,7 @@ void printSCFG(CFGWalker &Walker) {
til::SCFG *Scfg = SxBuilder.buildCFG(Walker);
TILPrinter::print(Scfg, llvm::errs());
}
+
+} // namespace threadSafety
+} // namespace clang
*/
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342823 - Eliminate some unneeded signed/unsigned conversions
Author: aaronpuchert
Date: Sat Sep 22 14:56:16 2018
New Revision: 342823
URL: http://llvm.org/viewvc/llvm-project?rev=342823&view=rev
Log:
Eliminate some unneeded signed/unsigned conversions
No functional change is intended, but generally this should be a bit
more safe.
Modified:
cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/lib/Analysis/ThreadSafetyTIL.cpp
Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h?rev=342823&r1=342822&r2=342823&view=diff
==
--- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h Sat Sep 22
14:56:16 2018
@@ -1643,10 +1643,10 @@ private:
friend class SCFG;
// assign unique ids to all instructions
- int renumberInstrs(int id);
+ unsigned renumberInstrs(unsigned id);
- int topologicalSort(SimpleArray &Blocks, int ID);
- int topologicalFinalSort(SimpleArray &Blocks, int ID);
+ unsigned topologicalSort(SimpleArray &Blocks, unsigned ID);
+ unsigned topologicalFinalSort(SimpleArray &Blocks, unsigned
ID);
void computeDominator();
void computePostDominator();
@@ -1657,7 +1657,7 @@ private:
SCFG *CFGPtr = nullptr;
// Unique ID for this BB in the containing CFG. IDs are in topological order.
- int BlockID : 31;
+ unsigned BlockID : 31;
// Bit to determine if a block has been visited during a traversal.
bool Visited : 1;
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=342823&r1=342822&r2=342823&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Sat Sep 22 14:56:16 2018
@@ -730,7 +730,7 @@ void LocalVariableMap::traverseCFG(CFG *
CtxIndices.resize(CFGraph->getNumBlockIDs());
for (const auto *CurrBlock : *SortedGraph) {
-int CurrBlockID = CurrBlock->getBlockID();
+unsigned CurrBlockID = CurrBlock->getBlockID();
CFGBlockInfo *CurrBlockInfo = &BlockInfo[CurrBlockID];
VisitedBlocks.insert(CurrBlock);
@@ -746,7 +746,7 @@ void LocalVariableMap::traverseCFG(CFG *
continue;
}
- int PrevBlockID = (*PI)->getBlockID();
+ unsigned PrevBlockID = (*PI)->getBlockID();
CFGBlockInfo *PrevBlockInfo = &BlockInfo[PrevBlockID];
if (CtxInit) {
@@ -2302,7 +2302,7 @@ void ThreadSafetyAnalyzer::runAnalysis(A
}
for (const auto *CurrBlock : *SortedGraph) {
-int CurrBlockID = CurrBlock->getBlockID();
+unsigned CurrBlockID = CurrBlock->getBlockID();
CFGBlockInfo *CurrBlockInfo = &BlockInfo[CurrBlockID];
// Use the default initial lockset in case there are no predecessors.
@@ -2329,7 +2329,7 @@ void ThreadSafetyAnalyzer::runAnalysis(A
if (*PI == nullptr || !VisitedBlocks.alreadySet(*PI))
continue;
- int PrevBlockID = (*PI)->getBlockID();
+ unsigned PrevBlockID = (*PI)->getBlockID();
CFGBlockInfo *PrevBlockInfo = &BlockInfo[PrevBlockID];
// Ignore edges from blocks that can't return.
@@ -2370,7 +2370,7 @@ void ThreadSafetyAnalyzer::runAnalysis(A
// Process continue and break blocks. Assume that the lockset for the
// resulting block is unaffected by any discrepancies in them.
for (const auto *PrevBlock : SpecialBlocks) {
- int PrevBlockID = PrevBlock->getBlockID();
+ unsigned PrevBlockID = PrevBlock->getBlockID();
CFGBlockInfo *PrevBlockInfo = &BlockInfo[PrevBlockID];
if (!LocksetInitialized) {
Modified: cfe/trunk/lib/Analysis/ThreadSafetyTIL.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafetyTIL.cpp?rev=342823&r1=342822&r2=342823&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafetyTIL.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafetyTIL.cpp Sat Sep 22 14:56:16 2018
@@ -150,7 +150,7 @@ void til::simplifyIncompleteArg(til::Phi
}
// Renumbers the arguments and instructions to have unique, sequential IDs.
-int BasicBlock::renumberInstrs(int ID) {
+unsigned BasicBlock::renumberInstrs(unsigned ID) {
for (auto *Arg : Args)
Arg->setID(this, ID++);
for (auto *Instr : Instrs)
@@ -163,7 +163,8 @@ int BasicBlock::renumberInstrs(int ID) {
// Each block will be written into the Blocks array in order, and its BlockID
// will be set to the index in the array. Sorting should start from the entry
// block, and ID should be the total number of blocks.
-int BasicBlock::topologicalSort(SimpleArray &Blocks, int ID) {
+unsigned BasicBlock::topologicalSort(SimpleArray &Blocks,
+ unsi
r343681 - Thread safety analysis: Unwrap __builtin_expect in getTrylockCallExpr
Author: aaronpuchert
Date: Wed Oct 3 04:58:19 2018
New Revision: 343681
URL: http://llvm.org/viewvc/llvm-project?rev=343681&view=rev
Log:
Thread safety analysis: Unwrap __builtin_expect in getTrylockCallExpr
Summary:
When people are really sure they'll get the lock they sometimes use
__builtin_expect. It's also used by some assertion implementations.
Asserting that try-lock succeeded is basically the same as asserting
that the lock is not held by anyone else (and acquiring it).
Reviewers: aaron.ballman, delesley
Reviewed By: aaron.ballman
Subscribers: kristina, cfe-commits
Differential Revision: https://reviews.llvm.org/D52398
Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=343681&r1=343680&r2=343681&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Wed Oct 3 04:58:19 2018
@@ -33,6 +33,7 @@
#include "clang/Analysis/Analyses/ThreadSafetyUtil.h"
#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Analysis/CFG.h"
+#include "clang/Basic/Builtins.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/OperatorKinds.h"
#include "clang/Basic/SourceLocation.h"
@@ -1388,8 +1389,11 @@ const CallExpr* ThreadSafetyAnalyzer::ge
if (!Cond)
return nullptr;
- if (const auto *CallExp = dyn_cast(Cond))
+ if (const auto *CallExp = dyn_cast(Cond)) {
+if (CallExp->getBuiltinCallee() == Builtin::BI__builtin_expect)
+ return getTrylockCallExpr(CallExp->getArg(0), C, Negate);
return CallExp;
+ }
else if (const auto *PE = dyn_cast(Cond))
return getTrylockCallExpr(PE->getSubExpr(), C, Negate);
else if (const auto *CE = dyn_cast(Cond))
Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp?rev=343681&r1=343680&r2=343681&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Wed Oct 3 04:58:19
2018
@@ -1754,12 +1754,27 @@ struct TestTryLock {
mu.Unlock();
}
+ void foo2_builtin_expect() {
+if (__builtin_expect(!mu.TryLock(), false))
+ return;
+a = 2;
+mu.Unlock();
+ }
+
void foo3() {
bool b = mu.TryLock();
if (b) {
a = 3;
mu.Unlock();
}
+ }
+
+ void foo3_builtin_expect() {
+bool b = mu.TryLock();
+if (__builtin_expect(b, true)) {
+ a = 3;
+ mu.Unlock();
+}
}
void foo4() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r343831 - Thread safety analysis: Examine constructor arguments
Author: aaronpuchert
Date: Thu Oct 4 16:51:14 2018
New Revision: 343831
URL: http://llvm.org/viewvc/llvm-project?rev=343831&view=rev
Log:
Thread safety analysis: Examine constructor arguments
Summary:
Instead of only examining call arguments, we also examine constructor
arguments applying the same rules.
That was an opportunity for refactoring the examination procedure to
work with iterators instead of integer indices. For the case of
CallExprs no functional change is intended.
Reviewers: aaron.ballman, delesley
Reviewed By: delesley
Subscribers: JonasToth, cfe-commits
Differential Revision: https://reviews.llvm.org/D52443
Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=343831&r1=343830&r2=343831&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Thu Oct 4 16:51:14 2018
@@ -1538,6 +1538,10 @@ class BuildLockset : public ConstStmtVis
ProtectedOperationKind POK = POK_VarAccess);
void handleCall(const Expr *Exp, const NamedDecl *D, VarDecl *VD = nullptr);
+ void examineArguments(const FunctionDecl *FD,
+CallExpr::const_arg_iterator ArgBegin,
+CallExpr::const_arg_iterator ArgEnd,
+bool SkipFirstParam = false);
public:
BuildLockset(ThreadSafetyAnalyzer *Anlzr, CFGBlockInfo &Info)
@@ -1938,10 +1942,37 @@ void BuildLockset::VisitCastExpr(const C
checkAccess(CE->getSubExpr(), AK_Read);
}
-void BuildLockset::VisitCallExpr(const CallExpr *Exp) {
- bool ExamineArgs = true;
- bool OperatorFun = false;
+void BuildLockset::examineArguments(const FunctionDecl *FD,
+CallExpr::const_arg_iterator ArgBegin,
+CallExpr::const_arg_iterator ArgEnd,
+bool SkipFirstParam) {
+ // Currently we can't do anything if we don't know the function declaration.
+ if (!FD)
+return;
+
+ // NO_THREAD_SAFETY_ANALYSIS does double duty here. Normally it
+ // only turns off checking within the body of a function, but we also
+ // use it to turn off checking in arguments to the function. This
+ // could result in some false negatives, but the alternative is to
+ // create yet another attribute.
+ if (FD->hasAttr())
+return;
+
+ const ArrayRef Params = FD->parameters();
+ auto Param = Params.begin();
+ if (SkipFirstParam)
+++Param;
+
+ // There can be default arguments, so we stop when one iterator is at end().
+ for (auto Arg = ArgBegin; Param != Params.end() && Arg != ArgEnd;
+ ++Param, ++Arg) {
+QualType Qt = (*Param)->getType();
+if (Qt->isReferenceType())
+ checkAccess(*Arg, AK_Read, POK_PassByRef);
+ }
+}
+void BuildLockset::VisitCallExpr(const CallExpr *Exp) {
if (const auto *CE = dyn_cast(Exp)) {
const auto *ME = dyn_cast(CE->getCallee());
// ME can be null when calling a method pointer
@@ -1960,13 +1991,12 @@ void BuildLockset::VisitCallExpr(const C
checkAccess(CE->getImplicitObjectArgument(), AK_Read);
}
}
- } else if (const auto *OE = dyn_cast(Exp)) {
-OperatorFun = true;
+examineArguments(CE->getDirectCallee(), CE->arg_begin(), CE->arg_end());
+ } else if (const auto *OE = dyn_cast(Exp)) {
auto OEop = OE->getOperator();
switch (OEop) {
case OO_Equal: {
-ExamineArgs = false;
const Expr *Target = OE->getArg(0);
const Expr *Source = OE->getArg(1);
checkAccess(Target, AK_Written);
@@ -1975,60 +2005,27 @@ void BuildLockset::VisitCallExpr(const C
}
case OO_Star:
case OO_Arrow:
- case OO_Subscript: {
-const Expr *Obj = OE->getArg(0);
-checkAccess(Obj, AK_Read);
+ case OO_Subscript:
if (!(OEop == OO_Star && OE->getNumArgs() > 1)) {
// Grrr. operator* can be multiplication...
- checkPtAccess(Obj, AK_Read);
+ checkPtAccess(OE->getArg(0), AK_Read);
}
-break;
- }
+LLVM_FALLTHROUGH;
default: {
// TODO: get rid of this, and rely on pass-by-ref instead.
const Expr *Obj = OE->getArg(0);
checkAccess(Obj, AK_Read);
+// Check the remaining arguments. For method operators, the first
+// argument is the implicit self argument, and doesn't appear in the
+// FunctionDecl, but for non-methods it does.
+const FunctionDecl *FD = OE->getDirectCallee();
+examineArguments(FD, std::next(OE->arg_begin()), OE->arg_end(),
+ /*SkipFirstParam*/ !isa(FD));
break;
}
}
- }
-
- if (ExamineArgs) {
-if (const FunctionDecl *FD = Exp->ge
r343902 - Thread safety analysis: Handle conditional expression in getTrylockCallExpr
Author: aaronpuchert
Date: Fri Oct 5 18:09:28 2018
New Revision: 343902
URL: http://llvm.org/viewvc/llvm-project?rev=343902&view=rev
Log:
Thread safety analysis: Handle conditional expression in getTrylockCallExpr
Summary:
We unwrap conditional expressions containing try-lock functions.
Additionally we don't acquire on conditional expression branches, since
that is usually not helpful. When joining the branches we would almost
certainly get a warning then.
Hopefully fixes an issue that was raised in D52398.
Reviewers: aaron.ballman, delesley, hokein
Reviewed By: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D52888
Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=343902&r1=343901&r2=343902&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Fri Oct 5 18:09:28 2018
@@ -1435,6 +1435,17 @@ const CallExpr* ThreadSafetyAnalyzer::ge
if (BOP->getOpcode() == BO_LOr)
return getTrylockCallExpr(BOP->getRHS(), C, Negate);
return nullptr;
+ } else if (const auto *COP = dyn_cast(Cond)) {
+bool TCond, FCond;
+if (getStaticBooleanValue(COP->getTrueExpr(), TCond) &&
+getStaticBooleanValue(COP->getFalseExpr(), FCond)) {
+ if (TCond && !FCond)
+return getTrylockCallExpr(COP->getCond(), C, Negate);
+ if (!TCond && FCond) {
+Negate = !Negate;
+return getTrylockCallExpr(COP->getCond(), C, Negate);
+ }
+}
}
return nullptr;
}
@@ -1449,7 +1460,8 @@ void ThreadSafetyAnalyzer::getEdgeLockse
Result = ExitSet;
const Stmt *Cond = PredBlock->getTerminatorCondition();
- if (!Cond)
+ // We don't acquire try-locks on ?: branches, only when its result is used.
+ if (!Cond || isa(PredBlock->getTerminator()))
return;
bool Negate = false;
Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp?rev=343902&r1=343901&r2=343902&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Fri Oct 5 18:09:28
2018
@@ -1873,6 +1873,23 @@ struct TestTryLock {
int i = a;
mu.Unlock();
}
+
+ // Test with conditional operator
+ void foo13() {
+if (mu.TryLock() ? 1 : 0)
+ mu.Unlock();
+ }
+
+ void foo14() {
+if (mu.TryLock() ? 0 : 1)
+ return;
+mu.Unlock();
+ }
+
+ void foo15() {
+if (mu.TryLock() ? 0 : 1) // expected-note{{mutex acquired here}}
+ mu.Unlock();// expected-warning{{releasing mutex 'mu' that
was not held}}
+ } // expected-warning{{mutex 'mu' is not held on
every path through here}}
}; // end TestTrylock
} // end namespace TrylockTest
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r362266 - Clarify when fix-it hints on warnings are appropriate
Author: aaronpuchert Date: Fri May 31 14:27:39 2019 New Revision: 362266 URL: http://llvm.org/viewvc/llvm-project?rev=362266&view=rev Log: Clarify when fix-it hints on warnings are appropriate Summary: This is not a change in the rules, it's meant as a clarification about warnings. Since the recovery from warnings is a no-op, the fix-it hints on warnings shouldn't change anything. Anything that doesn't just suppress the warning and changes the meaning of the code (even if it's for the better) should be on an additional note. Reviewers: rsmith, aaron.ballman Reviewed By: aaron.ballman Subscribers: cfe-commits, thakis Tags: #clang Differential Revision: https://reviews.llvm.org/D62470 Modified: cfe/trunk/docs/InternalsManual.rst Modified: cfe/trunk/docs/InternalsManual.rst URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/InternalsManual.rst?rev=362266&r1=362265&r2=362266&view=diff == --- cfe/trunk/docs/InternalsManual.rst (original) +++ cfe/trunk/docs/InternalsManual.rst Fri May 31 14:27:39 2019 @@ -423,6 +423,9 @@ Fix-it hints on errors and warnings need driver, they should only be used when it's very likely they match the user's intent. * Clang must recover from errors as if the fix-it had been applied. +* Fix-it hints on a warning must not change the meaning of the code. + However, a hint may clarify the meaning as intentional, for example by adding + parentheses when the precedence of operators isn't obvious. If a fix-it can't obey these rules, put the fix-it on a note. Fix-its on notes are not applied automatically. ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r363494 - [Clang] Rename -split-dwarf-file to -split-dwarf-output
Author: aaronpuchert
Date: Sat Jun 15 07:07:43 2019
New Revision: 363494
URL: http://llvm.org/viewvc/llvm-project?rev=363494&view=rev
Log:
[Clang] Rename -split-dwarf-file to -split-dwarf-output
Summary:
This is the first in a series of changes trying to align clang -cc1
flags for Split DWARF with those of llc. The unfortunate side effect of
having -split-dwarf-output for single file Split DWARF will disappear
again in a subsequent change.
The change is the result of a discussion in D59673.
Reviewers: dblaikie, echristo
Reviewed By: dblaikie
Differential Revision: https://reviews.llvm.org/D63130
Modified:
cfe/trunk/include/clang/Basic/CodeGenOptions.h
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/CodeGen/split-debug-filename.c
cfe/trunk/test/CodeGen/split-debug-single-file.c
cfe/trunk/test/CodeGen/thinlto-split-dwarf.c
cfe/trunk/test/Driver/fuchsia.c
cfe/trunk/test/Driver/split-debug.c
cfe/trunk/test/Driver/split-debug.s
cfe/trunk/test/Misc/cc1as-split-dwarf.s
cfe/trunk/test/Modules/pch_container.m
cfe/trunk/tools/driver/cc1as_main.cpp
Modified: cfe/trunk/include/clang/Basic/CodeGenOptions.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/CodeGenOptions.h?rev=363494&r1=363493&r2=363494&view=diff
==
--- cfe/trunk/include/clang/Basic/CodeGenOptions.h (original)
+++ cfe/trunk/include/clang/Basic/CodeGenOptions.h Sat Jun 15 07:07:43 2019
@@ -187,7 +187,7 @@ public:
/// The name for the split debug info file that we'll break out. This is used
/// in the backend for setting the name in the skeleton cu.
- std::string SplitDwarfFile;
+ std::string SplitDwarfOutput;
/// The name of the relocation model to use.
llvm::Reloc::Model RelocationModel;
Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=363494&r1=363493&r2=363494&view=diff
==
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)
+++ cfe/trunk/include/clang/Driver/CC1Options.td Sat Jun 15 07:07:43 2019
@@ -683,7 +683,7 @@ def version : Flag<["-"], "version">,
HelpText<"Print the compiler version">;
def main_file_name : Separate<["-"], "main-file-name">,
HelpText<"Main file name to use for debug info">;
-def split_dwarf_file : Separate<["-"], "split-dwarf-file">,
+def split_dwarf_output : Separate<["-"], "split-dwarf-output">,
HelpText<"File name to use for split dwarf debug info output">;
}
Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=363494&r1=363493&r2=363494&view=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Sat Jun 15 07:07:43 2019
@@ -472,7 +472,7 @@ static void initTargetOptions(llvm::Targ
Options.EmitAddrsig = CodeGenOpts.Addrsig;
if (CodeGenOpts.getSplitDwarfMode() != CodeGenOptions::NoFission)
-Options.MCOptions.SplitDwarfFile = CodeGenOpts.SplitDwarfFile;
+Options.MCOptions.SplitDwarfFile = CodeGenOpts.SplitDwarfOutput;
Options.MCOptions.MCRelaxAll = CodeGenOpts.RelaxAll;
Options.MCOptions.MCSaveTempLabels = CodeGenOpts.SaveTempLabels;
Options.MCOptions.MCUseDwarfDirectory = !CodeGenOpts.NoDwarfDirectoryAsm;
@@ -862,9 +862,9 @@ void EmitAssemblyHelper::EmitAssembly(Ba
break;
default:
-if (!CodeGenOpts.SplitDwarfFile.empty() &&
+if (!CodeGenOpts.SplitDwarfOutput.empty() &&
(CodeGenOpts.getSplitDwarfMode() == CodeGenOptions::SplitFileFission))
{
- DwoOS = openOutputFile(CodeGenOpts.SplitDwarfFile);
+ DwoOS = openOutputFile(CodeGenOpts.SplitDwarfOutput);
if (!DwoOS)
return;
}
@@ -1275,9 +1275,9 @@ void EmitAssemblyHelper::EmitAssemblyWit
NeedCodeGen = true;
CodeGenPasses.add(
createTargetTransformInfoWrapperPass(getTargetIRAnalysis()));
-if (!CodeGenOpts.SplitDwarfFile.empty() &&
+if (!CodeGenOpts.SplitDwarfOutput.empty() &&
CodeGenOpts.getSplitDwarfMode() == CodeGenOptions::SplitFileFission) {
- DwoOS = openOutputFile(CodeGenOpts.SplitDwarfFile);
+ DwoOS = openOutputFile(CodeGenOpts.SplitDwarfOutput);
if (!DwoOS)
return;
}
@@ -1428,7 +1428,7 @@ static void runThinLTOBackend(ModuleSumm
Conf.RemarksWithHotness = CGOpts.DiagnosticsWithHotness;
Conf.RemarksFilename = CGOpts.OptRecordFile;
Conf.RemarksPasses = CGOpts.OptRecordPasses;
- Conf.DwoPath = CGOpts.SplitDwarfFile;
+ Conf.DwoPath = CGOpts.SplitDwarfOutput;
switch (Action) {
r363496 - [Clang] Harmonize Split DWARF options with llc
Author: aaronpuchert
Date: Sat Jun 15 08:38:51 2019
New Revision: 363496
URL: http://llvm.org/viewvc/llvm-project?rev=363496&view=rev
Log:
[Clang] Harmonize Split DWARF options with llc
Summary:
With Split DWARF the resulting object file (then called skeleton CU)
contains the file name of another ("DWO") file with the debug info.
This can be a problem for remote compilation, as it will contain the
name of the file on the compilation server, not on the client.
To use Split DWARF with remote compilation, one needs to either
* make sure only relative paths are used, and mirror the build directory
structure of the client on the server,
* inject the desired file name on the client directly.
Since llc already supports the latter solution, we're just copying that
over. We allow setting the actual output filename separately from the
value of the DW_AT_[GNU_]dwo_name attribute in the skeleton CU.
Fixes PR40276.
Reviewers: dblaikie, echristo, tejohnson
Reviewed By: dblaikie
Differential Revision: https://reviews.llvm.org/D59673
Added:
cfe/trunk/test/CodeGen/split-debug-output.c
Modified:
cfe/trunk/include/clang/Basic/CodeGenOptions.h
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/CodeGen/split-debug-filename.c
cfe/trunk/test/CodeGen/split-debug-single-file.c
cfe/trunk/test/CodeGen/thinlto-split-dwarf.c
cfe/trunk/test/Driver/split-debug.c
Modified: cfe/trunk/include/clang/Basic/CodeGenOptions.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/CodeGenOptions.h?rev=363496&r1=363495&r2=363496&view=diff
==
--- cfe/trunk/include/clang/Basic/CodeGenOptions.h (original)
+++ cfe/trunk/include/clang/Basic/CodeGenOptions.h Sat Jun 15 08:38:51 2019
@@ -185,8 +185,11 @@ public:
/// file, for example with -save-temps.
std::string MainFileName;
- /// The name for the split debug info file that we'll break out. This is used
- /// in the backend for setting the name in the skeleton cu.
+ /// The name for the split debug info file used for the DW_AT_[GNU_]dwo_name
+ /// attribute in the skeleton CU.
+ std::string SplitDwarfFile;
+
+ /// Output filename for the split debug info, not used in the skeleton CU.
std::string SplitDwarfOutput;
/// The name of the relocation model to use.
Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=363496&r1=363495&r2=363496&view=diff
==
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)
+++ cfe/trunk/include/clang/Driver/CC1Options.td Sat Jun 15 08:38:51 2019
@@ -696,6 +696,8 @@ def enable_split_dwarf : Flag<["-"], "en
HelpText<"Use DWARF fission in 'split' mode">;
def enable_split_dwarf_EQ : Joined<["-"], "enable-split-dwarf=">,
HelpText<"Set DWARF fission mode to either 'split' or 'single'">,
Values<"split,single">;
+def split_dwarf_file : Separate<["-"], "split-dwarf-file">,
+ HelpText<"Name of the split dwarf debug info file to encode in the object
file">;
def fno_wchar : Flag<["-"], "fno-wchar">,
HelpText<"Disable C++ builtin type wchar_t">;
def fconstant_string_class : Separate<["-"], "fconstant-string-class">,
Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=363496&r1=363495&r2=363496&view=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Sat Jun 15 08:38:51 2019
@@ -472,7 +472,7 @@ static void initTargetOptions(llvm::Targ
Options.EmitAddrsig = CodeGenOpts.Addrsig;
if (CodeGenOpts.getSplitDwarfMode() != CodeGenOptions::NoFission)
-Options.MCOptions.SplitDwarfFile = CodeGenOpts.SplitDwarfOutput;
+Options.MCOptions.SplitDwarfFile = CodeGenOpts.SplitDwarfFile;
Options.MCOptions.MCRelaxAll = CodeGenOpts.RelaxAll;
Options.MCOptions.MCSaveTempLabels = CodeGenOpts.SaveTempLabels;
Options.MCOptions.MCUseDwarfDirectory = !CodeGenOpts.NoDwarfDirectoryAsm;
@@ -1428,7 +1428,8 @@ static void runThinLTOBackend(ModuleSumm
Conf.RemarksWithHotness = CGOpts.DiagnosticsWithHotness;
Conf.RemarksFilename = CGOpts.OptRecordFile;
Conf.RemarksPasses = CGOpts.OptRecordPasses;
- Conf.DwoPath = CGOpts.SplitDwarfOutput;
+ Conf.SplitDwarfFile = CGOpts.SplitDwarfFile;
+ Conf.SplitDwarfOutput = CGOpts.SplitDwarfOutput;
switch (Action) {
case Backend_EmitNothing:
Conf.PreCodeGenModuleHook = [](size_t Task, const Module &Mod) {
Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib
r363748 - Show note for -Wmissing-prototypes for functions with parameters
Author: aaronpuchert
Date: Tue Jun 18 15:52:39 2019
New Revision: 363748
URL: http://llvm.org/viewvc/llvm-project?rev=363748&view=rev
Log:
Show note for -Wmissing-prototypes for functions with parameters
Summary:
There was a search for non-prototype declarations for the function, but
we only showed the results for zero-parameter functions. Now we show the
note for functions with parameters as well, but we omit the fix-it hint
suggesting to add `void`.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D62750
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/test/Sema/warn-missing-prototypes.c
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=363748&r1=363747&r2=363748&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Jun 18 15:52:39
2019
@@ -4672,7 +4672,8 @@ def warn_missing_prototype : Warning<
"no previous prototype for function %0">,
InGroup>, DefaultIgnore;
def note_declaration_not_a_prototype : Note<
- "this declaration is not a prototype; add 'void' to make it a prototype for
a zero-parameter function">;
+ "this declaration is not a prototype; add %select{'void'|parameter
declarations}0 "
+ "to make it %select{a prototype for a zero-parameter function|one}0">;
def warn_strict_prototypes : Warning<
"this %select{function declaration is not|block declaration is not|"
"old-style function definition is not preceded by}0 a prototype">,
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=363748&r1=363747&r2=363748&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Jun 18 15:52:39 2019
@@ -12777,8 +12777,9 @@ void Sema::ActOnFinishInlineFunctionDef(
Consumer.HandleInlineFunctionDefinition(D);
}
-static bool ShouldWarnAboutMissingPrototype(const FunctionDecl *FD,
- const FunctionDecl*& PossibleZeroParamPrototype) {
+static bool
+ShouldWarnAboutMissingPrototype(const FunctionDecl *FD,
+const FunctionDecl *&PossiblePrototype) {
// Don't warn about invalid declarations.
if (FD->isInvalidDecl())
return false;
@@ -12815,7 +12816,6 @@ static bool ShouldWarnAboutMissingProtot
if (FD->isDeleted())
return false;
- bool MissingPrototype = true;
for (const FunctionDecl *Prev = FD->getPreviousDecl();
Prev; Prev = Prev->getPreviousDecl()) {
// Ignore any declarations that occur in function or method
@@ -12823,13 +12823,11 @@ static bool ShouldWarnAboutMissingProtot
if (Prev->getLexicalDeclContext()->isFunctionOrMethod())
continue;
-MissingPrototype = !Prev->getType()->isFunctionProtoType();
-if (FD->getNumParams() == 0)
- PossibleZeroParamPrototype = Prev;
-break;
+PossiblePrototype = Prev;
+return Prev->getType()->isFunctionNoProtoType();
}
- return MissingPrototype;
+ return true;
}
void
@@ -13349,21 +13347,22 @@ Decl *Sema::ActOnFinishFunctionBody(Decl
// prototype declaration. This warning is issued even if the
// definition itself provides a prototype. The aim is to detect
// global functions that fail to be declared in header files.
-const FunctionDecl *PossibleZeroParamPrototype = nullptr;
-if (ShouldWarnAboutMissingPrototype(FD, PossibleZeroParamPrototype)) {
+const FunctionDecl *PossiblePrototype = nullptr;
+if (ShouldWarnAboutMissingPrototype(FD, PossiblePrototype)) {
Diag(FD->getLocation(), diag::warn_missing_prototype) << FD;
- if (PossibleZeroParamPrototype) {
+ if (PossiblePrototype) {
// We found a declaration that is not a prototype,
// but that could be a zero-parameter prototype
-if (TypeSourceInfo *TI =
-PossibleZeroParamPrototype->getTypeSourceInfo()) {
+if (TypeSourceInfo *TI = PossiblePrototype->getTypeSourceInfo()) {
TypeLoc TL = TI->getTypeLoc();
if (FunctionNoProtoTypeLoc FTL = TL.getAs())
-Diag(PossibleZeroParamPrototype->getLocation(),
+Diag(PossiblePrototype->getLocation(),
diag::note_declaration_not_a_prototype)
-<< PossibleZeroParamPrototype
-<< FixItHint::CreateInsertion(FTL.getRParenLoc(), "void");
+<< (FD->getNumParams() != 0)
+<< (FD->getNumParams() == 0
+? FixItHint::CreateInsertion(FTL.getRParenLoc(),
"void")
+: FixItHint{});
}
}
Modified: cfe/trunk/test/Sem
r363749 - Suggestions to fix -Wmissing-{prototypes, variable-declarations}
Author: aaronpuchert
Date: Tue Jun 18 15:57:08 2019
New Revision: 363749
URL: http://llvm.org/viewvc/llvm-project?rev=363749&view=rev
Log:
Suggestions to fix -Wmissing-{prototypes,variable-declarations}
Summary:
I've found that most often the proper way to fix this warning is to add
`static`, because if the code otherwise compiles and links, the function
or variable is apparently not needed outside of the TU.
We can't provide a fix-it hint for variable declarations, because
multiple VarDecls can share the same type, and if we put static in front
of that, we affect all declared variables, some of which might have
previous declarations.
We also provide no fix-it hint for the rare case of an `extern` function
definition, because that would require removing `extern` and I have no
idea how to get the source location of the storage class specifier from
a FunctionDecl. I believe this information is only available earlier in
the AST construction from DeclSpec::getStorageClassSpecLoc(), but we
don't have that here.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D59402
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/test/Sema/warn-missing-prototypes.c
cfe/trunk/test/Sema/warn-missing-variable-declarations.c
cfe/trunk/test/SemaCXX/warn-missing-prototypes.cpp
cfe/trunk/test/SemaCXX/warn-missing-variable-declarations.cpp
cfe/trunk/test/SemaOpenCL/warn-missing-prototypes.cl
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=363749&r1=363748&r2=363749&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Jun 18 15:57:08
2019
@@ -4681,6 +4681,9 @@ def warn_strict_prototypes : Warning<
def warn_missing_variable_declarations : Warning<
"no previous extern declaration for non-static variable %0">,
InGroup>, DefaultIgnore;
+def note_static_for_internal_linkage : Note<
+ "declare 'static' if the %select{variable|function}0 is not intended to be "
+ "used outside of this translation unit">;
def err_static_data_member_reinitialization :
Error<"static data member %0 already has an initializer">;
def err_redefinition : Error<"redefinition of %0">;
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=363749&r1=363748&r2=363749&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Jun 18 15:57:08 2019
@@ -11899,8 +11899,11 @@ void Sema::CheckCompleteVariableDeclarat
while (prev && prev->isThisDeclarationADefinition())
prev = prev->getPreviousDecl();
-if (!prev)
+if (!prev) {
Diag(var->getLocation(), diag::warn_missing_variable_declarations) <<
var;
+ Diag(var->getTypeSpecStartLoc(), diag::note_static_for_internal_linkage)
+ << /* variable */ 0;
+}
}
// Cache the result of checking for constant initialization.
@@ -13364,6 +13367,13 @@ Decl *Sema::ActOnFinishFunctionBody(Decl
? FixItHint::CreateInsertion(FTL.getRParenLoc(),
"void")
: FixItHint{});
}
+ } else {
+Diag(FD->getTypeSpecStartLoc(), diag::note_static_for_internal_linkage)
+<< /* function */ 1
+<< (FD->getStorageClass() == SC_None
+? FixItHint::CreateInsertion(FD->getTypeSpecStartLoc(),
+ "static ")
+: FixItHint{});
}
// GNU warning -Wstrict-prototypes
Modified: cfe/trunk/test/Sema/warn-missing-prototypes.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-missing-prototypes.c?rev=363749&r1=363748&r2=363749&view=diff
==
--- cfe/trunk/test/Sema/warn-missing-prototypes.c (original)
+++ cfe/trunk/test/Sema/warn-missing-prototypes.c Tue Jun 18 15:57:08 2019
@@ -9,11 +9,17 @@ int f(int x) { return x; } // expected-w
static int g(int x) { return x; }
int h(int x) { return x; } // expected-warning{{no previous prototype for
function 'h'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be
used outside of this translation unit}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:1-[[@LINE-2]]:1}:"static "
static int g2();
int g2(int x) { return x; }
+extern int g3(int x) { return x; } // expected-warning{{no previous prototype
for function 'g3'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be
used outside of this translation unit}}
+// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:{{.*}}-[[@LINE-
r363754 - Fix tests after r363749
Author: aaronpuchert
Date: Tue Jun 18 16:40:17 2019
New Revision: 363754
URL: http://llvm.org/viewvc/llvm-project?rev=363754&view=rev
Log:
Fix tests after r363749
We changed -Wmissing-prototypes there, which was used in these tests via
-Weverything.
Modified:
cfe/trunk/test/Preprocessor/Weverything_pragma.c
cfe/trunk/test/Preprocessor/pragma_diagnostic.c
cfe/trunk/test/Preprocessor/pushable-diagnostics.c
cfe/trunk/test/SemaCXX/warn-everthing.cpp
Modified: cfe/trunk/test/Preprocessor/Weverything_pragma.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/Weverything_pragma.c?rev=363754&r1=363753&r2=363754&view=diff
==
--- cfe/trunk/test/Preprocessor/Weverything_pragma.c (original)
+++ cfe/trunk/test/Preprocessor/Weverything_pragma.c Tue Jun 18 16:40:17 2019
@@ -7,6 +7,7 @@
#define UNUSED_MACRO1 1 // expected-warning{{macro is not used}}
void foo() // expected-warning {{no previous prototype for function}}
+// expected-note@-1{{declare 'static' if the function is not intended to be
used outside of this translation unit}}
{
// A diagnostic without DefaultIgnore, and not part of a group.
(void) L'ab'; // expected-warning {{extraneous characters in character
constant ignored}}
Modified: cfe/trunk/test/Preprocessor/pragma_diagnostic.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/pragma_diagnostic.c?rev=363754&r1=363753&r2=363754&view=diff
==
--- cfe/trunk/test/Preprocessor/pragma_diagnostic.c (original)
+++ cfe/trunk/test/Preprocessor/pragma_diagnostic.c Tue Jun 18 16:40:17 2019
@@ -39,12 +39,15 @@ void ppo(){} // First test that we do no
#pragma clang diagnostic warning "-Weverything"
void ppp(){} // expected-warning {{no previous prototype for function 'ppp'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be
used outside of this translation unit}}
#pragma clang diagnostic ignored "-Weverything" // Reset it.
void ppq(){}
#pragma clang diagnostic error "-Weverything" // Now set to error
void ppr(){} // expected-error {{no previous prototype for function 'ppr'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be
used outside of this translation unit}}
#pragma clang diagnostic warning "-Weverything" // This should not be effective
void pps(){} // expected-error {{no previous prototype for function 'pps'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be
used outside of this translation unit}}
Modified: cfe/trunk/test/Preprocessor/pushable-diagnostics.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/pushable-diagnostics.c?rev=363754&r1=363753&r2=363754&view=diff
==
--- cfe/trunk/test/Preprocessor/pushable-diagnostics.c (original)
+++ cfe/trunk/test/Preprocessor/pushable-diagnostics.c Tue Jun 18 16:40:17 2019
@@ -23,17 +23,21 @@ void ppo0(){} // first verify that we do
#pragma clang diagnostic warning "-Weverything"
void ppr1(){} // expected-warning {{no previous prototype for function 'ppr1'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be
used outside of this translation unit}}
#pragma clang diagnostic push // push again
#pragma clang diagnostic ignored "-Weverything" // Set to ignore in this
level.
void pps2(){}
#pragma clang diagnostic warning "-Weverything" // Set to warning in this
level.
void ppt2(){} // expected-warning {{no previous prototype for function 'ppt2'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be
used outside of this translation unit}}
#pragma clang diagnostic error "-Weverything" // Set to error in this level.
void ppt3(){} // expected-error {{no previous prototype for function 'ppt3'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be
used outside of this translation unit}}
#pragma clang diagnostic pop // pop should go back to warning level
void pps1(){} // expected-warning {{no previous prototype for function 'pps1'}}
+// expected-note@-1{{declare 'static' if the function is not intended to be
used outside of this translation unit}}
#pragma clang diagnostic pop // Another pop should disble it again
Modified: cfe/trunk/test/SemaCXX/warn-everthing.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-everthing.cpp?rev=363754&r1=363753&r2=363754&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-everthing.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-everthing.cpp Tue Jun 18 16:40:17 2019
@@ -9,5 +9,6 @@ public:
};
void testPR12271() { // expected-warning {{no previous prototype for function
'testPR12271'}}
+// expected-note@-1{{declare 'static' if the function
[clang-tools-extra] r363760 - Fix more tests after r363749
Author: aaronpuchert
Date: Tue Jun 18 18:54:05 2019
New Revision: 363760
URL: http://llvm.org/viewvc/llvm-project?rev=363760&view=rev
Log:
Fix more tests after r363749
Apparently -Wmissing-prototypes is used for quite a few integration
tests.
Modified:
clang-tools-extra/trunk/test/clang-tidy/export-diagnostics.cpp
Modified: clang-tools-extra/trunk/test/clang-tidy/export-diagnostics.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/export-diagnostics.cpp?rev=363760&r1=363759&r2=363760&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/export-diagnostics.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/export-diagnostics.cpp Tue Jun 18
18:54:05 2019
@@ -8,6 +8,8 @@ X(f)
// CHECK-MESSAGES: -input.cpp:2:1: warning: no previous prototype for function
'ff' [clang-diagnostic-missing-prototypes]
// CHECK-MESSAGES: -input.cpp:1:19: note: expanded from macro 'X'
// CHECK-MESSAGES: {{^}}note: expanded from here{{$}}
+// CHECK-MESSAGES: -input.cpp:2:1: note: declare 'static' if the function is
not intended to be used outside of this translation unit
+// CHECK-MESSAGES: -input.cpp:1:14: note: expanded from macro 'X'
// CHECK-YAML: ---
// CHECK-YAML-NEXT: MainSourceFile: '{{.*}}-input.cpp'
@@ -28,4 +30,16 @@ X(f)
// CHECK-YAML-NEXT: FilePath:''
// CHECK-YAML-NEXT: FileOffset: 0
// CHECK-YAML-NEXT: Replacements:[]
+// CHECK-YAML-NEXT: - Message: 'declare ''static'' if the
function is not intended to be used outside of this translation unit'
+// CHECK-YAML-NEXT: FilePath:'{{.*}}-input.cpp'
+// CHECK-YAML-NEXT: FileOffset: 30
+// CHECK-YAML-NEXT: Replacements:
+// CHECK-YAML-NEXT: - FilePath:'{{.*}}-input.cpp'
+// CHECK-YAML-NEXT: Offset: 30
+// CHECK-YAML-NEXT: Length: 0
+// CHECK-YAML-NEXT: ReplacementText: 'static '
+// CHECK-YAML-NEXT: - Message: 'expanded from macro ''X'''
+// CHECK-YAML-NEXT: FilePath:'{{.*}}-input.cpp'
+// CHECK-YAML-NEXT: FileOffset: 13
+// CHECK-YAML-NEXT: Replacements:[]
// CHECK-YAML-NEXT: ...
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r364479 - [Clang] Remove unused -split-dwarf and obsolete -enable-split-dwarf
Author: aaronpuchert
Date: Wed Jun 26 14:36:35 2019
New Revision: 364479
URL: http://llvm.org/viewvc/llvm-project?rev=364479&view=rev
Log:
[Clang] Remove unused -split-dwarf and obsolete -enable-split-dwarf
Summary:
The changes in D59673 made the choice redundant, since we can achieve
single-file split DWARF just by not setting an output file name.
Like llc we can also derive whether to enable Split DWARF from whether
-split-dwarf-file is set, so we don't need the flag at all anymore.
The test CodeGen/split-debug-filename.c distinguished between having set
or not set -enable-split-dwarf with -split-dwarf-file, but we can
probably just always emit the metadata into the IR.
The flag -split-dwarf wasn't used at all anymore.
Reviewers: dblaikie, echristo
Reviewed By: dblaikie
Differential Revision: https://reviews.llvm.org/D63167
Modified:
cfe/trunk/include/clang/Basic/CodeGenOptions.def
cfe/trunk/include/clang/Basic/CodeGenOptions.h
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/CodeGen/split-debug-filename.c
cfe/trunk/test/CodeGen/split-debug-output.c
cfe/trunk/test/CodeGen/split-debug-single-file.c
cfe/trunk/test/Driver/split-debug.c
Modified: cfe/trunk/include/clang/Basic/CodeGenOptions.def
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/CodeGenOptions.def?rev=364479&r1=364478&r2=364479&view=diff
==
--- cfe/trunk/include/clang/Basic/CodeGenOptions.def (original)
+++ cfe/trunk/include/clang/Basic/CodeGenOptions.def Wed Jun 26 14:36:35 2019
@@ -261,8 +261,6 @@ CODEGENOPT(DebugExplicitImport, 1, 0) /
///< contain explicit imports for
///< anonymous namespaces
-ENUM_CODEGENOPT(SplitDwarfMode, DwarfFissionKind, 2, NoFission) ///< DWARF
fission mode to use.
-
CODEGENOPT(SplitDwarfInlining, 1, 1) ///< Whether to include inlining info in
the
///< skeleton CU to allow for
symbolication
///< of inline stack frames without .dwo
files.
Modified: cfe/trunk/include/clang/Basic/CodeGenOptions.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/CodeGenOptions.h?rev=364479&r1=364478&r2=364479&view=diff
==
--- cfe/trunk/include/clang/Basic/CodeGenOptions.h (original)
+++ cfe/trunk/include/clang/Basic/CodeGenOptions.h Wed Jun 26 14:36:35 2019
@@ -71,8 +71,6 @@ public:
LocalExecTLSModel
};
- enum DwarfFissionKind { NoFission, SplitFileFission, SingleFileFission };
-
/// Clang versions with different platform ABI conformance.
enum class ClangABI {
/// Attempt to be ABI-compatible with code generated by Clang 3.8.x
Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=364479&r1=364478&r2=364479&view=diff
==
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)
+++ cfe/trunk/include/clang/Driver/CC1Options.td Wed Jun 26 14:36:35 2019
@@ -230,8 +230,6 @@ def disable_red_zone : Flag<["-"], "disa
HelpText<"Do not emit code that uses the red zone.">;
def dwarf_column_info : Flag<["-"], "dwarf-column-info">,
HelpText<"Turn on column location information.">;
-def split_dwarf : Flag<["-"], "split-dwarf">,
- HelpText<"Split out the dwarf .dwo sections">;
def dwarf_ext_refs : Flag<["-"], "dwarf-ext-refs">,
HelpText<"Generate debug info with external references to clang modules"
" or precompiled headers">;
@@ -694,10 +692,6 @@ def fblocks_runtime_optional : Flag<["-"
HelpText<"Weakly link in the blocks runtime">;
def fexternc_nounwind : Flag<["-"], "fexternc-nounwind">,
HelpText<"Assume all functions with C linkage do not unwind">;
-def enable_split_dwarf : Flag<["-"], "enable-split-dwarf">,
- HelpText<"Use DWARF fission in 'split' mode">;
-def enable_split_dwarf_EQ : Joined<["-"], "enable-split-dwarf=">,
- HelpText<"Set DWARF fission mode to either 'split' or 'single'">,
Values<"split,single">;
def split_dwarf_file : Separate<["-"], "split-dwarf-file">,
HelpText<"Name of the split dwarf debug info file to encode in the object
file">;
def fno_wchar : Flag<["-"], "fno-wchar">,
Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=364479&r1=364478&r2=364479&view=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Wed Jun 26
r364480 - Fix formatting after r364479
Author: aaronpuchert
Date: Wed Jun 26 14:39:19 2019
New Revision: 364480
URL: http://llvm.org/viewvc/llvm-project?rev=364480&view=rev
Log:
Fix formatting after r364479
The reflowing obscurs the functional changes, so here is a separate
commit.
Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=364480&r1=364479&r2=364480&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Wed Jun 26 14:39:19 2019
@@ -614,10 +614,8 @@ void CGDebugInfo::CreateCompileUnit() {
TheCU = DBuilder.createCompileUnit(
LangTag, CUFile, CGOpts.EmitVersionIdentMetadata ? Producer : "",
LO.Optimize || CGOpts.PrepareForLTO || CGOpts.PrepareForThinLTO,
- CGOpts.DwarfDebugFlags, RuntimeVers,
- CGOpts.SplitDwarfFile,
- EmissionKind, DwoId, CGOpts.SplitDwarfInlining,
- CGOpts.DebugInfoForProfiling,
+ CGOpts.DwarfDebugFlags, RuntimeVers, CGOpts.SplitDwarfFile, EmissionKind,
+ DwoId, CGOpts.SplitDwarfInlining, CGOpts.DebugInfoForProfiling,
CGM.getTarget().getTriple().isNVPTX()
? llvm::DICompileUnit::DebugNameTableKind::None
: static_cast(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r349300 - Thread safety analysis: Allow scoped releasing of capabilities
Author: aaronpuchert
Date: Sun Dec 16 06:15:30 2018
New Revision: 349300
URL: http://llvm.org/viewvc/llvm-project?rev=349300&view=rev
Log:
Thread safety analysis: Allow scoped releasing of capabilities
Summary:
The pattern is problematic with C++ exceptions, and not as widespread as
scoped locks, but it's still used by some, for example Chromium.
We are a bit stricter here at join points, patterns that are allowed for
scoped locks aren't allowed here. That could still be changed in the
future, but I'd argue we should only relax this if people ask for it.
Fixes PR36162.
Reviewers: aaron.ballman, delesley, pwnall
Reviewed By: delesley, pwnall
Subscribers: pwnall, cfe-commits
Differential Revision: https://reviews.llvm.org/D52578
Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=349300&r1=349299&r2=349300&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Sun Dec 16 06:15:30 2018
@@ -42,6 +42,7 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/ImmutableMap.h"
#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/PointerIntPair.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
@@ -890,28 +891,46 @@ public:
class ScopedLockableFactEntry : public FactEntry {
private:
- SmallVector UnderlyingMutexes;
+ enum UnderlyingCapabilityKind {
+UCK_Acquired, ///< Any kind of acquired capability.
+UCK_ReleasedShared,///< Shared capability that was released.
+UCK_ReleasedExclusive, ///< Exclusive capability that was released.
+ };
+
+ using UnderlyingCapability =
+ llvm::PointerIntPair;
+
+ SmallVector UnderlyingMutexes;
public:
ScopedLockableFactEntry(const CapabilityExpr &CE, SourceLocation Loc,
- const CapExprSet &Excl, const CapExprSet &Shrd)
+ const CapExprSet &Excl, const CapExprSet &Shrd,
+ const CapExprSet &ExclRel, const CapExprSet &ShrdRel)
: FactEntry(CE, LK_Exclusive, Loc, false) {
for (const auto &M : Excl)
- UnderlyingMutexes.push_back(M.sexpr());
+ UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
for (const auto &M : Shrd)
- UnderlyingMutexes.push_back(M.sexpr());
+ UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
+for (const auto &M : ExclRel)
+ UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedExclusive);
+for (const auto &M : ShrdRel)
+ UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedShared);
}
void
handleRemovalFromIntersection(const FactSet &FSet, FactManager &FactMan,
SourceLocation JoinLoc, LockErrorKind LEK,
ThreadSafetyHandler &Handler) const override {
-for (const auto *UnderlyingMutex : UnderlyingMutexes) {
- if (FSet.findLock(FactMan, CapabilityExpr(UnderlyingMutex, false))) {
+for (const auto &UnderlyingMutex : UnderlyingMutexes) {
+ const auto *Entry = FSet.findLock(
+ FactMan, CapabilityExpr(UnderlyingMutex.getPointer(), false));
+ if ((UnderlyingMutex.getInt() == UCK_Acquired && Entry) ||
+ (UnderlyingMutex.getInt() != UCK_Acquired && !Entry)) {
// If this scoped lock manages another mutex, and if the underlying
-// mutex is still held, then warn about the underlying mutex.
+// mutex is still/not held, then warn about the underlying mutex.
Handler.handleMutexHeldEndOfScope(
-"mutex", sx::toString(UnderlyingMutex), loc(), JoinLoc, LEK);
+"mutex", sx::toString(UnderlyingMutex.getPointer()), loc(),
JoinLoc,
+LEK);
}
}
}
@@ -919,17 +938,14 @@ public:
void handleLock(FactSet &FSet, FactManager &FactMan, const FactEntry &entry,
ThreadSafetyHandler &Handler,
StringRef DiagKind) const override {
-for (const auto *UnderlyingMutex : UnderlyingMutexes) {
- CapabilityExpr UnderCp(UnderlyingMutex, false);
+for (const auto &UnderlyingMutex : UnderlyingMutexes) {
+ CapabilityExpr UnderCp(UnderlyingMutex.getPointer(), false);
- // We're relocking the underlying mutexes. Warn on double locking.
- if (FSet.findLock(FactMan, UnderCp)) {
-Handler.handleDoubleLock(DiagKind, UnderCp.toString(), entry.loc());
- } else {
-FSet.removeLock(FactMan, !UnderCp);
-FSet.addLock(FactMan, llvm::make_unique(
- UnderCp, entry.kind(), entry.loc()));
- }
+ if (UnderlyingMutex.getInt() == UCK_Acquired)
+lock(FSet, FactMan, UnderCp, entry.kind(), entry.loc(), &Handler,
+ DiagKind)
r349308 - Thread safety analysis: Avoid intermediate copies [NFC]
Author: aaronpuchert
Date: Sun Dec 16 08:19:11 2018
New Revision: 349308
URL: http://llvm.org/viewvc/llvm-project?rev=349308&view=rev
Log:
Thread safety analysis: Avoid intermediate copies [NFC]
The main reason is to reduce the number of constructor arguments though,
especially since many of them had the same type.
Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=349308&r1=349307&r2=349308&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Sun Dec 16 08:19:11 2018
@@ -903,18 +903,23 @@ private:
SmallVector UnderlyingMutexes;
public:
- ScopedLockableFactEntry(const CapabilityExpr &CE, SourceLocation Loc,
- const CapExprSet &Excl, const CapExprSet &Shrd,
- const CapExprSet &ExclRel, const CapExprSet &ShrdRel)
- : FactEntry(CE, LK_Exclusive, Loc, false) {
-for (const auto &M : Excl)
- UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
-for (const auto &M : Shrd)
- UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
-for (const auto &M : ExclRel)
- UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedExclusive);
-for (const auto &M : ShrdRel)
- UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedShared);
+ ScopedLockableFactEntry(const CapabilityExpr &CE, SourceLocation Loc)
+ : FactEntry(CE, LK_Exclusive, Loc, false) {}
+
+ void addExclusiveLock(const CapabilityExpr &M) {
+UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
+ }
+
+ void addSharedLock(const CapabilityExpr &M) {
+UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
+ }
+
+ void addExclusiveUnlock(const CapabilityExpr &M) {
+UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedExclusive);
+ }
+
+ void addSharedUnlock(const CapabilityExpr &M) {
+UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedShared);
}
void
@@ -1938,15 +1943,20 @@ void BuildLockset::handleCall(const Expr
// FIXME: does this store a pointer to DRE?
CapabilityExpr Scp = Analyzer->SxBuilder.translateAttrExpr(&DRE, nullptr);
-std::copy(ScopedExclusiveReqs.begin(), ScopedExclusiveReqs.end(),
- std::back_inserter(ExclusiveLocksToAdd));
-std::copy(ScopedSharedReqs.begin(), ScopedSharedReqs.end(),
- std::back_inserter(SharedLocksToAdd));
-Analyzer->addLock(FSet,
- llvm::make_unique(
- Scp, MLoc, ExclusiveLocksToAdd, SharedLocksToAdd,
- ExclusiveLocksToRemove, SharedLocksToRemove),
- CapDiagKind);
+auto ScopedEntry = llvm::make_unique(Scp, MLoc);
+for (const auto &M : ExclusiveLocksToAdd)
+ ScopedEntry->addExclusiveLock(M);
+for (const auto &M : ScopedExclusiveReqs)
+ ScopedEntry->addExclusiveLock(M);
+for (const auto &M : SharedLocksToAdd)
+ ScopedEntry->addSharedLock(M);
+for (const auto &M : ScopedSharedReqs)
+ ScopedEntry->addSharedLock(M);
+for (const auto &M : ExclusiveLocksToRemove)
+ ScopedEntry->addExclusiveUnlock(M);
+for (const auto &M : SharedLocksToRemove)
+ ScopedEntry->addSharedUnlock(M);
+Analyzer->addLock(FSet, std::move(ScopedEntry), CapDiagKind);
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r373148 - Don't install example analyzer plugins
Author: aaronpuchert
Date: Sat Sep 28 06:28:50 2019
New Revision: 373148
URL: http://llvm.org/viewvc/llvm-project?rev=373148&view=rev
Log:
Don't install example analyzer plugins
Summary: Fixes PR43430.
Reviewers: hintonda, NoQ, Szelethus, lebedev.ri
Reviewed By: lebedev.ri
Differential Revision: https://reviews.llvm.org/D68172
Modified:
cfe/trunk/lib/Analysis/plugins/CheckerDependencyHandling/CMakeLists.txt
cfe/trunk/lib/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
cfe/trunk/lib/Analysis/plugins/SampleAnalyzer/CMakeLists.txt
Modified:
cfe/trunk/lib/Analysis/plugins/CheckerDependencyHandling/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/plugins/CheckerDependencyHandling/CMakeLists.txt?rev=373148&r1=373147&r2=373148&view=diff
==
--- cfe/trunk/lib/Analysis/plugins/CheckerDependencyHandling/CMakeLists.txt
(original)
+++ cfe/trunk/lib/Analysis/plugins/CheckerDependencyHandling/CMakeLists.txt Sat
Sep 28 06:28:50 2019
@@ -3,7 +3,7 @@ set(LLVM_LINK_COMPONENTS
)
set(LLVM_EXPORTED_SYMBOL_FILE
${CMAKE_CURRENT_SOURCE_DIR}/CheckerDependencyHandlingAnalyzerPlugin.exports)
-add_llvm_library(CheckerDependencyHandlingAnalyzerPlugin MODULE
CheckerDependencyHandling.cpp PLUGIN_TOOL clang)
+add_llvm_library(CheckerDependencyHandlingAnalyzerPlugin MODULE BUILDTREE_ONLY
CheckerDependencyHandling.cpp PLUGIN_TOOL clang)
clang_target_link_libraries(CheckerDependencyHandlingAnalyzerPlugin PRIVATE
clangAnalysis
Modified: cfe/trunk/lib/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt?rev=373148&r1=373147&r2=373148&view=diff
==
--- cfe/trunk/lib/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
(original)
+++ cfe/trunk/lib/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt Sat Sep
28 06:28:50 2019
@@ -3,7 +3,7 @@ set(LLVM_LINK_COMPONENTS
)
set(LLVM_EXPORTED_SYMBOL_FILE
${CMAKE_CURRENT_SOURCE_DIR}/CheckerOptionHandlingAnalyzerPlugin.exports)
-add_llvm_library(CheckerOptionHandlingAnalyzerPlugin MODULE
CheckerOptionHandling.cpp PLUGIN_TOOL clang)
+add_llvm_library(CheckerOptionHandlingAnalyzerPlugin MODULE BUILDTREE_ONLY
CheckerOptionHandling.cpp PLUGIN_TOOL clang)
clang_target_link_libraries(CheckerOptionHandlingAnalyzerPlugin PRIVATE
clangAnalysis
Modified: cfe/trunk/lib/Analysis/plugins/SampleAnalyzer/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/plugins/SampleAnalyzer/CMakeLists.txt?rev=373148&r1=373147&r2=373148&view=diff
==
--- cfe/trunk/lib/Analysis/plugins/SampleAnalyzer/CMakeLists.txt (original)
+++ cfe/trunk/lib/Analysis/plugins/SampleAnalyzer/CMakeLists.txt Sat Sep 28
06:28:50 2019
@@ -3,7 +3,7 @@ set(LLVM_LINK_COMPONENTS
)
set(LLVM_EXPORTED_SYMBOL_FILE
${CMAKE_CURRENT_SOURCE_DIR}/SampleAnalyzerPlugin.exports)
-add_llvm_library(SampleAnalyzerPlugin MODULE MainCallChecker.cpp PLUGIN_TOOL
clang)
+add_llvm_library(SampleAnalyzerPlugin MODULE BUILDTREE_ONLY
MainCallChecker.cpp PLUGIN_TOOL clang)
clang_target_link_libraries(SampleAnalyzerPlugin PRIVATE
clangAnalysis
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 1850f56 - Thread safety analysis: Support deferring locks
Author: Aaron Puchert
Date: 2020-06-08T17:00:29+02:00
New Revision: 1850f56c8abae637c2cc1b8d27b8577c5700101a
URL:
https://github.com/llvm/llvm-project/commit/1850f56c8abae637c2cc1b8d27b8577c5700101a
DIFF:
https://github.com/llvm/llvm-project/commit/1850f56c8abae637c2cc1b8d27b8577c5700101a.diff
LOG: Thread safety analysis: Support deferring locks
Summary:
The standard std::unique_lock can be constructed to manage a lock without
initially acquiring it by passing std::defer_lock as second parameter.
It can be acquired later by calling lock().
To support this, we use the locks_excluded attribute. This might seem
like an odd choice at first, but its consistent with the other
annotations we support on scoped capability constructors. By excluding
the lock we state that it is currently not in use and the function
doesn't change that, which is exactly what the constructor does.
Along the way we slightly simplify handling of scoped capabilities.
Reviewers: aaron.ballman, delesley
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D81332
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index e0ff23df5ab4..6e518e7d38ef 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -905,11 +905,7 @@ class ScopedLockableFactEntry : public FactEntry {
ScopedLockableFactEntry(const CapabilityExpr &CE, SourceLocation Loc)
: FactEntry(CE, LK_Exclusive, Loc, false) {}
- void addExclusiveLock(const CapabilityExpr &M) {
-UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
- }
-
- void addSharedLock(const CapabilityExpr &M) {
+ void addLock(const CapabilityExpr &M) {
UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
}
@@ -1801,7 +1797,7 @@ void BuildLockset::handleCall(const Expr *Exp, const
NamedDecl *D,
SourceLocation Loc = Exp->getExprLoc();
CapExprSet ExclusiveLocksToAdd, SharedLocksToAdd;
CapExprSet ExclusiveLocksToRemove, SharedLocksToRemove, GenericLocksToRemove;
- CapExprSet ScopedExclusiveReqs, ScopedSharedReqs;
+ CapExprSet ScopedReqsAndExcludes;
StringRef CapDiagKind = "mutex";
// Figure out if we're constructing an object of scoped lockable class
@@ -1892,19 +1888,20 @@ void BuildLockset::handleCall(const Expr *Exp, const
NamedDecl *D,
POK_FunctionCall, ClassifyDiagnostic(A),
Exp->getExprLoc());
// use for adopting a lock
- if (isScopedVar) {
-Analyzer->getMutexIDs(A->isShared() ? ScopedSharedReqs
-: ScopedExclusiveReqs,
- A, Exp, D, VD);
- }
+ if (isScopedVar)
+Analyzer->getMutexIDs(ScopedReqsAndExcludes, A, Exp, D, VD);
}
break;
}
case attr::LocksExcluded: {
const auto *A = cast(At);
-for (auto *Arg : A->args())
+for (auto *Arg : A->args()) {
warnIfMutexHeld(D, Exp, Arg, ClassifyDiagnostic(A));
+ // use for deferring a lock
+ if (isScopedVar)
+Analyzer->getMutexIDs(ScopedReqsAndExcludes, A, Exp, D, VD);
+}
break;
}
@@ -1944,13 +1941,11 @@ void BuildLockset::handleCall(const Expr *Exp, const
NamedDecl *D,
auto ScopedEntry = std::make_unique(Scp, MLoc);
for (const auto &M : ExclusiveLocksToAdd)
- ScopedEntry->addExclusiveLock(M);
-for (const auto &M : ScopedExclusiveReqs)
- ScopedEntry->addExclusiveLock(M);
+ ScopedEntry->addLock(M);
for (const auto &M : SharedLocksToAdd)
- ScopedEntry->addSharedLock(M);
-for (const auto &M : ScopedSharedReqs)
- ScopedEntry->addSharedLock(M);
+ ScopedEntry->addLock(M);
+for (const auto &M : ScopedReqsAndExcludes)
+ ScopedEntry->addLock(M);
for (const auto &M : ExclusiveLocksToRemove)
ScopedEntry->addExclusiveUnlock(M);
for (const auto &M : SharedLocksToRemove)
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index cdb22cd22a99..02700147a032 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -2625,9 +2625,12 @@ void Foo::test5() {
namespace RelockableScopedLock {
+class DeferTraits {};
+
class SCOPED_LOCKABLE RelockableExclusiveMutexLock {
public:
RelockableExclusiveMutexLock(Mutex *mu) EXCLUSIVE_LOCK_FUNCTION(mu);
+ RelockableExclusiveMutexLock(Mutex *mu, DeferTraits) LOCKS_EXCLUDED(mu);
~RelockableExclusiveMutexLock() EXCLUSIVE_UNLOCK_FUNCTION();
void Lock() EXCLUSIVE_LOCK_FUNCTION();
@@ -2639,6 +2642,7 @@ struct ExclusiveTraits {};
c
[clang] f70912f - Thread safety analysis: Add note for double unlock
Author: Aaron Puchert
Date: 2020-06-08T17:00:29+02:00
New Revision: f70912f885f991d5af11d8ecb10b703f3cbed982
URL:
https://github.com/llvm/llvm-project/commit/f70912f885f991d5af11d8ecb10b703f3cbed982
DIFF:
https://github.com/llvm/llvm-project/commit/f70912f885f991d5af11d8ecb10b703f3cbed982.diff
LOG: Thread safety analysis: Add note for double unlock
Summary:
When getting a warning that we release a capability that isn't held it's
sometimes not clear why. So just like we do for double locking, we add a
note on the previous release operation, which marks the point since when
the capability isn't held any longer.
We can find this previous release operation by looking up the
corresponding negative capability.
Reviewers: aaron.ballman, delesley
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D81352
Added:
Modified:
clang/include/clang/Analysis/Analyses/ThreadSafety.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Analysis/ThreadSafety.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/test/Sema/warn-thread-safety-analysis.c
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
Removed:
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafety.h
b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
index 18659aa4e5bb..0d3dda1256fb 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafety.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
@@ -108,8 +108,10 @@ class ThreadSafetyHandler {
/// \param LockName -- A StringRef name for the lock expression, to be
printed
/// in the error message.
/// \param Loc -- The SourceLocation of the Unlock
+ /// \param LocPreviousUnlock -- If valid, the location of a previous Unlock.
virtual void handleUnmatchedUnlock(StringRef Kind, Name LockName,
- SourceLocation Loc) {}
+ SourceLocation Loc,
+ SourceLocation LocPreviousUnlock) {}
/// Warn about an unlock function call that attempts to unlock a lock with
/// the incorrect lock kind. For instance, a shared lock being unlocked
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 45a7f1c700b4..cce6dcc54c2f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3401,6 +3401,7 @@ def warn_expecting_lock_held_on_loop : Warning<
"expecting %0 '%1' to be held at start of each loop">,
InGroup, DefaultIgnore;
def note_locked_here : Note<"%0 acquired here">;
+def note_unlocked_here : Note<"%0 released here">;
def warn_lock_exclusive_and_shared : Warning<
"%0 '%1' is acquired exclusively and shared in the same scope">,
InGroup, DefaultIgnore;
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 6e518e7d38ef..1208eaf93e25 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -995,7 +995,10 @@ class ScopedLockableFactEntry : public FactEntry {
FSet.addLock(FactMan, std::make_unique(
!Cp, LK_Exclusive, loc));
} else if (Handler) {
- Handler->handleUnmatchedUnlock(DiagKind, Cp.toString(), loc);
+ SourceLocation PrevLoc;
+ if (const FactEntry *Neg = FSet.findLock(FactMan, !Cp))
+PrevLoc = Neg->loc();
+ Handler->handleUnmatchedUnlock(DiagKind, Cp.toString(), loc, PrevLoc);
}
}
};
@@ -1322,7 +1325,10 @@ void ThreadSafetyAnalyzer::removeLock(FactSet &FSet,
const CapabilityExpr &Cp,
const FactEntry *LDat = FSet.findLock(FactMan, Cp);
if (!LDat) {
-Handler.handleUnmatchedUnlock(DiagKind, Cp.toString(), UnlockLoc);
+SourceLocation PrevLoc;
+if (const FactEntry *Neg = FSet.findLock(FactMan, !Cp))
+ PrevLoc = Neg->loc();
+Handler.handleUnmatchedUnlock(DiagKind, Cp.toString(), UnlockLoc, PrevLoc);
return;
}
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp
b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 995d776d6565..3b7356893833 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1700,6 +1700,14 @@ class ThreadSafetyReporter : public
clang::threadSafety::ThreadSafetyHandler {
: getNotes();
}
+ OptionalNotes makeUnlockedHereNote(SourceLocation LocUnlocked,
+ StringRef Kind) {
+return LocUnlocked.isValid()
+ ? getNotes(PartialDiagnosticAt(
+ LocUnlocked, S.PDiag(diag::note_unlocked_here) << Kind))
+ : getNotes();
+ }
+
public:
ThreadSafetyReporter(Sema &S, SourceLocation FL, SourceLocation FEL)
: S(S), FunLocation(FL), FunEndLocation(FEL),
@@ -1726,13 +1734,14 @@ class ThreadSafetyReporter : public
clang::thre
[clang] f43859a - PR45000: Let Sema::SubstParmVarDecl handle default args of lambdas in initializers
Author: Aaron Puchert
Date: 2020-04-22T22:37:21+02:00
New Revision: f43859a099fa3587123717be941fa63ba8d0d4f2
URL:
https://github.com/llvm/llvm-project/commit/f43859a099fa3587123717be941fa63ba8d0d4f2
DIFF:
https://github.com/llvm/llvm-project/commit/f43859a099fa3587123717be941fa63ba8d0d4f2.diff
LOG: PR45000: Let Sema::SubstParmVarDecl handle default args of lambdas in
initializers
Summary:
We extend the behavior for local functions and methods of local classes
to lambdas in variable initializers. The initializer is not a separate
scope, but we treat it as such.
We also remove the (faulty) instantiation of default arguments in
TreeTransform::TransformLambdaExpr, because it doesn't do proper
initialization, and if it did, we would do it twice (and thus also emit
eventual errors twice).
Reviewed By: rsmith
Differential Revision: https://reviews.llvm.org/D76038
Added:
Modified:
clang/include/clang/AST/DeclBase.h
clang/lib/AST/DeclBase.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/lib/Sema/TreeTransform.h
clang/test/SemaCXX/vartemplate-lambda.cpp
clang/test/SemaTemplate/instantiate-local-class.cpp
Removed:
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index b48f7e4f9308..91875377679d 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -869,14 +869,15 @@ class alignas(8) Decl {
return getParentFunctionOrMethod() == nullptr;
}
- /// Returns true if this declaration lexically is inside a function.
- /// It recognizes non-defining declarations as well as members of local
- /// classes:
+ /// Returns true if this declaration is lexically inside a function or inside
+ /// a variable initializer. It recognizes non-defining declarations as well
+ /// as members of local classes:
/// \code
/// void foo() { void bar(); }
/// void foo2() { class ABC { void bar(); }; }
+ /// inline int x = [](){ return 0; };
/// \endcode
- bool isLexicallyWithinFunctionOrMethod() const;
+ bool isInLocalScope() const;
/// If this decl is defined inside a function/method/block it returns
/// the corresponding DeclContext, otherwise it returns null.
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index 8a18de1e8248..2aab53f4fa90 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -332,13 +332,16 @@ void Decl::setDeclContextsImpl(DeclContext *SemaDC,
DeclContext *LexicalDC,
}
}
-bool Decl::isLexicallyWithinFunctionOrMethod() const {
+bool Decl::isInLocalScope() const {
const DeclContext *LDC = getLexicalDeclContext();
while (true) {
if (LDC->isFunctionOrMethod())
return true;
if (!isa(LDC))
return false;
+if (const auto *CRD = dyn_cast(LDC))
+ if (CRD->isLambda())
+return true;
LDC = LDC->getLexicalParent();
}
return false;
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp
b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index e2b3ebba6bbe..4b6b92ccab2c 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2380,7 +2380,7 @@ ParmVarDecl *Sema::SubstParmVarDecl(ParmVarDecl *OldParm,
UnparsedDefaultArgInstantiations[OldParm].push_back(NewParm);
} else if (Expr *Arg = OldParm->getDefaultArg()) {
FunctionDecl *OwningFunc = cast(OldParm->getDeclContext());
-if (OwningFunc->isLexicallyWithinFunctionOrMethod()) {
+if (OwningFunc->isInLocalScope()) {
// Instantiate default arguments for methods of local classes (DR1484)
// and non-defining declarations.
Sema::ContextRAII SavedContext(*this, OwningFunc);
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 551517581063..a6541dabe6b2 100755
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4363,7 +4363,7 @@
TemplateDeclInstantiator::InitFunctionInstantiation(FunctionDecl *New,
EPI.ExceptionSpec.Type != EST_None &&
EPI.ExceptionSpec.Type != EST_DynamicNone &&
EPI.ExceptionSpec.Type != EST_BasicNoexcept &&
-!Tmpl->isLexicallyWithinFunctionOrMethod()) {
+!Tmpl->isInLocalScope()) {
FunctionDecl *ExceptionSpecTemplate = Tmpl;
if (EPI.ExceptionSpec.Type == EST_Uninstantiated)
ExceptionSpecTemplate = EPI.ExceptionSpec.SourceTemplate;
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index abde968bed8c..da6ff8e5cea8 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12219,19 +12219,6 @@ TreeTransform::TransformLambdaExpr(LambdaExpr
*E) {
LSI->CallOperator = NewCallOperator;
- for (unsigned I = 0, NumParams = NewCallOperator->ge
[clang] 391c15f - [NFC] Correct typo in comment after D76038
Author: Aaron Puchert
Date: 2020-04-23T02:26:02+02:00
New Revision: 391c15fccdc6b0d33bb651a298c07216e532904e
URL:
https://github.com/llvm/llvm-project/commit/391c15fccdc6b0d33bb651a298c07216e532904e
DIFF:
https://github.com/llvm/llvm-project/commit/391c15fccdc6b0d33bb651a298c07216e532904e.diff
LOG: [NFC] Correct typo in comment after D76038
Added:
Modified:
clang/include/clang/AST/DeclBase.h
Removed:
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index 91875377679d..da8b25b497b0 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -871,11 +871,11 @@ class alignas(8) Decl {
/// Returns true if this declaration is lexically inside a function or inside
/// a variable initializer. It recognizes non-defining declarations as well
- /// as members of local classes:
+ /// as members of local classes and lambdas:
/// \code
/// void foo() { void bar(); }
/// void foo2() { class ABC { void bar(); }; }
- /// inline int x = [](){ return 0; };
+ /// inline int x = [](){ return 0; }();
/// \endcode
bool isInLocalScope() const;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] ce7eb72 - Thread safety analysis: Reword warning after D72635
Author: Aaron Puchert
Date: 2020-04-27T22:23:52+02:00
New Revision: ce7eb72a3c87a9d15ba4962fa7a23aad24f98156
URL:
https://github.com/llvm/llvm-project/commit/ce7eb72a3c87a9d15ba4962fa7a23aad24f98156
DIFF:
https://github.com/llvm/llvm-project/commit/ce7eb72a3c87a9d15ba4962fa7a23aad24f98156.diff
LOG: Thread safety analysis: Reword warning after D72635
We allow arbitrary names for capabilities now, and the name didn't play
a role for this anyway.
Added:
Modified:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/test/SemaCXX/warn-thread-safety-parsing.cpp
Removed:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c9aa49a31ed8..9c1b6c593857 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3336,7 +3336,7 @@ def warn_thread_attribute_argument_not_lockable : Warning<
InGroup, DefaultIgnore;
def warn_thread_attribute_decl_not_lockable : Warning<
"%0 attribute can only be applied in a context annotated "
- "with 'capability(\"mutex\")' attribute">,
+ "with 'capability' attribute">,
InGroup, DefaultIgnore;
def warn_thread_attribute_decl_not_pointer : Warning<
"%0 only applies to pointer types; type here is %1">,
diff --git a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
index 198d02e1f076..6ad0f877a11d 100644
--- a/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-parsing.cpp
@@ -496,7 +496,7 @@ Mutex aa_var_arg_bad_3 ACQUIRED_AFTER(muDoublePointer); // \
Mutex aa_var_arg_bad_4 ACQUIRED_AFTER(umu); // \
// expected-warning {{'acquired_after' attribute requires arguments whose
type is annotated with 'capability' attribute}}
UnlockableMu aa_var_arg_bad_5 ACQUIRED_AFTER(mu_aa); // \
- // expected-warning {{'acquired_after' attribute can only be applied in a
context annotated with 'capability("mutex")' attribute}}
+ // expected-warning {{'acquired_after' attribute can only be applied in a
context annotated with 'capability' attribute}}
//-//
// Acquired Before (ab)
@@ -559,7 +559,7 @@ Mutex ab_var_arg_bad_3 ACQUIRED_BEFORE(muDoublePointer); //
\
Mutex ab_var_arg_bad_4 ACQUIRED_BEFORE(umu); // \
// expected-warning {{'acquired_before' attribute requires arguments whose
type is annotated with 'capability' attribute}}
UnlockableMu ab_var_arg_bad_5 ACQUIRED_BEFORE(mu_ab); // \
- // expected-warning {{'acquired_before' attribute can only be applied in a
context annotated with 'capability("mutex")' attribute}}
+ // expected-warning {{'acquired_before' attribute can only be applied in a
context annotated with 'capability' attribute}}
//-//
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D78853: [Analysis] Fix null pointer dereference warnings [1/n]
Am 29.04.20 um 23:11 schrieb Mandeep Singh Grang: > My previous email details why we are doing > this:Â http://lists.llvm.org/pipermail/llvm-dev/2020-April/141167.html > Basically, we ran the PREfast static analysis tool on LLVM/Clang and > it reported a lot of warnings. I guess some of them are false > positives after all. Thanks for the link. There is nothing wrong with running additional tools on LLVM. > As David suggests that maybe I should validate these warnings by also > running the clang static analyzer. You could have a look at http://llvm.org/reports/scan-build/. It's not terribly up-to-date though, so you might also run it yourself. > There are a lot of other warnings reported by the tool. Here is the > full > list:Â > https://docs.google.com/spreadsheets/d/1h_3tHxsgBampxb7PXoB5lgwiBSpTty9RLe5maIQxnTk/edit?usp=sharing. In my opinion you're probably not losing a lot by filtering out the types of issues that the Clang static analyzer can find as well. So for example, ignore the null dereferences since Clang has essentially the same check. Of course it could be that the tool finds additional issues, I can't really say that. But you can see in the report that there quite a few issues of a similar kind. (This one isn't among them, however.) For analyzing the issues in the list, it would be good to note the git commit of your analysis run, otherwise it might be hard to follow the reports. > If the community is interested in getting those fixed I can upstream > patches. Improvements are always welcome, but unfortunately no static analysis tool only reports actual issues. Otherwise one could hope that the Clang findings would have been all fixed by now. So I think you need to carefully inspect the reports. It's not a bad idea to start with a random sample of the issues reported by every check. Avoid stylistic issues like shadowing: people have different opinions on that. Then try to find out if there is a real issue, or if you can think of a way to improve the code. (That's often subjective, of course. But if you can rewrite code a bit to make it more obvious that a certain bad thing can't happen, you'll find open ears.) The best thing is of course if you can use the report to construct failing test cases, but I wouldn't put the bar that high. Best regards, Aaron ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: Fix a bug in handling temporary constructors (PR #74020)
@@ -1530,7 +1532,6 @@ void ThreadSafetyAnalyzer::getEdgeLockset(FactSet& Result,
}
namespace {
-
aaronpuchert wrote:
Nitpick: can you undo the whitespace change?
https://github.com/llvm/llvm-project/pull/74020
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: Fix a bug in handling temporary constructors (PR #74020)
@@ -2487,15 +2486,15 @@ void
ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
// Clean up constructed object even if there are no attributes to
// keep the number of objects in limbo as small as possible.
- if (auto Object = LocksetBuilder.ConstructedObjects.find(
+ if (auto Object = LocksetBuilder.Analyzer->ConstructedObjects.find(
TD.getBindTemporaryExpr()->getSubExpr());
- Object != LocksetBuilder.ConstructedObjects.end()) {
+ Object != LocksetBuilder.Analyzer->ConstructedObjects.end()) {
const auto *DD = TD.getDestructorDecl(AC.getASTContext());
if (DD->hasAttrs())
// TODO: the location here isn't quite correct.
LocksetBuilder.handleCall(nullptr, DD, Object->second,
TD.getBindTemporaryExpr()->getEndLoc());
-LocksetBuilder.ConstructedObjects.erase(Object);
+LocksetBuilder.Analyzer->ConstructedObjects.erase(Object);
aaronpuchert wrote:
We're in `ThreadSafetyAnalyzer` here, so you should be able to drop
`LocksetBuilder.Analyzer->` and refer to the member directly.
https://github.com/llvm/llvm-project/pull/74020
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: Fix a bug in handling temporary constructors (PR #74020)
https://github.com/aaronpuchert approved this pull request. Thanks, looks good to me! https://github.com/llvm/llvm-project/pull/74020 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -0,0 +1,143 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-format-attribute %s
+
+#include
+#include
+#include
+#include
+
+__attribute__((__format__ (__scanf__, 1, 4)))
+void f1(char *out, const size_t len, const char *format, ... /* args */)
+{
+va_list args;
+vsnprintf(out, len, format, args); // expected-warning {{diagnostic
behavior may be improved by adding the 'printf' format attribute to the
declaration of 'f1'}}
+ // CHECK-FIXES:
__attribute__((format(printf, 1, 4)))
aaronpuchert wrote:
Shouldn't it be `__attribute__((format(printf, 3, 4)))`, since the format
string is the third argument? The first argument is just the target buffer.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -6849,6 +6849,73 @@ static void handleSwiftAsyncAttr(Sema &S, Decl *D, const
ParsedAttr &AL) {
checkSwiftAsyncErrorBlock(S, D, ErrorAttr, AsyncAttr);
}
+// Warn if parent function misses format attribute. Parent function misses
+// format attribute if there is an argument format string forwarded to calling
+// function with format attribute, parent function has a parameter which type
+// is either string or pointer to char and parent function format attribute
+// type does not match with calling function format attribute type.
+void Sema::DiagnoseMissingFormatAttributes(const FunctionDecl *FDecl,
+ ArrayRef Args,
+ SourceLocation Loc) {
+ assert(FDecl);
+
+ // Check if function has format attribute with forwarded format string.
+ IdentifierInfo *AttrType;
+ if (!llvm::any_of(
+ FDecl->specific_attrs(), [&](const FormatAttr *Attr) {
+if (!Args[Attr->getFirstArg()]->getReferencedDeclOfCallee())
+ return false;
+
+AttrType = Attr->getType();
+return true;
+ }))
+return;
+
+ const FunctionDecl *ParentFuncDecl = getCurFunctionDecl();
+ if (!ParentFuncDecl)
+return;
+
+ // Check if parent function has string or pointer to char parameter.
+ unsigned int StringIndex = 0;
+ if (!llvm::any_of(
+ ParentFuncDecl->parameters(), [&](const ParmVarDecl *Param) {
+StringIndex = Param->getFunctionScopeIndex() + 1;
+QualType Ty = Param->getType();
+if (isNSStringType(Ty, Context, true))
+ return true;
+if (isCFStringType(Ty, Context))
+ return true;
+if (Ty->isPointerType() &&
+Ty->castAs()
+->getPointeeType()
+->isSpecificBuiltinType(BuiltinType::Char_S))
+ return true;
+return false;
+ }))
+return;
+
+ // Check if there is parent function format attribute which type matches
+ // calling function format attribute type.
+ if (llvm::any_of(
+ ParentFuncDecl->specific_attrs(),
+ [&](const FormatAttr *Attr) { return Attr->getType() == AttrType; }))
+return;
aaronpuchert wrote:
The index arguments could still be off, should we warn about that?
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
https://github.com/aaronpuchert commented: Two additional checks that might be interesting: * Look at the `FormatIdx` argument. Is it a `DeclRefExpr` referring to a `ParmVarDecl`, perhaps with some conversions? (There is probably going to be an `LValueToRValue` conversion at least. There are special `Ignore*` functions on `Expr` to unwrap that.) If it is, we should propagate the attribute, if not then not. * For `FirstArg` it seems that GCC only warns when we call a `v*` function, or the called function has `FirstArg == 0`, probably because forwarding to a variadic function isn't really possible in C. It doesn't seem to check whether we're actually forwarding. What you've implemented here (deciding based on `ParentFuncDecl->isVariadic()`) seems like a reasonable heuristic. But it probably makes sense to additionally restrict based on the archetype or `FirstArg`. Or is that what you're doing with `Args[Attr->getFirstArg()]`? https://github.com/llvm/llvm-project/pull/70024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -6849,6 +6849,73 @@ static void handleSwiftAsyncAttr(Sema &S, Decl *D, const
ParsedAttr &AL) {
checkSwiftAsyncErrorBlock(S, D, ErrorAttr, AsyncAttr);
}
+// Warn if parent function misses format attribute. Parent function misses
+// format attribute if there is an argument format string forwarded to calling
+// function with format attribute, parent function has a parameter which type
+// is either string or pointer to char and parent function format attribute
+// type does not match with calling function format attribute type.
+void Sema::DiagnoseMissingFormatAttributes(const FunctionDecl *FDecl,
+ ArrayRef Args,
+ SourceLocation Loc) {
+ assert(FDecl);
+
+ // Check if function has format attribute with forwarded format string.
+ IdentifierInfo *AttrType;
+ if (!llvm::any_of(
+ FDecl->specific_attrs(), [&](const FormatAttr *Attr) {
+if (!Args[Attr->getFirstArg()]->getReferencedDeclOfCallee())
aaronpuchert wrote:
Do we need to do a bounds check before we access into the array or has this
already been checked?
Also, the attribute arguments start counting at 1, but array access starts at
0, so I would assume we're off by one here?
And shouldn't we look up `Attr->getFormatIdx()` instead? The "first argument"
can be 0, if the caller takes a `va_list` for example.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -0,0 +1,132 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-format-attribute %s
+
+#include
+#include
+
+void f1(const std::string &str, ... /* args */)
+{
+va_list args;
+vscanf(str.c_str(), args); // no warning
+vprintf(str.c_str(), args); // no warning
+}
+
+__attribute__((format(printf, 1, 2))) // expected-error: {{format argument not
a string type}}
+void f2(const std::string &str, ... /* args */);
+
+void f3(std::string_view str, ... /* args */)
+{
+va_list args;
+vscanf(std::string(str).c_str(), args); // no warning
+vprintf(std::string(str).c_str(), args); // no warning
+}
+
+__attribute__((format(printf, 1, 2))) // expected-error {{format argument not
a string type}}
+void f4(std::string_view str, ... /* args */);
+
+void f5(const std::wstring &str, ... /* args */)
+{
+va_list args;
+vscanf((const char *)str.c_str(), args); // no warning
+vprintf((const char *)str.c_str(), args); // no warning
+}
+
+__attribute__((format(printf, 1, 2))) // expected-error {{format argument not
a string type}}
+void f6(const std::wstring &str, ... /* args */);
+
+void f7(std::wstring_view str, ... /* args */)
+{
+va_list args;
+vscanf((const char *) std::wstring(str).c_str(), args); // no warning
+vprintf((const char *) std::wstring(str).c_str(), args); // no warning
+}
+
+__attribute__((format(printf, 1, 2))) // expected-error {{format argument not
a string type}}
+void f8(std::wstring_view str, ... /* args */);
+
+void f9(const wchar_t *out, ... /* args */)
+{
+va_list args;
+vprintf(out, args); // expected-error {{no matching function for call to
'vprintf'}}
+vscanf((const char *) out, args); // no warning
+vscanf((char *) out, args); // no warning
+}
+
+__attribute__((format(printf, 1, 2))) // expected-error {{format argument not
a string type}}
+void f10(const wchar_t *out, ... /* args */);
+
+void f11(const char16_t *out, ... /* args */)
+{
+va_list args;
+vscanf(out, args); // expected-error {{no matching function for call to
'vscanf'}}
+}
+
+__attribute__((format(printf, 1, 2))) // expected-error {{format argument not
a string type}}
+void f12(const char16_t *out, ... /* args */);
+
+void f13(const char32_t *out, ... /* args */)
+{
+va_list args;
+vscanf(out, args); // expected-error {{no matching function for call to
'vscanf'}}
+}
+
+__attribute__((format(scanf, 1, 2))) // expected-error {{format argument not a
string type}}
+void f14(const char32_t *out, ... /* args */);
+
+void f15(const char *out, ... /* args */)
+{
+va_list args;
+vscanf(out, args); // expected-warning {{diagnostic behavior may be
improved by adding the 'scanf' format attribute to the declaration of 'f15'}}
+}
+
+__attribute__((format(scanf, 1, 2)))
+void f16(const char *out, ... /* args */)
+{
+va_list args;
+vscanf(out, args); // no warning
+}
+
+void f17(const unsigned char *out, ... /* args */)
+{
+va_list args;
+vscanf(out, args); // expected-error {{no matching function for call to
'vscanf'}}
+}
+
+__attribute__((format(scanf, 1, 2)))
+void f18(const unsigned char *out, ... /* args */)
+{
+va_list args;
+vscanf(out, args); // expected-error {{no matching function for call to
'vscanf'}}
+}
+
+void f19(const signed char *out, ... /* args */)
+{
+va_list args;
+vscanf(out, args); // expected-error {{no matching function for call to
'vscanf'}}
+}
+
+__attribute__((format(scanf, 1, 2)))
+void f20(const signed char *out, ... /* args */)
+{
+va_list args;
+vscanf(out, args); // expected-error {{no matching function for call to
'vscanf'}}
+}
+
+void f21(const char out[], ... /* args */)
+{
+va_list args;
+vscanf(out, args); // expected-warning {{diagnostic behavior may be
improved by adding the 'scanf' format attribute to the declaration of 'f21'}}
+}
+
+__attribute__((format(scanf, 1, 2)))
+void f22(const char out[], ... /* args */)
+{
+va_list args;
+vscanf(out, args); // no warning
+}
+
+template
+void func(char (&str)[N], ...)
+{
+va_list args;
+vprintf(str, args); // no warning
+}
aaronpuchert wrote:
What makes C++ interesting is the implicit `this` parameter in non-static
member functions, maybe you can add tests for that? The existing tests here
seem to be mostly about cases where the attribute does not apply due to some
technicality.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -0,0 +1,143 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-format-attribute %s
+
+#include
+#include
+#include
+#include
+
+__attribute__((__format__ (__scanf__, 1, 4)))
+void f1(char *out, const size_t len, const char *format, ... /* args */)
+{
+va_list args;
+vsnprintf(out, len, format, args); // expected-warning {{diagnostic
behavior may be improved by adding the 'printf' format attribute to the
declaration of 'f1'}}
+ // CHECK-FIXES:
__attribute__((format(printf, 1, 4)))
+}
+
+void f2(char *out, va_list args)
+{
+vprintf(out, args); // expected-warning {{diagnostic behavior may be
improved by adding the 'printf' format attribute to the declaration of 'f2'}}
+// CHECK-FIXES: __attribute__((format(printf, 1, 0)))
+vscanf(out, args); // expected-warning {{diagnostic behavior may be
improved by adding the 'scanf' format attribute to the declaration of 'f2'}}
+ // CHECK-FIXES: __attribute__((format(scanf, 1, 0)))
+}
+
+void f3(char* out, ... /* args */)
+{
+va_list args;
+vprintf("test", args); // no warning
+}
+
+void f4(char *out, ... /* args */)
+{
+const char *ch;
+va_list args;
+vscanf(ch, args); // expected-warning {{diagnostic behavior may be
improved by adding the 'scanf' format attribute to the declaration of 'f4'}}
+ // CHECK-FIXES: __attribute__((format(scanf, 1, 2)))
aaronpuchert wrote:
I don't think we can propagate the attribute here, since the format string is a
local variable. It doesn't come from the caller.
The `out` parameter seems unused here, and has no relation to the format string
`ch`.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
https://github.com/aaronpuchert edited https://github.com/llvm/llvm-project/pull/70024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -6849,6 +6849,73 @@ static void handleSwiftAsyncAttr(Sema &S, Decl *D, const
ParsedAttr &AL) {
checkSwiftAsyncErrorBlock(S, D, ErrorAttr, AsyncAttr);
}
+// Warn if parent function misses format attribute. Parent function misses
+// format attribute if there is an argument format string forwarded to calling
+// function with format attribute, parent function has a parameter which type
+// is either string or pointer to char and parent function format attribute
+// type does not match with calling function format attribute type.
+void Sema::DiagnoseMissingFormatAttributes(const FunctionDecl *FDecl,
+ ArrayRef Args,
+ SourceLocation Loc) {
+ assert(FDecl);
+
+ // Check if function has format attribute with forwarded format string.
+ IdentifierInfo *AttrType;
+ if (!llvm::any_of(
+ FDecl->specific_attrs(), [&](const FormatAttr *Attr) {
+if (!Args[Attr->getFirstArg()]->getReferencedDeclOfCallee())
+ return false;
+
+AttrType = Attr->getType();
+return true;
+ }))
+return;
+
+ const FunctionDecl *ParentFuncDecl = getCurFunctionDecl();
+ if (!ParentFuncDecl)
+return;
+
+ // Check if parent function has string or pointer to char parameter.
+ unsigned int StringIndex = 0;
+ if (!llvm::any_of(
+ ParentFuncDecl->parameters(), [&](const ParmVarDecl *Param) {
+StringIndex = Param->getFunctionScopeIndex() + 1;
+QualType Ty = Param->getType();
+if (isNSStringType(Ty, Context, true))
+ return true;
+if (isCFStringType(Ty, Context))
+ return true;
+if (Ty->isPointerType() &&
+Ty->castAs()
+->getPointeeType()
+->isSpecificBuiltinType(BuiltinType::Char_S))
+ return true;
+return false;
+ }))
+return;
aaronpuchert wrote:
I don't think we can decide this based on the type. We should look at the
argument and check if it's a `DeclRefExpr` referring to a parameter of the
parent function. There can be other (unrelated) string arguments. (See also my
comments in the tests.)
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -6849,6 +6849,73 @@ static void handleSwiftAsyncAttr(Sema &S, Decl *D, const
ParsedAttr &AL) {
checkSwiftAsyncErrorBlock(S, D, ErrorAttr, AsyncAttr);
}
+// Warn if parent function misses format attribute. Parent function misses
+// format attribute if there is an argument format string forwarded to calling
+// function with format attribute, parent function has a parameter which type
+// is either string or pointer to char and parent function format attribute
+// type does not match with calling function format attribute type.
+void Sema::DiagnoseMissingFormatAttributes(const FunctionDecl *FDecl,
+ ArrayRef Args,
+ SourceLocation Loc) {
+ assert(FDecl);
+
+ // Check if function has format attribute with forwarded format string.
+ IdentifierInfo *AttrType;
+ if (!llvm::any_of(
+ FDecl->specific_attrs(), [&](const FormatAttr *Attr) {
+if (!Args[Attr->getFirstArg()]->getReferencedDeclOfCallee())
+ return false;
+
+AttrType = Attr->getType();
+return true;
+ }))
+return;
+
+ const FunctionDecl *ParentFuncDecl = getCurFunctionDecl();
+ if (!ParentFuncDecl)
+return;
+
+ // Check if parent function has string or pointer to char parameter.
+ unsigned int StringIndex = 0;
+ if (!llvm::any_of(
+ ParentFuncDecl->parameters(), [&](const ParmVarDecl *Param) {
+StringIndex = Param->getFunctionScopeIndex() + 1;
+QualType Ty = Param->getType();
+if (isNSStringType(Ty, Context, true))
+ return true;
+if (isCFStringType(Ty, Context))
+ return true;
+if (Ty->isPointerType() &&
+Ty->castAs()
+->getPointeeType()
+->isSpecificBuiltinType(BuiltinType::Char_S))
+ return true;
+return false;
+ }))
+return;
+
+ // Check if there is parent function format attribute which type matches
+ // calling function format attribute type.
+ if (llvm::any_of(
+ ParentFuncDecl->specific_attrs(),
+ [&](const FormatAttr *Attr) { return Attr->getType() == AttrType; }))
+return;
+
+ unsigned int FirstToCheck =
+ ParentFuncDecl->isVariadic() ? (ParentFuncDecl->getNumParams() + 1) : 0;
+
+ // Diagnose missing format attributes
+ std::string InsertionText;
+ llvm::raw_string_ostream OS(InsertionText);
+ OS << "__attribute__((format(" << AttrType->getName() << ", "
+ << std::to_string(StringIndex) << ", " << std::to_string(FirstToCheck)
aaronpuchert wrote:
I'm surprised that we need `std::to_string` here, can
`llvm::raw_string_ostream` not print integers directly?
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -6849,6 +6849,73 @@ static void handleSwiftAsyncAttr(Sema &S, Decl *D, const
ParsedAttr &AL) {
checkSwiftAsyncErrorBlock(S, D, ErrorAttr, AsyncAttr);
}
+// Warn if parent function misses format attribute. Parent function misses
+// format attribute if there is an argument format string forwarded to calling
+// function with format attribute, parent function has a parameter which type
+// is either string or pointer to char and parent function format attribute
+// type does not match with calling function format attribute type.
+void Sema::DiagnoseMissingFormatAttributes(const FunctionDecl *FDecl,
+ ArrayRef Args,
+ SourceLocation Loc) {
+ assert(FDecl);
+
+ // Check if function has format attribute with forwarded format string.
+ IdentifierInfo *AttrType;
+ if (!llvm::any_of(
+ FDecl->specific_attrs(), [&](const FormatAttr *Attr) {
+if (!Args[Attr->getFirstArg()]->getReferencedDeclOfCallee())
+ return false;
+
+AttrType = Attr->getType();
+return true;
+ }))
+return;
+
+ const FunctionDecl *ParentFuncDecl = getCurFunctionDecl();
+ if (!ParentFuncDecl)
+return;
+
+ // Check if parent function has string or pointer to char parameter.
+ unsigned int StringIndex = 0;
+ if (!llvm::any_of(
+ ParentFuncDecl->parameters(), [&](const ParmVarDecl *Param) {
+StringIndex = Param->getFunctionScopeIndex() + 1;
+QualType Ty = Param->getType();
+if (isNSStringType(Ty, Context, true))
+ return true;
+if (isCFStringType(Ty, Context))
+ return true;
+if (Ty->isPointerType() &&
+Ty->castAs()
+->getPointeeType()
+->isSpecificBuiltinType(BuiltinType::Char_S))
+ return true;
+return false;
+ }))
+return;
aaronpuchert wrote:
Hmm, it seems that GCC is also warning in cases where we're not forwarding the
format string, for example here:
```c
void forward_tgt(char* tgt) {
va_list va;
const char* fmt;
vsprintf(tgt, fmt, va);
}
```
produces "warning: function 'void forward_tgt(char*)' might be a candidate for
'gnu_printf' format attribute [-Wsuggest-attribute=format]". But I think that's
a false positive, and we can easily check for proper forwarding instead.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Add -Wswitch-default warning option (PR #73077)
aaronpuchert wrote:
@gendalph, this warning is meant to always warn if a {{default}} label is
missing, even if all enumeration values are covered. If you don't want a
warning on enumerations, use the previously mentioned clang-tidy check
[bugprone-switch-missing-default-case](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/switch-missing-default-case.html).
We can probably discuss adding this as warning, though I doubt we'll get
consensus on that. This warning was added despite never going to be always-on
because GCC has it (under the same name) and some code styles ask for it.
https://github.com/llvm/llvm-project/pull/73077
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Fix Wswitch-default bad warning in template (PR #76007)
https://github.com/aaronpuchert approved this pull request. For now I guess this is Ok, although I think the better fix would be to diagnose missing or duplicate `default` labels even in the dependent case. Because `default` labels themselves are never dependent. https://github.com/llvm/llvm-project/pull/76007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: Fix a bug in handling temporary constructors (PR #74020)
@@ -1544,7 +1544,10 @@ class BuildLockset : public
ConstStmtVisitor {
// The fact set for the function on exit.
const FactSet &FunctionExitFSet;
/// Maps constructed objects to `this` placeholder prior to initialization.
- llvm::SmallDenseMap ConstructedObjects;
+ /// The map lives longer than this builder so this is a reference to an
+ /// existing one. The map lives so long as the CFG while this builder is for
+ /// one CFG block.
aaronpuchert wrote:
It's probably sufficient to explain this in the commit message.
https://github.com/llvm/llvm-project/pull/74020
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: Fix a bug in handling temporary constructors (PR #74020)
https://github.com/aaronpuchert edited https://github.com/llvm/llvm-project/pull/74020 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: Fix a bug in handling temporary constructors (PR #74020)
@@ -1718,6 +1720,13 @@ struct TestScopedLockable {
MutexLock{&mu1}, a = 5;
}
+ void temporary2(int x) {
+if (check(MutexLock{&mu1}) || x) {
+
+}
+check(MutexLock{&mu1});
aaronpuchert wrote:
The `check` here doesn't do anything, right?
https://github.com/llvm/llvm-project/pull/74020
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: Fix a bug in handling temporary constructors (PR #74020)
@@ -1718,6 +1720,13 @@ struct TestScopedLockable {
MutexLock{&mu1}, a = 5;
}
+ void temporary2(int x) {
aaronpuchert wrote:
I would suggest a speaking name for the test, like `temporary_cfg`.
https://github.com/llvm/llvm-project/pull/74020
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: Fix a bug in handling temporary constructors (PR #74020)
https://github.com/aaronpuchert commented: I have some suggestions, but in principle this is absolutely right. Thanks for finding this and providing a fix! > The issue is that the map lives within a CFG block. It didn't cross my mind to check how long `BuildLockset` lived, I always assumed it lived for the entire function. But you're right, it does not, and is therefore an unsuitable home for the map. https://github.com/llvm/llvm-project/pull/74020 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: Fix a bug in handling temporary constructors (PR #74020)
@@ -2392,6 +2397,8 @@ void
ThreadSafetyAnalyzer::runAnalysis(AnalysisDeclContext &AC) {
for (const auto &Lock : LocksReleased)
ExpectedFunctionExitSet.removeLock(FactMan, Lock);
+ ConstructedObjectMapTy ConstructedObjects;
aaronpuchert wrote:
I wonder if we should put it into the `ThreadSafetyAnalyzer`, where we already
have the similar `LocalVariableMap`. Then we don't need to pass it into
`BuildLockset`, because that already has a pointer to the
`ThreadSafetyAnalyzer`.
https://github.com/llvm/llvm-project/pull/74020
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: Fix a bug in handling temporary constructors (PR #74020)
@@ -1718,6 +1720,13 @@ struct TestScopedLockable {
MutexLock{&mu1}, a = 5;
}
+ void temporary2(int x) {
+if (check(MutexLock{&mu1}) || x) {
+
+}
aaronpuchert wrote:
The `if` is probably not needed here, right? We could just write
```suggestion
check(MutexLock{&mu1}) || x;
```
and should have the same CFG artifact.
https://github.com/llvm/llvm-project/pull/74020
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Add -Wswitch-default warning option (PR #73077)
aaronpuchert wrote: > There is one clang-tidy check (bugprone-switch-missing-default-case) also for > this feature. This excludes enumeration types though, while GCC `-Wswitch-default` warns on them as well. (Even if all enumeration values are covered.) > whether switch statements should have a default is not something the industry > seem to agree on, so it would be a very opinionated warning. Yes. Though I have to say, even though I'm squarely in the `-Wcovered-switch-default` camp (i.e. no `default` label in switches that cover all enumeration values), I think it's still valuable to have this warning because some style guides/policies demand a `default`, and it is very little effort on our side to maintain this. I understand the policy against off-by-default warnings to avoid crowding the core compiler, but this is essentially just two lines of code, and if GCC has it we're not going out on a limb here. And since we already seem to support the flag for GCC compatibility, we might as well make it work if it's that easy. > LLVM development heavily relies on `-Wswitch-enum`, for example. I think we're using `-Wswitch`, as `-Wswitch-enum` requires to cover all enumeration values even if there is a default label. Our policy is: cover all enumeration values or have a `default`. This is "enforced" by `-Wswitch` in combination with `-Wcovered-switch-default`. https://github.com/llvm/llvm-project/pull/73077 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Add -Wswitch-default warning option (PR #73077)
aaronpuchert wrote: In the end it probably boils down to how you view an enumeration type. Is it a type with the declared enumerators as inhabitants, or is it an integer type of some length with associated symbolic constants of that type? In other words, is an enumeration a "strong" type or a "weak" type? The standard itself seems more in the "weak" camp. Yes, it does not allow integer values larger than what the largest enumeration value requires (if the underlying integer type isn't declared), but otherwise integer values that don't correspond to enumeration values can be cast to the enumeration type just fine. In [[dcl.enum]p8](https://eel.is/c++draft/enum#dcl.enum-8): > [For an enumeration whose underlying type is not fixed,] the values of the > enumeration are the values representable by a hypothetical integer type with > minimal width _M_ such that all enumerators can be represented. However, nothing prevents a developer from assuming a stricter model, where only the declared enumeration values are allowed and everything else is undefined behavior. (If not in terms of the language, then in terms of the program.) That is what we have done in the LLVM code base, and it's also the model that I personally prefer. But it is probably legitimate to follow the weaker model implied by the standard and assume that enumerations can have non-enumerator values, which requires a default label, enforced by this warning. https://github.com/llvm/llvm-project/pull/73077 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Add -Wswitch-default warning option (PR #73077)
aaronpuchert wrote: > Aaron is the real decision maker here Specifically @AaronBallman, not me. https://github.com/llvm/llvm-project/pull/73077 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Fix Wswitch-default bad warning in template (PR #76007)
https://github.com/aaronpuchert approved this pull request. https://github.com/llvm/llvm-project/pull/76007 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 1721d52 - Let clang-repl link privately against Clang components
Author: Aaron Puchert Date: 2022-03-28T23:53:53+02:00 New Revision: 1721d52a62067b8a5ceec58b417b2c73ad870b13 URL: https://github.com/llvm/llvm-project/commit/1721d52a62067b8a5ceec58b417b2c73ad870b13 DIFF: https://github.com/llvm/llvm-project/commit/1721d52a62067b8a5ceec58b417b2c73ad870b13.diff LOG: Let clang-repl link privately against Clang components First of all, this is the convention: all other tools have their dependencies private. While it does not have an effect on linking (there is no linking against executables), it does have an effect on exporting: having the targets private allows installing the tools without the libraries in a statically linked build, or a build against libclang-cpp.so. Reviewed By: v.g.vassilev Differential Revision: https://reviews.llvm.org/D122546 Added: Modified: clang/tools/clang-repl/CMakeLists.txt Removed: diff --git a/clang/tools/clang-repl/CMakeLists.txt b/clang/tools/clang-repl/CMakeLists.txt index 30e3b2be9ed37..b51a18c10cdca 100644 --- a/clang/tools/clang-repl/CMakeLists.txt +++ b/clang/tools/clang-repl/CMakeLists.txt @@ -11,7 +11,7 @@ add_clang_tool(clang-repl ClangRepl.cpp ) -clang_target_link_libraries(clang-repl PUBLIC +clang_target_link_libraries(clang-repl PRIVATE clangBasic clangFrontend clangInterpreter ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 0e64a52 - Thread safety analysis: Mock getter for private mutexes can be undefined
Author: Aaron Puchert
Date: 2021-07-23T14:46:02+02:00
New Revision: 0e64a525c12a0822683d3bdc51b6294b5265f860
URL:
https://github.com/llvm/llvm-project/commit/0e64a525c12a0822683d3bdc51b6294b5265f860
DIFF:
https://github.com/llvm/llvm-project/commit/0e64a525c12a0822683d3bdc51b6294b5265f860.diff
LOG: Thread safety analysis: Mock getter for private mutexes can be undefined
Usage in an annotation is no odr-use, so I think there needs to be no
definition. Upside is that in practice one will get linker errors if it
is actually odr-used instead of calling a function that returns 0.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D106375
Added:
Modified:
clang/docs/ThreadSafetyAnalysis.rst
Removed:
diff --git a/clang/docs/ThreadSafetyAnalysis.rst
b/clang/docs/ThreadSafetyAnalysis.rst
index 651229f01d03..69046ba18b8c 100644
--- a/clang/docs/ThreadSafetyAnalysis.rst
+++ b/clang/docs/ThreadSafetyAnalysis.rst
@@ -640,8 +640,8 @@ mutex. For example:
Mutex mu;
public:
-// For thread safety analysis only. Does not actually return mu.
-Mutex* getMu() RETURN_CAPABILITY(mu) { return 0; }
+// For thread safety analysis only. Does not need to be defined.
+Mutex* getMu() RETURN_CAPABILITY(mu);
void doSomething() REQUIRES(mu);
};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] c9a16e8 - Drop '* text=auto' from .gitattributes and normalize
Author: Aaron Puchert
Date: 2022-04-28T03:05:10+02:00
New Revision: c9a16e8c3d99173c7525e576d78eed57110d2b08
URL:
https://github.com/llvm/llvm-project/commit/c9a16e8c3d99173c7525e576d78eed57110d2b08
DIFF:
https://github.com/llvm/llvm-project/commit/c9a16e8c3d99173c7525e576d78eed57110d2b08.diff
LOG: Drop '* text=auto' from .gitattributes and normalize
Git wants to check in 'text' files with LF endings, so this changes them
in the repository but not in the checkout, where they keep CRLF endings.
Differential Revision: https://reviews.llvm.org/D124563
Added:
Modified:
clang-tools-extra/test/.gitattributes
clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp
clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp.expected
Removed:
diff --git a/clang-tools-extra/test/.gitattributes
b/clang-tools-extra/test/.gitattributes
index 9d17a32c7b261..42567e3196d61 100644
--- a/clang-tools-extra/test/.gitattributes
+++ b/clang-tools-extra/test/.gitattributes
@@ -2,10 +2,6 @@
# rely on or check fixed character -offset, Offset: or FileOffset: locations
# will fail when run on input files checked out with
diff erent line endings.
-# Most test input files should use native line endings, to ensure that we run
-# tests against both line ending types.
-* text=auto
-
# These test input files rely on one-byte Unix (LF) line-endings, as they use
# fixed -offset, FileOffset:, or Offset: numbers in their tests.
clang-apply-replacements/ClangRenameClassReplacements.cpp text eol=lf
diff --git
a/clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp
b/clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp
index 26f79968f556d..09e50c292cc91 100644
--- a/clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp
+++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp
@@ -1,6 +1,6 @@
-
-// This file intentionally uses a CRLF newlines!
-
-void foo() {
- int *x = 0;
-}
+
+// This file intentionally uses a CRLF newlines!
+
+void foo() {
+ int *x = 0;
+}
diff --git
a/clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp.expected
b/clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp.expected
index ad8e907856283..7a5e11354748d 100644
---
a/clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp.expected
+++
b/clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp.expected
@@ -1,6 +1,6 @@
-
-// This file intentionally uses a CRLF newlines!
-
-void foo() {
- int *x = nullptr;
-}
+
+// This file intentionally uses a CRLF newlines!
+
+void foo() {
+ int *x = nullptr;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] dd1790c - Thread safety analysis: Pack CapabilityExpr using PointerIntPair (NFC)
Author: Aaron Puchert
Date: 2022-04-29T22:30:33+02:00
New Revision: dd1790cd05ae124e9e5d57dfe9279ff54f34b488
URL:
https://github.com/llvm/llvm-project/commit/dd1790cd05ae124e9e5d57dfe9279ff54f34b488
DIFF:
https://github.com/llvm/llvm-project/commit/dd1790cd05ae124e9e5d57dfe9279ff54f34b488.diff
LOG: Thread safety analysis: Pack CapabilityExpr using PointerIntPair (NFC)
We're storing these quite frequently: FactEntry inherits from
CapabilityExpr, and the FactManager has a vector of such entries.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D124127
Added:
Modified:
clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
Removed:
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
index 2f6a78126a1d..392eecdb1897 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
@@ -30,6 +30,7 @@
#include "clang/Analysis/CFG.h"
#include "clang/Basic/LLVM.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/PointerIntPair.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Casting.h"
#include
@@ -269,28 +270,27 @@ class CFGWalker {
// translateAttrExpr needs it, but that should be moved too.
class CapabilityExpr {
private:
- /// The capability expression.
- const til::SExpr* CapExpr;
-
- /// True if this is a negative capability.
- bool Negated;
+ /// The capability expression and whether it's negated.
+ llvm::PointerIntPair CapExpr;
public:
- CapabilityExpr(const til::SExpr *E, bool Neg) : CapExpr(E), Negated(Neg) {}
+ CapabilityExpr(const til::SExpr *E, bool Neg) : CapExpr(E, Neg) {}
- const til::SExpr* sexpr() const { return CapExpr; }
- bool negative() const { return Negated; }
+ const til::SExpr *sexpr() const { return CapExpr.getPointer(); }
+ bool negative() const { return CapExpr.getInt(); }
CapabilityExpr operator!() const {
-return CapabilityExpr(CapExpr, !Negated);
+return CapabilityExpr(CapExpr.getPointer(), !CapExpr.getInt());
}
bool equals(const CapabilityExpr &other) const {
-return (Negated == other.Negated) && sx::equals(CapExpr, other.CapExpr);
+return (negative() == other.negative()) &&
+ sx::equals(sexpr(), other.sexpr());
}
bool matches(const CapabilityExpr &other) const {
-return (Negated == other.Negated) && sx::matches(CapExpr, other.CapExpr);
+return (negative() == other.negative()) &&
+ sx::matches(sexpr(), other.sexpr());
}
bool matchesUniv(const CapabilityExpr &CapE) const {
@@ -298,27 +298,27 @@ class CapabilityExpr {
}
bool partiallyMatches(const CapabilityExpr &other) const {
-return (Negated == other.Negated) &&
-sx::partiallyMatches(CapExpr, other.CapExpr);
+return (negative() == other.negative()) &&
+ sx::partiallyMatches(sexpr(), other.sexpr());
}
const ValueDecl* valueDecl() const {
-if (Negated || CapExpr == nullptr)
+if (negative() || sexpr() == nullptr)
return nullptr;
-if (const auto *P = dyn_cast(CapExpr))
+if (const auto *P = dyn_cast(sexpr()))
return P->clangDecl();
-if (const auto *P = dyn_cast(CapExpr))
+if (const auto *P = dyn_cast(sexpr()))
return P->clangDecl();
return nullptr;
}
std::string toString() const {
-if (Negated)
- return "!" + sx::toString(CapExpr);
-return sx::toString(CapExpr);
+if (negative())
+ return "!" + sx::toString(sexpr());
+return sx::toString(sexpr());
}
- bool shouldIgnore() const { return CapExpr == nullptr; }
+ bool shouldIgnore() const { return sexpr() == nullptr; }
bool isInvalid() const { return sexpr() && isa(sexpr()); }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] d65c922 - Thread safety analysis: Store CapabilityExprs in ScopedLockableFactEntry (NFC)
Author: Aaron Puchert
Date: 2022-04-29T22:30:33+02:00
New Revision: d65c922450d1fdf0f44f4a10a8f33b11c6c01bf5
URL:
https://github.com/llvm/llvm-project/commit/d65c922450d1fdf0f44f4a10a8f33b11c6c01bf5
DIFF:
https://github.com/llvm/llvm-project/commit/d65c922450d1fdf0f44f4a10a8f33b11c6c01bf5.diff
LOG: Thread safety analysis: Store CapabilityExprs in ScopedLockableFactEntry
(NFC)
For now this doesn't make a whole lot of sense, but it will allow us to
store the capability kind in a CapabilityExpr and make sure it doesn't
get lost. The capabilities managed by a scoped lockable can of course be
of different kind, so we'll need to store that per entry.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D124128
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 9cc990bd35a3..2b461a72dc2e 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -41,7 +41,6 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/ImmutableMap.h"
#include "llvm/ADT/Optional.h"
-#include "llvm/ADT/PointerIntPair.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
@@ -897,25 +896,27 @@ class ScopedLockableFactEntry : public FactEntry {
UCK_ReleasedExclusive, ///< Exclusive capability that was released.
};
- using UnderlyingCapability =
- llvm::PointerIntPair;
+ struct UnderlyingCapability {
+CapabilityExpr Cap;
+UnderlyingCapabilityKind Kind;
+ };
- SmallVector UnderlyingMutexes;
+ SmallVector UnderlyingMutexes;
public:
ScopedLockableFactEntry(const CapabilityExpr &CE, SourceLocation Loc)
: FactEntry(CE, LK_Exclusive, Loc, Acquired) {}
void addLock(const CapabilityExpr &M) {
-UnderlyingMutexes.emplace_back(M.sexpr(), UCK_Acquired);
+UnderlyingMutexes.push_back(UnderlyingCapability{M, UCK_Acquired});
}
void addExclusiveUnlock(const CapabilityExpr &M) {
-UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedExclusive);
+UnderlyingMutexes.push_back(UnderlyingCapability{M,
UCK_ReleasedExclusive});
}
void addSharedUnlock(const CapabilityExpr &M) {
-UnderlyingMutexes.emplace_back(M.sexpr(), UCK_ReleasedShared);
+UnderlyingMutexes.push_back(UnderlyingCapability{M, UCK_ReleasedShared});
}
void
@@ -923,15 +924,13 @@ class ScopedLockableFactEntry : public FactEntry {
SourceLocation JoinLoc, LockErrorKind LEK,
ThreadSafetyHandler &Handler) const override {
for (const auto &UnderlyingMutex : UnderlyingMutexes) {
- const auto *Entry = FSet.findLock(
- FactMan, CapabilityExpr(UnderlyingMutex.getPointer(), false));
- if ((UnderlyingMutex.getInt() == UCK_Acquired && Entry) ||
- (UnderlyingMutex.getInt() != UCK_Acquired && !Entry)) {
+ const auto *Entry = FSet.findLock(FactMan, UnderlyingMutex.Cap);
+ if ((UnderlyingMutex.Kind == UCK_Acquired && Entry) ||
+ (UnderlyingMutex.Kind != UCK_Acquired && !Entry)) {
// If this scoped lock manages another mutex, and if the underlying
// mutex is still/not held, then warn about the underlying mutex.
Handler.handleMutexHeldEndOfScope(
-"mutex", sx::toString(UnderlyingMutex.getPointer()), loc(),
JoinLoc,
-LEK);
+"mutex", UnderlyingMutex.Cap.toString(), loc(), JoinLoc, LEK);
}
}
}
@@ -940,13 +939,12 @@ class ScopedLockableFactEntry : public FactEntry {
ThreadSafetyHandler &Handler,
StringRef DiagKind) const override {
for (const auto &UnderlyingMutex : UnderlyingMutexes) {
- CapabilityExpr UnderCp(UnderlyingMutex.getPointer(), false);
-
- if (UnderlyingMutex.getInt() == UCK_Acquired)
-lock(FSet, FactMan, UnderCp, entry.kind(), entry.loc(), &Handler,
- DiagKind);
+ if (UnderlyingMutex.Kind == UCK_Acquired)
+lock(FSet, FactMan, UnderlyingMutex.Cap, entry.kind(), entry.loc(),
+ &Handler, DiagKind);
else
-unlock(FSet, FactMan, UnderCp, entry.loc(), &Handler, DiagKind);
+unlock(FSet, FactMan, UnderlyingMutex.Cap, entry.loc(), &Handler,
+ DiagKind);
}
}
@@ -956,18 +954,18 @@ class ScopedLockableFactEntry : public FactEntry {
StringRef DiagKind) const override {
assert(!Cp.negative() && "Managing object cannot be negative.");
for (const auto &UnderlyingMutex : UnderlyingMutexes) {
- CapabilityExpr UnderCp(UnderlyingMutex.getPointer(), false);
-
// Remove/lock the underlying mutex if it exists/is still unlocked; warn
// on double unlocking/locking if we're not destroying the scoped object.
ThreadSafetyHandler *TSH
[clang] f8afb8f - Thread safety analysis: Store capability kind in CapabilityExpr
Author: Aaron Puchert
Date: 2022-04-29T22:30:33+02:00
New Revision: f8afb8fdedae04ad2670857c97925c439d47d862
URL:
https://github.com/llvm/llvm-project/commit/f8afb8fdedae04ad2670857c97925c439d47d862
DIFF:
https://github.com/llvm/llvm-project/commit/f8afb8fdedae04ad2670857c97925c439d47d862.diff
LOG: Thread safety analysis: Store capability kind in CapabilityExpr
This should make us print the right capability kind in many more cases,
especially when attributes name multiple capabilities of different kinds.
Previously we were trying to deduce the capability kind from the
original attribute, but most attributes can name multiple capabilities,
which could be of different kinds. So instead we derive the kind when
translating the attribute expression, and then store it in the returned
CapabilityExpr. Then we can extract the corresponding capability name
when we need it, which saves us lots of plumbing and almost guarantees
that the name is right.
I didn't bother adding any tests for this because it's just a usability
improvement and it's pretty much evident from the code that we don't
fall back to "mutex" anymore (save for a few cases that I'll address in
a separate change).
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D124131
Added:
Modified:
clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
clang/lib/Analysis/ThreadSafety.cpp
clang/lib/Analysis/ThreadSafetyCommon.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
Removed:
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
index 392eecdb1897e..da69348ea9388 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyCommon.h
@@ -273,14 +273,23 @@ class CapabilityExpr {
/// The capability expression and whether it's negated.
llvm::PointerIntPair CapExpr;
+ /// The kind of capability as specified by @ref CapabilityAttr::getName.
+ StringRef CapKind;
+
public:
- CapabilityExpr(const til::SExpr *E, bool Neg) : CapExpr(E, Neg) {}
+ CapabilityExpr() : CapExpr(nullptr, false) {}
+ CapabilityExpr(const til::SExpr *E, StringRef Kind, bool Neg)
+ : CapExpr(E, Neg), CapKind(Kind) {}
+
+ // Don't allow implicitly-constructed StringRefs since we'll capture them.
+ template CapabilityExpr(const til::SExpr *, T, bool) = delete;
const til::SExpr *sexpr() const { return CapExpr.getPointer(); }
+ StringRef getKind() const { return CapKind; }
bool negative() const { return CapExpr.getInt(); }
CapabilityExpr operator!() const {
-return CapabilityExpr(CapExpr.getPointer(), !CapExpr.getInt());
+return CapabilityExpr(CapExpr.getPointer(), CapKind, !CapExpr.getInt());
}
bool equals(const CapabilityExpr &other) const {
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 2b461a72dc2eb..63cc61b07c14a 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -139,12 +139,12 @@ class FactEntry : public CapabilityExpr {
SourceLocation JoinLoc, LockErrorKind LEK,
ThreadSafetyHandler &Handler) const = 0;
virtual void handleLock(FactSet &FSet, FactManager &FactMan,
- const FactEntry &entry, ThreadSafetyHandler &Handler,
- StringRef DiagKind) const = 0;
+ const FactEntry &entry,
+ ThreadSafetyHandler &Handler) const = 0;
virtual void handleUnlock(FactSet &FSet, FactManager &FactMan,
const CapabilityExpr &Cp, SourceLocation UnlockLoc,
-bool FullyRemove, ThreadSafetyHandler &Handler,
-StringRef DiagKind) const = 0;
+bool FullyRemove,
+ThreadSafetyHandler &Handler) const = 0;
// Return true if LKind >= LK, where exclusive > shared
bool isAtLeast(LockKind LK) const {
@@ -865,21 +865,21 @@ class LockableFactEntry : public FactEntry {
SourceLocation JoinLoc, LockErrorKind LEK,
ThreadSafetyHandler &Handler) const override {
if (!asserted() && !negative() && !isUniversal()) {
- Handler.handleMutexHeldEndOfScope("mutex", toString(), loc(), JoinLoc,
+ Handler.handleMutexHeldEndOfScope(getKind(), toString(), loc(), JoinLoc,
LEK);
}
}
void handleLock(FactSet &FSet, FactManager &FactMan, const FactEntry &entry,
- ThreadSafetyHandler &Handler,
- StringRef DiagKind) const override {
-Handler.handleDoubleLock(DiagKind, entry.toString(), loc(), entry.loc());
+ ThreadS
[clang] 0314dba - Thread safety analysis: Don't pass capability kind where not needed (NFC)
Author: Aaron Puchert
Date: 2022-04-29T22:30:33+02:00
New Revision: 0314dbac026f58aaaf0a9ee4515f401f0d43ee76
URL:
https://github.com/llvm/llvm-project/commit/0314dbac026f58aaaf0a9ee4515f401f0d43ee76
DIFF:
https://github.com/llvm/llvm-project/commit/0314dbac026f58aaaf0a9ee4515f401f0d43ee76.diff
LOG: Thread safety analysis: Don't pass capability kind where not needed (NFC)
If no capability is held, or the capability expression is invalid, there
is obviously no capability kind and so none would be reported.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D124132
Added:
Modified:
clang/include/clang/Analysis/Analyses/ThreadSafety.h
clang/lib/Analysis/ThreadSafety.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
Removed:
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafety.h
b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
index bfa9870a1e1f..1808d1d71e05 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafety.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
@@ -98,9 +98,8 @@ class ThreadSafetyHandler {
virtual ~ThreadSafetyHandler();
/// Warn about lock expressions which fail to resolve to lockable objects.
- /// \param Kind -- the capability's name parameter (role, mutex, etc).
/// \param Loc -- the SourceLocation of the unresolved expression.
- virtual void handleInvalidLockExp(StringRef Kind, SourceLocation Loc) {}
+ virtual void handleInvalidLockExp(SourceLocation Loc) {}
/// Warn about unlock function calls that do not have a prior matching lock
/// expression.
@@ -169,14 +168,12 @@ class ThreadSafetyHandler {
SourceLocation Loc2) {}
/// Warn when a protected operation occurs while no locks are held.
- /// \param Kind -- the capability's name parameter (role, mutex, etc).
/// \param D -- The decl for the protected variable or function
/// \param POK -- The kind of protected operation (e.g. variable access)
/// \param AK -- The kind of access (i.e. read or write) that occurred
/// \param Loc -- The location of the protected operation.
- virtual void handleNoMutexHeld(StringRef Kind, const NamedDecl *D,
- ProtectedOperationKind POK, AccessKind AK,
- SourceLocation Loc) {}
+ virtual void handleNoMutexHeld(const NamedDecl *D, ProtectedOperationKind
POK,
+ AccessKind AK, SourceLocation Loc) {}
/// Warn when a protected operation occurs while the specific mutex
protecting
/// the operation is not locked.
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 63cc61b07c14..b8fe6000d971 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -74,7 +74,7 @@ static void warnInvalidLock(ThreadSafetyHandler &Handler,
// FIXME: add a note about the attribute location in MutexExp or D
if (Loc.isValid())
-Handler.handleInvalidLockExp(Kind, Loc);
+Handler.handleInvalidLockExp(Loc);
}
namespace {
@@ -1696,7 +1696,7 @@ void BuildLockset::checkAccess(const Expr *Exp,
AccessKind AK,
return;
if (D->hasAttr() && FSet.isEmpty(Analyzer->FactMan)) {
-Analyzer->Handler.handleNoMutexHeld("mutex", D, POK, AK, Loc);
+Analyzer->Handler.handleNoMutexHeld(D, POK, AK, Loc);
}
for (const auto *I : D->specific_attrs())
@@ -1734,8 +1734,7 @@ void BuildLockset::checkPtAccess(const Expr *Exp,
AccessKind AK,
return;
if (D->hasAttr() && FSet.isEmpty(Analyzer->FactMan))
-Analyzer->Handler.handleNoMutexHeld("mutex", D, PtPOK, AK,
-Exp->getExprLoc());
+Analyzer->Handler.handleNoMutexHeld(D, PtPOK, AK, Exp->getExprLoc());
for (auto const *I : D->specific_attrs())
warnIfMutexNotHeld(D, Exp, AK, I->getArg(), PtPOK, Exp->getExprLoc());
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp
b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index ac5ad52c0b1d..bf282bbb400d 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1844,7 +1844,7 @@ class ThreadSafetyReporter : public
clang::threadSafety::ThreadSafetyHandler {
}
}
- void handleInvalidLockExp(StringRef Kind, SourceLocation Loc) override {
+ void handleInvalidLockExp(SourceLocation Loc) override {
PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_cannot_resolve_lock)
<< Loc);
Warnings.emplace_back(std::move(Warning), getNotes());
@@ -1922,9 +1922,8 @@ class ThreadSafetyReporter : public
clang::threadSafety::ThreadSafetyHandler {
Warnings.emplace_back(std::move(Warning), getNotes(Note));
}
- void handleNoMutexHeld(StringRef Kind, const NamedDecl *D,
- ProtectedOperationKind POK, AccessKind AK,
-
[clang] bfe63ab - Thread safety analysis: Support builtin pointer-to-member operators
Author: Aaron Puchert
Date: 2022-07-14T13:36:14+02:00
New Revision: bfe63ab63e22b61bd5898c65425e8ebe43189913
URL:
https://github.com/llvm/llvm-project/commit/bfe63ab63e22b61bd5898c65425e8ebe43189913
DIFF:
https://github.com/llvm/llvm-project/commit/bfe63ab63e22b61bd5898c65425e8ebe43189913.diff
LOG: Thread safety analysis: Support builtin pointer-to-member operators
We consider an access to x.*pm as access of the same kind into x, and
an access to px->*pm as access of the same kind into *px. Previously we
missed reads and writes in the .* case, and operations to the pointed-to
data for ->* (we didn't miss accesses to the pointer itself, because
that requires an LValueToRValue cast that we treat independently).
We added support for overloaded operator->* in D124966.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D129514
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 03bbf078d7e89..32d950864ce78 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1679,6 +1679,17 @@ void BuildLockset::checkAccess(const Expr *Exp,
AccessKind AK,
return;
}
+ if (const auto *BO = dyn_cast(Exp)) {
+switch (BO->getOpcode()) {
+case BO_PtrMemD: // .*
+ return checkAccess(BO->getLHS(), AK, POK);
+case BO_PtrMemI: // ->*
+ return checkPtAccess(BO->getLHS(), AK, POK);
+default:
+ return;
+}
+ }
+
if (const auto *AE = dyn_cast(Exp)) {
checkPtAccess(AE->getLHS(), AK, POK);
return;
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index ea229fef649b9..ac854dce0f7b0 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -4870,6 +4870,8 @@ class PtGuardedByCorrectnessTest {
intsa[10] GUARDED_BY(mu1);
Cell sc[10] GUARDED_BY(mu1);
+ static constexpr int Cell::*pa = &Cell::a;
+
void test1() {
mu1.Lock();
if (a == 0) doSomething(); // OK, we don't dereference.
@@ -4889,9 +4891,11 @@ class PtGuardedByCorrectnessTest {
if (c->a == 0) doSomething();// expected-warning {{reading the value
pointed to by 'c' requires holding mutex 'mu2'}}
c->a = 0;// expected-warning {{writing the value
pointed to by 'c' requires holding mutex 'mu2' exclusively}}
+c->*pa = 0; // expected-warning {{writing the value
pointed to by 'c' requires holding mutex 'mu2' exclusively}}
if ((*c).a == 0) doSomething(); // expected-warning {{reading the value
pointed to by 'c' requires holding mutex 'mu2'}}
(*c).a = 0; // expected-warning {{writing the value
pointed to by 'c' requires holding mutex 'mu2' exclusively}}
+(*c).*pa = 0;// expected-warning {{writing the value
pointed to by 'c' requires holding mutex 'mu2' exclusively}}
if (a[0] == 42) doSomething(); // expected-warning {{reading the value
pointed to by 'a' requires holding mutex 'mu2'}}
a[0] = 57; // expected-warning {{writing the value
pointed to by 'a' requires holding mutex 'mu2' exclusively}}
@@ -4923,6 +4927,7 @@ class PtGuardedByCorrectnessTest {
sa[0] = 57; // expected-warning {{writing variable
'sa' requires holding mutex 'mu1' exclusively}}
if (sc[0].a == 42) doSomething(); // expected-warning {{reading variable
'sc' requires holding mutex 'mu1'}}
sc[0].a = 57; // expected-warning {{writing variable
'sc' requires holding mutex 'mu1' exclusively}}
+sc[0].*pa = 57; // expected-warning {{writing variable
'sc' requires holding mutex 'mu1' exclusively}}
if (*sa == 42) doSomething(); // expected-warning {{reading variable
'sa' requires holding mutex 'mu1'}}
*sa = 57; // expected-warning {{writing variable
'sa' requires holding mutex 'mu1' exclusively}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] e0c66c6 - Thread safety analysis: Don't erase TIL_Opcode type (NFC)
Author: Aaron Puchert
Date: 2022-07-14T13:36:35+02:00
New Revision: e0c66c699eb000f604e24b1c4e73b899b4d942d3
URL:
https://github.com/llvm/llvm-project/commit/e0c66c699eb000f604e24b1c4e73b899b4d942d3
DIFF:
https://github.com/llvm/llvm-project/commit/e0c66c699eb000f604e24b1c4e73b899b4d942d3.diff
LOG: Thread safety analysis: Don't erase TIL_Opcode type (NFC)
This is mainly for debugging, but it also eliminates some casts.
Added:
Modified:
clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
Removed:
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
b/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
index 77a800c28754c..65556c8d584c9 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
@@ -75,7 +75,7 @@ namespace til {
class BasicBlock;
/// Enum for the
diff erent distinct classes of SExpr
-enum TIL_Opcode {
+enum TIL_Opcode : unsigned char {
#define TIL_OPCODE_DEF(X) COP_##X,
#include "ThreadSafetyOps.def"
#undef TIL_OPCODE_DEF
@@ -278,7 +278,7 @@ class SExpr {
public:
SExpr() = delete;
- TIL_Opcode opcode() const { return static_cast(Opcode); }
+ TIL_Opcode opcode() const { return Opcode; }
// Subclasses of SExpr must define the following:
//
@@ -321,7 +321,7 @@ class SExpr {
SExpr(TIL_Opcode Op) : Opcode(Op) {}
SExpr(const SExpr &E) : Opcode(E.Opcode), Flags(E.Flags) {}
- const unsigned char Opcode;
+ const TIL_Opcode Opcode;
unsigned char Reserved = 0;
unsigned short Flags = 0;
unsigned SExprID = 0;
@@ -332,7 +332,7 @@ class SExpr {
namespace ThreadSafetyTIL {
inline bool isTrivial(const SExpr *E) {
- unsigned Op = E->opcode();
+ TIL_Opcode Op = E->opcode();
return Op == COP_Variable || Op == COP_Literal || Op == COP_LiteralPtr;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 44ae49e - Thread safety analysis: Handle compound assignment and ->* overloads
Author: Aaron Puchert
Date: 2022-05-09T15:35:43+02:00
New Revision: 44ae49e1a72576ca6aa8835b3f72df9605516403
URL:
https://github.com/llvm/llvm-project/commit/44ae49e1a72576ca6aa8835b3f72df9605516403
DIFF:
https://github.com/llvm/llvm-project/commit/44ae49e1a72576ca6aa8835b3f72df9605516403.diff
LOG: Thread safety analysis: Handle compound assignment and ->* overloads
Like regular assignment, compound assignment operators can be assumed to
write to their left-hand side operand. So we strengthen the requirements
there. (Previously only the default read access had been required.)
Just like operator->, operator->* can also be assumed to dereference the
left-hand side argument, so we require read access to the pointee. This
will generate new warnings if the left-hand side has a pt_guarded_by
attribute. This overload is rarely used, but it was trivial to add, so
why not. (Supporting the builtin operator requires changes to the TIL.)
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D124966
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/Analysis/ThreadSafety.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
Removed:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 73fb5a4f3c1f5..a82ae8845e65c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -204,6 +204,9 @@ Improvements to Clang's diagnostics
``-Wimplicit-int``.
- ``-Wmisexpect`` warns when the branch weights collected during profiling
conflict with those added by ``llvm.expect``.
+- ``-Wthread-safety-analysis`` now considers overloaded compound assignment and
+ increment/decrement operators as writing to their first argument, thus
+ requiring an exclusive lock if the argument is guarded.
Non-comprehensive list of changes in this release
-
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index b8fe6000d9716..03bbf078d7e89 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1988,16 +1988,27 @@ void BuildLockset::VisitCallExpr(const CallExpr *Exp) {
examineArguments(CE->getDirectCallee(), CE->arg_begin(), CE->arg_end());
} else if (const auto *OE = dyn_cast(Exp)) {
-auto OEop = OE->getOperator();
+OverloadedOperatorKind OEop = OE->getOperator();
switch (OEop) {
- case OO_Equal: {
-const Expr *Target = OE->getArg(0);
-const Expr *Source = OE->getArg(1);
-checkAccess(Target, AK_Written);
-checkAccess(Source, AK_Read);
+ case OO_Equal:
+ case OO_PlusEqual:
+ case OO_MinusEqual:
+ case OO_StarEqual:
+ case OO_SlashEqual:
+ case OO_PercentEqual:
+ case OO_CaretEqual:
+ case OO_AmpEqual:
+ case OO_PipeEqual:
+ case OO_LessLessEqual:
+ case OO_GreaterGreaterEqual:
+checkAccess(OE->getArg(1), AK_Read);
+LLVM_FALLTHROUGH;
+ case OO_PlusPlus:
+ case OO_MinusMinus:
+checkAccess(OE->getArg(0), AK_Written);
break;
- }
case OO_Star:
+ case OO_ArrowStar:
case OO_Arrow:
case OO_Subscript:
if (!(OEop == OO_Star && OE->getNumArgs() > 1)) {
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index 9cd44fb07230d..ea229fef649b9 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -82,6 +82,9 @@ class SmartPtr {
T* ptr_;
};
+template
+U& operator->*(const SmartPtr& ptr, U T::*p) { return ptr->*p; }
+
// For testing destructor calls and cleanup.
class MyString {
@@ -4338,6 +4341,21 @@ class Data {
void operator()() { }
+ Data& operator+=(int);
+ Data& operator-=(int);
+ Data& operator*=(int);
+ Data& operator/=(int);
+ Data& operator%=(int);
+ Data& operator^=(int);
+ Data& operator&=(int);
+ Data& operator|=(int);
+ Data& operator<<=(int);
+ Data& operator>>=(int);
+ Data& operator++();
+ Data& operator++(int);
+ Data& operator--();
+ Data& operator--(int);
+
private:
int dat;
};
@@ -4404,6 +4422,20 @@ class Foo {
// expected-warning {{reading variable 'datap1_'
requires holding mutex 'mu_'}}
data_ = *datap2_; // expected-warning {{writing variable 'data_'
requires holding mutex 'mu_' exclusively}} \
// expected-warning {{reading the value pointed to
by 'datap2_' requires holding mutex 'mu_'}}
+data_ += 1; // expected-warning {{writing variable 'data_'
requires holding mutex 'mu_' exclusively}}
+data_ -= 1; // expected-warning {{writing variable 'data_'
requires holding mutex 'mu_' exclusively}}
+data_ *= 1; // expected-warning {{writing variable 'data_'
requ
[clang] 99d3582 - Comment parsing: Specify argument numbers for some block commands
Author: Aaron Puchert
Date: 2022-05-13T13:48:46+02:00
New Revision: 99d35826a043916b259a0e440a2aa5cabbad2773
URL:
https://github.com/llvm/llvm-project/commit/99d35826a043916b259a0e440a2aa5cabbad2773
DIFF:
https://github.com/llvm/llvm-project/commit/99d35826a043916b259a0e440a2aa5cabbad2773.diff
LOG: Comment parsing: Specify argument numbers for some block commands
The command traits have a member NumArgs for which all the parsing
infrastructure is in place, but no command was setting it to a value
other than 0. By doing so we get warnings when passing an empty
paragraph to \retval (the first argument is the return value, then comes
the description). We also take \xrefitem along for the ride, although as
the documentation states it's unlikely to be used directly.
Reviewed By: gribozavr2
Differential Revision: https://reviews.llvm.org/D125422
Added:
Modified:
clang/include/clang/AST/CommentCommands.td
clang/test/AST/ast-dump-comment.cpp
clang/test/Sema/warn-documentation.cpp
Removed:
diff --git a/clang/include/clang/AST/CommentCommands.td
b/clang/include/clang/AST/CommentCommands.td
index 7e962a4b4171b..d357ec1cf8b99 100644
--- a/clang/include/clang/AST/CommentCommands.td
+++ b/clang/include/clang/AST/CommentCommands.td
@@ -154,7 +154,7 @@ def Post : BlockCommand<"post">;
def Pre: BlockCommand<"pre">;
def Remark : BlockCommand<"remark">;
def Remarks: BlockCommand<"remarks">;
-def Retval : BlockCommand<"retval">;
+def Retval : BlockCommand<"retval"> { let NumArgs = 1; }
def Sa : BlockCommand<"sa">;
def See: BlockCommand<"see">;
def Since : BlockCommand<"since">;
@@ -162,7 +162,7 @@ def Test : BlockCommand<"test">;
def Todo : BlockCommand<"todo">;
def Version: BlockCommand<"version">;
def Warning: BlockCommand<"warning">;
-def XRefItem : BlockCommand<"xrefitem">;
+def XRefItem : BlockCommand<"xrefitem"> { let NumArgs = 3; }
// HeaderDoc commands
def Abstract : BlockCommand<"abstract"> { let IsBriefCommand = 1; }
def ClassDesign : RecordLikeDetailCommand<"classdesign">;
diff --git a/clang/test/AST/ast-dump-comment.cpp
b/clang/test/AST/ast-dump-comment.cpp
index 11c96024546e0..1936c732cb989 100644
--- a/clang/test/AST/ast-dump-comment.cpp
+++ b/clang/test/AST/ast-dump-comment.cpp
@@ -32,6 +32,13 @@ int Test_BlockCommandComment;
// CHECK-NEXT: ParagraphComment
// CHECK-NEXT: TextComment{{.*}} Text=" Aaa"
+/// \retval 42 Aaa
+int Test_BlockCommandComment_WithArgs();
+// CHECK: FunctionDecl{{.*}}Test_BlockCommandComment_WithArgs
+// CHECK:BlockCommandComment{{.*}} Name="retval" Arg[0]="42"
+// CHECK-NEXT: ParagraphComment
+// CHECK-NEXT: TextComment{{.*}} Text=" Aaa"
+
/// \param Aaa xxx
/// \param [in,out] Bbb yyy
void Test_ParamCommandComment(int Aaa, int Bbb);
diff --git a/clang/test/Sema/warn-documentation.cpp
b/clang/test/Sema/warn-documentation.cpp
index 353c94a47eb6f..570b5baf54029 100644
--- a/clang/test/Sema/warn-documentation.cpp
+++ b/clang/test/Sema/warn-documentation.cpp
@@ -189,6 +189,14 @@ int test_multiple_returns3(int);
int test_multiple_returns4(int);
+/// expected-warning@+1 {{empty paragraph passed to '\retval' command}}
+/// \retval 0
+int test_retval_no_paragraph();
+
+/// \retval 0 Everything is fine.
+int test_retval_fine();
+
+
// expected-warning@+1 {{'\param' command used in a comment that is not
attached to a function declaration}}
/// \param a Blah blah.
int test_param1_backslash;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] d3a4033 - Comment parsing: Allow inline commands to have 0 or more than 1 argument
Author: Aaron Puchert
Date: 2022-05-13T13:48:46+02:00
New Revision: d3a4033d6ee1d017e216ff7caeeeb5ca2e18a783
URL:
https://github.com/llvm/llvm-project/commit/d3a4033d6ee1d017e216ff7caeeeb5ca2e18a783
DIFF:
https://github.com/llvm/llvm-project/commit/d3a4033d6ee1d017e216ff7caeeeb5ca2e18a783.diff
LOG: Comment parsing: Allow inline commands to have 0 or more than 1 argument
That's required to support `\n`, but can also be used for other commands.
We already had the infrastructure in place to parse a varying number of
arguments, we simply needed to generalize it so that it would work not
only for block commands.
This should fix #55319.
Reviewed By: gribozavr2
Differential Revision: https://reviews.llvm.org/D125429
Added:
Modified:
clang/include/clang/AST/Comment.h
clang/include/clang/AST/CommentCommands.td
clang/include/clang/AST/CommentParser.h
clang/include/clang/AST/CommentSema.h
clang/include/clang/Basic/DiagnosticCommentKinds.td
clang/lib/AST/CommentParser.cpp
clang/lib/AST/CommentSema.cpp
clang/test/AST/ast-dump-comment.cpp
clang/test/Headers/x86-intrinsics-headers-clean.cpp
clang/test/Sema/warn-documentation.cpp
Removed:
diff --git a/clang/include/clang/AST/Comment.h
b/clang/include/clang/AST/Comment.h
index 5ecc35791b7b0..0b68c11316649 100644
--- a/clang/include/clang/AST/Comment.h
+++ b/clang/include/clang/AST/Comment.h
@@ -194,6 +194,11 @@ class Comment {
#include "clang/AST/CommentNodes.inc"
};
+ struct Argument {
+SourceRange Range;
+StringRef Text;
+ };
+
Comment(CommentKind K,
SourceLocation LocBegin,
SourceLocation LocEnd) :
@@ -296,13 +301,6 @@ class TextComment : public InlineContentComment {
/// A command with word-like arguments that is considered inline content.
class InlineCommandComment : public InlineContentComment {
public:
- struct Argument {
-SourceRange Range;
-StringRef Text;
-
-Argument(SourceRange Range, StringRef Text) : Range(Range), Text(Text) { }
- };
-
/// The most appropriate rendering mode for this command, chosen on command
/// semantics in Doxygen.
enum RenderKind {
@@ -588,15 +586,6 @@ class ParagraphComment : public BlockContentComment {
/// arguments depends on command name) and a paragraph as an argument
/// (e. g., \\brief).
class BlockCommandComment : public BlockContentComment {
-public:
- struct Argument {
-SourceRange Range;
-StringRef Text;
-
-Argument() { }
-Argument(SourceRange Range, StringRef Text) : Range(Range), Text(Text) { }
- };
-
protected:
/// Word-like arguments.
ArrayRef Args;
diff --git a/clang/include/clang/AST/CommentCommands.td
b/clang/include/clang/AST/CommentCommands.td
index d357ec1cf8b99..a3b9eb313fcf5 100644
--- a/clang/include/clang/AST/CommentCommands.td
+++ b/clang/include/clang/AST/CommentCommands.td
@@ -31,6 +31,7 @@ class Command {
}
class InlineCommand : Command {
+ let NumArgs = 1;
let IsInlineCommand = 1;
}
@@ -86,6 +87,7 @@ def C : InlineCommand<"c">;
def P : InlineCommand<"p">;
def A : InlineCommand<"a">;
def E : InlineCommand<"e">;
+def N : InlineCommand<"n"> { let NumArgs = 0; }
def Em : InlineCommand<"em">;
def Emoji : InlineCommand<"emoji">;
diff --git a/clang/include/clang/AST/CommentParser.h
b/clang/include/clang/AST/CommentParser.h
index 1a0cfb06e52b7..e11e818b1af0a 100644
--- a/clang/include/clang/AST/CommentParser.h
+++ b/clang/include/clang/AST/CommentParser.h
@@ -97,9 +97,8 @@ class Parser {
void parseTParamCommandArgs(TParamCommandComment *TPC,
TextTokenRetokenizer &Retokenizer);
- void parseBlockCommandArgs(BlockCommandComment *BC,
- TextTokenRetokenizer &Retokenizer,
- unsigned NumArgs);
+ ArrayRef
+ parseCommandArgs(TextTokenRetokenizer &Retokenizer, unsigned NumArgs);
BlockCommandComment *parseBlockCommand();
InlineCommandComment *parseInlineCommand();
diff --git a/clang/include/clang/AST/CommentSema.h
b/clang/include/clang/AST/CommentSema.h
index 015ce8f8652ac..5e30ff8adb915 100644
--- a/clang/include/clang/AST/CommentSema.h
+++ b/clang/include/clang/AST/CommentSema.h
@@ -128,16 +128,10 @@ class Sema {
void actOnTParamCommandFinish(TParamCommandComment *Command,
ParagraphComment *Paragraph);
- InlineCommandComment *actOnInlineCommand(SourceLocation CommandLocBegin,
- SourceLocation CommandLocEnd,
- unsigned CommandID);
-
InlineCommandComment *actOnInlineCommand(SourceLocation CommandLocBegin,
SourceLocation CommandLocEnd,
unsigned CommandID,
- SourceLocation ArgLocB
[clang] d2396d8 - Comment parsing: Treat properties as zero-argument inline commands
Author: Aaron Puchert
Date: 2022-05-13T13:48:46+02:00
New Revision: d2396d896ee12ad20bc740174edfce2120d742b2
URL:
https://github.com/llvm/llvm-project/commit/d2396d896ee12ad20bc740174edfce2120d742b2
DIFF:
https://github.com/llvm/llvm-project/commit/d2396d896ee12ad20bc740174edfce2120d742b2.diff
LOG: Comment parsing: Treat properties as zero-argument inline commands
That is more accurate, and using a separate class in TableGen seems
appropriate since these are not parts of the text but properties of the
declaration itself.
Reviewed By: gribozavr2
Differential Revision: https://reviews.llvm.org/D125473
Added:
Modified:
clang/include/clang/AST/CommentCommands.td
Removed:
diff --git a/clang/include/clang/AST/CommentCommands.td
b/clang/include/clang/AST/CommentCommands.td
index a3b9eb313fcf..e839031752cd 100644
--- a/clang/include/clang/AST/CommentCommands.td
+++ b/clang/include/clang/AST/CommentCommands.td
@@ -63,6 +63,11 @@ class VerbatimLineCommand : Command {
let IsVerbatimLineCommand = 1;
}
+class PropertyCommand : Command {
+ let NumArgs = 0;
+ let IsInlineCommand = 1;
+}
+
class DeclarationVerbatimLineCommand :
VerbatimLineCommand {
let IsDeclarationCommand = 1;
@@ -275,31 +280,6 @@ def Until: VerbatimLineCommand<"until">;
def NoOp : VerbatimLineCommand<"noop">;
-// These have actually no arguments, but we can treat them as line commands.
-def CallGraph : VerbatimLineCommand<"callgraph">;
-def HideCallGraph : VerbatimLineCommand<"hidecallgraph">;
-def CallerGraph : VerbatimLineCommand<"callergraph">;
-def HideCallerGraph : VerbatimLineCommand<"hidecallergraph">;
-def ShowInitializer : VerbatimLineCommand<"showinitializer">;
-def HideInitializer : VerbatimLineCommand<"hideinitializer">;
-def ShowRefBy : VerbatimLineCommand<"showrefby">;
-def HideRefBy : VerbatimLineCommand<"hiderefby">;
-def ShowRefs: VerbatimLineCommand<"showrefs">;
-def HideRefs: VerbatimLineCommand<"hiderefs">;
-
-// These also have no argument.
-def Private : VerbatimLineCommand<"private">;
-def Protected : VerbatimLineCommand<"protected">;
-def Public: VerbatimLineCommand<"public">;
-def Pure : VerbatimLineCommand<"pure">;
-def Static: VerbatimLineCommand<"static">;
-
-// These also have no argument.
-def NoSubgrouping: VerbatimLineCommand<"nosubgrouping">;
-def PrivateSection : VerbatimLineCommand<"privatesection">;
-def ProtectedSection : VerbatimLineCommand<"protectedsection">;
-def PublicSection: VerbatimLineCommand<"publicsection">;
-
// We might also build proper support for if/ifnot/else/elseif/endif.
def If : VerbatimLineCommand<"if">;
def IfNot : VerbatimLineCommand<"ifnot">;
@@ -311,6 +291,32 @@ def Endif : VerbatimLineCommand<"endif">;
def Cond: VerbatimLineCommand<"cond">;
def EndCond : VerbatimLineCommand<"endcond">;
+//===--===//
+// PropertyCommand
+//===--===//
+
+def CallGraph : PropertyCommand<"callgraph">;
+def HideCallGraph : PropertyCommand<"hidecallgraph">;
+def CallerGraph : PropertyCommand<"callergraph">;
+def HideCallerGraph : PropertyCommand<"hidecallergraph">;
+def ShowInitializer : PropertyCommand<"showinitializer">;
+def HideInitializer : PropertyCommand<"hideinitializer">;
+def ShowRefBy : PropertyCommand<"showrefby">;
+def HideRefBy : PropertyCommand<"hiderefby">;
+def ShowRefs: PropertyCommand<"showrefs">;
+def HideRefs: PropertyCommand<"hiderefs">;
+
+def Private : PropertyCommand<"private">;
+def Protected : PropertyCommand<"protected">;
+def Public: PropertyCommand<"public">;
+def Pure : PropertyCommand<"pure">;
+def Static: PropertyCommand<"static">;
+
+def NoSubgrouping: PropertyCommand<"nosubgrouping">;
+def PrivateSection : PropertyCommand<"privatesection">;
+def ProtectedSection : PropertyCommand<"protectedsection">;
+def PublicSection: PropertyCommand<"publicsection">;
+
//===--===//
// DeclarationVerbatimLineCommand
//===--===//
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 25862f5 - Try to disambiguate between overloads on Mac
Author: Aaron Puchert
Date: 2022-05-13T16:29:02+02:00
New Revision: 25862f53cce966cef2957825095861dec631d4f1
URL:
https://github.com/llvm/llvm-project/commit/25862f53cce966cef2957825095861dec631d4f1
DIFF:
https://github.com/llvm/llvm-project/commit/25862f53cce966cef2957825095861dec631d4f1.diff
LOG: Try to disambiguate between overloads on Mac
Presumably Mac has a different understanding of how long `long` is.
Should fix a build error introduced by D125429 that's not visible on
other architectures.
Added:
Modified:
clang/lib/AST/CommentParser.cpp
Removed:
diff --git a/clang/lib/AST/CommentParser.cpp b/clang/lib/AST/CommentParser.cpp
index 7bac1fb99b88..d78b3ace2bb8 100644
--- a/clang/lib/AST/CommentParser.cpp
+++ b/clang/lib/AST/CommentParser.cpp
@@ -414,7 +414,7 @@ InlineCommandComment *Parser::parseInlineCommand() {
if (Args.size() < Info->NumArgs) {
Diag(CommandTok.getEndLocation().getLocWithOffset(1),
diag::warn_doc_inline_command_not_enough_arguments)
-<< CommandTok.is(tok::at_command) << Info->Name << Args.size()
+<< CommandTok.is(tok::at_command) << Info->Name <<
(uint64_t)Args.size()
<< Info->NumArgs
<< SourceRange(CommandTok.getLocation(), CommandTok.getEndLocation());
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] ac7a9ef - Resolve overload ambiguity on Mac OS when printing size_t in diagnostics
Author: Aaron Puchert
Date: 2022-05-14T12:37:36+02:00
New Revision: ac7a9ef0ae3a5c63dc4e641f9912d8b659ebd720
URL:
https://github.com/llvm/llvm-project/commit/ac7a9ef0ae3a5c63dc4e641f9912d8b659ebd720
DIFF:
https://github.com/llvm/llvm-project/commit/ac7a9ef0ae3a5c63dc4e641f9912d8b659ebd720.diff
LOG: Resolve overload ambiguity on Mac OS when printing size_t in diagnostics
Precommit builds cover Linux and Windows, but this ambiguity would only
show up on Mac OS: there we have int32_t = int, int64_t = long long and
size_t = unsigned long. So printing a size_t, while successful on the
other two architectures, cannot be unambiguously resolved on Mac OS.
This is not really meant to support printing arguments of type long or
size_t, but more as a way to prevent build breakage that would not be
detected in precommit builds, as happened in D125429.
Technically we have no guarantee that one of these types has the 64 bits
that afdac5fbcb6a3 wanted to provide, so proposals are welcome. We do
have a guarantee though that these three types are different, so we
should be fine with overload resolution.
Reviewed By: aeubanks
Differential Revision: https://reviews.llvm.org/D125580
Added:
Modified:
clang/include/clang/Basic/Diagnostic.h
clang/lib/AST/CommentParser.cpp
Removed:
diff --git a/clang/include/clang/Basic/Diagnostic.h
b/clang/include/clang/Basic/Diagnostic.h
index dc1a0efe1c47..33ad0827c0ca 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -1404,7 +1404,13 @@ inline const StreamingDiagnostic &operator<<(const
StreamingDiagnostic &DB,
}
inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
- int64_t I) {
+ long I) {
+ DB.AddTaggedVal(I, DiagnosticsEngine::ak_sint);
+ return DB;
+}
+
+inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
+ long long I) {
DB.AddTaggedVal(I, DiagnosticsEngine::ak_sint);
return DB;
}
@@ -1426,7 +1432,13 @@ inline const StreamingDiagnostic &operator<<(const
StreamingDiagnostic &DB,
}
inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
- uint64_t I) {
+ unsigned long I) {
+ DB.AddTaggedVal(I, DiagnosticsEngine::ak_uint);
+ return DB;
+}
+
+inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
+ unsigned long long I) {
DB.AddTaggedVal(I, DiagnosticsEngine::ak_uint);
return DB;
}
diff --git a/clang/lib/AST/CommentParser.cpp b/clang/lib/AST/CommentParser.cpp
index d78b3ace2bb8..7bac1fb99b88 100644
--- a/clang/lib/AST/CommentParser.cpp
+++ b/clang/lib/AST/CommentParser.cpp
@@ -414,7 +414,7 @@ InlineCommandComment *Parser::parseInlineCommand() {
if (Args.size() < Info->NumArgs) {
Diag(CommandTok.getEndLocation().getLocWithOffset(1),
diag::warn_doc_inline_command_not_enough_arguments)
-<< CommandTok.is(tok::at_command) << Info->Name <<
(uint64_t)Args.size()
+<< CommandTok.is(tok::at_command) << Info->Name << Args.size()
<< Info->NumArgs
<< SourceRange(CommandTok.getLocation(), CommandTok.getEndLocation());
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Thread safety analysis: provide printSCFG definition. (PR #80277)
https://github.com/aaronpuchert approved this pull request. Thanks, looks good to me! https://github.com/llvm/llvm-project/pull/80277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
https://github.com/aaronpuchert edited https://github.com/llvm/llvm-project/pull/70024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -435,6 +435,86 @@ bool Sema::ConstantFoldAttrArgs(const AttributeCommonInfo
&CI,
return true;
}
+// Warn if parent function does not have builtin function format attribute.
+void Sema::DiagnoseMissingFormatAttributes(NamedDecl *FDecl,
+ SourceLocation Loc) {
+ if (!FDecl)
+return;
+
+ auto *FD = dyn_cast_or_null(FDecl);
+ if (!FD)
+return;
aaronpuchert wrote:
Unless I'm missing something, the only caller is already passing a non-null
`FunctionDecl`.
```suggestion
void Sema::DiagnoseMissingFormatAttributes(FunctionDecl *FD,
SourceLocation Loc) {
```
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
https://github.com/aaronpuchert commented: I assume this is meant to imitate the GCC warning of the same name, which I found pretty useful. Would be nice to have the same in Clang! https://github.com/llvm/llvm-project/pull/70024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-format-attribute %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wmissing-format-attribute %s
+
+#include
+#include
+
+va_list args;
+
+__attribute__((__format__ (__scanf__, 1, 4)))
+void foo(char *out, const size_t len, const char *format, ... /* args */)
+{
+vsnprintf(out, len, format, args); // expected-warning {{Function 'foo'
might be candidate for 'printf' format attribute}}
aaronpuchert wrote:
Add a test where we're calling the function with non-argument format string and
variadic arguments. Then the warning should not be emitted. Not sure about
mixing argument format string and non-argument variadic arguments and the other
way around, but these should probably not warn either, since we can't propagate
the attribute.
Also where we're taking a `va_list` as argument and forwarding that. This
should also warn.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -435,6 +435,86 @@ bool Sema::ConstantFoldAttrArgs(const AttributeCommonInfo
&CI,
return true;
}
+// Warn if parent function does not have builtin function format attribute.
+void Sema::DiagnoseMissingFormatAttributes(NamedDecl *FDecl,
+ SourceLocation Loc) {
+ if (!FDecl)
+return;
+
+ auto *FD = dyn_cast_or_null(FDecl);
+ if (!FD)
+return;
+
+ unsigned BuiltinID = FD->getBuiltinID(/*ConsiderWrappers=*/true);
+
+ // Function is not builtin if it's builtin ID is 0.
+ if (!BuiltinID)
+return;
+
+ // Check if function is one with format attribute.
+ switch (BuiltinID) {
+ case Builtin::BIprintf:
+ case Builtin::BIfprintf:
+ case Builtin::BIsprintf:
+ case Builtin::BIscanf:
+ case Builtin::BIfscanf:
+ case Builtin::BIsscanf:
+ case Builtin::BIvprintf:
+ case Builtin::BIvfprintf:
+ case Builtin::BIvsprintf:
+break;
+ default: {
+// In C99 mode check functions below.
+if (!getLangOpts().C99)
+ return;
+switch (BuiltinID) {
+case Builtin::BIsnprintf:
+case Builtin::BIvsnprintf:
+case Builtin::BIvscanf:
+case Builtin::BIvfscanf:
+case Builtin::BIvsscanf:
+ break;
+default:
+ return;
+}
+ }
+ }
+
+ Scope *ParentScope = getCurScope() ? getCurScope()->getFnParent() : nullptr;
+ if (!ParentScope)
+return;
+
+ DeclContext *ParentScopeEntity = ParentScope->getEntity();
+ if (!ParentScopeEntity)
+return;
+ if (ParentScopeEntity->getDeclKind() != Decl::Kind::Function)
+return;
+
+ FunctionDecl *ParentFuncDecl = static_cast(ParentScopeEntity);
+ if (!ParentFuncDecl)
+return;
+ if (!ParentFuncDecl->isVariadic())
+return;
aaronpuchert wrote:
Couldn't we use `getCurFunctionDecl`?
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-format-attribute %s +// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wmissing-format-attribute %s + +#include +#include + +va_list args; aaronpuchert wrote: This should be in the function body together with `va_start`. https://github.com/llvm/llvm-project/pull/70024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -435,6 +435,86 @@ bool Sema::ConstantFoldAttrArgs(const AttributeCommonInfo
&CI,
return true;
}
+// Warn if parent function does not have builtin function format attribute.
+void Sema::DiagnoseMissingFormatAttributes(NamedDecl *FDecl,
+ SourceLocation Loc) {
+ if (!FDecl)
+return;
+
+ auto *FD = dyn_cast_or_null(FDecl);
+ if (!FD)
+return;
+
+ unsigned BuiltinID = FD->getBuiltinID(/*ConsiderWrappers=*/true);
+
+ // Function is not builtin if it's builtin ID is 0.
+ if (!BuiltinID)
+return;
aaronpuchert wrote:
What about calling a regular function that also has a `format` attribute? The
GCC warning catches that as well.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -435,6 +435,86 @@ bool Sema::ConstantFoldAttrArgs(const AttributeCommonInfo
&CI,
return true;
}
+// Warn if parent function does not have builtin function format attribute.
+void Sema::DiagnoseMissingFormatAttributes(NamedDecl *FDecl,
+ SourceLocation Loc) {
+ if (!FDecl)
+return;
+
+ auto *FD = dyn_cast_or_null(FDecl);
+ if (!FD)
+return;
+
+ unsigned BuiltinID = FD->getBuiltinID(/*ConsiderWrappers=*/true);
+
+ // Function is not builtin if it's builtin ID is 0.
+ if (!BuiltinID)
+return;
+
+ // Check if function is one with format attribute.
+ switch (BuiltinID) {
+ case Builtin::BIprintf:
+ case Builtin::BIfprintf:
+ case Builtin::BIsprintf:
+ case Builtin::BIscanf:
+ case Builtin::BIfscanf:
+ case Builtin::BIsscanf:
+ case Builtin::BIvprintf:
+ case Builtin::BIvfprintf:
+ case Builtin::BIvsprintf:
+break;
+ default: {
+// In C99 mode check functions below.
+if (!getLangOpts().C99)
+ return;
+switch (BuiltinID) {
+case Builtin::BIsnprintf:
+case Builtin::BIvsnprintf:
+case Builtin::BIvscanf:
+case Builtin::BIvfscanf:
+case Builtin::BIvsscanf:
+ break;
+default:
+ return;
+}
+ }
+ }
+
+ Scope *ParentScope = getCurScope() ? getCurScope()->getFnParent() : nullptr;
+ if (!ParentScope)
+return;
+
+ DeclContext *ParentScopeEntity = ParentScope->getEntity();
+ if (!ParentScopeEntity)
+return;
+ if (ParentScopeEntity->getDeclKind() != Decl::Kind::Function)
+return;
+
+ FunctionDecl *ParentFuncDecl = static_cast(ParentScopeEntity);
+ if (!ParentFuncDecl)
+return;
+ if (!ParentFuncDecl->isVariadic())
+return;
+
+ // Iterate through builtin function format attributes. Then check
+ // if parent function has these attributes. If parent function does
+ // not have builtin function format attribut, emit warning.
+ for (const FormatAttr *Attr : FD->specific_attrs()) {
+bool hasFormatAttr = false;
+for (const FormatAttr *ParentAttr :
+ ParentFuncDecl->specific_attrs()) {
+ if (ParentAttr->getType() == Attr->getType()) {
+hasFormatAttr = true;
+break;
+ }
+}
+if (!hasFormatAttr) {
+ Diag(Loc, diag::warn_missing_format_attribute)
+ << ParentFuncDecl << Attr->getType();
aaronpuchert wrote:
Would be cool if we could add a fix-it. There are even ways to find macros that
wrap the attribute, see the implementation for `-Wimplicit-fallthrough`.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -435,6 +435,86 @@ bool Sema::ConstantFoldAttrArgs(const AttributeCommonInfo
&CI,
return true;
}
+// Warn if parent function does not have builtin function format attribute.
+void Sema::DiagnoseMissingFormatAttributes(NamedDecl *FDecl,
+ SourceLocation Loc) {
+ if (!FDecl)
+return;
+
+ auto *FD = dyn_cast_or_null(FDecl);
+ if (!FD)
+return;
+
+ unsigned BuiltinID = FD->getBuiltinID(/*ConsiderWrappers=*/true);
+
+ // Function is not builtin if it's builtin ID is 0.
+ if (!BuiltinID)
+return;
+
+ // Check if function is one with format attribute.
+ switch (BuiltinID) {
+ case Builtin::BIprintf:
+ case Builtin::BIfprintf:
+ case Builtin::BIsprintf:
+ case Builtin::BIscanf:
+ case Builtin::BIfscanf:
+ case Builtin::BIsscanf:
+ case Builtin::BIvprintf:
+ case Builtin::BIvfprintf:
+ case Builtin::BIvsprintf:
+break;
+ default: {
+// In C99 mode check functions below.
+if (!getLangOpts().C99)
+ return;
+switch (BuiltinID) {
+case Builtin::BIsnprintf:
+case Builtin::BIvsnprintf:
+case Builtin::BIvscanf:
+case Builtin::BIvfscanf:
+case Builtin::BIvsscanf:
+ break;
+default:
+ return;
+}
+ }
+ }
+
+ Scope *ParentScope = getCurScope() ? getCurScope()->getFnParent() : nullptr;
+ if (!ParentScope)
+return;
+
+ DeclContext *ParentScopeEntity = ParentScope->getEntity();
+ if (!ParentScopeEntity)
+return;
+ if (ParentScopeEntity->getDeclKind() != Decl::Kind::Function)
+return;
+
+ FunctionDecl *ParentFuncDecl = static_cast(ParentScopeEntity);
+ if (!ParentFuncDecl)
+return;
+ if (!ParentFuncDecl->isVariadic())
+return;
+
+ // Iterate through builtin function format attributes. Then check
+ // if parent function has these attributes. If parent function does
+ // not have builtin function format attribut, emit warning.
+ for (const FormatAttr *Attr : FD->specific_attrs()) {
+bool hasFormatAttr = false;
+for (const FormatAttr *ParentAttr :
+ ParentFuncDecl->specific_attrs()) {
+ if (ParentAttr->getType() == Attr->getType()) {
+hasFormatAttr = true;
+break;
+ }
+}
aaronpuchert wrote:
Seems like you could use `llvm::any_of`.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -435,6 +435,86 @@ bool Sema::ConstantFoldAttrArgs(const AttributeCommonInfo
&CI,
return true;
}
+// Warn if parent function does not have builtin function format attribute.
+void Sema::DiagnoseMissingFormatAttributes(NamedDecl *FDecl,
+ SourceLocation Loc) {
+ if (!FDecl)
+return;
+
+ auto *FD = dyn_cast_or_null(FDecl);
+ if (!FD)
+return;
+
+ unsigned BuiltinID = FD->getBuiltinID(/*ConsiderWrappers=*/true);
+
+ // Function is not builtin if it's builtin ID is 0.
+ if (!BuiltinID)
+return;
+
+ // Check if function is one with format attribute.
+ switch (BuiltinID) {
+ case Builtin::BIprintf:
+ case Builtin::BIfprintf:
+ case Builtin::BIsprintf:
+ case Builtin::BIscanf:
+ case Builtin::BIfscanf:
+ case Builtin::BIsscanf:
+ case Builtin::BIvprintf:
+ case Builtin::BIvfprintf:
+ case Builtin::BIvsprintf:
+break;
+ default: {
+// In C99 mode check functions below.
+if (!getLangOpts().C99)
+ return;
+switch (BuiltinID) {
+case Builtin::BIsnprintf:
+case Builtin::BIvsnprintf:
+case Builtin::BIvscanf:
+case Builtin::BIvfscanf:
+case Builtin::BIvsscanf:
+ break;
+default:
+ return;
+}
+ }
+ }
+
+ Scope *ParentScope = getCurScope() ? getCurScope()->getFnParent() : nullptr;
+ if (!ParentScope)
+return;
+
+ DeclContext *ParentScopeEntity = ParentScope->getEntity();
+ if (!ParentScopeEntity)
+return;
+ if (ParentScopeEntity->getDeclKind() != Decl::Kind::Function)
+return;
+
+ FunctionDecl *ParentFuncDecl = static_cast(ParentScopeEntity);
+ if (!ParentFuncDecl)
+return;
+ if (!ParentFuncDecl->isVariadic())
+return;
+
+ // Iterate through builtin function format attributes. Then check
+ // if parent function has these attributes. If parent function does
+ // not have builtin function format attribut, emit warning.
aaronpuchert wrote:
This seems to be missing an important ingredient, unless I'm overlooking it: we
only want to diagnose if the format string and the var args are forwarded.
Simply calling one of these functions does not mean that we need to propagate
the attribute, e.g. if we're calling it with a string literal.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -435,6 +435,86 @@ bool Sema::ConstantFoldAttrArgs(const AttributeCommonInfo
&CI,
return true;
}
+// Warn if parent function does not have builtin function format attribute.
+void Sema::DiagnoseMissingFormatAttributes(NamedDecl *FDecl,
+ SourceLocation Loc) {
+ if (!FDecl)
+return;
+
+ auto *FD = dyn_cast_or_null(FDecl);
+ if (!FD)
+return;
+
+ unsigned BuiltinID = FD->getBuiltinID(/*ConsiderWrappers=*/true);
+
+ // Function is not builtin if it's builtin ID is 0.
+ if (!BuiltinID)
+return;
+
+ // Check if function is one with format attribute.
+ switch (BuiltinID) {
+ case Builtin::BIprintf:
+ case Builtin::BIfprintf:
+ case Builtin::BIsprintf:
+ case Builtin::BIscanf:
+ case Builtin::BIfscanf:
+ case Builtin::BIsscanf:
+ case Builtin::BIvprintf:
+ case Builtin::BIvfprintf:
+ case Builtin::BIvsprintf:
+break;
+ default: {
+// In C99 mode check functions below.
+if (!getLangOpts().C99)
+ return;
+switch (BuiltinID) {
+case Builtin::BIsnprintf:
+case Builtin::BIvsnprintf:
+case Builtin::BIvscanf:
+case Builtin::BIvfscanf:
+case Builtin::BIvsscanf:
+ break;
aaronpuchert wrote:
Why don't we move them into the switch above? Presumably they can simply not be
used prior to C99? If we need this at all.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Catch missing format attributes (PR #70024)
@@ -435,6 +435,86 @@ bool Sema::ConstantFoldAttrArgs(const AttributeCommonInfo
&CI,
return true;
}
+// Warn if parent function does not have builtin function format attribute.
+void Sema::DiagnoseMissingFormatAttributes(NamedDecl *FDecl,
+ SourceLocation Loc) {
+ if (!FDecl)
+return;
+
+ auto *FD = dyn_cast_or_null(FDecl);
+ if (!FD)
+return;
+
+ unsigned BuiltinID = FD->getBuiltinID(/*ConsiderWrappers=*/true);
+
+ // Function is not builtin if it's builtin ID is 0.
+ if (!BuiltinID)
+return;
+
+ // Check if function is one with format attribute.
+ switch (BuiltinID) {
+ case Builtin::BIprintf:
+ case Builtin::BIfprintf:
+ case Builtin::BIsprintf:
+ case Builtin::BIscanf:
+ case Builtin::BIfscanf:
+ case Builtin::BIsscanf:
+ case Builtin::BIvprintf:
+ case Builtin::BIvfprintf:
+ case Builtin::BIvsprintf:
+break;
+ default: {
+// In C99 mode check functions below.
+if (!getLangOpts().C99)
+ return;
+switch (BuiltinID) {
+case Builtin::BIsnprintf:
+case Builtin::BIvsnprintf:
+case Builtin::BIvscanf:
+case Builtin::BIvfscanf:
+case Builtin::BIvsscanf:
+ break;
+default:
+ return;
+}
+ }
+ }
+
+ Scope *ParentScope = getCurScope() ? getCurScope()->getFnParent() : nullptr;
+ if (!ParentScope)
+return;
+
+ DeclContext *ParentScopeEntity = ParentScope->getEntity();
+ if (!ParentScopeEntity)
+return;
+ if (ParentScopeEntity->getDeclKind() != Decl::Kind::Function)
+return;
+
+ FunctionDecl *ParentFuncDecl = static_cast(ParentScopeEntity);
+ if (!ParentFuncDecl)
+return;
+ if (!ParentFuncDecl->isVariadic())
+return;
+
+ // Iterate through builtin function format attributes. Then check
+ // if parent function has these attributes. If parent function does
+ // not have builtin function format attribut, emit warning.
+ for (const FormatAttr *Attr : FD->specific_attrs()) {
+bool hasFormatAttr = false;
+for (const FormatAttr *ParentAttr :
+ ParentFuncDecl->specific_attrs()) {
+ if (ParentAttr->getType() == Attr->getType()) {
aaronpuchert wrote:
We probably also want to check the other attribute arguments.
https://github.com/llvm/llvm-project/pull/70024
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] f702a6f - Thread safety analysis: Improve documentation for ASSERT_CAPABILITY
Author: Russell Yanofsky Date: 2020-09-26T22:16:50+02:00 New Revision: f702a6fa7c9e4c0e2871b3d6657ce4dfa525ce52 URL: https://github.com/llvm/llvm-project/commit/f702a6fa7c9e4c0e2871b3d6657ce4dfa525ce52 DIFF: https://github.com/llvm/llvm-project/commit/f702a6fa7c9e4c0e2871b3d6657ce4dfa525ce52.diff LOG: Thread safety analysis: Improve documentation for ASSERT_CAPABILITY Previous description didn't actually state the effect the attribute has on thread safety analysis (causing analysis to assume the capability is held). Previous description was also ambiguous about (or slightly overstated) the noreturn assumption made by thread safety analysis, implying the assumption had to be true about the function's behavior in general, and not just its behavior in places where it's used. Stating the assumption specifically should avoid a perceived need to disable thread safety analysis in places where only asserting that a specific capability is held would be better. Reviewed By: aaronpuchert, vasild Differential Revision: https://reviews.llvm.org/D87629 Added: Modified: clang/docs/ThreadSafetyAnalysis.rst Removed: diff --git a/clang/docs/ThreadSafetyAnalysis.rst b/clang/docs/ThreadSafetyAnalysis.rst index e4a3342c02bd..651229f01d03 100644 --- a/clang/docs/ThreadSafetyAnalysis.rst +++ b/clang/docs/ThreadSafetyAnalysis.rst @@ -144,6 +144,9 @@ and data members. Users are *strongly advised* to define macros for the various attributes; example definitions can be found in :ref:`mutexheader`, below. The following documentation assumes the use of macros. +The attributes only control assumptions made by thread safety analysis and the +warnings it issues. They don't affect generated code or behavior at run-time. + For historical reasons, prior versions of thread safety used macro names that were very lock-centric. These macros have since been renamed to fit a more general capability model. The prior names are still in use, and will be @@ -447,10 +450,11 @@ ASSERT_CAPABILITY(...) and ASSERT_SHARED_CAPABILITY(...) *Previously:* ``ASSERT_EXCLUSIVE_LOCK``, ``ASSERT_SHARED_LOCK`` -These are attributes on a function or method that does a run-time test to see -whether the calling thread holds the given capability. The function is assumed -to fail (no return) if the capability is not held. See :ref:`mutexheader`, -below, for example uses. +These are attributes on a function or method which asserts the calling thread +already holds the given capability, for example by performing a run-time test +and terminating if the capability is not held. Presence of this annotation +causes the analysis to assume the capability is held after calls to the +annotated function. See :ref:`mutexheader`, below, for example uses. GUARDED_VAR and PT_GUARDED_VAR ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 4855018 - Fix sphinx warnings in AttributeReference, NFC
Author: Aaron Puchert Date: 2020-09-27T00:52:36+02:00 New Revision: 485501899d6c752ff05f4e045f7f89ace39ec413 URL: https://github.com/llvm/llvm-project/commit/485501899d6c752ff05f4e045f7f89ace39ec413 DIFF: https://github.com/llvm/llvm-project/commit/485501899d6c752ff05f4e045f7f89ace39ec413.diff LOG: Fix sphinx warnings in AttributeReference, NFC The previous attempt in d34c8c70 didn't help (the problem was missing indentation), and another issue was introduced by a51d51a0. Added: Modified: clang/include/clang/Basic/AttrDocs.td Removed: diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 644795d41c8c..46b6b643e3de 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -2949,7 +2949,7 @@ Attribute ``trivial_abi`` has no effect in the following cases: - Copy constructors and move constructors of the class are all deleted. - The class has a base class that is non-trivial for the purposes of calls. - The class has a non-static data member whose type is non-trivial for the -purposes of calls, which includes: + purposes of calls, which includes: - classes that are non-trivial for the purposes of calls - __weak-qualified types in Objective-C++ @@ -3694,11 +3694,10 @@ deprecated, only exists for compatibility purposes, and should not be used in new code. * ``swift_newtype(struct)`` means that a Swift struct will be created for this -typedef. + typedef. * ``swift_newtype(enum)`` means that a Swift enum will be created for this -ypedef. - + typedef. .. code-block:: c ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 916b750 - [CodeGen] Use existing EmitLambdaVLACapture (NFC)
Author: Aaron Puchert
Date: 2020-08-19T15:20:05+02:00
New Revision: 916b750a8d1ab47d41939b42bf1d6eeddbdef686
URL:
https://github.com/llvm/llvm-project/commit/916b750a8d1ab47d41939b42bf1d6eeddbdef686
DIFF:
https://github.com/llvm/llvm-project/commit/916b750a8d1ab47d41939b42bf1d6eeddbdef686.diff
LOG: [CodeGen] Use existing EmitLambdaVLACapture (NFC)
Added:
Modified:
clang/lib/CodeGen/CGStmt.cpp
Removed:
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index c2bd17a238d0..9dd79469b544 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -2418,8 +2418,7 @@ LValue CodeGenFunction::InitCapturedStruct(const
CapturedStmt &S) {
I != E; ++I, ++CurField) {
LValue LV = EmitLValueForFieldInitialization(SlotLV, *CurField);
if (CurField->hasCapturedVLAType()) {
- auto VAT = CurField->getCapturedVLAType();
- EmitStoreThroughLValue(RValue::get(VLASizeMap[VAT->getSizeExpr()]), LV);
+ EmitLambdaVLACapture(CurField->getCapturedVLAType(), LV);
} else {
EmitInitializerForField(*CurField, LV, *I);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 85fce44 - [Sema] Simplify ShouldDiagnoseUnusedDecl, NFC
Author: Aaron Puchert
Date: 2020-08-29T18:42:58+02:00
New Revision: 85fce449dc43447bf9d75163bda81e157f5b73e7
URL:
https://github.com/llvm/llvm-project/commit/85fce449dc43447bf9d75163bda81e157f5b73e7
DIFF:
https://github.com/llvm/llvm-project/commit/85fce449dc43447bf9d75163bda81e157f5b73e7.diff
LOG: [Sema] Simplify ShouldDiagnoseUnusedDecl, NFC
Instead of writing to a flag and then returning based on that flag we
can also return directly. The flag name also doesn't provide additional
information, it just reflects the name of the function (isReferenced).
Added:
Modified:
clang/lib/Sema/SemaDecl.cpp
Removed:
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index fba590f44c20..a9e6113dc7bb 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -1763,25 +1763,20 @@ static bool ShouldDiagnoseUnusedDecl(const NamedDecl
*D) {
if (D->isInvalidDecl())
return false;
- bool Referenced = false;
if (auto *DD = dyn_cast(D)) {
// For a decomposition declaration, warn if none of the bindings are
// referenced, instead of if the variable itself is referenced (which
// it is, by the bindings' expressions).
-for (auto *BD : DD->bindings()) {
- if (BD->isReferenced()) {
-Referenced = true;
-break;
- }
-}
+for (auto *BD : DD->bindings())
+ if (BD->isReferenced())
+return false;
} else if (!D->getDeclName()) {
return false;
} else if (D->isReferenced() || D->isUsed()) {
-Referenced = true;
+return false;
}
- if (Referenced || D->hasAttr() ||
- D->hasAttr())
+ if (D->hasAttr() || D->hasAttr())
return false;
if (isa(D))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] b4a2d36 - [Sema] ICK_Function_Conversion is a third kind conversion
Author: Aaron Puchert
Date: 2020-08-29T18:42:36+02:00
New Revision: b4a2d36c3f74ea5574cd03a9c1a704bcffb1869e
URL:
https://github.com/llvm/llvm-project/commit/b4a2d36c3f74ea5574cd03a9c1a704bcffb1869e
DIFF:
https://github.com/llvm/llvm-project/commit/b4a2d36c3f74ea5574cd03a9c1a704bcffb1869e.diff
LOG: [Sema] ICK_Function_Conversion is a third kind conversion
Not sure if this has any effect, but it was inconsistent before.
Reviewed By: rsmith
Differential Revision: https://reviews.llvm.org/D67113
Added:
Modified:
clang/lib/Sema/SemaOverload.cpp
Removed:
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index ec7c41e8ed09..21a9ad04d500 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -5515,7 +5515,6 @@ static bool CheckConvertedConstantConversions(Sema &S,
// conversions are fine.
switch (SCS.Second) {
case ICK_Identity:
- case ICK_Function_Conversion:
case ICK_Integral_Promotion:
case ICK_Integral_Conversion: // Narrowing conversions are checked elsewhere.
case ICK_Zero_Queue_Conversion:
@@ -5562,6 +5561,7 @@ static bool CheckConvertedConstantConversions(Sema &S,
case ICK_Function_To_Pointer:
llvm_unreachable("found a first conversion kind in Second");
+ case ICK_Function_Conversion:
case ICK_Qualification:
llvm_unreachable("found a third conversion kind in Second");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 8ca00c5 - Thread safety analysis: More consistent warning message
Author: Aaron Puchert
Date: 2020-09-01T23:16:05+02:00
New Revision: 8ca00c5cdc0b86a433b80db633f3ff46e6547895
URL:
https://github.com/llvm/llvm-project/commit/8ca00c5cdc0b86a433b80db633f3ff46e6547895
DIFF:
https://github.com/llvm/llvm-project/commit/8ca00c5cdc0b86a433b80db633f3ff46e6547895.diff
LOG: Thread safety analysis: More consistent warning message
Other warning messages for negative capabilities also mention their
kind, and the double space was ugly.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D84603
Added:
Modified:
clang/include/clang/Analysis/Analyses/ThreadSafety.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Analysis/ThreadSafety.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/test/SemaCXX/warn-thread-safety-analysis.cpp
clang/test/SemaCXX/warn-thread-safety-negative.cpp
Removed:
diff --git a/clang/include/clang/Analysis/Analyses/ThreadSafety.h
b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
index 0d3dda1256fbc..bfa9870a1e1f0 100644
--- a/clang/include/clang/Analysis/Analyses/ThreadSafety.h
+++ b/clang/include/clang/Analysis/Analyses/ThreadSafety.h
@@ -202,6 +202,14 @@ class ThreadSafetyHandler {
virtual void handleNegativeNotHeld(StringRef Kind, Name LockName, Name Neg,
SourceLocation Loc) {}
+ /// Warn when calling a function that a negative capability is not held.
+ /// \param D -- The decl for the function requiring the negative capability.
+ /// \param LockName -- The name for the lock expression, to be printed in the
+ /// diagnostic.
+ /// \param Loc -- The location of the protected operation.
+ virtual void handleNegativeNotHeld(const NamedDecl *D, Name LockName,
+ SourceLocation Loc) {}
+
/// Warn when a function is called while an excluded mutex is locked. For
/// example, the mutex may be locked inside the function.
/// \param Kind -- the capability's name parameter (role, mutex, etc).
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 7683d24d2821b..d856f784e0eea 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3477,6 +3477,9 @@ def warn_acquired_before_after_cycle : Warning<
def warn_acquire_requires_negative_cap : Warning<
"acquiring %0 '%1' requires negative capability '%2'">,
InGroup, DefaultIgnore;
+def warn_fun_requires_negative_cap : Warning<
+ "calling function %0 requires negative capability '%1'">,
+ InGroup, DefaultIgnore;
// Thread safety warnings on pass by reference
def warn_guarded_pass_by_reference : Warning<
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 1208eaf93e25d..64e0da9e64b12 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1641,8 +1641,7 @@ void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D,
const Expr *Exp,
// Otherwise the negative requirement must be propagated to the caller.
LDat = FSet.findLock(Analyzer->FactMan, Cp);
if (!LDat) {
- Analyzer->Handler.handleMutexNotHeld("", D, POK, Cp.toString(),
- LK_Shared, Loc);
+ Analyzer->Handler.handleNegativeNotHeld(D, Cp.toString(), Loc);
}
return;
}
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp
b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 5cc215e08ea83..37fd26d7c22d7 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -1892,6 +1892,13 @@ class ThreadSafetyReporter : public
clang::threadSafety::ThreadSafetyHandler {
Warnings.emplace_back(std::move(Warning), getNotes());
}
+ void handleNegativeNotHeld(const NamedDecl *D, Name LockName,
+ SourceLocation Loc) override {
+PartialDiagnosticAt Warning(
+Loc, S.PDiag(diag::warn_fun_requires_negative_cap) << D << LockName);
+Warnings.emplace_back(std::move(Warning), getNotes());
+ }
+
void handleFunExcludesLock(StringRef Kind, Name FunName, Name LockName,
SourceLocation Loc) override {
PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_fun_excludes_mutex)
diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
index bbcdd5b9e5f0c..91bd15def577d 100644
--- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -4985,7 +4985,7 @@ class Foo {
}
void bar() {
-bar2(); // expected-warning {{calling function 'bar2' requires
holding '!mu'}}
+bar2(); // expected-warning {{calling function 'bar2' requires
negative capability '!mu'}}
}
void bar2() EXCLUSIVE
[clang] 8544def - Thread safety analysis: Document how try-acquire is handled
Author: Aaron Puchert
Date: 2020-09-05T14:26:43+02:00
New Revision: 8544defdcb09c25c5958e5f5b5762e9b9046
URL:
https://github.com/llvm/llvm-project/commit/8544defdcb09c25c5958e5f5b5762e9b9046
DIFF:
https://github.com/llvm/llvm-project/commit/8544defdcb09c25c5958e5f5b5762e9b9046.diff
LOG: Thread safety analysis: Document how try-acquire is handled
I don't think this is obvious, since try-acquire seemingly contradicts
our usual requirements of "no conditional locking".
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D87065
Added:
Modified:
clang/docs/ThreadSafetyAnalysis.rst
Removed:
diff --git a/clang/docs/ThreadSafetyAnalysis.rst
b/clang/docs/ThreadSafetyAnalysis.rst
index ea8e98a1884b..b8d7d24275b9 100644
--- a/clang/docs/ThreadSafetyAnalysis.rst
+++ b/clang/docs/ThreadSafetyAnalysis.rst
@@ -414,6 +414,26 @@ The first argument must be ``true`` or ``false``, to
specify which return value
indicates success, and the remaining arguments are interpreted in the same way
as ``ACQUIRE``. See :ref:`mutexheader`, below, for example uses.
+Because the analysis doesn't support conditional locking, a capability is
+treated as acquired after the first branch on the return value of a try-acquire
+function.
+
+.. code-block:: c++
+
+ Mutex mu;
+ int a GUARDED_BY(mu);
+
+ void foo() {
+bool success = mu.TryLock();
+a = 0; // Warning, mu is not locked.
+if (success) {
+ a = 0; // Ok.
+ mu.Unlock();
+} else {
+ a = 0; // Warning, mu is not locked.
+}
+ }
+
ASSERT_CAPABILITY(...) and ASSERT_SHARED_CAPABILITY(...)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 16975a6 - Set InvalidDecl directly when deserializing a Decl
Author: Aaron Puchert
Date: 2020-09-05T14:26:43+02:00
New Revision: 16975a638df3cda95c677055120b23e689d96dcd
URL:
https://github.com/llvm/llvm-project/commit/16975a638df3cda95c677055120b23e689d96dcd
DIFF:
https://github.com/llvm/llvm-project/commit/16975a638df3cda95c677055120b23e689d96dcd.diff
LOG: Set InvalidDecl directly when deserializing a Decl
When parsing a C++17 binding declaration, we first create the
BindingDecls in Sema::ActOnDecompositionDeclarator, and then build the
DecompositionDecl in Sema::ActOnVariableDeclarator, so the contained
BindingDecls are never null. But when deserializing, we read the
DecompositionDecl with all properties before filling in the Bindings.
Among other things, reading a declaration reads whether it's invalid,
then calling setInvalidDecl which assumes that all bindings of the
DecompositionDecl are available, but that isn't the case.
Deserialization should just set all properties directly without invoking
subsequent functions, so we just set the flag without using the setter.
Fixes PR34960.
Reviewed By: rsmith
Differential Revision: https://reviews.llvm.org/D86207
Added:
Modified:
clang/lib/Serialization/ASTReaderDecl.cpp
clang/test/PCH/cxx1z-decomposition.cpp
Removed:
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 47b378f5727b..f5a66dc3c2d1 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -585,7 +585,7 @@ void ASTDeclReader::VisitDecl(Decl *D) {
Reader.getContext());
}
D->setLocation(ThisDeclLoc);
- D->setInvalidDecl(Record.readInt());
+ D->InvalidDecl = Record.readInt();
if (Record.readInt()) { // hasAttrs
AttrVec Attrs;
Record.readAttributes(Attrs);
diff --git a/clang/test/PCH/cxx1z-decomposition.cpp
b/clang/test/PCH/cxx1z-decomposition.cpp
index 2f817b4280de..914ce80c550d 100644
--- a/clang/test/PCH/cxx1z-decomposition.cpp
+++ b/clang/test/PCH/cxx1z-decomposition.cpp
@@ -2,11 +2,11 @@
// RUN: %clang_cc1 -pedantic -std=c++1z -include %s -verify %s
//
// With PCH:
-// RUN: %clang_cc1 -pedantic -std=c++1z -emit-pch %s -o %t
-// RUN: %clang_cc1 -pedantic -std=c++1z -include-pch %t -verify %s
+// RUN: %clang_cc1 -pedantic -std=c++1z -emit-pch
-fallow-pch-with-compiler-errors %s -o %t
+// RUN: %clang_cc1 -pedantic -std=c++1z -include-pch %t
-fallow-pch-with-compiler-errors -verify %s
-// RUN: %clang_cc1 -pedantic -std=c++1z -emit-pch -fpch-instantiate-templates
%s -o %t
-// RUN: %clang_cc1 -pedantic -std=c++1z -include-pch %t -verify %s
+// RUN: %clang_cc1 -pedantic -std=c++1z -emit-pch
-fallow-pch-with-compiler-errors -fpch-instantiate-templates %s -o %t
+// RUN: %clang_cc1 -pedantic -std=c++1z -include-pch %t
-fallow-pch-with-compiler-errors -verify %s
#ifndef HEADER
#define HEADER
@@ -22,6 +22,8 @@ constexpr int foo(Q &&q) {
return a * 10 + b;
}
+auto [noinit]; // expected-error{{decomposition declaration '[noinit]'
requires an initializer}}
+
#else
int arr[2];
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] b2ce79e - Thread safety analysis: ValueDecl in Project is non-null
Author: Aaron Puchert
Date: 2020-09-05T17:26:12+02:00
New Revision: b2ce79ef66157dd752e3864ece57915e23a73f5d
URL:
https://github.com/llvm/llvm-project/commit/b2ce79ef66157dd752e3864ece57915e23a73f5d
DIFF:
https://github.com/llvm/llvm-project/commit/b2ce79ef66157dd752e3864ece57915e23a73f5d.diff
LOG: Thread safety analysis: ValueDecl in Project is non-null
The constructor asserts that, use it in the ThreadSafetyAnalyzer.
Also note that the result of a cast<> cannot be null.
Added:
Modified:
clang/lib/Analysis/ThreadSafety.cpp
clang/lib/Analysis/ThreadSafetyCommon.cpp
Removed:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp
b/clang/lib/Analysis/ThreadSafety.cpp
index 1d4aabaaeb57..5b97265a6d8a 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1277,9 +1277,8 @@ bool ThreadSafetyAnalyzer::inCurrentScope(const
CapabilityExpr &CapE) {
if (const auto *P = dyn_cast(SExp)) {
if (!CurrentMethod)
return false;
-const auto *VD = P->clangDecl();
-if (VD)
- return VD->getDeclContext() == CurrentMethod->getDeclContext();
+const ValueDecl *VD = P->clangDecl();
+return VD->getDeclContext() == CurrentMethod->getDeclContext();
}
return false;
diff --git a/clang/lib/Analysis/ThreadSafetyCommon.cpp
b/clang/lib/Analysis/ThreadSafetyCommon.cpp
index 1b8c55e56d47..aee918576007 100644
--- a/clang/lib/Analysis/ThreadSafetyCommon.cpp
+++ b/clang/lib/Analysis/ThreadSafetyCommon.cpp
@@ -274,7 +274,7 @@ til::SExpr *SExprBuilder::translateDeclRefExpr(const
DeclRefExpr *DRE,
const auto *VD = cast(DRE->getDecl()->getCanonicalDecl());
// Function parameters require substitution and/or renaming.
- if (const auto *PV = dyn_cast_or_null(VD)) {
+ if (const auto *PV = dyn_cast(VD)) {
unsigned I = PV->getFunctionScopeIndex();
const DeclContext *D = PV->getDeclContext();
if (Ctx && Ctx->FunArgs) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
