[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -4912,3 +4922,279 @@ void AutoType::Profile(llvm::FoldingSetNodeID &ID, 
const ASTContext &Context) {
   Profile(ID, Context, getDeducedType(), getKeyword(), isDependentType(),
   getTypeConstraintConcept(), getTypeConstraintArguments());
 }
+
+FunctionEffect::~FunctionEffect() = default;
+
+bool FunctionEffect::diagnoseConversion(bool Adding, QualType OldType,
+FunctionEffectSet OldFX,
+QualType NewType,
+FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseRedeclaration(bool Adding,
+   const FunctionDecl &OldFunction,
+   FunctionEffectSet OldFX,
+   const FunctionDecl &NewFunction,
+   FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseMethodOverride(bool Adding,
+const CXXMethodDecl &OldMethod,
+FunctionEffectSet OldFX,
+const CXXMethodDecl &NewMethod,
+FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::canInferOnDecl(const Decl *Caller,
+FunctionEffectSet CallerFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseFunctionCall(bool Direct, const Decl *Caller,
+  FunctionEffectSet CallerFX,
+  CalleeDeclOrType Callee,
+  FunctionEffectSet CalleeFX) const {
+  return false;
+}
+
+const NoLockNoAllocEffect &NoLockNoAllocEffect::nolock_instance() {
+  static NoLockNoAllocEffect global(kNoLockTrue, "nolock");
+  return global;
+}
+
+const NoLockNoAllocEffect &NoLockNoAllocEffect::noalloc_instance() {
+  static NoLockNoAllocEffect global(kNoAllocTrue, "noalloc");
+  return global;
+}
+
+// TODO: Separate flags for noalloc
+NoLockNoAllocEffect::NoLockNoAllocEffect(EffectType Ty, const char *Name)
+: FunctionEffect(Ty,
+ kRequiresVerification | kVerifyCalls |
+ kInferrableOnCallees | kExcludeThrow | kExcludeCatch |
+ kExcludeObjCMessageSend | kExcludeStaticLocalVars |
+ kExcludeThreadLocalVars,
+ Name) {}
+
+NoLockNoAllocEffect::~NoLockNoAllocEffect() = default;
+
+std::string NoLockNoAllocEffect::attribute() const {
+  return std::string{"__attribute__((clang_"} + name().str() + "))";
+}
+
+bool NoLockNoAllocEffect::diagnoseConversion(bool Adding, QualType OldType,
+ FunctionEffectSet OldFX,
+ QualType NewType,
+ FunctionEffectSet NewFX) const {
+  // noalloc can't be added (spoofed) during a conversion, unless we have 
nolock
+  if (Adding) {
+if (!isNoLock()) {
+  for (const auto *Effect : OldFX) {
+if (Effect->type() == kNoLockTrue)
+  return false;
+  }
+}
+// nolock can't be added (spoofed) during a conversion.
+return true;
+  }
+  return false;
+}
+
+bool NoLockNoAllocEffect::diagnoseRedeclaration(bool Adding,
+const FunctionDecl 
&OldFunction,
+FunctionEffectSet OldFX,
+const FunctionDecl 
&NewFunction,
+FunctionEffectSet NewFX) const 
{
+  // nolock/noalloc can't be removed in a redeclaration
+  // adding -> false, removing -> true (diagnose)
+  return !Adding;
+}
+
+bool NoLockNoAllocEffect::diagnoseMethodOverride(
+bool Adding, const CXXMethodDecl &OldMethod, FunctionEffectSet OldFX,
+const CXXMethodDecl &NewMethod, FunctionEffectSet NewFX) const {
+  // nolock/noalloc can't be removed from an override
+  return !Adding;
+}
+
+bool NoLockNoAllocEffect::canInferOnDecl(const Decl *Caller,
+ FunctionEffectSet CallerFX) const {
+  // Does the Decl have nolock(false) / noalloc(false) ?
+  QualType QT;
+  if (isa(Caller)) {
+const auto *TSI = cast(Caller)->getSignatureAsWritten();
+QT = TSI->getType();
+  } else if (isa(Caller)) {
+QT = cast(Caller)->getType();
+  } else {
+return false;
+  }
+  if (QT->hasAttr(isNoLock() ? attr::Kind::NoLock : attr::Kind::NoAlloc)) {
+return false;
+  }
+
+  return true;
+}
+
+// TODO: Notice that we don't care about some of the parameters. Is the
+// interface overly general?
+bool NoLockNoAllocEffect::diagnoseFunctionCall(
+bool Direct, const Decl *Caller, FunctionEffectSet CallerFX,
+CalleeD

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -4912,3 +4922,279 @@ void AutoType::Profile(llvm::FoldingSetNodeID &ID, 
const ASTContext &Context) {
   Profile(ID, Context, getDeducedType(), getKeyword(), isDependentType(),
   getTypeConstraintConcept(), getTypeConstraintArguments());
 }
+
+FunctionEffect::~FunctionEffect() = default;
+
+bool FunctionEffect::diagnoseConversion(bool Adding, QualType OldType,
+FunctionEffectSet OldFX,
+QualType NewType,
+FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseRedeclaration(bool Adding,
+   const FunctionDecl &OldFunction,
+   FunctionEffectSet OldFX,
+   const FunctionDecl &NewFunction,
+   FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseMethodOverride(bool Adding,
+const CXXMethodDecl &OldMethod,
+FunctionEffectSet OldFX,
+const CXXMethodDecl &NewMethod,
+FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::canInferOnDecl(const Decl *Caller,
+FunctionEffectSet CallerFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseFunctionCall(bool Direct, const Decl *Caller,
+  FunctionEffectSet CallerFX,
+  CalleeDeclOrType Callee,
+  FunctionEffectSet CalleeFX) const {
+  return false;
+}
+
+const NoLockNoAllocEffect &NoLockNoAllocEffect::nolock_instance() {
+  static NoLockNoAllocEffect global(kNoLockTrue, "nolock");
+  return global;
+}

dougsonos wrote:

I did one whole implementation with both the true/false forms of the attributes 
as `AttributedType`. Along the way I found and had to hack around two bugs 
where the sugar gets stripped, e.g. on an `auto` lambda.

I got some guidance that the `true` forms of the attributes made sense as part 
of the canonical type, so (confusingly, I know):
- the `true` attribute triggers the addition of a global instance of a 
`NoLockNoAllocEffect` into a `FunctionEffectSet` as an optional part of the 
`FunctionProtoType`. It does not currently put an attribute on the type.
- the `false` attribute uses `AttributedType`. BTW another limitation of 
`AttributedType` is that it doesn't seem to hold anything more than the 
attribute kind, so if we went that way we'd probably need 3 kinds for `nolock` 
(true, false, type-dependent expression) and 3 for `noalloc`. The 
type-dependent expression might end up needing a whole new type-sugar class.

Even if `FunctionEffect` turns out to be just some bits controlling details of 
the behavior, rather than a vtable, it's still attractive to me to encapsulate 
it into a uniqued object with methods, to centralize all those behaviors.

There's a fair amount of code prepared for the possibility of new, unrelated 
effects (e.g. "TCB + types"), anticipating that FunctionEffectSet could hold 
multiple effects. Considering for example a codebase that with several TCB's, 
they would all have the same behaviors about calls within themselves, but need 
to be considered unique effects, e.g. a function in TCB A can't call a function 
that's only part of TCB Z. This is the motivation for the FunctionEffect 
abstraction.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -11100,6 +11136,48 @@ Attr 
*Sema::getImplicitCodeSegOrSectionAttrForFunction(const FunctionDecl *FD,
   return nullptr;
 }
 
+// Should only be called when getFunctionEffects() returns a non-empty set.
+// Decl should be a FunctionDecl or BlockDecl.
+void Sema::CheckAddCallableWithEffects(const Decl *D, FunctionEffectSet FX) {
+  if (!D->hasBody()) {
+if (const auto *FD = D->getAsFunction()) {
+  if (!FD->willHaveBody()) {
+return;
+  }
+}
+  }
+
+  if (Diags.getIgnoreAllWarnings() ||
+  (Diags.getSuppressSystemWarnings() &&
+   SourceMgr.isInSystemHeader(D->getLocation(
+return;
+
+  if (hasUncompilableErrorOccurred())
+return;
+
+  // For code in dependent contexts, we'll do this at instantiation time
+  // (??? This was copied from something else in AnalysisBasedWarnings ???)
+  if (cast(D)->isDependentContext()) {
+return;
+  }

dougsonos wrote:

Thanks. For now I've disabled this with a TODO and will see what happens when I 
next try the branch on our codebase.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -0,0 +1,84 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify %s

dougsonos wrote:

OK, maybe "syntax" then...

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

dougsonos wrote:

> Since you mention it is attached to the type, is it mangled then differently. 
> e.g.:
> Does the above f calls to 2 different functions?

This example is problematic because in this construct, `g` and `h` degenerate 
into function pointers, and inference doesn't work across function pointers. 
The RFC shows a technique for avoiding this (search for `nolock_fp`).

But let's try this:

```
template 
void f(T a) [[clang::nolock]] { a(); }

void m() {
auto g = []() [[clang::nolock]] {};
auto h = []() [[clang::nolock(false)]] {};

f(g);
f(h);
}

This showed:
- in `Sema::CheckAddCallableWithEffects`, I really do want to keep that line of 
code that skips functions in dependent contexts; otherwise `f` gets analyzed 
with some sort of placeholder type.
- there are two template instantiations of `f` (according to logging), for the 
separate lambdas
- the bug where `AttributedType` sugar gets lost on lambdas (when the 
"inferred" return type gets converted to a concrete one) happens here and the 
`nolock(false)` attribute is lost from `h`.

That bug defeats `h`'s wish to not have `nolock` inferred on it, so the 
analysis decides:
- `g` is verified safe
- `f` is verified safe
- `h` is inferred safe
- `f(h)` is verified safe
- `m` is safe

Good questions, thanks. I'm working on the others.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -11100,6 +11136,48 @@ Attr 
*Sema::getImplicitCodeSegOrSectionAttrForFunction(const FunctionDecl *FD,
   return nullptr;
 }
 
+// Should only be called when getFunctionEffects() returns a non-empty set.
+// Decl should be a FunctionDecl or BlockDecl.
+void Sema::CheckAddCallableWithEffects(const Decl *D, FunctionEffectSet FX) {
+  if (!D->hasBody()) {
+if (const auto *FD = D->getAsFunction()) {
+  if (!FD->willHaveBody()) {
+return;
+  }
+}
+  }
+
+  if (Diags.getIgnoreAllWarnings() ||
+  (Diags.getSuppressSystemWarnings() &&
+   SourceMgr.isInSystemHeader(D->getLocation(
+return;
+
+  if (hasUncompilableErrorOccurred())
+return;
+
+  // For code in dependent contexts, we'll do this at instantiation time
+  // (??? This was copied from something else in AnalysisBasedWarnings ???)
+  if (cast(D)->isDependentContext()) {
+return;
+  }

dougsonos wrote:

This actually did turn out to be important -- without this check, a templated 
`nolock` function would get checked using placeholder template parameters, 
leading to spurious diagnostics.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

dougsonos wrote:

> > * the bug where `AttributedType` sugar gets lost on lambdas (when the 
> > "inferred" return type gets converted to a concrete one) happens here and 
> > the `nolock(false)` attribute is lost from `h`.
> 
> Opened an issue for this (#85120) because it really does seem like a bug to 
> me.

Thanks, I have a stab at a fix for that, here, somewhere... will push it soon.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Rough first stab at addressing #85120 (PR #85147)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos created 
https://github.com/llvm/llvm-project/pull/85147

I pointed out this issue in the review for nolock/noalloc, and @Sirraide opened 
#85120 

Here are some (very rough) bits of code I'd written to try to address the loss 
of type sugar, plus a subsequent crash involving lambdas (can't remember 
details now, and the crash may not exist any more on main).

Just in case it helps.

>From 613f04e311f083c129acaa4598cbfd9894fe3805 Mon Sep 17 00:00:00 2001
From: Doug Wyatt 
Date: Wed, 13 Mar 2024 16:02:14 -0700
Subject: [PATCH] Rough first stab at addressing #85120

---
 clang/include/clang/AST/ASTContext.h |  5 +++
 clang/lib/AST/ASTContext.cpp | 62 +++-
 clang/lib/Sema/SemaExpr.cpp  |  2 +-
 clang/lib/Sema/TreeTransform.h   | 61 ++-
 4 files changed, 125 insertions(+), 5 deletions(-)

diff --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index ff6b64c7f72d57..ca90417c9bf70b 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1301,6 +1301,11 @@ class ASTContext : public RefCountedBase {
   /// Change the result type of a function type once it is deduced.
   void adjustDeducedFunctionResultType(FunctionDecl *FD, QualType ResultType);
 
+  /// Transform a function type to have the provided result type, preserving
+  /// AttributedType and MacroQualifiedType sugar.
+  QualType getFunctionTypeWithResultType(QualType OrigFuncType, QualType 
ResultType,
+const FunctionProtoType::ExtProtoInfo &EPI) const;
+
   /// Get a function type and produce the equivalent function type with the
   /// specified exception specification. Type sugar that can be present on a
   /// declaration of a function with an exception specification is permitted
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 5a8fae76a43a4d..035dc19ba7011d 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -3140,13 +3140,71 @@ const FunctionType 
*ASTContext::adjustFunctionType(const FunctionType *T,
   return cast(Result.getTypePtr());
 }
 
+// EPI is provided by the caller because in the case of 
adjustDeducedFunctionResultType, it
+// is copied entirely from the previous FunctionType, but with a block 
(ActOnBlockStmtExpr),
+// it is more complicated...
+QualType ASTContext::getFunctionTypeWithResultType(QualType OrigFuncType, 
QualType ResultType,
+  const FunctionProtoType::ExtProtoInfo &EPI) const
+{
+  // Might be wrapped in a macro qualified type.
+  if (const auto *MQT = dyn_cast(OrigFuncType)) {
+return getMacroQualifiedType(
+getFunctionTypeWithResultType(MQT->getUnderlyingType(), ResultType, 
EPI),
+MQT->getMacroIdentifier());
+  }
+
+  // Might have a calling-convention attribute.
+  if (const auto *AT = dyn_cast(OrigFuncType)) {
+return getAttributedType(
+AT->getAttrKind(),
+getFunctionTypeWithResultType(AT->getModifiedType(), ResultType, EPI),
+getFunctionTypeWithResultType(AT->getEquivalentType(), ResultType, 
EPI));
+  }
+  
+  // Anything else must be a function type. Rebuild it with the new return 
value.
+  const auto *FPT = OrigFuncType->castAs();
+  return getFunctionType(ResultType, FPT->getParamTypes(), EPI);
+}
+
+#if 0
+static void examineType(const char* prefix, QualType QT, const char* term)
+{
+  llvm::outs() << prefix;
+  if (const auto *MQT = dyn_cast(QT)) {
+examineType( "MacroQualifiedType <", MQT->getUnderlyingType(), ">");
+  } else if (const auto *AT = dyn_cast(QT)) {
+examineType("AttributedType <", AT->getEquivalentType(), ">");
+  } else {
+const auto *FPT = QT->castAs();
+assert(FPT);
+llvm::outs() << QT;
+  }
+  llvm::outs() << term;
+}
+#endif
+
 void ASTContext::adjustDeducedFunctionResultType(FunctionDecl *FD,
  QualType ResultType) {
   FD = FD->getMostRecentDecl();
   while (true) {
-const auto *FPT = FD->getType()->castAs();
+QualType OrigFuncType = FD->getType();
+const auto *FPT = OrigFuncType->castAs();
 FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
-FD->setType(getFunctionType(ResultType, FPT->getParamTypes(), EPI));
+#if 1 // my new way
+QualType NewFuncType = getFunctionTypeWithResultType(OrigFuncType, 
ResultType, EPI);
+#else // original way
+QualType NewFuncType = getFunctionType(ResultType, FPT->getParamTypes(), 
EPI);
+#endif
+/*llvm::outs() << "transform " << OrigFuncType << " -> " << NewFuncType << 
"\n";
+llvm::outs() << " isConstQualified " << OrigFuncType.isConstQualified() << 
NewFuncType.isConstQualified() << "\n";
+llvm::outs() << " isLocalConstQualified " << 
OrigFuncType.isLocalConstQualified() << NewFuncType.isLocalConstQualified() << 
"\n";
+llvm::outs() << " const method " << FPT->isConst() << 
NewFuncType->castAs()->isConst() << "\n";
+llvm::outs() << " canonical " << Ne

[clang] Rough first stab at addressing #85120 (PR #85147)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos converted_to_draft 
https://github.com/llvm/llvm-project/pull/85147
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Rough first stab at addressing #85120 (PR #85147)

[Doug Wyatt via cfe-commits]

dougsonos wrote:

> @dougsonos I may have some time to look into this since there are probably 
> (as always) some annoying edge cases—particularly w/ templates. Would you be 
> fine with me committing to this branch or would it be easier for you if I 
> made a separate branch for that? Either is fine by me.

It might be simplest if you make your own branch, borrowing whatever from mine 
helps. Thanks.

https://github.com/llvm/llvm-project/pull/85147
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -4912,3 +4922,279 @@ void AutoType::Profile(llvm::FoldingSetNodeID &ID, 
const ASTContext &Context) {
   Profile(ID, Context, getDeducedType(), getKeyword(), isDependentType(),
   getTypeConstraintConcept(), getTypeConstraintArguments());
 }
+
+FunctionEffect::~FunctionEffect() = default;
+
+bool FunctionEffect::diagnoseConversion(bool Adding, QualType OldType,
+FunctionEffectSet OldFX,
+QualType NewType,
+FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseRedeclaration(bool Adding,
+   const FunctionDecl &OldFunction,
+   FunctionEffectSet OldFX,
+   const FunctionDecl &NewFunction,
+   FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseMethodOverride(bool Adding,
+const CXXMethodDecl &OldMethod,
+FunctionEffectSet OldFX,
+const CXXMethodDecl &NewMethod,
+FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::canInferOnDecl(const Decl *Caller,
+FunctionEffectSet CallerFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseFunctionCall(bool Direct, const Decl *Caller,
+  FunctionEffectSet CallerFX,
+  CalleeDeclOrType Callee,
+  FunctionEffectSet CalleeFX) const {
+  return false;
+}
+
+const NoLockNoAllocEffect &NoLockNoAllocEffect::nolock_instance() {
+  static NoLockNoAllocEffect global(kNoLockTrue, "nolock");
+  return global;
+}

dougsonos wrote:

> The attribute itself appears to usually be stored in an AttributedTypeLoc 
> rather than as part of the type itself

Ah right, I remember now. I think my difficulty with getting to the attribute 
itself stemmed from `Expr` only having a `QualType`, no `TypeSourceInfo`. And 
for example, `TypePrinter` only has a `QualType` and thus can't print the 
details of an `Attr::AnnotateType`.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -10778,6 +10778,101 @@ def warn_imp_cast_drops_unaligned : Warning<
   "implicit cast from type %0 to type %1 drops __unaligned qualifier">,
   InGroup>;
 
+def warn_func_effect_allocates : Warning<
+  "'%0' function '%1' must not allocate or deallocate memory">,
+  InGroup;
+
+def note_func_effect_allocates : Note<
+  "'%1' cannot be inferred '%0' because it allocates/deallocates memory">;

dougsonos wrote:

Thanks. I think I added function names to diagnostics as a hack around the 
difficulty with template instantiation notes (which we discussed later in this 
file). Once that's resolved I should be able to remove them here.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -10778,6 +10778,101 @@ def warn_imp_cast_drops_unaligned : Warning<
   "implicit cast from type %0 to type %1 drops __unaligned qualifier">,
   InGroup>;
 
+def warn_func_effect_allocates : Warning<
+  "'%0' function '%1' must not allocate or deallocate memory">,
+  InGroup;
+
+def note_func_effect_allocates : Note<
+  "'%1' cannot be inferred '%0' because it allocates/deallocates memory">;
+
+def warn_func_effect_throws_or_catches : Warning<
+  "'%0' function '%1' must not throw or catch exceptions">,
+  InGroup;
+
+def note_func_effect_throws_or_catches : Note<
+  "'%1' cannot be inferred '%0' because it throws or catches exceptions">;
+
+def warn_func_effect_has_static_local : Warning<
+  "'%0' function '%1' must not have static locals">,
+  InGroup;
+
+def note_func_effect_has_static_local : Note<
+  "'%1' cannot be inferred '%0' because it has a static local">;
+
+def warn_func_effect_uses_thread_local : Warning<
+  "'%0' function '%1' must not use thread-local variables">,
+  InGroup;
+
+def note_func_effect_uses_thread_local : Note<
+  "'%1' cannot be inferred '%0' because it uses a thread-local variable">;
+
+def warn_func_effect_calls_objc : Warning<
+  "'%0' function '%1' must not access an ObjC method or property">,
+  InGroup;
+
+def note_func_effect_calls_objc : Note<
+  "'%1' cannot be inferred '%0' because it accesses an ObjC method or 
property">;
+
+def warn_func_effect_calls_disallowed_func : Warning<
+  "'%0' function '%1' must not call non-'%0' function '%2'">,
+  InGroup;
+
+// UNTESTED
+def warn_func_effect_calls_disallowed_expr : Warning<
+  "'%0' function '%1' must not call non-'%0' expression">,
+  InGroup;
+
+def note_func_effect_calls_disallowed_func : Note<
+  "'%1' cannot be inferred '%0' because it calls non-'%0' function '%2'">;
+
+def note_func_effect_call_extern : Note<
+  "'%1' cannot be inferred '%0' because it has no definition in this 
translation unit">;
+
+def note_func_effect_call_not_inferrable : Note<
+  "'%1' does not permit inference of '%0'">;
+
+def note_func_effect_call_virtual : Note<
+  "'%1' cannot be inferred '%0' because it is virtual">;
+
+def note_func_effect_call_func_ptr : Note<
+  "'%1' cannot be inferred '%0' because it is a function pointer">;
+
+// TODO: Not currently being generated
+// def warn_perf_annotation_implies_noexcept : Warning<
+//  "'%0' function should be declared noexcept">,
+//  InGroup;
+
+// TODO: Not currently being generated
+def warn_func_effect_false_on_type : Warning<
+  "only functions/methods/blocks may be declared %0(false)">,
+  InGroup;
+
+// TODO: can the usual template expansion notes be used?
+def note_func_effect_from_template : Note<
+  "in template expansion here">;

dougsonos wrote:

Right, the difficulty is that the analysis currently happens at the very end of 
Sema. Sema saves up all the `FunctionDecl` and `BlockDecl`'s to be checked, 
then at the end (AnalysisBasedWarnings), makes a pass through that list, only 
doing an AST traversal of those functions to be verified -- the list of which 
grows according to the need for inference. In that context I haven't worked out 
a way to obtain template instantiation notes for a given `FunctionDecl`.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -3922,6 +3922,42 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, 
NamedDecl *&OldD, Scope *S,
 return true;
   }
 
+  const auto OldFX = Old->getFunctionEffects();
+  const auto NewFX = New->getFunctionEffects();
+  if (OldFX != NewFX) {
+const auto Diffs = FunctionEffectSet::differences(OldFX, NewFX);
+for (const auto &Item : Diffs) {
+  const FunctionEffect *Effect = Item.first;
+  const bool Adding = Item.second;
+  if (Effect->diagnoseRedeclaration(Adding, *Old, OldFX, *New, NewFX)) {
+Diag(New->getLocation(),
+ diag::warn_mismatched_func_effect_redeclaration)
+<< Effect->name();
+Diag(Old->getLocation(), diag::note_previous_declaration);
+  }
+}
+
+const auto MergedFX = OldFX | NewFX;
+
+// Having diagnosed any problems, prevent further errors by applying the
+// merged set of effects to both declarations.
+auto applyMergedFX = [&](FunctionDecl *FD) {
+  const auto *FPT = FD->getType()->getAs();
+  FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
+  EPI.FunctionEffects = MergedFX;
+  QualType ModQT = Context.getFunctionType(FD->getReturnType(),
+   FPT->getParamTypes(), EPI);
+
+  FD->setType(ModQT);
+};
+
+applyMergedFX(Old);
+applyMergedFX(New);
+
+OldQType = Old->getType();

dougsonos wrote:

In prototyping the feature, it was really helpful to be able to redeclare 
things like `pthread_self()` as `nolock`. This was also envisioned as a viable 
bootstrapping technique.

Maybe, however, this is an undesirable hack. A workaround would be to create 
wrapper functions that are declared safe but call the unsafe function with 
diagnostics disabled. To facilitate this a bit more cleanly, we could add the 
concept of a "leaf" function (as with TCB). `nolock_leaf` or `nolock(leaf)` (?) 
could mean "tell my callers I am `nolock`, but don't verify me."

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)

[Doug Wyatt via cfe-commits]

@@ -3140,13 +3140,35 @@ const FunctionType 
*ASTContext::adjustFunctionType(const FunctionType *T,
   return cast(Result.getTypePtr());
 }
 
+QualType ASTContext::adjustFunctionResultType(QualType FunctionType,
+  QualType ResultType) {
+  // Might be wrapped in a macro qualified type.
+  if (const auto *MQT = dyn_cast(FunctionType)) {
+return getMacroQualifiedType(
+adjustFunctionResultType(MQT->getUnderlyingType(), ResultType),
+MQT->getMacroIdentifier());
+  }
+
+  // Might have a calling-convention attribute.
+  if (const auto *AT = dyn_cast(FunctionType)) {
+return getAttributedType(
+AT->getAttrKind(),
+adjustFunctionResultType(AT->getModifiedType(), ResultType),
+adjustFunctionResultType(AT->getEquivalentType(), ResultType));
+  }
+
+  // Anything else must be a function type. Rebuild it with the new return
+  // value.

dougsonos wrote:

> Also, I just noticed, this code seems to be modelled after 
> `getFunctionTypeWithExceptionSpec()`, which is like two functions down in the 
> same file and which does pretty much the same thing that this is doing here. 
> It might make sense to merge the two.

I was hacking just enough to make it work, apologies.

https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Bugfixes and improved support for `AttributedType`s in lambdas (PR #85325)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/85325
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

dougsonos wrote:

> > * Conversions between functions / function pointers with and without the 
> > attribute work in C++ but not in C and I'm a bit lost (there's a bunch of 
> > debug logging around this e.g. Sema::IsFunctionConversion and 
> > Sema::ImpCastExprToType)

I've probably been staring at this way too long, but here's what's going on. My 
test is:

```
void nolock(int) [[clang::nolock]];
void x() {
void (*fp_plain)(int);
fp_plain = nolock;
}
```

At the bottom of `checkPointerTypesForAssignment`:
```
  llvm::outs() << "checkPointerTypesForAssignment calling IsFunctionConversion 
LHS " << ltrans << "   RHS " << rtrans << "\n";
  if (!S.getLangOpts().CPlusPlus &&
  S.IsFunctionConversion(ltrans, rtrans, ltrans))
```
This prints `LHS void (int)   RHS void (int) __attribute__((clang_nolock))`

Then inside isFunctionConversion, I immediately log:
```
  llvm::outs() << "IsFunctionConversion: " << FromType << " -> " << ToType
   << "\n";
```
and it's `void (int) -> void (int) __attribute__((clang_nolock))`

Reconciliation of the FunctionEffectSets on the two types is needed, but I'm 
confused right from the beginning here; the naming of "From" and "To" seems 
backwards. Would appreciate any pointers.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

dougsonos wrote:

> > have you tried maybe simply not setting `Changed` to `true` (perhaps only 
> > in C mode)?
> 
> It’s worth pointing out that C also warns (and C23 straight-up _errors_) if 
> you replace `nolock` w/ `noreturn` iirc, so either that’s also a bug or 
> that’s intended; I think I was going to ask Aaron about this as he’s the C 
> expert, but I forgot about that too, again because of the `AttributedType` 
> stuff.

I was testing with `noreturn` earlier. It's hard to compare, because (at least 
in C23):
```
../clang/test/Sema/attr-nolock-wip.cpp:15:19: error: 'noreturn' attribute 
cannot be applied to types
   15 | void noret(int) [[noreturn]];
  |   ^
```

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

dougsonos wrote:

> > have you tried maybe simply not setting `Changed` to `true` (perhaps only 
> > in C mode)?

Just tried. In C mode, it works to do nothing about effect changes in 
isFunctionConversion (!).

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

dougsonos wrote:

> I would maybe try going with that then for now (and maybe add a comment about 
> that too); I’m not sure my function pointer example is really the same 
> situation, but I remember finding an example that was analogous but for 
> `noreturn` _somwhere_ in the test cases, but I don’t remember where.

Thanks. I just opened #85415 about the apparently reversed parameters to 
`IsFunctionConversion`

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -3922,6 +3922,42 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, 
NamedDecl *&OldD, Scope *S,
 return true;
   }
 
+  const auto OldFX = Old->getFunctionEffects();
+  const auto NewFX = New->getFunctionEffects();
+  if (OldFX != NewFX) {
+const auto Diffs = FunctionEffectSet::differences(OldFX, NewFX);
+for (const auto &Item : Diffs) {
+  const FunctionEffect *Effect = Item.first;
+  const bool Adding = Item.second;
+  if (Effect->diagnoseRedeclaration(Adding, *Old, OldFX, *New, NewFX)) {
+Diag(New->getLocation(),
+ diag::warn_mismatched_func_effect_redeclaration)
+<< Effect->name();
+Diag(Old->getLocation(), diag::note_previous_declaration);
+  }
+}
+
+const auto MergedFX = OldFX | NewFX;
+
+// Having diagnosed any problems, prevent further errors by applying the
+// merged set of effects to both declarations.
+auto applyMergedFX = [&](FunctionDecl *FD) {
+  const auto *FPT = FD->getType()->getAs();
+  FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
+  EPI.FunctionEffects = MergedFX;
+  QualType ModQT = Context.getFunctionType(FD->getReturnType(),
+   FPT->getParamTypes(), EPI);
+
+  FD->setType(ModQT);
+};
+
+applyMergedFX(Old);
+applyMergedFX(New);
+
+OldQType = Old->getType();

dougsonos wrote:

> > A workaround would be to create wrapper functions that are declared safe 
> > but call the unsafe function with diagnostics disabled.
> 
> I could see there being situations where you might want to be able to do 
> something like that (i.e. declare a function as safe even if the compiler 
> might think it’s unsafe because it calls unsafe functions), but I wonder if 
> you couldn’t just disable the diagnostics locally in that function (or even 
> just parts thereof) only using a `#pragma`.

Yes, though that could make it difficult to distinguish between
- calls that are truly unsafe but where disabling warnings is needed in the 
short term
- calls that are truly safe but where annotation hasn't yet caught up (e.g. 
https://developer.apple.com/documentation/kernel/1532191-vdsp_vadd )

On the other hand, it is practical to disable diagnostics through a macro 
including pragmas. The user could employ two different macros to disable 
diagnostics, and the macro names would express the difference between the two 
situations.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -3922,6 +3922,42 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, 
NamedDecl *&OldD, Scope *S,
 return true;
   }
 
+  const auto OldFX = Old->getFunctionEffects();
+  const auto NewFX = New->getFunctionEffects();
+  if (OldFX != NewFX) {
+const auto Diffs = FunctionEffectSet::differences(OldFX, NewFX);
+for (const auto &Item : Diffs) {
+  const FunctionEffect *Effect = Item.first;
+  const bool Adding = Item.second;
+  if (Effect->diagnoseRedeclaration(Adding, *Old, OldFX, *New, NewFX)) {
+Diag(New->getLocation(),
+ diag::warn_mismatched_func_effect_redeclaration)
+<< Effect->name();
+Diag(Old->getLocation(), diag::note_previous_declaration);
+  }
+}
+
+const auto MergedFX = OldFX | NewFX;
+
+// Having diagnosed any problems, prevent further errors by applying the
+// merged set of effects to both declarations.
+auto applyMergedFX = [&](FunctionDecl *FD) {
+  const auto *FPT = FD->getType()->getAs();
+  FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
+  EPI.FunctionEffects = MergedFX;
+  QualType ModQT = Context.getFunctionType(FD->getReturnType(),
+   FPT->getParamTypes(), EPI);
+
+  FD->setType(ModQT);
+};
+
+applyMergedFX(Old);
+applyMergedFX(New);
+
+OldQType = Old->getType();

dougsonos wrote:

> it’s better to avoid modifying the `Old` declaration

OK, I agree that that will make more sense.

> look at all declarations of a function (starting at the most recent one, 
> because that one will most likely have the attribute on it) 

There are quite a number of places that ask this question but I'll work through 
this. This is an interesting example:
```
void foo() {
auto* x = new int;
}
void foo() [[clang::nolock]];
void bar() [[clang::nolock]] {
foo();
}
```
I think that `foo()` ought to get verified because by the time verification 
happens, the redeclaration has been seen.

I'm having difficulty testing this though, because now that the `Old` 
declaration is untouched, `MergeFunctionDecl` is unhappy that `New` has a 
different canonical type. Would it make sense to add another check near the end 
like this one?
```
  // Check if the function types are compatible when pointer size address
  // spaces are ignored.
  if (Context.hasSameFunctionTypeIgnoringPtrSizes(OldQType, NewQType))
return false;
```

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -3922,6 +3922,42 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, 
NamedDecl *&OldD, Scope *S,
 return true;
   }
 
+  const auto OldFX = Old->getFunctionEffects();
+  const auto NewFX = New->getFunctionEffects();
+  if (OldFX != NewFX) {
+const auto Diffs = FunctionEffectSet::differences(OldFX, NewFX);
+for (const auto &Item : Diffs) {
+  const FunctionEffect *Effect = Item.first;
+  const bool Adding = Item.second;
+  if (Effect->diagnoseRedeclaration(Adding, *Old, OldFX, *New, NewFX)) {
+Diag(New->getLocation(),
+ diag::warn_mismatched_func_effect_redeclaration)
+<< Effect->name();
+Diag(Old->getLocation(), diag::note_previous_declaration);
+  }
+}
+
+const auto MergedFX = OldFX | NewFX;
+
+// Having diagnosed any problems, prevent further errors by applying the
+// merged set of effects to both declarations.
+auto applyMergedFX = [&](FunctionDecl *FD) {
+  const auto *FPT = FD->getType()->getAs();
+  FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
+  EPI.FunctionEffects = MergedFX;
+  QualType ModQT = Context.getFunctionType(FD->getReturnType(),
+   FPT->getParamTypes(), EPI);
+
+  FD->setType(ModQT);
+};
+
+applyMergedFX(Old);
+applyMergedFX(New);
+
+OldQType = Old->getType();

dougsonos wrote:

I've made `Sema::MergeFunctionDecl` use `OldQTypeForComparison` earlier so as 
to ignore effect differences in redeclarations, and 
`FunctionDecl::getFunctionEffects()` now collects and returns the union of 
effects present in all redeclarations.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -2380,6 +2382,1239 @@ class UnsafeBufferUsageReporter : public 
UnsafeBufferUsageHandler {
 };
 } // namespace
 
+// 
=
+
+// Temporary debugging option
+#define FX_ANALYZER_VERIFY_DECL_LIST 1
+
+namespace FXAnalysis {
+
+enum class DiagnosticID : uint8_t {
+  None = 0, // sentinel for an empty Diagnostic
+  Throws,
+  Catches,
+  CallsObjC,
+  AllocatesMemory,
+  HasStaticLocal,
+  AccessesThreadLocal,
+
+  // These only apply to callees, where the analysis stops at the Decl
+  DeclWithoutConstraintOrInference,
+
+  CallsUnsafeDecl,
+  CallsDisallowedExpr,
+};
+
+struct Diagnostic {
+  const FunctionEffect *Effect = nullptr;
+  const Decl *Callee = nullptr; // only valid for Calls*
+  SourceLocation Loc;
+  DiagnosticID ID = DiagnosticID::None;
+
+  Diagnostic() = default;
+
+  Diagnostic(const FunctionEffect *Effect, DiagnosticID ID, SourceLocation Loc,
+ const Decl *Callee = nullptr)
+  : Effect(Effect), Callee(Callee), Loc(Loc), ID(ID) {}
+};
+
+enum class SpecialFuncType : uint8_t { None, OperatorNew, OperatorDelete };
+enum class CallType {
+  Unknown,
+  Function,
+  Virtual,
+  Block
+  // unknown: probably function pointer
+};
+
+// Return whether the function CAN be verified.
+// The question of whether it SHOULD be verified is independent.
+static bool functionIsVerifiable(const FunctionDecl *FD) {
+  if (!(FD->hasBody() || FD->isInlined())) {
+// externally defined; we couldn't verify if we wanted to.
+return false;
+  }
+  if (FD->isTrivial()) {
+// Otherwise `struct x { int a; };` would have an unverifiable default
+// constructor.
+return true;
+  }
+  return true;
+}
+
+// Transitory, more extended information about a callable, which can be a
+// function, block, function pointer...
+struct CallableInfo {
+  const Decl *CDecl;
+  mutable std::optional
+  MaybeName; // mutable because built on demand in const method
+  SpecialFuncType FuncType = SpecialFuncType::None;
+  FunctionEffectSet Effects;
+  CallType CType = CallType::Unknown;
+
+  CallableInfo(const Decl &CD, SpecialFuncType FT = SpecialFuncType::None)
+  : CDecl(&CD), FuncType(FT) {
+// llvm::errs() << "CallableInfo " << name() << "\n";
+
+if (auto *FD = dyn_cast(CDecl)) {
+  assert(FD->getCanonicalDecl() == FD);
+  // Use the function's definition, if any.
+  if (auto *Def = FD->getDefinition()) {
+CDecl = FD = Def;
+  }
+  CType = CallType::Function;
+  if (auto *Method = dyn_cast(FD)) {
+if (Method->isVirtual()) {
+  CType = CallType::Virtual;
+}
+  }
+  Effects = FD->getFunctionEffects();
+} else if (auto *BD = dyn_cast(CDecl)) {
+  CType = CallType::Block;
+  Effects = BD->getFunctionEffects();
+} else if (auto *VD = dyn_cast(CDecl)) {
+  // ValueDecl is function, enum, or variable, so just look at the type.
+  Effects = FunctionEffectSet::get(*VD->getType());
+}
+  }
+
+  bool isDirectCall() const {
+return CType == CallType::Function || CType == CallType::Block;
+  }
+
+  bool isVerifiable() const {
+switch (CType) {
+case CallType::Unknown:
+case CallType::Virtual:
+  break;
+case CallType::Block:
+  return true;
+case CallType::Function:
+  return functionIsVerifiable(dyn_cast(CDecl));
+}
+return false;
+  }
+
+  /// Generate a name for logging and diagnostics.
+  std::string name(Sema &Sem) const {
+if (!MaybeName) {
+  std::string Name;
+  llvm::raw_string_ostream OS(Name);
+
+  if (auto *FD = dyn_cast(CDecl)) {
+FD->getNameForDiagnostic(OS, Sem.getPrintingPolicy(),
+ /*Qualified=*/true);
+  } else if (auto *BD = dyn_cast(CDecl)) {
+OS << "(block " << BD->getBlockManglingNumber() << ")";
+  } else if (auto *VD = dyn_cast(CDecl)) {
+VD->printQualifiedName(OS);
+  }
+  MaybeName = Name;
+}
+return *MaybeName;
+  }
+};
+
+// --
+// Map effects to single diagnostics.
+class EffectToDiagnosticMap {
+  // Since we currently only have a tiny number of effects (typically no more
+  // than 1), use a sorted SmallVector.

dougsonos wrote:

Done

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -4912,3 +4922,279 @@ void AutoType::Profile(llvm::FoldingSetNodeID &ID, 
const ASTContext &Context) {
   Profile(ID, Context, getDeducedType(), getKeyword(), isDependentType(),
   getTypeConstraintConcept(), getTypeConstraintArguments());
 }
+
+FunctionEffect::~FunctionEffect() = default;
+
+bool FunctionEffect::diagnoseConversion(bool Adding, QualType OldType,
+FunctionEffectSet OldFX,
+QualType NewType,
+FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseRedeclaration(bool Adding,
+   const FunctionDecl &OldFunction,
+   FunctionEffectSet OldFX,
+   const FunctionDecl &NewFunction,
+   FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseMethodOverride(bool Adding,
+const CXXMethodDecl &OldMethod,
+FunctionEffectSet OldFX,
+const CXXMethodDecl &NewMethod,
+FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::canInferOnDecl(const Decl *Caller,
+FunctionEffectSet CallerFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseFunctionCall(bool Direct, const Decl *Caller,
+  FunctionEffectSet CallerFX,
+  CalleeDeclOrType Callee,
+  FunctionEffectSet CalleeFX) const {
+  return false;
+}
+
+const NoLockNoAllocEffect &NoLockNoAllocEffect::nolock_instance() {
+  static NoLockNoAllocEffect global(kNoLockTrue, "nolock");
+  return global;
+}
+
+const NoLockNoAllocEffect &NoLockNoAllocEffect::noalloc_instance() {
+  static NoLockNoAllocEffect global(kNoAllocTrue, "noalloc");
+  return global;
+}
+
+// TODO: Separate flags for noalloc
+NoLockNoAllocEffect::NoLockNoAllocEffect(EffectType Ty, const char *Name)
+: FunctionEffect(Ty,
+ kRequiresVerification | kVerifyCalls |
+ kInferrableOnCallees | kExcludeThrow | kExcludeCatch |
+ kExcludeObjCMessageSend | kExcludeStaticLocalVars |
+ kExcludeThreadLocalVars,
+ Name) {}
+
+NoLockNoAllocEffect::~NoLockNoAllocEffect() = default;
+
+std::string NoLockNoAllocEffect::attribute() const {
+  return std::string{"__attribute__((clang_"} + name().str() + "))";
+}
+
+bool NoLockNoAllocEffect::diagnoseConversion(bool Adding, QualType OldType,
+ FunctionEffectSet OldFX,
+ QualType NewType,
+ FunctionEffectSet NewFX) const {
+  // noalloc can't be added (spoofed) during a conversion, unless we have 
nolock
+  if (Adding) {
+if (!isNoLock()) {
+  for (const auto *Effect : OldFX) {
+if (Effect->type() == kNoLockTrue)
+  return false;
+  }
+}
+// nolock can't be added (spoofed) during a conversion.
+return true;
+  }
+  return false;
+}
+
+bool NoLockNoAllocEffect::diagnoseRedeclaration(bool Adding,
+const FunctionDecl 
&OldFunction,
+FunctionEffectSet OldFX,
+const FunctionDecl 
&NewFunction,
+FunctionEffectSet NewFX) const 
{
+  // nolock/noalloc can't be removed in a redeclaration
+  // adding -> false, removing -> true (diagnose)
+  return !Adding;
+}
+
+bool NoLockNoAllocEffect::diagnoseMethodOverride(
+bool Adding, const CXXMethodDecl &OldMethod, FunctionEffectSet OldFX,
+const CXXMethodDecl &NewMethod, FunctionEffectSet NewFX) const {
+  // nolock/noalloc can't be removed from an override
+  return !Adding;
+}
+
+bool NoLockNoAllocEffect::canInferOnDecl(const Decl *Caller,
+ FunctionEffectSet CallerFX) const {
+  // Does the Decl have nolock(false) / noalloc(false) ?
+  QualType QT;
+  if (isa(Caller)) {
+const auto *TSI = cast(Caller)->getSignatureAsWritten();
+QT = TSI->getType();
+  } else if (isa(Caller)) {
+QT = cast(Caller)->getType();
+  } else {
+return false;
+  }
+  if (QT->hasAttr(isNoLock() ? attr::Kind::NoLock : attr::Kind::NoAlloc)) {
+return false;
+  }
+
+  return true;
+}
+
+// TODO: Notice that we don't care about some of the parameters. Is the
+// interface overly general?
+bool NoLockNoAllocEffect::diagnoseFunctionCall(
+bool Direct, const Decl *Caller, FunctionEffectSet CallerFX,
+CalleeD

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos converted_to_draft 
https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos closed 
https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

dougsonos wrote:

> > * Does the FunctionEffect / FunctionEffectSet abstraction make sense 
> > (Type.h)? The idea is that an abstract effect system for functions could 
> > easily support things like "TCB with types" or adding "nowait" to the 
> > "nolock/noalloc" group of effects.
> 
> Hmm, this is a difficult question. Generally, I would probably prefer to just 
> hard-code whatever effects we need right now, perhaps as something like a 
> bitmask; from what I recall, none of the effects take arguments or anything 
> like that, so that would be possible in that case (these are just my personal 
> observations, though; I don’t have every last detail of this feature 
> memorised, so if I’m missing something obvious, please let me know); my main 
> concern is that the current implementation might be a bit too general. In 
> particular, this seems like a lot of—if not too much—machinery for 
> implementing a grand total of two function attributes.
> 
> Because, sure, making it extensible doesn’t sound like a bad idea on paper, 
> but adding any effects would likely require significant modifications to 
> other parts of the compiler as well, so if a situation should arise where we 
> do need to handle more complex effects, we should be able to refactor it 
> whenever that comes up. For the time being, a simpler effect system may serve 
> us better (and should also be more straight-forward to refactor if need be).
> 
> Perhaps, let’s say that, unless it’s likely that we may end up adding effects 
> that require more storage than a single bit or two in the foreseeable future, 
> then I’d rather just have it be a bitmask. The way this is implemented 
> currently does remind me at least in part of attributes, but attributes are 
> very varied and _do_ take arguments, so I’m not sure the two systems are 
> really comparable.
> 
> @AaronBallman I recall you having some opinions on adding data to 
> `FunctionProtoType`s the other day, wdyt about this?



https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos reopened 
https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -1402,6 +1402,29 @@ def CXX11NoReturn : InheritableAttr {
   let Documentation = [CXX11NoReturnDocs];
 }
 
+def NoLock : DeclOrTypeAttr {
+  let Spellings = [CXX11<"clang", "nolock">,
+   C23<"clang", "nolock">,
+   GNU<"clang_nolock">];
+
+  // Subjects - not needed?
+  //let Subjects = SubjectList<[FunctionLike, Block, TypedefName], ErrorDiag>;

dougsonos wrote:

I was cribbing from AnnotateType which also has no Subjects. I added these 
tests, which seem to indicate that type attributes have some built-in defense.
```
int nl_var [[clang::nolock]]; // expected-warning {{ 'nolock' only applies to 
function types; type here is 'int' }}
struct nl_struct {} [[clang::nolock]]; // expected-warning {{ attribute 
'nolock' is ignored, place it after "struct" to apply attribute to type 
declaration }}
struct [[clang::nolock]] nl_struct2 {}; // expected-error {{ 'nolock' attribute 
cannot be applied to a declaration }}
```

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -10778,6 +10778,101 @@ def warn_imp_cast_drops_unaligned : Warning<
   "implicit cast from type %0 to type %1 drops __unaligned qualifier">,
   InGroup>;
 
+def warn_func_effect_allocates : Warning<
+  "'%0' function '%1' must not allocate or deallocate memory">,
+  InGroup;
+
+def note_func_effect_allocates : Note<
+  "'%1' cannot be inferred '%0' because it allocates/deallocates memory">;

dougsonos wrote:

Yeah, the code generating warnings/notes always puts the effect before the 
function name, so there's consistency there and inconsistency here:
```
def warn_func_effect_allocates : Warning<
  "'%0' function '%1' must not allocate or deallocate memory">,
  InGroup;

def note_func_effect_allocates : Note<
  "'%1' cannot be inferred '%0' because it allocates/deallocates memory">;
```

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -10778,6 +10778,101 @@ def warn_imp_cast_drops_unaligned : Warning<
   "implicit cast from type %0 to type %1 drops __unaligned qualifier">,
   InGroup>;
 
+def warn_func_effect_allocates : Warning<
+  "'%0' function '%1' must not allocate or deallocate memory">,
+  InGroup;
+
+def note_func_effect_allocates : Note<
+  "'%1' cannot be inferred '%0' because it allocates/deallocates memory">;
+
+def warn_func_effect_throws_or_catches : Warning<
+  "'%0' function '%1' must not throw or catch exceptions">,
+  InGroup;
+
+def note_func_effect_throws_or_catches : Note<
+  "'%1' cannot be inferred '%0' because it throws or catches exceptions">;
+
+def warn_func_effect_has_static_local : Warning<
+  "'%0' function '%1' must not have static locals">,
+  InGroup;
+
+def note_func_effect_has_static_local : Note<
+  "'%1' cannot be inferred '%0' because it has a static local">;
+
+def warn_func_effect_uses_thread_local : Warning<
+  "'%0' function '%1' must not use thread-local variables">,
+  InGroup;
+
+def note_func_effect_uses_thread_local : Note<
+  "'%1' cannot be inferred '%0' because it uses a thread-local variable">;
+
+def warn_func_effect_calls_objc : Warning<
+  "'%0' function '%1' must not access an ObjC method or property">,
+  InGroup;
+
+def note_func_effect_calls_objc : Note<
+  "'%1' cannot be inferred '%0' because it accesses an ObjC method or 
property">;
+
+def warn_func_effect_calls_disallowed_func : Warning<
+  "'%0' function '%1' must not call non-'%0' function '%2'">,
+  InGroup;
+
+// UNTESTED
+def warn_func_effect_calls_disallowed_expr : Warning<
+  "'%0' function '%1' must not call non-'%0' expression">,
+  InGroup;
+
+def note_func_effect_calls_disallowed_func : Note<
+  "'%1' cannot be inferred '%0' because it calls non-'%0' function '%2'">;
+
+def note_func_effect_call_extern : Note<
+  "'%1' cannot be inferred '%0' because it has no definition in this 
translation unit">;
+
+def note_func_effect_call_not_inferrable : Note<
+  "'%1' does not permit inference of '%0'">;
+
+def note_func_effect_call_virtual : Note<
+  "'%1' cannot be inferred '%0' because it is virtual">;
+
+def note_func_effect_call_func_ptr : Note<
+  "'%1' cannot be inferred '%0' because it is a function pointer">;
+
+// TODO: Not currently being generated
+// def warn_perf_annotation_implies_noexcept : Warning<
+//  "'%0' function should be declared noexcept">,
+//  InGroup;
+
+// TODO: Not currently being generated
+def warn_func_effect_false_on_type : Warning<
+  "only functions/methods/blocks may be declared %0(false)">,
+  InGroup;
+
+// TODO: can the usual template expansion notes be used?
+def note_func_effect_from_template : Note<
+  "in template expansion here">;

dougsonos wrote:

I do want to use those existing notes, but I couldn't figure out how to get the 
instantiation location (from the late stage of Sema where the diagnostics are 
being generated).

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -3666,11 +3673,14 @@ void FunctionProtoType::Profile(llvm::FoldingSetNodeID 
&ID, QualType Result,
   // Finally we have a trailing return type flag (bool)
   // combined with AArch64 SME Attributes, to save space:
   //  int
+  // Then add the FunctionEffects
   //
   // There is no ambiguity between the consumed arguments and an empty EH
   // spec because of the leading 'bool' which unambiguously indicates
   // whether the following bool is the EH spec or part of the arguments.
 
+  ID.AddPointer(epi.FunctionEffects.getOpaqueValue()); // TODO: Where???

dougsonos wrote:

Thanks; this call seems to be taking a lot of care about this but it does seem 
to relate to how various parts are optional.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -4912,3 +4922,279 @@ void AutoType::Profile(llvm::FoldingSetNodeID &ID, 
const ASTContext &Context) {
   Profile(ID, Context, getDeducedType(), getKeyword(), isDependentType(),
   getTypeConstraintConcept(), getTypeConstraintArguments());
 }
+
+FunctionEffect::~FunctionEffect() = default;
+
+bool FunctionEffect::diagnoseConversion(bool Adding, QualType OldType,
+FunctionEffectSet OldFX,
+QualType NewType,
+FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseRedeclaration(bool Adding,
+   const FunctionDecl &OldFunction,
+   FunctionEffectSet OldFX,
+   const FunctionDecl &NewFunction,
+   FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseMethodOverride(bool Adding,
+const CXXMethodDecl &OldMethod,
+FunctionEffectSet OldFX,
+const CXXMethodDecl &NewMethod,
+FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::canInferOnDecl(const Decl *Caller,
+FunctionEffectSet CallerFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseFunctionCall(bool Direct, const Decl *Caller,
+  FunctionEffectSet CallerFX,
+  CalleeDeclOrType Callee,
+  FunctionEffectSet CalleeFX) const {
+  return false;
+}
+
+const NoLockNoAllocEffect &NoLockNoAllocEffect::nolock_instance() {
+  static NoLockNoAllocEffect global(kNoLockTrue, "nolock");
+  return global;
+}

dougsonos wrote:

I would like it to be this simple, though of the current behaviors of 
noalloc/nolock, the one that seems extra-special is to detect that a function 
is declared `nolock(false)` or `noalloc(false)` (by examining the function's 
type sugar) and short-circuit any attempt to infer the attribute.

Of course that could just be one more bit of behavior, with the identity of the 
sugar attribute a hardcoded function of the effect type.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -4912,3 +4922,279 @@ void AutoType::Profile(llvm::FoldingSetNodeID &ID, 
const ASTContext &Context) {
   Profile(ID, Context, getDeducedType(), getKeyword(), isDependentType(),
   getTypeConstraintConcept(), getTypeConstraintArguments());
 }
+
+FunctionEffect::~FunctionEffect() = default;
+
+bool FunctionEffect::diagnoseConversion(bool Adding, QualType OldType,
+FunctionEffectSet OldFX,
+QualType NewType,
+FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseRedeclaration(bool Adding,
+   const FunctionDecl &OldFunction,
+   FunctionEffectSet OldFX,
+   const FunctionDecl &NewFunction,
+   FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseMethodOverride(bool Adding,
+const CXXMethodDecl &OldMethod,
+FunctionEffectSet OldFX,
+const CXXMethodDecl &NewMethod,
+FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::canInferOnDecl(const Decl *Caller,
+FunctionEffectSet CallerFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseFunctionCall(bool Direct, const Decl *Caller,
+  FunctionEffectSet CallerFX,
+  CalleeDeclOrType Callee,
+  FunctionEffectSet CalleeFX) const {
+  return false;
+}
+
+const NoLockNoAllocEffect &NoLockNoAllocEffect::nolock_instance() {
+  static NoLockNoAllocEffect global(kNoLockTrue, "nolock");
+  return global;
+}
+
+const NoLockNoAllocEffect &NoLockNoAllocEffect::noalloc_instance() {
+  static NoLockNoAllocEffect global(kNoAllocTrue, "noalloc");
+  return global;
+}
+
+// TODO: Separate flags for noalloc
+NoLockNoAllocEffect::NoLockNoAllocEffect(EffectType Ty, const char *Name)
+: FunctionEffect(Ty,
+ kRequiresVerification | kVerifyCalls |
+ kInferrableOnCallees | kExcludeThrow | kExcludeCatch |
+ kExcludeObjCMessageSend | kExcludeStaticLocalVars |
+ kExcludeThreadLocalVars,
+ Name) {}
+
+NoLockNoAllocEffect::~NoLockNoAllocEffect() = default;
+
+std::string NoLockNoAllocEffect::attribute() const {
+  return std::string{"__attribute__((clang_"} + name().str() + "))";
+}

dougsonos wrote:

TypePrinter.cpp
I thought about using the square bracket spelling but two (admittedly dumb) 
things discouraged me:
- in the Clang branch where I originally did the work, square bracket spellings 
weren't enabled in the default C dialect
- matching square brackets (in the test that checks an AST dump) requires extra 
backslashes

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -4912,3 +4922,279 @@ void AutoType::Profile(llvm::FoldingSetNodeID &ID, 
const ASTContext &Context) {
   Profile(ID, Context, getDeducedType(), getKeyword(), isDependentType(),
   getTypeConstraintConcept(), getTypeConstraintArguments());
 }
+
+FunctionEffect::~FunctionEffect() = default;
+
+bool FunctionEffect::diagnoseConversion(bool Adding, QualType OldType,
+FunctionEffectSet OldFX,
+QualType NewType,
+FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseRedeclaration(bool Adding,
+   const FunctionDecl &OldFunction,
+   FunctionEffectSet OldFX,
+   const FunctionDecl &NewFunction,
+   FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseMethodOverride(bool Adding,
+const CXXMethodDecl &OldMethod,
+FunctionEffectSet OldFX,
+const CXXMethodDecl &NewMethod,
+FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::canInferOnDecl(const Decl *Caller,
+FunctionEffectSet CallerFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseFunctionCall(bool Direct, const Decl *Caller,
+  FunctionEffectSet CallerFX,
+  CalleeDeclOrType Callee,
+  FunctionEffectSet CalleeFX) const {
+  return false;
+}
+
+const NoLockNoAllocEffect &NoLockNoAllocEffect::nolock_instance() {
+  static NoLockNoAllocEffect global(kNoLockTrue, "nolock");
+  return global;
+}
+
+const NoLockNoAllocEffect &NoLockNoAllocEffect::noalloc_instance() {
+  static NoLockNoAllocEffect global(kNoAllocTrue, "noalloc");
+  return global;
+}
+
+// TODO: Separate flags for noalloc
+NoLockNoAllocEffect::NoLockNoAllocEffect(EffectType Ty, const char *Name)
+: FunctionEffect(Ty,
+ kRequiresVerification | kVerifyCalls |
+ kInferrableOnCallees | kExcludeThrow | kExcludeCatch |
+ kExcludeObjCMessageSend | kExcludeStaticLocalVars |
+ kExcludeThreadLocalVars,
+ Name) {}
+
+NoLockNoAllocEffect::~NoLockNoAllocEffect() = default;
+
+std::string NoLockNoAllocEffect::attribute() const {
+  return std::string{"__attribute__((clang_"} + name().str() + "))";
+}
+
+bool NoLockNoAllocEffect::diagnoseConversion(bool Adding, QualType OldType,
+ FunctionEffectSet OldFX,
+ QualType NewType,
+ FunctionEffectSet NewFX) const {
+  // noalloc can't be added (spoofed) during a conversion, unless we have 
nolock
+  if (Adding) {
+if (!isNoLock()) {
+  for (const auto *Effect : OldFX) {
+if (Effect->type() == kNoLockTrue)
+  return false;
+  }
+}
+// nolock can't be added (spoofed) during a conversion.
+return true;
+  }
+  return false;
+}
+
+bool NoLockNoAllocEffect::diagnoseRedeclaration(bool Adding,
+const FunctionDecl 
&OldFunction,
+FunctionEffectSet OldFX,
+const FunctionDecl 
&NewFunction,
+FunctionEffectSet NewFX) const 
{
+  // nolock/noalloc can't be removed in a redeclaration
+  // adding -> false, removing -> true (diagnose)
+  return !Adding;
+}
+
+bool NoLockNoAllocEffect::diagnoseMethodOverride(
+bool Adding, const CXXMethodDecl &OldMethod, FunctionEffectSet OldFX,
+const CXXMethodDecl &NewMethod, FunctionEffectSet NewFX) const {
+  // nolock/noalloc can't be removed from an override
+  return !Adding;
+}
+
+bool NoLockNoAllocEffect::canInferOnDecl(const Decl *Caller,
+ FunctionEffectSet CallerFX) const {
+  // Does the Decl have nolock(false) / noalloc(false) ?
+  QualType QT;
+  if (isa(Caller)) {
+const auto *TSI = cast(Caller)->getSignatureAsWritten();
+QT = TSI->getType();
+  } else if (isa(Caller)) {
+QT = cast(Caller)->getType();
+  } else {
+return false;
+  }
+  if (QT->hasAttr(isNoLock() ? attr::Kind::NoLock : attr::Kind::NoAlloc)) {
+return false;
+  }
+
+  return true;
+}
+
+// TODO: Notice that we don't care about some of the parameters. Is the
+// interface overly general?
+bool NoLockNoAllocEffect::diagnoseFunctionCall(
+bool Direct, const Decl *Caller, FunctionEffectSet CallerFX,
+CalleeD

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -2380,6 +2382,1239 @@ class UnsafeBufferUsageReporter : public 
UnsafeBufferUsageHandler {
 };
 } // namespace
 
+// 
=
+
+// Temporary debugging option
+#define FX_ANALYZER_VERIFY_DECL_LIST 1
+
+namespace FXAnalysis {
+
+enum class DiagnosticID : uint8_t {
+  None = 0, // sentinel for an empty Diagnostic
+  Throws,
+  Catches,
+  CallsObjC,
+  AllocatesMemory,
+  HasStaticLocal,
+  AccessesThreadLocal,
+
+  // These only apply to callees, where the analysis stops at the Decl
+  DeclWithoutConstraintOrInference,
+
+  CallsUnsafeDecl,
+  CallsDisallowedExpr,
+};
+
+struct Diagnostic {
+  const FunctionEffect *Effect = nullptr;
+  const Decl *Callee = nullptr; // only valid for Calls*
+  SourceLocation Loc;
+  DiagnosticID ID = DiagnosticID::None;
+
+  Diagnostic() = default;
+
+  Diagnostic(const FunctionEffect *Effect, DiagnosticID ID, SourceLocation Loc,
+ const Decl *Callee = nullptr)
+  : Effect(Effect), Callee(Callee), Loc(Loc), ID(ID) {}
+};
+
+enum class SpecialFuncType : uint8_t { None, OperatorNew, OperatorDelete };
+enum class CallType {
+  Unknown,
+  Function,
+  Virtual,
+  Block
+  // unknown: probably function pointer
+};
+
+// Return whether the function CAN be verified.
+// The question of whether it SHOULD be verified is independent.
+static bool functionIsVerifiable(const FunctionDecl *FD) {
+  if (!(FD->hasBody() || FD->isInlined())) {
+// externally defined; we couldn't verify if we wanted to.
+return false;
+  }
+  if (FD->isTrivial()) {
+// Otherwise `struct x { int a; };` would have an unverifiable default
+// constructor.
+return true;
+  }
+  return true;
+}
+
+// Transitory, more extended information about a callable, which can be a
+// function, block, function pointer...
+struct CallableInfo {
+  const Decl *CDecl;
+  mutable std::optional
+  MaybeName; // mutable because built on demand in const method
+  SpecialFuncType FuncType = SpecialFuncType::None;
+  FunctionEffectSet Effects;
+  CallType CType = CallType::Unknown;
+
+  CallableInfo(const Decl &CD, SpecialFuncType FT = SpecialFuncType::None)
+  : CDecl(&CD), FuncType(FT) {
+// llvm::errs() << "CallableInfo " << name() << "\n";
+
+if (auto *FD = dyn_cast(CDecl)) {
+  assert(FD->getCanonicalDecl() == FD);
+  // Use the function's definition, if any.
+  if (auto *Def = FD->getDefinition()) {
+CDecl = FD = Def;
+  }
+  CType = CallType::Function;
+  if (auto *Method = dyn_cast(FD)) {
+if (Method->isVirtual()) {
+  CType = CallType::Virtual;
+}
+  }
+  Effects = FD->getFunctionEffects();
+} else if (auto *BD = dyn_cast(CDecl)) {
+  CType = CallType::Block;
+  Effects = BD->getFunctionEffects();
+} else if (auto *VD = dyn_cast(CDecl)) {
+  // ValueDecl is function, enum, or variable, so just look at the type.
+  Effects = FunctionEffectSet::get(*VD->getType());
+}
+  }
+
+  bool isDirectCall() const {
+return CType == CallType::Function || CType == CallType::Block;
+  }
+
+  bool isVerifiable() const {
+switch (CType) {
+case CallType::Unknown:
+case CallType::Virtual:
+  break;
+case CallType::Block:
+  return true;
+case CallType::Function:
+  return functionIsVerifiable(dyn_cast(CDecl));
+}
+return false;
+  }
+
+  /// Generate a name for logging and diagnostics.
+  std::string name(Sema &Sem) const {
+if (!MaybeName) {
+  std::string Name;
+  llvm::raw_string_ostream OS(Name);
+
+  if (auto *FD = dyn_cast(CDecl)) {
+FD->getNameForDiagnostic(OS, Sem.getPrintingPolicy(),
+ /*Qualified=*/true);
+  } else if (auto *BD = dyn_cast(CDecl)) {
+OS << "(block " << BD->getBlockManglingNumber() << ")";
+  } else if (auto *VD = dyn_cast(CDecl)) {
+VD->printQualifiedName(OS);
+  }
+  MaybeName = Name;
+}
+return *MaybeName;
+  }
+};
+
+// --
+// Map effects to single diagnostics.
+class EffectToDiagnosticMap {
+  // Since we currently only have a tiny number of effects (typically no more
+  // than 1), use a sorted SmallVector.
+  using Element = std::pair;
+  using ImplVec = llvm::SmallVector;
+  std::unique_ptr Impl;
+
+public:
+  Diagnostic &getOrInsertDefault(const FunctionEffect *Key) {
+if (Impl == nullptr) {
+  Impl = std::make_unique>();
+  auto &Item = Impl->emplace_back();
+  Item.first = Key;
+  return Item.second;
+}
+Element Elem(Key, {});
+auto Iter = _find(Elem);
+if (Iter != Impl->end() && Iter->first == Key) {
+  return Iter->second;
+}
+Iter = Impl->insert(Iter, Elem);
+return Iter->second;
+  }
+
+  const Diagnostic *lookup(const FunctionEffect *key) {
+if (Impl == nullptr) {
+  return nullptr;
+}
+Element elem(key, {});
+auto iter = _find(elem);
+if (iter != Im

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -395,6 +395,33 @@ bool Sema::checkStringLiteralArgumentAttr(const ParsedAttr 
&AL, unsigned ArgNum,
   return checkStringLiteralArgumentAttr(AL, ArgExpr, Str, ArgLocation);
 }
 
+/// Check if the argument \p ArgNum of \p Attr is a compile-time constant
+/// integer (boolean) expression. If not, emit an error and return false.
+bool Sema::checkBoolExprArgumentAttr(const ParsedAttr &AL, unsigned ArgNum,
+ bool &Value) {
+  if (AL.isInvalid()) {
+return false;
+  }
+  Expr *ArgExpr = AL.getArgAsExpr(ArgNum);
+  SourceLocation ErrorLoc(AL.getLoc());
+
+  if (AL.isArgIdent(ArgNum)) {
+IdentifierLoc *IL = AL.getArgAsIdent(ArgNum);
+ErrorLoc = IL->Loc;
+  } else if (ArgExpr != nullptr) {
+if (const std::optional MaybeVal =
+ArgExpr->getIntegerConstantExpr(Context, &ErrorLoc)) {
+  Value = MaybeVal->getBoolValue();
+  return true;
+}
+  }

dougsonos wrote:

Thank you! I was looking for precedents for this and couldn't find it on any 
attributes. It bugged me that I was writing something like this.

Yes, the parameter can be an arbitrary expression.

But if it is dependent: then how would we represent `nolock(expression)` before 
we can evaluate the expression? And when would it be collapsed back into a 
concretely true or false form?

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -339,6 +349,11 @@ namespace {
 
 bool didParseNoDeref() const { return parsedNoDeref; }
 
+void setParsedNolock(unsigned char v) { parsedNolock = v; }
+unsigned char getParsedNolock() const { return parsedNolock; }
+void setParsedNoalloc(unsigned char v) { parsedNoalloc = v; }
+unsigned char getParsedNoalloc() const { return parsedNoalloc; }

dougsonos wrote:

Yeah I thought I was trying to hide implementation details but this struct is 
local to the (giant) source file and it would definitely be better to just use 
the enum everywhere.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify %s
+// R UN: %clang_cc1 -fsyntax-only -fblocks -verify -x c -std=c23 %s
+
+#if !__has_attribute(clang_nolock)
+#error "the 'nolock' attribute is not available"
+#endif
+
+void unannotated(void);
+void nolock(void) [[clang::nolock]];

dougsonos wrote:

Wow, I never thought I'd see this day!

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -0,0 +1,84 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify %s

dougsonos wrote:

Thanks, yes, I will merge them (other than the one that is checking the AST 
dump).

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -4912,3 +4922,279 @@ void AutoType::Profile(llvm::FoldingSetNodeID &ID, 
const ASTContext &Context) {
   Profile(ID, Context, getDeducedType(), getKeyword(), isDependentType(),
   getTypeConstraintConcept(), getTypeConstraintArguments());
 }
+
+FunctionEffect::~FunctionEffect() = default;
+
+bool FunctionEffect::diagnoseConversion(bool Adding, QualType OldType,
+FunctionEffectSet OldFX,
+QualType NewType,
+FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseRedeclaration(bool Adding,
+   const FunctionDecl &OldFunction,
+   FunctionEffectSet OldFX,
+   const FunctionDecl &NewFunction,
+   FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseMethodOverride(bool Adding,
+const CXXMethodDecl &OldMethod,
+FunctionEffectSet OldFX,
+const CXXMethodDecl &NewMethod,
+FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::canInferOnDecl(const Decl *Caller,
+FunctionEffectSet CallerFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseFunctionCall(bool Direct, const Decl *Caller,
+  FunctionEffectSet CallerFX,
+  CalleeDeclOrType Callee,
+  FunctionEffectSet CalleeFX) const {
+  return false;
+}
+
+const NoLockNoAllocEffect &NoLockNoAllocEffect::nolock_instance() {
+  static NoLockNoAllocEffect global(kNoLockTrue, "nolock");
+  return global;
+}

dougsonos wrote:

Since `nolock(true)` needs to be part of a type, it becomes a `FunctionEffect` 
in a `FunctionEffectSet` attached to a `FunctionProtoType`. I looked at ways to 
defer `nolock(false)` off of the type and onto the `Decl`, but this violates a 
hard rule about placement of square-bracket attributes. So `nolock(false)` is 
implemented as type sugar, via `AttributedType`, even though it's ignored on 
types and only has meaning when attached to Decls.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -10778,6 +10778,101 @@ def warn_imp_cast_drops_unaligned : Warning<
   "implicit cast from type %0 to type %1 drops __unaligned qualifier">,
   InGroup>;
 
+def warn_func_effect_allocates : Warning<
+  "'%0' function '%1' must not allocate or deallocate memory">,
+  InGroup;
+
+def note_func_effect_allocates : Note<
+  "'%1' cannot be inferred '%0' because it allocates/deallocates memory">;

dougsonos wrote:

I decided I preferred inconsistency in DiagnosticSemaKinds.td than in the code 
that emits the diags:

```
S.Diag(Diag.Loc, diag::warn_func_effect_allocates)
<< effectName << TopFuncName;
...
S.Diag(Diag2.Loc, diag::note_func_effect_allocates)
<< effectName << CalleeName;
```

But maybe that's just a bit precious. As I was trying to word the diagnostics, 
there were times when they didn't always come out in consistent orders and I 
had bugs in passing the parameters in the right order.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -10778,6 +10778,101 @@ def warn_imp_cast_drops_unaligned : Warning<
   "implicit cast from type %0 to type %1 drops __unaligned qualifier">,
   InGroup>;
 
+def warn_func_effect_allocates : Warning<
+  "'%0' function '%1' must not allocate or deallocate memory">,
+  InGroup;
+
+def note_func_effect_allocates : Note<
+  "'%1' cannot be inferred '%0' because it allocates/deallocates memory">;

dougsonos wrote:

Another way to put it: when one reads through the ~100 lines of diagnostics in 
DiagnosticSemaKinds.td, I find it helpful that %0 is always the name of the 
effect and %1 is always a function (or block) name.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -395,6 +395,33 @@ bool Sema::checkStringLiteralArgumentAttr(const ParsedAttr 
&AL, unsigned ArgNum,
   return checkStringLiteralArgumentAttr(AL, ArgExpr, Str, ArgLocation);
 }
 
+/// Check if the argument \p ArgNum of \p Attr is a compile-time constant
+/// integer (boolean) expression. If not, emit an error and return false.
+bool Sema::checkBoolExprArgumentAttr(const ParsedAttr &AL, unsigned ArgNum,
+ bool &Value) {
+  if (AL.isInvalid()) {
+return false;
+  }
+  Expr *ArgExpr = AL.getArgAsExpr(ArgNum);
+  SourceLocation ErrorLoc(AL.getLoc());
+
+  if (AL.isArgIdent(ArgNum)) {
+IdentifierLoc *IL = AL.getArgAsIdent(ArgNum);
+ErrorLoc = IL->Loc;
+  } else if (ArgExpr != nullptr) {
+if (const std::optional MaybeVal =
+ArgExpr->getIntegerConstantExpr(Context, &ErrorLoc)) {
+  Value = MaybeVal->getBoolValue();
+  return true;
+}
+  }

dougsonos wrote:

Wow. I'm going to have to build a test to explore this. Thanks.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos deleted 
https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -4912,3 +4922,279 @@ void AutoType::Profile(llvm::FoldingSetNodeID &ID, 
const ASTContext &Context) {
   Profile(ID, Context, getDeducedType(), getKeyword(), isDependentType(),
   getTypeConstraintConcept(), getTypeConstraintArguments());
 }
+
+FunctionEffect::~FunctionEffect() = default;
+
+bool FunctionEffect::diagnoseConversion(bool Adding, QualType OldType,
+FunctionEffectSet OldFX,
+QualType NewType,
+FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseRedeclaration(bool Adding,
+   const FunctionDecl &OldFunction,
+   FunctionEffectSet OldFX,
+   const FunctionDecl &NewFunction,
+   FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseMethodOverride(bool Adding,
+const CXXMethodDecl &OldMethod,
+FunctionEffectSet OldFX,
+const CXXMethodDecl &NewMethod,
+FunctionEffectSet NewFX) const {
+  return false;
+}
+
+bool FunctionEffect::canInferOnDecl(const Decl *Caller,
+FunctionEffectSet CallerFX) const {
+  return false;
+}
+
+bool FunctionEffect::diagnoseFunctionCall(bool Direct, const Decl *Caller,
+  FunctionEffectSet CallerFX,
+  CalleeDeclOrType Callee,
+  FunctionEffectSet CalleeFX) const {
+  return false;
+}
+
+const NoLockNoAllocEffect &NoLockNoAllocEffect::nolock_instance() {
+  static NoLockNoAllocEffect global(kNoLockTrue, "nolock");
+  return global;
+}

dougsonos wrote:

The GNU syntax is more liberal, and yes, this is supported (and tested as of my 
next push):
```
// Alternate placement on the FunctionDecl
__attribute__((clang_nolock)) void nl_function();
// CHECK: FunctionDecl {{.*}} nl_function 'void () 
__attribute__((clang_nolock))'
```

One reason I prefer the square-bracket syntax is that it is not only consistent 
with `noexcept`, but it becomes practical to apply both `nolock` and `noexcept` 
with a single macro (not sure yet I want to do that to my codebase, but it's 
possible).

Another is that the placement rules seem more consistent and sensible, to me 
anyhow.

The `nolock/noalloc(true)` attributes do need to apply to function types, not 
declarations, so as to be able to analyze indirect calls. They only applies to 
declarations as a consequence of declarations having function types. 
`nolock/noalloc(false)` have to be treated the same way as the `true` form for 
parsing, but for analysis purposes, they end up being ignored until they land 
on a `Decl`, since indirect calls prevent inference regardless.

As for the rule about square brackets, in SemaType.cpp:
```
FUNCTION_TYPE_ATTRS_CASELIST:
  attr.setUsedAsTypeAttr();

  // Attributes with standard syntax have strict rules for what they
  // appertain to and hence should not use the "distribution" logic below.
  if (attr.isStandardAttributeSyntax() ||
  attr.isRegularKeywordAttribute()) {
if (!handleFunctionTypeAttr(state, attr, type, CFT)) {
  diagnoseBadTypeAttribute(state.getSema(), attr, type);
  attr.setInvalid();
}
break;
  }
```
So with square-bracket syntax, the attribute has to follow the right 
parenthesis enclosing the parameter list (and after const).

Incidentally, I realized that in Attr.td the attributes can derive from 
`TypeAttr` rather than `DeclOrTypeAttr`.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nolock/noalloc attributes (PR #84983)

[Doug Wyatt via cfe-commits]

@@ -0,0 +1,84 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify %s

dougsonos wrote:

Hm, the standard diagnostic for diagnosing incompatible attributes produces an 
error, and the last-pass constraint analysis is skipped when there are errors. 
I'm going to keep those constraint analysis tests in a separate file and give 
the files more descriptive names, e.g. -parsing, -sema, -constraints.

https://github.com/llvm/llvm-project/pull/84983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -4932,6 +4938,78 @@ class FunctionEffectsRef {
   void dump(llvm::raw_ostream &OS) const;
 };
 
+/// A mutable set of FunctionEffect::Kind.
+class FunctionEffectKindSet {
+  // For now this only needs to be a bitmap.
+  constexpr static size_t EndBitPos = 8;
+  using KindBitsT = std::bitset;
+
+  KindBitsT KindBits{};
+
+  explicit FunctionEffectKindSet(KindBitsT KB) : KindBits(KB) {}
+
+  constexpr static size_t kindToPos(FunctionEffect::Kind K) {
+return size_t(K);

dougsonos wrote:

Oops, committed that before realizing it doesn't build -- and I'm not quite 
sure what you intended.

https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -10950,6 +10950,51 @@ def warn_imp_cast_drops_unaligned : Warning<
   InGroup>;
 
 // Function effects
+def warn_func_effect_violation : Warning<
+  "'%0' %select{function|constructor|destructor|lambda|block|constructor's 
member initializer}1 "

dougsonos wrote:

The `%0` at the beginning is the name of the constraining attribute, so the 
diagnostic looks something like:

'nonblocking' lambda must not allocate or deallocate memory

Is that sufficient?

https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -15099,6 +15037,106 @@ class Sema final : public SemaBase {
   std::string getFixItZeroLiteralForType(QualType T, SourceLocation Loc) const;
 
   ///@}
+
+  //

dougsonos wrote:

It's copied from the other groupings which separate methods (etc) which are 
isolated to a single source file.

https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -5137,47 +5137,41 @@ StringRef FunctionEffect::name() const {
   llvm_unreachable("unknown effect kind");
 }
 
-bool FunctionEffect::canInferOnFunction(const Decl &Callee) const {
+std::optional FunctionEffect::effectProhibitingInference(
+const Decl &Callee, const FunctionEffectKindSet &CalleeFX) const {
   switch (kind()) {
   case Kind::NonAllocating:
   case Kind::NonBlocking: {
-FunctionEffectsRef CalleeFX;
-if (auto *FD = Callee.getAsFunction())
-  CalleeFX = FD->getFunctionEffects();
-else if (auto *BD = dyn_cast(&Callee))
-  CalleeFX = BD->getFunctionEffects();
-else
-  return false;
-for (const FunctionEffectWithCondition &CalleeEC : CalleeFX) {
+for (const FunctionEffect &Effect : CalleeFX) {
   // nonblocking/nonallocating cannot call allocating.
-  if (CalleeEC.Effect.kind() == Kind::Allocating)
-return false;
+  if (Effect.kind() == Kind::Allocating)
+return Effect;
   // nonblocking cannot call blocking.
-  if (kind() == Kind::NonBlocking &&
-  CalleeEC.Effect.kind() == Kind::Blocking)
-return false;
+  if (kind() == Kind::NonBlocking && Effect.kind() == Kind::Blocking)
+return Effect;
 }
-return true;
+return std::nullopt;
   }
 
   case Kind::Allocating:
   case Kind::Blocking:
-return false;
+assert(0 && "effectProhibitingInference with non-inferable effect kind");

dougsonos wrote:

Ah, I removed a more obviously superfluous assert for Kind::None, letting it 
fall into the llvm_unreachable. I didn't see this one at the time, and it does 
have a more useful message. Hm.

https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -8392,6 +8397,20 @@ void ASTReader::InitializeSema(Sema &S) {
 NewOverrides.applyOverrides(SemaObj->getLangOpts());
   }
 
+  if (!DeclsWithEffectsToVerify.empty()) {
+for (GlobalDeclID ID : DeclsWithEffectsToVerify) {
+  Decl *D = GetDecl(ID);
+  FunctionEffectsRef FX;
+  if (auto *FD = dyn_cast(D))
+FX = FD->getFunctionEffects();
+  else if (auto *BD = dyn_cast(D))
+FX = BD->getFunctionEffects();

dougsonos wrote:

If it were just one call, I wouldn't care so much but that would be a 
repetition of two lines and I'm a bit OCD about DRY. If the concern is about 
the test of `FX.empty()` being applied in the case where it's never been 
assigned, a `continue` could fix that.

https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -10950,6 +10950,51 @@ def warn_imp_cast_drops_unaligned : Warning<
   InGroup>;
 
 // Function effects
+def warn_func_effect_violation : Warning<
+  "'%0' %select{function|constructor|destructor|lambda|block|constructor's 
member initializer}1 "

dougsonos wrote:

How about

lambda with 'nonblocking' attribute must not allocate or deallocate memory

?

https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -0,0 +1,1566 @@
+//=== SemaFunctionEffects.cpp - Sema handling of function effects 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file implements Sema handling of function effects.
+//
+//===--===//
+
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/Type.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Sema/SemaInternal.h"
+
+#define DEBUG_TYPE "effectanalysis"

dougsonos wrote:

This works with LLVM_DEBUG to enable debug logging and only in debug builds.

https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -0,0 +1,1566 @@
+//=== SemaFunctionEffects.cpp - Sema handling of function effects 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file implements Sema handling of function effects.
+//
+//===--===//
+
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/Type.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Sema/SemaInternal.h"
+
+#define DEBUG_TYPE "effectanalysis"
+
+using namespace clang;
+
+namespace {
+
+enum class ViolationID : uint8_t {
+  None = 0, // Sentinel for an empty Violation.
+  // These first few map to a %select{} in a diagnostic.
+  BaseDiagnosticIndex,
+  AllocatesMemory = BaseDiagnosticIndex,
+  ThrowsOrCatchesExceptions,
+  HasStaticLocalVariable,
+  AccessesThreadLocalVariable,
+  AccessesObjCMethodOrProperty,
+
+  // These only apply to callees, where the analysis stops at the Decl.
+  DeclDisallowsInference,
+
+  // These both apply to indirect calls. The difference is that sometimes
+  // we have an actual Decl (generally a variable) which is the function
+  // pointer being called, and sometimes, typically due to a cast, we only
+  // have an expression.
+  CallsDeclWithoutEffect,
+  CallsExprWithoutEffect,
+};
+
+// Information about the AST context in which a violation was found, so
+// that diagnostics can point to the correct source.
+class ViolationSite {
+public:
+  enum class Kind : uint8_t {
+Default = 0, // Function body.
+MemberInitializer = 1,
+DefaultArgExpr = 2
+  };
+
+private:
+  llvm::PointerIntPair Impl;
+
+public:
+  ViolationSite() = default;
+
+  explicit ViolationSite(CXXDefaultArgExpr *E)
+  : Impl(E, Kind::DefaultArgExpr) {}
+
+  Kind kind() const { return static_cast(Impl.getInt()); }
+  CXXDefaultArgExpr *defaultArgExpr() const { return Impl.getPointer(); }
+
+  void setKind(Kind K) { Impl.setPointerAndInt(nullptr, K); }
+};
+
+// Represents a violation of the rules, potentially for the entire duration of
+// the analysis phase, in order to refer to it when explaining why a caller has
+// been made unsafe by a callee. Can be transformed into either a Diagnostic
+// (warning or a note), depending on whether the violation pertains to a
+// function failing to be verifed as holding an effect vs. a function failing 
to
+// be inferred as holding that effect.
+struct Violation {
+  FunctionEffect Effect;
+  FunctionEffect
+  CalleeEffectPreventingInference; // Only for certain IDs; can be None.
+  ViolationID ID = ViolationID::None;
+  ViolationSite Site;
+  SourceLocation Loc;
+  const Decl *Callee = nullptr; // Only valid for Calls*.
+
+  Violation() = default;
+
+  Violation(FunctionEffect Effect, ViolationID ID, ViolationSite VS,
+SourceLocation Loc, const Decl *Callee = nullptr,
+std::optional CalleeEffect = std::nullopt)
+  : Effect(Effect), ID(ID), Site(VS), Loc(Loc), Callee(Callee) {
+if (CalleeEffect)
+  CalleeEffectPreventingInference = *CalleeEffect;
+  }
+
+  unsigned diagnosticSelectIndex() const {
+return unsigned(ID) - unsigned(ViolationID::BaseDiagnosticIndex);
+  }
+};
+
+enum class SpecialFuncType : uint8_t { None, OperatorNew, OperatorDelete };
+enum class CallableType : uint8_t {
+  // Unknown: probably function pointer
+  Unknown,
+  Function,
+  Virtual,
+  Block
+};
+
+// Return whether a function's effects CAN be verified.
+// The question of whether it SHOULD be verified is independent.
+static bool functionIsVerifiable(const FunctionDecl *FD) {
+  if (FD->isTrivial()) {
+// Otherwise `struct x { int a; };` would have an unverifiable default
+// constructor.
+return true;
+  }
+  return FD->hasBody();
+}
+
+static bool isNoexcept(const FunctionDecl *FD) {
+  const auto *FPT = FD->getType()->castAs();
+  if (FPT->isNothrow() || FD->hasAttr())
+return true;
+  return false;
+}
+
+// This list is probably incomplete.
+// FIXME: Investigate:
+// __builtin_eh_return?
+// __builtin_allow_runtime_check?
+// __builtin_unwind_init and other similar things that sound exception-related.
+// va_copy?
+// coroutines?
+static FunctionEffectKindSet getBuiltinFunctionEffects(unsigned BuiltinID) {
+  FunctionEffectKindSet Result;
+
+  switch (BuiltinID) {
+  case 0:  // Not builtin.
+  default: // By default, builtins have no known effects.
+break;
+
+  // These allocate/deallocate heap memory.
+  case Builtin::ID::BI__builtin_calloc:
+  case Builtin::ID::BI__builtin_malloc:
+  case Builtin::ID::BI__builtin_realloc:
+  case Builtin::ID::BI__builtin_free:
+  case Builtin::ID::BI__builtin_operator_delete:
+

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -0,0 +1,1566 @@
+//=== SemaFunctionEffects.cpp - Sema handling of function effects 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file implements Sema handling of function effects.
+//
+//===--===//
+
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/Type.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Sema/SemaInternal.h"
+
+#define DEBUG_TYPE "effectanalysis"
+
+using namespace clang;
+
+namespace {
+
+enum class ViolationID : uint8_t {
+  None = 0, // Sentinel for an empty Violation.
+  // These first few map to a %select{} in a diagnostic.
+  BaseDiagnosticIndex,
+  AllocatesMemory = BaseDiagnosticIndex,
+  ThrowsOrCatchesExceptions,
+  HasStaticLocalVariable,
+  AccessesThreadLocalVariable,
+  AccessesObjCMethodOrProperty,
+
+  // These only apply to callees, where the analysis stops at the Decl.
+  DeclDisallowsInference,
+
+  // These both apply to indirect calls. The difference is that sometimes
+  // we have an actual Decl (generally a variable) which is the function
+  // pointer being called, and sometimes, typically due to a cast, we only
+  // have an expression.
+  CallsDeclWithoutEffect,
+  CallsExprWithoutEffect,
+};
+
+// Information about the AST context in which a violation was found, so
+// that diagnostics can point to the correct source.
+class ViolationSite {
+public:
+  enum class Kind : uint8_t {
+Default = 0, // Function body.
+MemberInitializer = 1,
+DefaultArgExpr = 2
+  };
+
+private:
+  llvm::PointerIntPair Impl;
+
+public:
+  ViolationSite() = default;
+
+  explicit ViolationSite(CXXDefaultArgExpr *E)
+  : Impl(E, Kind::DefaultArgExpr) {}
+
+  Kind kind() const { return static_cast(Impl.getInt()); }
+  CXXDefaultArgExpr *defaultArgExpr() const { return Impl.getPointer(); }
+
+  void setKind(Kind K) { Impl.setPointerAndInt(nullptr, K); }
+};
+
+// Represents a violation of the rules, potentially for the entire duration of
+// the analysis phase, in order to refer to it when explaining why a caller has
+// been made unsafe by a callee. Can be transformed into either a Diagnostic
+// (warning or a note), depending on whether the violation pertains to a
+// function failing to be verifed as holding an effect vs. a function failing 
to
+// be inferred as holding that effect.
+struct Violation {
+  FunctionEffect Effect;
+  FunctionEffect
+  CalleeEffectPreventingInference; // Only for certain IDs; can be None.
+  ViolationID ID = ViolationID::None;
+  ViolationSite Site;
+  SourceLocation Loc;
+  const Decl *Callee = nullptr; // Only valid for Calls*.
+
+  Violation() = default;
+
+  Violation(FunctionEffect Effect, ViolationID ID, ViolationSite VS,
+SourceLocation Loc, const Decl *Callee = nullptr,
+std::optional CalleeEffect = std::nullopt)
+  : Effect(Effect), ID(ID), Site(VS), Loc(Loc), Callee(Callee) {
+if (CalleeEffect)
+  CalleeEffectPreventingInference = *CalleeEffect;
+  }
+
+  unsigned diagnosticSelectIndex() const {
+return unsigned(ID) - unsigned(ViolationID::BaseDiagnosticIndex);
+  }
+};
+
+enum class SpecialFuncType : uint8_t { None, OperatorNew, OperatorDelete };
+enum class CallableType : uint8_t {
+  // Unknown: probably function pointer
+  Unknown,
+  Function,
+  Virtual,
+  Block
+};
+
+// Return whether a function's effects CAN be verified.
+// The question of whether it SHOULD be verified is independent.
+static bool functionIsVerifiable(const FunctionDecl *FD) {
+  if (FD->isTrivial()) {
+// Otherwise `struct x { int a; };` would have an unverifiable default
+// constructor.
+return true;
+  }
+  return FD->hasBody();
+}
+
+static bool isNoexcept(const FunctionDecl *FD) {
+  const auto *FPT = FD->getType()->castAs();
+  if (FPT->isNothrow() || FD->hasAttr())
+return true;
+  return false;
+}
+
+// This list is probably incomplete.
+// FIXME: Investigate:
+// __builtin_eh_return?
+// __builtin_allow_runtime_check?
+// __builtin_unwind_init and other similar things that sound exception-related.
+// va_copy?
+// coroutines?
+static FunctionEffectKindSet getBuiltinFunctionEffects(unsigned BuiltinID) {
+  FunctionEffectKindSet Result;
+
+  switch (BuiltinID) {
+  case 0:  // Not builtin.
+  default: // By default, builtins have no known effects.
+break;
+
+  // These allocate/deallocate heap memory.
+  case Builtin::ID::BI__builtin_calloc:
+  case Builtin::ID::BI__builtin_malloc:
+  case Builtin::ID::BI__builtin_realloc:
+  case Builtin::ID::BI__builtin_free:
+  case Builtin::ID::BI__builtin_operator_delete:
+

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -0,0 +1,1566 @@
+//=== SemaFunctionEffects.cpp - Sema handling of function effects 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file implements Sema handling of function effects.
+//
+//===--===//
+
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/Type.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Sema/SemaInternal.h"
+
+#define DEBUG_TYPE "effectanalysis"
+
+using namespace clang;
+
+namespace {
+
+enum class ViolationID : uint8_t {
+  None = 0, // Sentinel for an empty Violation.
+  // These first few map to a %select{} in a diagnostic.
+  BaseDiagnosticIndex,
+  AllocatesMemory = BaseDiagnosticIndex,
+  ThrowsOrCatchesExceptions,
+  HasStaticLocalVariable,
+  AccessesThreadLocalVariable,
+  AccessesObjCMethodOrProperty,
+
+  // These only apply to callees, where the analysis stops at the Decl.
+  DeclDisallowsInference,
+
+  // These both apply to indirect calls. The difference is that sometimes
+  // we have an actual Decl (generally a variable) which is the function
+  // pointer being called, and sometimes, typically due to a cast, we only
+  // have an expression.
+  CallsDeclWithoutEffect,
+  CallsExprWithoutEffect,
+};
+
+// Information about the AST context in which a violation was found, so
+// that diagnostics can point to the correct source.
+class ViolationSite {
+public:
+  enum class Kind : uint8_t {
+Default = 0, // Function body.
+MemberInitializer = 1,
+DefaultArgExpr = 2
+  };
+
+private:
+  llvm::PointerIntPair Impl;
+
+public:
+  ViolationSite() = default;
+
+  explicit ViolationSite(CXXDefaultArgExpr *E)
+  : Impl(E, Kind::DefaultArgExpr) {}
+
+  Kind kind() const { return static_cast(Impl.getInt()); }
+  CXXDefaultArgExpr *defaultArgExpr() const { return Impl.getPointer(); }
+
+  void setKind(Kind K) { Impl.setPointerAndInt(nullptr, K); }
+};
+
+// Represents a violation of the rules, potentially for the entire duration of
+// the analysis phase, in order to refer to it when explaining why a caller has
+// been made unsafe by a callee. Can be transformed into either a Diagnostic
+// (warning or a note), depending on whether the violation pertains to a
+// function failing to be verifed as holding an effect vs. a function failing 
to
+// be inferred as holding that effect.
+struct Violation {
+  FunctionEffect Effect;
+  FunctionEffect
+  CalleeEffectPreventingInference; // Only for certain IDs; can be None.
+  ViolationID ID = ViolationID::None;
+  ViolationSite Site;
+  SourceLocation Loc;
+  const Decl *Callee = nullptr; // Only valid for Calls*.
+
+  Violation() = default;
+
+  Violation(FunctionEffect Effect, ViolationID ID, ViolationSite VS,
+SourceLocation Loc, const Decl *Callee = nullptr,
+std::optional CalleeEffect = std::nullopt)
+  : Effect(Effect), ID(ID), Site(VS), Loc(Loc), Callee(Callee) {
+if (CalleeEffect)
+  CalleeEffectPreventingInference = *CalleeEffect;
+  }
+
+  unsigned diagnosticSelectIndex() const {
+return unsigned(ID) - unsigned(ViolationID::BaseDiagnosticIndex);
+  }
+};
+
+enum class SpecialFuncType : uint8_t { None, OperatorNew, OperatorDelete };
+enum class CallableType : uint8_t {
+  // Unknown: probably function pointer
+  Unknown,
+  Function,
+  Virtual,
+  Block
+};
+
+// Return whether a function's effects CAN be verified.
+// The question of whether it SHOULD be verified is independent.
+static bool functionIsVerifiable(const FunctionDecl *FD) {
+  if (FD->isTrivial()) {
+// Otherwise `struct x { int a; };` would have an unverifiable default
+// constructor.
+return true;
+  }
+  return FD->hasBody();
+}
+
+static bool isNoexcept(const FunctionDecl *FD) {
+  const auto *FPT = FD->getType()->castAs();
+  if (FPT->isNothrow() || FD->hasAttr())
+return true;
+  return false;
+}
+
+// This list is probably incomplete.
+// FIXME: Investigate:
+// __builtin_eh_return?
+// __builtin_allow_runtime_check?
+// __builtin_unwind_init and other similar things that sound exception-related.
+// va_copy?
+// coroutines?
+static FunctionEffectKindSet getBuiltinFunctionEffects(unsigned BuiltinID) {
+  FunctionEffectKindSet Result;
+
+  switch (BuiltinID) {
+  case 0:  // Not builtin.
+  default: // By default, builtins have no known effects.
+break;
+
+  // These allocate/deallocate heap memory.
+  case Builtin::ID::BI__builtin_calloc:
+  case Builtin::ID::BI__builtin_malloc:
+  case Builtin::ID::BI__builtin_realloc:
+  case Builtin::ID::BI__builtin_free:
+  case Builtin::ID::BI__builtin_operator_delete:
+

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -8392,6 +8397,20 @@ void ASTReader::InitializeSema(Sema &S) {
 NewOverrides.applyOverrides(SemaObj->getLangOpts());
   }
 
+  if (!DeclsWithEffectsToVerify.empty()) {
+for (GlobalDeclID ID : DeclsWithEffectsToVerify) {
+  Decl *D = GetDecl(ID);
+  FunctionEffectsRef FX;
+  if (auto *FD = dyn_cast(D))
+FX = FD->getFunctionEffects();
+  else if (auto *BD = dyn_cast(D))
+FX = BD->getFunctionEffects();

dougsonos wrote:

Yeah, it's guarding against a pathological case where a function or block in 
`DeclsWithEffectsToVerify` turns out to have no effects. It happens that 
(despite comments) `addDeclWithEffects()` will still guard against this, so OK.

https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -0,0 +1,1566 @@
+//=== SemaFunctionEffects.cpp - Sema handling of function effects 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file implements Sema handling of function effects.
+//
+//===--===//
+
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/Type.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Sema/SemaInternal.h"
+
+#define DEBUG_TYPE "effectanalysis"
+
+using namespace clang;
+
+namespace {
+
+enum class ViolationID : uint8_t {
+  None = 0, // Sentinel for an empty Violation.
+  // These first few map to a %select{} in a diagnostic.
+  BaseDiagnosticIndex,
+  AllocatesMemory = BaseDiagnosticIndex,
+  ThrowsOrCatchesExceptions,
+  HasStaticLocalVariable,
+  AccessesThreadLocalVariable,
+  AccessesObjCMethodOrProperty,
+
+  // These only apply to callees, where the analysis stops at the Decl.
+  DeclDisallowsInference,
+
+  // These both apply to indirect calls. The difference is that sometimes
+  // we have an actual Decl (generally a variable) which is the function
+  // pointer being called, and sometimes, typically due to a cast, we only
+  // have an expression.
+  CallsDeclWithoutEffect,
+  CallsExprWithoutEffect,
+};
+
+// Information about the AST context in which a violation was found, so
+// that diagnostics can point to the correct source.
+class ViolationSite {
+public:
+  enum class Kind : uint8_t {
+Default = 0, // Function body.
+MemberInitializer = 1,
+DefaultArgExpr = 2
+  };
+
+private:
+  llvm::PointerIntPair Impl;
+
+public:
+  ViolationSite() = default;
+
+  explicit ViolationSite(CXXDefaultArgExpr *E)
+  : Impl(E, Kind::DefaultArgExpr) {}
+
+  Kind kind() const { return static_cast(Impl.getInt()); }
+  CXXDefaultArgExpr *defaultArgExpr() const { return Impl.getPointer(); }
+
+  void setKind(Kind K) { Impl.setPointerAndInt(nullptr, K); }
+};
+
+// Represents a violation of the rules, potentially for the entire duration of
+// the analysis phase, in order to refer to it when explaining why a caller has
+// been made unsafe by a callee. Can be transformed into either a Diagnostic
+// (warning or a note), depending on whether the violation pertains to a
+// function failing to be verifed as holding an effect vs. a function failing 
to
+// be inferred as holding that effect.
+struct Violation {
+  FunctionEffect Effect;
+  FunctionEffect
+  CalleeEffectPreventingInference; // Only for certain IDs; can be None.
+  ViolationID ID = ViolationID::None;
+  ViolationSite Site;
+  SourceLocation Loc;
+  const Decl *Callee = nullptr; // Only valid for Calls*.
+
+  Violation() = default;
+
+  Violation(FunctionEffect Effect, ViolationID ID, ViolationSite VS,
+SourceLocation Loc, const Decl *Callee = nullptr,
+std::optional CalleeEffect = std::nullopt)
+  : Effect(Effect), ID(ID), Site(VS), Loc(Loc), Callee(Callee) {
+if (CalleeEffect)
+  CalleeEffectPreventingInference = *CalleeEffect;
+  }
+
+  unsigned diagnosticSelectIndex() const {
+return unsigned(ID) - unsigned(ViolationID::BaseDiagnosticIndex);
+  }
+};
+
+enum class SpecialFuncType : uint8_t { None, OperatorNew, OperatorDelete };
+enum class CallableType : uint8_t {
+  // Unknown: probably function pointer
+  Unknown,
+  Function,
+  Virtual,
+  Block
+};
+
+// Return whether a function's effects CAN be verified.
+// The question of whether it SHOULD be verified is independent.
+static bool functionIsVerifiable(const FunctionDecl *FD) {
+  if (FD->isTrivial()) {
+// Otherwise `struct x { int a; };` would have an unverifiable default
+// constructor.
+return true;
+  }
+  return FD->hasBody();
+}
+
+static bool isNoexcept(const FunctionDecl *FD) {
+  const auto *FPT = FD->getType()->castAs();
+  if (FPT->isNothrow() || FD->hasAttr())
+return true;
+  return false;
+}
+
+// This list is probably incomplete.
+// FIXME: Investigate:
+// __builtin_eh_return?
+// __builtin_allow_runtime_check?
+// __builtin_unwind_init and other similar things that sound exception-related.
+// va_copy?
+// coroutines?
+static FunctionEffectKindSet getBuiltinFunctionEffects(unsigned BuiltinID) {
+  FunctionEffectKindSet Result;
+
+  switch (BuiltinID) {
+  case 0:  // Not builtin.
+  default: // By default, builtins have no known effects.
+break;
+
+  // These allocate/deallocate heap memory.
+  case Builtin::ID::BI__builtin_calloc:
+  case Builtin::ID::BI__builtin_malloc:
+  case Builtin::ID::BI__builtin_realloc:
+  case Builtin::ID::BI__builtin_free:
+  case Builtin::ID::BI__builtin_operator_delete:
+

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

dougsonos wrote:

> Hi @dougsonos
> 
> We’re experiencing an unforeseen pain point trying to use rtsan without 
> function effects, and wanted to ask **how you would feel about making 
> function effect warnings opt-in rather than opt-out.**
> 
> While users can easily opt in to function effects and not rtsan, the problem 
> is that they can’t easily opt in to rtsan and not function effects.
> 
> Here’s why: someone wanting to try out rtsan can add the `[[nonblocking]]` 
> attribute, but this automatically opts them in to function effect warnings. 
> For users who compile with `-Werror`, this means they will likely be unable 
> to compile the code they wish to test with rtsan unless they explicitly turn 
> off function effects warnings with -Wno-function-effects. If they’re not 
> familiar with function effects they won’t know this, and we’re worried about 
> an education gap causing them to blame rtsan and give up on it before 
> realizing they can flick the function effects warnings off.
> 
> By disabling these warnings by default, both tools have the same "activation" 
> of attribute + compile time flag, and it is equally easy to run either tool 
> in isolation, or together.

My current understanding is that Clang has groups of warnings like `-Wmost` 
`-Wall` and `-Wextra` though I haven't seen yet how those work under the hood. 
My sense is that it would be weird for `-Wall` not to include 
`-Wfunction-effects`, but that it would be OK for `-Wfunction-effects` not to 
be enabled by default.

I do wonder, though, would it be that difficult to tell users to include 
`-Wno-function-effects` with `-fsanitize=realtime` (or whatever it is)?

Hopefully one of the experienced maintainers will chime in here.

https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

dougsonos wrote:

In working with this version of the compiler, I've discovered a pain point with 
libc++, `std::__libcpp_verbose_abort()`. Many things which one would expect to 
be nonblocking, e.g. `std::vector::operator[]`, have hardening paths which 
call `__libcpp_verbose_abort()` on failure.

I chatted with a libc++ maintainer about this. A first thought was to simply 
declare `__libcpp_verbose_abort()` as `nonblocking`. But that feels like a 
weird lie. Possibly most ideally, functions like this would have an attribute 
to exempt them from nonblocking analysis.

A quick hack would be to synthesize that attribute from a combination of the 
`noreturn` attribute and the function name containing "abort" or "terminate". 
(`noreturn` on its own is initially attractive, but it can also apply to a 
wrapper around `throw`).

The only workaround is to redeclare `__libcpp_verbose_abort()` with 
`[[clang::nonblocking]]`. This is tricky because the redeclaration has to 
follow the one in `<__verbose_abort>` but precede its use from other headers 
like ``. That leads to having to include `<__verbose_abort>` directly.

Would appreciate any thoughts!

https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

dougsonos wrote:

> I do agree, it seems reasonable for it to be in `-Wall` or similar! That is 
> absolutely what I'd expect as a user :)

I guess my thinking was colored a bit by the way many of our codebases use 
`-Wall`.

I did come around to thinking that without `-Wall`, it's cleaner for the 
diagnostics to be disabled by default. Pushed a change.

Thanks, Chris.

https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][rtsan] Introduce realtime sanitizer codegen and driver (PR #102622)

[Doug Wyatt via cfe-commits]

@@ -845,6 +845,12 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, 
QualType RetTy,
   if (SanOpts.has(SanitizerKind::ShadowCallStack))
 Fn->addFnAttr(llvm::Attribute::ShadowCallStack);
 
+  if (SanOpts.has(SanitizerKind::Realtime)) {
+for (const FunctionEffectWithCondition &Fe : FD->getFunctionEffects())
+  if (Fe.Effect.kind() == FunctionEffect::Kind::NonBlocking)
+Fn->addFnAttr(llvm::Attribute::SanitizeRealtime);
+  }

dougsonos wrote:

`getFunctionEffects()` is cheap, but when investigating the performance 
regression introduced by effects, it turned out to be worthwhile to test 
`Context.hasAnyFunctionEffects()` first, to optimize for the case of no effects 
ever having been seen.

https://github.com/llvm/llvm-project/pull/102622
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -2397,6 +2397,1262 @@ class UnsafeBufferUsageReporter : public 
UnsafeBufferUsageHandler {
 };
 } // namespace
 
+// 
=
+
+namespace FXAnalysis {
+
+enum class DiagnosticID : uint8_t {
+  None = 0, // sentinel for an empty Diagnostic
+  Throws,
+  Catches,
+  CallsObjC,
+  AllocatesMemory,
+  HasStaticLocal,
+  AccessesThreadLocal,
+
+  // These only apply to callees, where the analysis stops at the Decl
+  DeclDisallowsInference,
+
+  CallsDeclWithoutEffect,
+  CallsExprWithoutEffect,
+};
+
+// Holds an effect diagnosis, potentially for the entire duration of the
+// analysis phase, in order to refer to it when explaining why a caller has 
been
+// made unsafe by a callee.
+struct Diagnostic {
+  FunctionEffect Effect;
+  DiagnosticID ID = DiagnosticID::None;
+  SourceLocation Loc;
+  const Decl *Callee = nullptr; // only valid for Calls*
+
+  Diagnostic() = default;
+
+  Diagnostic(const FunctionEffect &Effect, DiagnosticID ID, SourceLocation Loc,
+ const Decl *Callee = nullptr)
+  : Effect(Effect), ID(ID), Loc(Loc), Callee(Callee) {}
+};
+
+enum class SpecialFuncType : uint8_t { None, OperatorNew, OperatorDelete };
+enum class CallType {
+  // unknown: probably function pointer
+  Unknown,
+  Function,
+  Virtual,
+  Block
+};
+
+// Return whether a function's effects CAN be verified.
+// The question of whether it SHOULD be verified is independent.
+static bool functionIsVerifiable(const FunctionDecl *FD) {
+  if (!(FD->hasBody() || FD->isInlined())) {
+// externally defined; we couldn't verify if we wanted to.
+return false;
+  }
+  if (FD->isTrivial()) {
+// Otherwise `struct x { int a; };` would have an unverifiable default
+// constructor.
+return true;
+  }
+  return true;
+}
+
+/// A mutable set of FunctionEffect, for use in places where any conditions
+/// have been resolved or can be ignored.
+class EffectSet {
+  // This implementation optimizes footprint, since we hold one of these for
+  // every function visited, which, due to inference, can be many more 
functions
+  // than have declared effects.
+
+  template  struct FixedVector {
+SizeT Count = 0;
+T Items[Capacity] = {};
+
+using value_type = T;
+
+using iterator = T *;
+using const_iterator = const T *;
+iterator begin() { return &Items[0]; }
+iterator end() { return &Items[Count]; }
+const_iterator begin() const { return &Items[0]; }
+const_iterator end() const { return &Items[Count]; }
+const_iterator cbegin() const { return &Items[0]; }
+const_iterator cend() const { return &Items[Count]; }
+
+void insert(iterator I, const T &Value) {
+  assert(Count < Capacity);
+  iterator E = end();
+  if (I != E)
+std::copy_backward(I, E, E + 1);
+  *I = Value;
+  ++Count;
+}
+
+void push_back(const T &Value) {
+  assert(Count < Capacity);
+  Items[Count++] = Value;
+}
+  };
+
+  // As long as FunctionEffect is only 1 byte, and there are only 2 verifiable
+  // effects, this fixed-size vector with a capacity of 7 is more than
+  // sufficient and is only 8 bytes.
+  FixedVector Impl;
+
+public:
+  EffectSet() = default;
+  explicit EffectSet(FunctionEffectsRef FX) { insert(FX); }
+
+  operator ArrayRef() const {
+return ArrayRef(Impl.cbegin(), Impl.cend());
+  }
+
+  using iterator = const FunctionEffect *;
+  iterator begin() const { return Impl.cbegin(); }
+  iterator end() const { return Impl.cend(); }
+
+  void insert(const FunctionEffect &Effect) {
+FunctionEffect *Iter = Impl.begin();
+FunctionEffect *End = Impl.end();
+// linear search; lower_bound is overkill for a tiny vector like this
+for (; Iter != End; ++Iter) {
+  if (*Iter == Effect)
+return;
+  if (Effect < *Iter)
+break;
+}
+Impl.insert(Iter, Effect);
+  }
+  void insert(const EffectSet &Set) {
+for (const FunctionEffect &Item : Set) {
+  // push_back because set is already sorted
+  Impl.push_back(Item);
+}
+  }
+  void insert(FunctionEffectsRef FX) {
+for (const FunctionEffectWithCondition &EC : FX) {
+  assert(EC.Cond.getCondition() ==
+ nullptr); // should be resolved by now, right?
+  // push_back because set is already sorted
+  Impl.push_back(EC.Effect);
+}
+  }
+  bool contains(const FunctionEffect::Kind EK) const {
+for (const FunctionEffect &E : Impl)
+  if (E.kind() == EK)
+return true;
+return false;
+  }
+
+  void dump(llvm::raw_ostream &OS) const;
+
+  static EffectSet difference(ArrayRef LHS,
+  ArrayRef RHS) {
+EffectSet Result;
+std::set_difference(LHS.begin(), LHS.end(), RHS.begin(), RHS.end(),
+std::back_inserter(Result.Impl));
+return Result;
+  }
+};
+
+LLVM_DUMP_METHOD void EffectSet::dump(llvm::raw_ostream &OS) const {
+  OS << "Effects{";
+  bool Fi

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

dougsonos wrote:

Thanks for all of the feedback so far; I believe I've addressed it all. Would 
appreciate another look.
@Sirraide 

https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

dougsonos wrote:

> ```
> #include 
> float* nb4() noexcept [[clang::nonallocating]]
> {
>   float* ptr = (float*)malloc(100 * sizeof(float));
>   return ptr;
> }
> ```
> 
> Produces no warnings:

Hi Chris,
Thanks. That turned out to be the impetus to start carving out exceptions to 
"built-in functions are always safe", since the `malloc` obtained by `cstdlib` 
gets recognized as a built-in. Pushed a first crack at handling that.

https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

dougsonos wrote:

> aha! I missed this was part of the plan to begin with, that makes sense.

Well, there was some head-scratching before debugging revealed that the Clang 
was treating `malloc()` as a built-in. It doesn't do that if you declare it 
locally.

https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -0,0 +1,1199 @@
+//=== EffectAnalysis.cpp - Sema warnings for function effects 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file implements caller/callee analysis for function effects.
+//
+//===--===//
+
+#include "clang/AST/Decl.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/Type.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Sema/SemaInternal.h"
+
+#define DEBUG_TYPE "fxanalysis"
+
+using namespace clang;
+
+namespace {
+
+enum class ViolationID : uint8_t {
+  None = 0, // sentinel for an empty Violation
+  Throws,
+  Catches,
+  CallsObjC,
+  AllocatesMemory,
+  HasStaticLocal,
+  AccessesThreadLocal,
+
+  // These only apply to callees, where the analysis stops at the Decl
+  DeclDisallowsInference,
+
+  CallsDeclWithoutEffect,
+  CallsExprWithoutEffect,
+};
+
+// Represents a violation of the rules, potentially for the entire duration of
+// the analysis phase, in order to refer to it when explaining why a caller has
+// been made unsafe by a callee. Can be transformed into either a Diagnostic
+// (warning or a note), depending on whether the violation pertains to a
+// function failing to be verifed as holding an effect vs. a function failing 
to
+// be inferred as holding that effect.
+struct Violation {
+  FunctionEffect Effect;
+  FunctionEffect CalleeEffectPreventingInference; // only for certain IDs
+  ViolationID ID = ViolationID::None;
+  SourceLocation Loc;
+  const Decl *Callee = nullptr; // only valid for Calls*
+
+  Violation() = default;
+
+  Violation(const FunctionEffect &Effect, ViolationID ID, SourceLocation Loc,
+const Decl *Callee = nullptr,
+const FunctionEffect *CalleeEffect = nullptr)
+  : Effect(Effect), ID(ID), Loc(Loc), Callee(Callee) {
+if (CalleeEffect != nullptr)
+  CalleeEffectPreventingInference = *CalleeEffect;
+  }
+};
+
+enum class SpecialFuncType : uint8_t { None, OperatorNew, OperatorDelete };
+enum class CallableType {
+  // unknown: probably function pointer
+  Unknown,
+  Function,
+  Virtual,
+  Block
+};
+
+// Return whether a function's effects CAN be verified.
+// The question of whether it SHOULD be verified is independent.
+static bool functionIsVerifiable(const FunctionDecl *FD) {
+  if (FD->isTrivial()) {
+// Otherwise `struct x { int a; };` would have an unverifiable default
+// constructor.
+return true;
+  }
+  return FD->hasBody();
+}
+
+static bool isNoexcept(const FunctionDecl *FD) {
+  const auto *FPT = FD->getType()->castAs();
+  if (FPT->isNothrow() || FD->hasAttr())
+return true;
+  return false;
+}
+
+// Transitory, more extended information about a callable, which can be a
+// function, block, or function pointer.
+struct CallableInfo {
+  // CDecl holds the function's definition, if any.
+  // FunctionDecl if CallableType::Function or Virtual
+  // BlockDecl if CallableType::Block
+  const Decl *CDecl;
+
+  // Remember whether the callable is a function, block, virtual method,
+  // or (presumed) function pointer.
+  CallableType CType = CallableType::Unknown;
+
+  // Remember whether the callable is an operator new or delete function,
+  // so that calls to them are reported more meaningfully, as memory
+  // allocations.
+  SpecialFuncType FuncType = SpecialFuncType::None;
+
+  // We inevitably want to know the callable's declared effects, so cache them.
+  FunctionEffectKindSet Effects;
+
+  CallableInfo(const Decl &CD, SpecialFuncType FT = SpecialFuncType::None)
+  : CDecl(&CD), FuncType(FT) {
+FunctionEffectsRef DeclEffects;
+if (auto *FD = dyn_cast(CDecl)) {
+  // Use the function's definition, if any.
+  if (const FunctionDecl *Def = FD->getDefinition())
+CDecl = FD = Def;
+  CType = CallableType::Function;
+  if (auto *Method = dyn_cast(FD);
+  Method && Method->isVirtual())
+CType = CallableType::Virtual;
+  DeclEffects = FD->getFunctionEffects();
+} else if (auto *BD = dyn_cast(CDecl)) {
+  CType = CallableType::Block;
+  DeclEffects = BD->getFunctionEffects();
+} else if (auto *VD = dyn_cast(CDecl)) {
+  // ValueDecl is function, enum, or variable, so just look at its type.
+  DeclEffects = FunctionEffectsRef::get(VD->getType());
+}
+Effects = FunctionEffectKindSet(DeclEffects);
+  }
+
+  CallableType type() const { return CType; }
+
+  bool isCalledDirectly() const {
+return CType == CallableType::Function || CType == CallableType::Block;
+  }
+
+  bool isVerifiable() const {
+switch (CType) {
+case CallableType::Unknown:
+case CallableType::Virtual:

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -0,0 +1,1199 @@
+//=== EffectAnalysis.cpp - Sema warnings for function effects 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file implements caller/callee analysis for function effects.
+//
+//===--===//
+
+#include "clang/AST/Decl.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/Type.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Sema/SemaInternal.h"
+
+#define DEBUG_TYPE "fxanalysis"
+
+using namespace clang;
+
+namespace {
+
+enum class ViolationID : uint8_t {
+  None = 0, // sentinel for an empty Violation
+  Throws,
+  Catches,
+  CallsObjC,
+  AllocatesMemory,
+  HasStaticLocal,
+  AccessesThreadLocal,
+
+  // These only apply to callees, where the analysis stops at the Decl
+  DeclDisallowsInference,
+
+  CallsDeclWithoutEffect,
+  CallsExprWithoutEffect,
+};
+
+// Represents a violation of the rules, potentially for the entire duration of
+// the analysis phase, in order to refer to it when explaining why a caller has
+// been made unsafe by a callee. Can be transformed into either a Diagnostic
+// (warning or a note), depending on whether the violation pertains to a
+// function failing to be verifed as holding an effect vs. a function failing 
to
+// be inferred as holding that effect.
+struct Violation {
+  FunctionEffect Effect;
+  FunctionEffect CalleeEffectPreventingInference; // only for certain IDs
+  ViolationID ID = ViolationID::None;
+  SourceLocation Loc;
+  const Decl *Callee = nullptr; // only valid for Calls*
+
+  Violation() = default;
+
+  Violation(const FunctionEffect &Effect, ViolationID ID, SourceLocation Loc,
+const Decl *Callee = nullptr,
+const FunctionEffect *CalleeEffect = nullptr)
+  : Effect(Effect), ID(ID), Loc(Loc), Callee(Callee) {
+if (CalleeEffect != nullptr)
+  CalleeEffectPreventingInference = *CalleeEffect;
+  }
+};
+
+enum class SpecialFuncType : uint8_t { None, OperatorNew, OperatorDelete };
+enum class CallableType {
+  // unknown: probably function pointer
+  Unknown,
+  Function,
+  Virtual,
+  Block
+};
+
+// Return whether a function's effects CAN be verified.
+// The question of whether it SHOULD be verified is independent.
+static bool functionIsVerifiable(const FunctionDecl *FD) {
+  if (FD->isTrivial()) {
+// Otherwise `struct x { int a; };` would have an unverifiable default
+// constructor.
+return true;
+  }
+  return FD->hasBody();
+}
+
+static bool isNoexcept(const FunctionDecl *FD) {
+  const auto *FPT = FD->getType()->castAs();
+  if (FPT->isNothrow() || FD->hasAttr())
+return true;
+  return false;
+}
+
+// Transitory, more extended information about a callable, which can be a
+// function, block, or function pointer.
+struct CallableInfo {
+  // CDecl holds the function's definition, if any.
+  // FunctionDecl if CallableType::Function or Virtual
+  // BlockDecl if CallableType::Block
+  const Decl *CDecl;
+
+  // Remember whether the callable is a function, block, virtual method,
+  // or (presumed) function pointer.
+  CallableType CType = CallableType::Unknown;
+
+  // Remember whether the callable is an operator new or delete function,
+  // so that calls to them are reported more meaningfully, as memory
+  // allocations.
+  SpecialFuncType FuncType = SpecialFuncType::None;
+
+  // We inevitably want to know the callable's declared effects, so cache them.
+  FunctionEffectKindSet Effects;
+
+  CallableInfo(const Decl &CD, SpecialFuncType FT = SpecialFuncType::None)
+  : CDecl(&CD), FuncType(FT) {
+FunctionEffectsRef DeclEffects;
+if (auto *FD = dyn_cast(CDecl)) {
+  // Use the function's definition, if any.
+  if (const FunctionDecl *Def = FD->getDefinition())
+CDecl = FD = Def;
+  CType = CallableType::Function;
+  if (auto *Method = dyn_cast(FD);
+  Method && Method->isVirtual())
+CType = CallableType::Virtual;
+  DeclEffects = FD->getFunctionEffects();
+} else if (auto *BD = dyn_cast(CDecl)) {
+  CType = CallableType::Block;
+  DeclEffects = BD->getFunctionEffects();
+} else if (auto *VD = dyn_cast(CDecl)) {
+  // ValueDecl is function, enum, or variable, so just look at its type.
+  DeclEffects = FunctionEffectsRef::get(VD->getType());
+}
+Effects = FunctionEffectKindSet(DeclEffects);
+  }
+
+  CallableType type() const { return CType; }
+
+  bool isCalledDirectly() const {
+return CType == CallableType::Function || CType == CallableType::Block;
+  }
+
+  bool isVerifiable() const {
+switch (CType) {
+case CallableType::Unknown:
+case CallableType::Virtual:

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -953,6 +953,9 @@ class ASTReader
   /// Sema tracks these to emit deferred diags.
   llvm::SmallSetVector DeclsToCheckForDeferredDiags;
 
+  /// The IDs of all decls with function effects to be checked.
+  SmallVector DeclsWithEffectsToVerify;

dougsonos wrote:

My original implementation made a separate complete AST traversal to find all 
of the Decls to verify. I think it was @rjmccall who suggested the current 
approach of building a list as they are encountered. IIRC he mentioned at that 
time that the list would have to be serialized as part of the AST, to properly 
support situations where a precompiled header defines a function with an effect 
that needs verifying.

https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -5137,47 +5137,41 @@ StringRef FunctionEffect::name() const {
   llvm_unreachable("unknown effect kind");
 }
 
-bool FunctionEffect::canInferOnFunction(const Decl &Callee) const {
+std::optional FunctionEffect::effectProhibitingInference(
+const Decl &Callee, const FunctionEffectKindSet &CalleeFX) const {
   switch (kind()) {
   case Kind::NonAllocating:
   case Kind::NonBlocking: {
-FunctionEffectsRef CalleeFX;
-if (auto *FD = Callee.getAsFunction())
-  CalleeFX = FD->getFunctionEffects();
-else if (auto *BD = dyn_cast(&Callee))
-  CalleeFX = BD->getFunctionEffects();
-else
-  return false;
-for (const FunctionEffectWithCondition &CalleeEC : CalleeFX) {
+for (const FunctionEffect &Effect : CalleeFX) {
   // nonblocking/nonallocating cannot call allocating.
-  if (CalleeEC.Effect.kind() == Kind::Allocating)
-return false;
+  if (Effect.kind() == Kind::Allocating)
+return Effect;
   // nonblocking cannot call blocking.
-  if (kind() == Kind::NonBlocking &&
-  CalleeEC.Effect.kind() == Kind::Blocking)
-return false;
+  if (kind() == Kind::NonBlocking && Effect.kind() == Kind::Blocking)
+return Effect;
 }
-return true;
+return std::nullopt;
   }
 
   case Kind::Allocating:
   case Kind::Blocking:
-return false;
+assert(0 && "effectProhibitingInference with non-inferable effect kind");

dougsonos wrote:

Hard experience has trained me to think of `assert()` as a potential no-op, 
thus the `break`, but yeah, the combination with `llvm_unreachable()` is no 
good.

https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -0,0 +1,1199 @@
+//=== EffectAnalysis.cpp - Sema warnings for function effects 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file implements caller/callee analysis for function effects.
+//
+//===--===//
+
+#include "clang/AST/Decl.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/Type.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Sema/SemaInternal.h"
+
+#define DEBUG_TYPE "fxanalysis"
+
+using namespace clang;
+
+namespace {
+
+enum class ViolationID : uint8_t {
+  None = 0, // sentinel for an empty Violation
+  Throws,
+  Catches,
+  CallsObjC,
+  AllocatesMemory,
+  HasStaticLocal,
+  AccessesThreadLocal,
+
+  // These only apply to callees, where the analysis stops at the Decl
+  DeclDisallowsInference,
+
+  CallsDeclWithoutEffect,
+  CallsExprWithoutEffect,
+};
+
+// Represents a violation of the rules, potentially for the entire duration of
+// the analysis phase, in order to refer to it when explaining why a caller has
+// been made unsafe by a callee. Can be transformed into either a Diagnostic
+// (warning or a note), depending on whether the violation pertains to a
+// function failing to be verifed as holding an effect vs. a function failing 
to
+// be inferred as holding that effect.
+struct Violation {
+  FunctionEffect Effect;
+  FunctionEffect CalleeEffectPreventingInference; // only for certain IDs
+  ViolationID ID = ViolationID::None;
+  SourceLocation Loc;
+  const Decl *Callee = nullptr; // only valid for Calls*
+
+  Violation() = default;
+
+  Violation(const FunctionEffect &Effect, ViolationID ID, SourceLocation Loc,
+const Decl *Callee = nullptr,
+const FunctionEffect *CalleeEffect = nullptr)
+  : Effect(Effect), ID(ID), Loc(Loc), Callee(Callee) {
+if (CalleeEffect != nullptr)
+  CalleeEffectPreventingInference = *CalleeEffect;
+  }
+};
+
+enum class SpecialFuncType : uint8_t { None, OperatorNew, OperatorDelete };
+enum class CallableType {
+  // unknown: probably function pointer
+  Unknown,
+  Function,
+  Virtual,
+  Block
+};
+
+// Return whether a function's effects CAN be verified.
+// The question of whether it SHOULD be verified is independent.
+static bool functionIsVerifiable(const FunctionDecl *FD) {
+  if (FD->isTrivial()) {
+// Otherwise `struct x { int a; };` would have an unverifiable default
+// constructor.
+return true;
+  }
+  return FD->hasBody();
+}
+
+static bool isNoexcept(const FunctionDecl *FD) {
+  const auto *FPT = FD->getType()->castAs();
+  if (FPT->isNothrow() || FD->hasAttr())
+return true;
+  return false;
+}
+
+// Transitory, more extended information about a callable, which can be a
+// function, block, or function pointer.
+struct CallableInfo {
+  // CDecl holds the function's definition, if any.
+  // FunctionDecl if CallableType::Function or Virtual
+  // BlockDecl if CallableType::Block
+  const Decl *CDecl;
+
+  // Remember whether the callable is a function, block, virtual method,
+  // or (presumed) function pointer.
+  CallableType CType = CallableType::Unknown;
+
+  // Remember whether the callable is an operator new or delete function,
+  // so that calls to them are reported more meaningfully, as memory
+  // allocations.
+  SpecialFuncType FuncType = SpecialFuncType::None;
+
+  // We inevitably want to know the callable's declared effects, so cache them.
+  FunctionEffectKindSet Effects;
+
+  CallableInfo(const Decl &CD, SpecialFuncType FT = SpecialFuncType::None)
+  : CDecl(&CD), FuncType(FT) {
+FunctionEffectsRef DeclEffects;
+if (auto *FD = dyn_cast(CDecl)) {
+  // Use the function's definition, if any.
+  if (const FunctionDecl *Def = FD->getDefinition())
+CDecl = FD = Def;
+  CType = CallableType::Function;
+  if (auto *Method = dyn_cast(FD);
+  Method && Method->isVirtual())
+CType = CallableType::Virtual;
+  DeclEffects = FD->getFunctionEffects();
+} else if (auto *BD = dyn_cast(CDecl)) {
+  CType = CallableType::Block;
+  DeclEffects = BD->getFunctionEffects();
+} else if (auto *VD = dyn_cast(CDecl)) {
+  // ValueDecl is function, enum, or variable, so just look at its type.
+  DeclEffects = FunctionEffectsRef::get(VD->getType());
+}
+Effects = FunctionEffectKindSet(DeclEffects);
+  }
+
+  CallableType type() const { return CType; }
+
+  bool isCalledDirectly() const {
+return CType == CallableType::Function || CType == CallableType::Block;
+  }
+
+  bool isVerifiable() const {
+switch (CType) {
+case CallableType::Unknown:
+case CallableType::Virtual:

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -0,0 +1,1199 @@
+//=== EffectAnalysis.cpp - Sema warnings for function effects 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file implements caller/callee analysis for function effects.
+//
+//===--===//
+
+#include "clang/AST/Decl.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/Type.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Sema/SemaInternal.h"
+
+#define DEBUG_TYPE "fxanalysis"
+
+using namespace clang;
+
+namespace {
+
+enum class ViolationID : uint8_t {
+  None = 0, // sentinel for an empty Violation
+  Throws,
+  Catches,
+  CallsObjC,
+  AllocatesMemory,
+  HasStaticLocal,
+  AccessesThreadLocal,
+
+  // These only apply to callees, where the analysis stops at the Decl
+  DeclDisallowsInference,
+
+  CallsDeclWithoutEffect,
+  CallsExprWithoutEffect,
+};
+
+// Represents a violation of the rules, potentially for the entire duration of
+// the analysis phase, in order to refer to it when explaining why a caller has
+// been made unsafe by a callee. Can be transformed into either a Diagnostic
+// (warning or a note), depending on whether the violation pertains to a
+// function failing to be verifed as holding an effect vs. a function failing 
to
+// be inferred as holding that effect.
+struct Violation {
+  FunctionEffect Effect;
+  FunctionEffect CalleeEffectPreventingInference; // only for certain IDs
+  ViolationID ID = ViolationID::None;
+  SourceLocation Loc;
+  const Decl *Callee = nullptr; // only valid for Calls*
+
+  Violation() = default;
+
+  Violation(const FunctionEffect &Effect, ViolationID ID, SourceLocation Loc,
+const Decl *Callee = nullptr,
+const FunctionEffect *CalleeEffect = nullptr)
+  : Effect(Effect), ID(ID), Loc(Loc), Callee(Callee) {
+if (CalleeEffect != nullptr)
+  CalleeEffectPreventingInference = *CalleeEffect;
+  }
+};
+
+enum class SpecialFuncType : uint8_t { None, OperatorNew, OperatorDelete };
+enum class CallableType {
+  // unknown: probably function pointer
+  Unknown,
+  Function,
+  Virtual,
+  Block
+};
+
+// Return whether a function's effects CAN be verified.
+// The question of whether it SHOULD be verified is independent.
+static bool functionIsVerifiable(const FunctionDecl *FD) {
+  if (FD->isTrivial()) {
+// Otherwise `struct x { int a; };` would have an unverifiable default
+// constructor.
+return true;
+  }
+  return FD->hasBody();
+}
+
+static bool isNoexcept(const FunctionDecl *FD) {
+  const auto *FPT = FD->getType()->castAs();
+  if (FPT->isNothrow() || FD->hasAttr())
+return true;
+  return false;
+}
+
+// Transitory, more extended information about a callable, which can be a
+// function, block, or function pointer.
+struct CallableInfo {
+  // CDecl holds the function's definition, if any.
+  // FunctionDecl if CallableType::Function or Virtual
+  // BlockDecl if CallableType::Block
+  const Decl *CDecl;
+
+  // Remember whether the callable is a function, block, virtual method,
+  // or (presumed) function pointer.
+  CallableType CType = CallableType::Unknown;
+
+  // Remember whether the callable is an operator new or delete function,
+  // so that calls to them are reported more meaningfully, as memory
+  // allocations.
+  SpecialFuncType FuncType = SpecialFuncType::None;
+
+  // We inevitably want to know the callable's declared effects, so cache them.
+  FunctionEffectKindSet Effects;
+
+  CallableInfo(const Decl &CD, SpecialFuncType FT = SpecialFuncType::None)
+  : CDecl(&CD), FuncType(FT) {
+FunctionEffectsRef DeclEffects;
+if (auto *FD = dyn_cast(CDecl)) {
+  // Use the function's definition, if any.
+  if (const FunctionDecl *Def = FD->getDefinition())
+CDecl = FD = Def;
+  CType = CallableType::Function;
+  if (auto *Method = dyn_cast(FD);
+  Method && Method->isVirtual())
+CType = CallableType::Virtual;
+  DeclEffects = FD->getFunctionEffects();
+} else if (auto *BD = dyn_cast(CDecl)) {
+  CType = CallableType::Block;
+  DeclEffects = BD->getFunctionEffects();
+} else if (auto *VD = dyn_cast(CDecl)) {
+  // ValueDecl is function, enum, or variable, so just look at its type.
+  DeclEffects = FunctionEffectsRef::get(VD->getType());
+}
+Effects = FunctionEffectKindSet(DeclEffects);
+  }
+
+  CallableType type() const { return CType; }
+
+  bool isCalledDirectly() const {
+return CType == CallableType::Function || CType == CallableType::Block;
+  }
+
+  bool isVerifiable() const {
+switch (CType) {
+case CallableType::Unknown:
+case CallableType::Virtual:

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -0,0 +1,1199 @@
+//=== EffectAnalysis.cpp - Sema warnings for function effects 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file implements caller/callee analysis for function effects.
+//
+//===--===//
+
+#include "clang/AST/Decl.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/Type.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Sema/SemaInternal.h"
+
+#define DEBUG_TYPE "fxanalysis"
+
+using namespace clang;
+
+namespace {
+
+enum class ViolationID : uint8_t {
+  None = 0, // sentinel for an empty Violation
+  Throws,
+  Catches,
+  CallsObjC,
+  AllocatesMemory,
+  HasStaticLocal,
+  AccessesThreadLocal,
+
+  // These only apply to callees, where the analysis stops at the Decl
+  DeclDisallowsInference,
+
+  CallsDeclWithoutEffect,
+  CallsExprWithoutEffect,
+};
+
+// Represents a violation of the rules, potentially for the entire duration of
+// the analysis phase, in order to refer to it when explaining why a caller has
+// been made unsafe by a callee. Can be transformed into either a Diagnostic
+// (warning or a note), depending on whether the violation pertains to a
+// function failing to be verifed as holding an effect vs. a function failing 
to
+// be inferred as holding that effect.
+struct Violation {
+  FunctionEffect Effect;
+  FunctionEffect CalleeEffectPreventingInference; // only for certain IDs
+  ViolationID ID = ViolationID::None;
+  SourceLocation Loc;
+  const Decl *Callee = nullptr; // only valid for Calls*
+
+  Violation() = default;
+
+  Violation(const FunctionEffect &Effect, ViolationID ID, SourceLocation Loc,
+const Decl *Callee = nullptr,
+const FunctionEffect *CalleeEffect = nullptr)
+  : Effect(Effect), ID(ID), Loc(Loc), Callee(Callee) {
+if (CalleeEffect != nullptr)
+  CalleeEffectPreventingInference = *CalleeEffect;
+  }
+};
+
+enum class SpecialFuncType : uint8_t { None, OperatorNew, OperatorDelete };
+enum class CallableType {
+  // unknown: probably function pointer
+  Unknown,
+  Function,
+  Virtual,
+  Block
+};
+
+// Return whether a function's effects CAN be verified.
+// The question of whether it SHOULD be verified is independent.
+static bool functionIsVerifiable(const FunctionDecl *FD) {
+  if (FD->isTrivial()) {
+// Otherwise `struct x { int a; };` would have an unverifiable default
+// constructor.
+return true;
+  }
+  return FD->hasBody();
+}
+
+static bool isNoexcept(const FunctionDecl *FD) {
+  const auto *FPT = FD->getType()->castAs();
+  if (FPT->isNothrow() || FD->hasAttr())
+return true;
+  return false;
+}
+
+// Transitory, more extended information about a callable, which can be a
+// function, block, or function pointer.
+struct CallableInfo {
+  // CDecl holds the function's definition, if any.
+  // FunctionDecl if CallableType::Function or Virtual
+  // BlockDecl if CallableType::Block
+  const Decl *CDecl;
+
+  // Remember whether the callable is a function, block, virtual method,
+  // or (presumed) function pointer.
+  CallableType CType = CallableType::Unknown;
+
+  // Remember whether the callable is an operator new or delete function,
+  // so that calls to them are reported more meaningfully, as memory
+  // allocations.
+  SpecialFuncType FuncType = SpecialFuncType::None;
+
+  // We inevitably want to know the callable's declared effects, so cache them.
+  FunctionEffectKindSet Effects;
+
+  CallableInfo(const Decl &CD, SpecialFuncType FT = SpecialFuncType::None)
+  : CDecl(&CD), FuncType(FT) {
+FunctionEffectsRef DeclEffects;
+if (auto *FD = dyn_cast(CDecl)) {
+  // Use the function's definition, if any.
+  if (const FunctionDecl *Def = FD->getDefinition())
+CDecl = FD = Def;
+  CType = CallableType::Function;
+  if (auto *Method = dyn_cast(FD);
+  Method && Method->isVirtual())
+CType = CallableType::Virtual;
+  DeclEffects = FD->getFunctionEffects();
+} else if (auto *BD = dyn_cast(CDecl)) {
+  CType = CallableType::Block;
+  DeclEffects = BD->getFunctionEffects();
+} else if (auto *VD = dyn_cast(CDecl)) {
+  // ValueDecl is function, enum, or variable, so just look at its type.
+  DeclEffects = FunctionEffectsRef::get(VD->getType());
+}
+Effects = FunctionEffectKindSet(DeclEffects);
+  }
+
+  CallableType type() const { return CType; }
+
+  bool isCalledDirectly() const {
+return CType == CallableType::Function || CType == CallableType::Block;
+  }
+
+  bool isVerifiable() const {
+switch (CType) {
+case CallableType::Unknown:
+case CallableType::Virtual:

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -4914,6 +4920,78 @@ class FunctionEffectsRef {
   void dump(llvm::raw_ostream &OS) const;
 };
 
+/// A mutable set of FunctionEffect::Kind.
+class FunctionEffectKindSet {

dougsonos wrote:

I agree that this can and should use `std::bitset` instead of reinventing it, 
as a representation, but that doesn't help with the problem of iterating 
through the set bits, and a lot of code wants to iterate through these sets.

The only alternative I can think of is a `for_each()` method. Sometimes that 
can be messier than it's worth and sometimes it is better than a special 
iterator. Will try it.

https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -0,0 +1,1199 @@
+//=== EffectAnalysis.cpp - Sema warnings for function effects 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file implements caller/callee analysis for function effects.
+//
+//===--===//
+
+#include "clang/AST/Decl.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/Type.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Sema/SemaInternal.h"
+
+#define DEBUG_TYPE "fxanalysis"
+
+using namespace clang;
+
+namespace {
+
+enum class ViolationID : uint8_t {
+  None = 0, // sentinel for an empty Violation
+  Throws,
+  Catches,
+  CallsObjC,
+  AllocatesMemory,
+  HasStaticLocal,
+  AccessesThreadLocal,
+
+  // These only apply to callees, where the analysis stops at the Decl
+  DeclDisallowsInference,
+
+  CallsDeclWithoutEffect,
+  CallsExprWithoutEffect,
+};
+
+// Represents a violation of the rules, potentially for the entire duration of
+// the analysis phase, in order to refer to it when explaining why a caller has
+// been made unsafe by a callee. Can be transformed into either a Diagnostic
+// (warning or a note), depending on whether the violation pertains to a
+// function failing to be verifed as holding an effect vs. a function failing 
to
+// be inferred as holding that effect.
+struct Violation {
+  FunctionEffect Effect;
+  FunctionEffect CalleeEffectPreventingInference; // only for certain IDs
+  ViolationID ID = ViolationID::None;
+  SourceLocation Loc;
+  const Decl *Callee = nullptr; // only valid for Calls*
+
+  Violation() = default;
+
+  Violation(const FunctionEffect &Effect, ViolationID ID, SourceLocation Loc,
+const Decl *Callee = nullptr,
+const FunctionEffect *CalleeEffect = nullptr)
+  : Effect(Effect), ID(ID), Loc(Loc), Callee(Callee) {
+if (CalleeEffect != nullptr)
+  CalleeEffectPreventingInference = *CalleeEffect;
+  }
+};
+
+enum class SpecialFuncType : uint8_t { None, OperatorNew, OperatorDelete };
+enum class CallableType {
+  // unknown: probably function pointer
+  Unknown,
+  Function,
+  Virtual,
+  Block
+};
+
+// Return whether a function's effects CAN be verified.
+// The question of whether it SHOULD be verified is independent.
+static bool functionIsVerifiable(const FunctionDecl *FD) {
+  if (FD->isTrivial()) {
+// Otherwise `struct x { int a; };` would have an unverifiable default
+// constructor.
+return true;
+  }
+  return FD->hasBody();
+}
+
+static bool isNoexcept(const FunctionDecl *FD) {
+  const auto *FPT = FD->getType()->castAs();
+  if (FPT->isNothrow() || FD->hasAttr())
+return true;
+  return false;
+}
+
+// Transitory, more extended information about a callable, which can be a
+// function, block, or function pointer.
+struct CallableInfo {
+  // CDecl holds the function's definition, if any.
+  // FunctionDecl if CallableType::Function or Virtual
+  // BlockDecl if CallableType::Block
+  const Decl *CDecl;
+
+  // Remember whether the callable is a function, block, virtual method,
+  // or (presumed) function pointer.
+  CallableType CType = CallableType::Unknown;
+
+  // Remember whether the callable is an operator new or delete function,
+  // so that calls to them are reported more meaningfully, as memory
+  // allocations.
+  SpecialFuncType FuncType = SpecialFuncType::None;
+
+  // We inevitably want to know the callable's declared effects, so cache them.
+  FunctionEffectKindSet Effects;
+
+  CallableInfo(const Decl &CD, SpecialFuncType FT = SpecialFuncType::None)
+  : CDecl(&CD), FuncType(FT) {
+FunctionEffectsRef DeclEffects;
+if (auto *FD = dyn_cast(CDecl)) {
+  // Use the function's definition, if any.
+  if (const FunctionDecl *Def = FD->getDefinition())
+CDecl = FD = Def;
+  CType = CallableType::Function;
+  if (auto *Method = dyn_cast(FD);
+  Method && Method->isVirtual())
+CType = CallableType::Virtual;
+  DeclEffects = FD->getFunctionEffects();
+} else if (auto *BD = dyn_cast(CDecl)) {
+  CType = CallableType::Block;
+  DeclEffects = BD->getFunctionEffects();
+} else if (auto *VD = dyn_cast(CDecl)) {
+  // ValueDecl is function, enum, or variable, so just look at its type.
+  DeclEffects = FunctionEffectsRef::get(VD->getType());
+}
+Effects = FunctionEffectKindSet(DeclEffects);
+  }
+
+  CallableType type() const { return CType; }
+
+  bool isCalledDirectly() const {
+return CType == CallableType::Function || CType == CallableType::Block;
+  }
+
+  bool isVerifiable() const {
+switch (CType) {
+case CallableType::Unknown:
+case CallableType::Virtual:

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -0,0 +1,1199 @@
+//=== EffectAnalysis.cpp - Sema warnings for function effects 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file implements caller/callee analysis for function effects.
+//
+//===--===//
+
+#include "clang/AST/Decl.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/Type.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Sema/SemaInternal.h"
+
+#define DEBUG_TYPE "fxanalysis"
+
+using namespace clang;
+
+namespace {
+
+enum class ViolationID : uint8_t {
+  None = 0, // sentinel for an empty Violation
+  Throws,
+  Catches,
+  CallsObjC,
+  AllocatesMemory,
+  HasStaticLocal,
+  AccessesThreadLocal,
+
+  // These only apply to callees, where the analysis stops at the Decl
+  DeclDisallowsInference,
+
+  CallsDeclWithoutEffect,
+  CallsExprWithoutEffect,
+};
+
+// Represents a violation of the rules, potentially for the entire duration of
+// the analysis phase, in order to refer to it when explaining why a caller has
+// been made unsafe by a callee. Can be transformed into either a Diagnostic
+// (warning or a note), depending on whether the violation pertains to a
+// function failing to be verifed as holding an effect vs. a function failing 
to
+// be inferred as holding that effect.
+struct Violation {
+  FunctionEffect Effect;
+  FunctionEffect CalleeEffectPreventingInference; // only for certain IDs
+  ViolationID ID = ViolationID::None;
+  SourceLocation Loc;
+  const Decl *Callee = nullptr; // only valid for Calls*
+
+  Violation() = default;
+
+  Violation(const FunctionEffect &Effect, ViolationID ID, SourceLocation Loc,
+const Decl *Callee = nullptr,
+const FunctionEffect *CalleeEffect = nullptr)
+  : Effect(Effect), ID(ID), Loc(Loc), Callee(Callee) {
+if (CalleeEffect != nullptr)
+  CalleeEffectPreventingInference = *CalleeEffect;
+  }
+};
+
+enum class SpecialFuncType : uint8_t { None, OperatorNew, OperatorDelete };
+enum class CallableType {
+  // unknown: probably function pointer
+  Unknown,
+  Function,
+  Virtual,
+  Block
+};
+
+// Return whether a function's effects CAN be verified.
+// The question of whether it SHOULD be verified is independent.
+static bool functionIsVerifiable(const FunctionDecl *FD) {
+  if (FD->isTrivial()) {
+// Otherwise `struct x { int a; };` would have an unverifiable default
+// constructor.
+return true;
+  }
+  return FD->hasBody();
+}
+
+static bool isNoexcept(const FunctionDecl *FD) {
+  const auto *FPT = FD->getType()->castAs();
+  if (FPT->isNothrow() || FD->hasAttr())
+return true;
+  return false;
+}
+
+// Transitory, more extended information about a callable, which can be a
+// function, block, or function pointer.
+struct CallableInfo {
+  // CDecl holds the function's definition, if any.
+  // FunctionDecl if CallableType::Function or Virtual
+  // BlockDecl if CallableType::Block
+  const Decl *CDecl;
+
+  // Remember whether the callable is a function, block, virtual method,
+  // or (presumed) function pointer.
+  CallableType CType = CallableType::Unknown;
+
+  // Remember whether the callable is an operator new or delete function,
+  // so that calls to them are reported more meaningfully, as memory
+  // allocations.
+  SpecialFuncType FuncType = SpecialFuncType::None;
+
+  // We inevitably want to know the callable's declared effects, so cache them.
+  FunctionEffectKindSet Effects;
+
+  CallableInfo(const Decl &CD, SpecialFuncType FT = SpecialFuncType::None)
+  : CDecl(&CD), FuncType(FT) {
+FunctionEffectsRef DeclEffects;
+if (auto *FD = dyn_cast(CDecl)) {
+  // Use the function's definition, if any.
+  if (const FunctionDecl *Def = FD->getDefinition())
+CDecl = FD = Def;
+  CType = CallableType::Function;
+  if (auto *Method = dyn_cast(FD);
+  Method && Method->isVirtual())
+CType = CallableType::Virtual;
+  DeclEffects = FD->getFunctionEffects();
+} else if (auto *BD = dyn_cast(CDecl)) {
+  CType = CallableType::Block;
+  DeclEffects = BD->getFunctionEffects();
+} else if (auto *VD = dyn_cast(CDecl)) {
+  // ValueDecl is function, enum, or variable, so just look at its type.
+  DeclEffects = FunctionEffectsRef::get(VD->getType());
+}
+Effects = FunctionEffectKindSet(DeclEffects);
+  }
+
+  CallableType type() const { return CType; }
+
+  bool isCalledDirectly() const {
+return CType == CallableType::Function || CType == CallableType::Block;
+  }
+
+  bool isVerifiable() const {
+switch (CType) {
+case CallableType::Unknown:
+case CallableType::Virtual:

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -0,0 +1,256 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -fcxx-exceptions -std=c++20 -verify 
%s
+// These are in a separate file because errors (e.g. incompatible attributes) 
currently prevent
+// the FXAnalysis pass from running at all.
+
+// This diagnostic is re-enabled and exercised in isolation later in this file.
+#pragma clang diagnostic ignored "-Wperf-constraint-implies-noexcept"
+
+// --- CONSTRAINTS ---
+
+void nb1() [[clang::nonblocking]]
+{
+   int *pInt = new int; // expected-warning {{'nonblocking' function must 
not allocate or deallocate memory}}
+   delete pInt; // expected-warning {{'nonblocking' function must not 
allocate or deallocate memory}}
+}
+
+void nb2() [[clang::nonblocking]]
+{
+   static int global; // expected-warning {{'nonblocking' function must 
not have static locals}}
+}
+
+void nb3() [[clang::nonblocking]]
+{
+   try {
+   throw 42; // expected-warning {{'nonblocking' function must not 
throw or catch exceptions}}
+   }
+   catch (...) { // expected-warning {{'nonblocking' function must not 
throw or catch exceptions}}
+   }
+}
+

dougsonos wrote:

Without making any changes specific to these tests, they currently generate:

```
void catches() try {} catch (...) {} // expected-note {{function cannot be 
inferred 'nonblocking' because it throws or catches exceptions}}

void nb20() [[clang::nonblocking]] {
catches(); // expected-warning {{'nonblocking' function must not call 
non-'nonblocking' function 'catches'}}
}

struct S {
int x;
S(int x) try : x(x) {} catch (...) {} // expected-note {{function cannot be 
inferred 'nonblocking' because it throws or catches exceptions}}
S(double) : x((throw 3, 3)) {} // expected-note {{function cannot be 
inferred 'nonblocking' because it throws or catches exceptions}}
};

int badi(); // expected-note {{declaration cannot be inferred 'nonblocking' 
because it has no definition in this translation unit}}

struct A {
int x = (throw 3, 3); // expected-note {{function cannot be inferred 
'nonblocking' because it throws or catches exceptions}}
};

struct B {
int y = badi(); // expected-note {{function cannot be inferred 
'nonblocking' because it calls non-'nonblocking' function 'badi'}}
};

void f() [[clang::nonblocking]] {
S s1(3);   // expected-warning {{'nonblocking' function must not call 
non-'nonblocking' function 'S::S'}}
S s2(3.0); // expected-warning {{'nonblocking' function must not call 
non-'nonblocking' function 'S::S'}}
A a;   // expected-warning {{'nonblocking' function must not call 
non-'nonblocking' function 'A::A'}}
B b;   // expected-warning {{ 'nonblocking' function must not call 
non-'nonblocking' function 'B::B'}}
}
```

https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

https://github.com/dougsonos edited 
https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] nonblocking/nonallocating attributes: 2nd pass caller/callee analysis (PR #99656)

[Doug Wyatt via cfe-commits]

@@ -0,0 +1,256 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -fcxx-exceptions -std=c++20 -verify 
%s
+// These are in a separate file because errors (e.g. incompatible attributes) 
currently prevent
+// the FXAnalysis pass from running at all.
+
+// This diagnostic is re-enabled and exercised in isolation later in this file.
+#pragma clang diagnostic ignored "-Wperf-constraint-implies-noexcept"
+
+// --- CONSTRAINTS ---
+
+void nb1() [[clang::nonblocking]]
+{
+   int *pInt = new int; // expected-warning {{'nonblocking' function must 
not allocate or deallocate memory}}
+   delete pInt; // expected-warning {{'nonblocking' function must not 
allocate or deallocate memory}}
+}
+
+void nb2() [[clang::nonblocking]]
+{
+   static int global; // expected-warning {{'nonblocking' function must 
not have static locals}}
+}
+
+void nb3() [[clang::nonblocking]]
+{
+   try {
+   throw 42; // expected-warning {{'nonblocking' function must not 
throw or catch exceptions}}
+   }
+   catch (...) { // expected-warning {{'nonblocking' function must not 
throw or catch exceptions}}
+   }
+}
+

dougsonos wrote:

In the last one, the diagnostic currently attaches to:
```
int x = badi();
```
without the context that one of the constructors is calling this. Hmm...

https://github.com/llvm/llvm-project/pull/99656
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits