[clang] nolock/noalloc attributes (PR #84983)
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)
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)
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)
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)
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)
@@ -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)
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)
@@ -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)
@@ -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)
@@ -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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
@@ -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)
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)
@@ -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)
@@ -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] nolock/noalloc attributes (PR #84983)
@@ -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)
@@ -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)
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)
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)
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)
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)
@@ -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)
@@ -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)
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)
@@ -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)
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)
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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
@@ -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)
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)
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)
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)
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)
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)
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)
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)
@@ -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)
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)
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)
@@ -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)
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)
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)
@@ -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)
@@ -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)
@@ -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] nonblocking/nonallocating attributes (was: nolock/noalloc) (PR #84983)
@@ -464,6 +466,16 @@ class ASTContext : public RefCountedBase {
/// This is the top-level (C++20) Named module we are building.
Module *CurrentCXXNamedModule = nullptr;
+ class FunctionEffectSetUniquing {
+llvm::DenseSet> Set;
+
+ public:
+FunctionEffectSet getUniqued(llvm::ArrayRef FX);
+
+~FunctionEffectSetUniquing();
+ };
+ FunctionEffectSetUniquing UniquedFunctionEffectSet;
+
dougsonos wrote:
The idea here was that since the set elements are `ArrayRef`s pointing to
separately allocated memory, the destructor of this object could do all the
destruction. Moot if this uniquing business goes away though.
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 (was: nolock/noalloc) (PR #84983)
@@ -4429,6 +4433,218 @@ class FunctionNoProtoType : public FunctionType, public
llvm::FoldingSetNode {
}
};
+//
--
+
+// TODO: Should FunctionEffect be located elsewhere, where Decl is not
+// forward-declared?
+class Decl;
+class CXXMethodDecl;
+class FunctionEffectSet;
+class TypeSourceInfo;
+
+/// Represents an abstract function effect.
+class FunctionEffect {
+public:
+ /// Identifies the particular type of effect.
+ enum class Type {
+None = 0,
+NonBlocking,
+NonAllocating,
+ };
+
+ /// Flags describing behaviors of the effect.
+ // (Why not a struct with bitfields? There's one function that would like to
+ // test a caller-specified bit. There are some potential optimizations that
+ // would OR together the bits of multiple effects.)
+ using Flags = unsigned;
+ enum FlagBit : unsigned {
+// Can verification inspect callees' implementations? (e.g. nonblocking:
+// yes, tcb+types: no)
+FE_InferrableOnCallees = 0x1,
+
+// Language constructs which effects can diagnose as disallowed.
+FE_ExcludeThrow = 0x2,
+FE_ExcludeCatch = 0x4,
+FE_ExcludeObjCMessageSend = 0x8,
+FE_ExcludeStaticLocalVars = 0x10,
+FE_ExcludeThreadLocalVars = 0x20
+ };
+
+ /// Describes the result of effects differing between a base class's virtual
+ /// method and an overriding method in a subclass.
+ enum class OverrideResult {
+Ignore,
+Warn,
+Propagate // Base method's effects are merged with those of the override.
+ };
+
+private:
+ // For uniqueness, currently only Type_ is significant.
+
+ LLVM_PREFERRED_TYPE(Type)
+ unsigned Type_ : 2;
+ Flags Flags_ : 8; // A constant function of Type but cached here.
+
+ // Expansion: for hypothetical TCB+types, there could be one Type for TCB,
+ // then ~16(?) bits "Subtype" to map to a specific named TCB. Subtype would
+ // be considered for uniqueness.
+
+ // Since this struct is serialized as if it were a uint32_t, it's important
+ // to pad and explicitly zero the extra bits.
+ [[maybe_unused]] unsigned Padding : 22;
+
+public:
+ using CalleeDeclOrType =
+ llvm::PointerUnion;
dougsonos wrote:
Thanks. It turns out to be unused, an artifact of a previous implementation.
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 (was: nolock/noalloc) (PR #84983)
@@ -4429,6 +4433,218 @@ class FunctionNoProtoType : public FunctionType, public
llvm::FoldingSetNode {
}
};
+//
--
+
+// TODO: Should FunctionEffect be located elsewhere, where Decl is not
+// forward-declared?
+class Decl;
+class CXXMethodDecl;
+class FunctionEffectSet;
+class TypeSourceInfo;
+
+/// Represents an abstract function effect.
+class FunctionEffect {
+public:
+ /// Identifies the particular type of effect.
+ enum class Type {
+None = 0,
+NonBlocking,
+NonAllocating,
+ };
+
+ /// Flags describing behaviors of the effect.
+ // (Why not a struct with bitfields? There's one function that would like to
+ // test a caller-specified bit. There are some potential optimizations that
+ // would OR together the bits of multiple effects.)
+ using Flags = unsigned;
+ enum FlagBit : unsigned {
+// Can verification inspect callees' implementations? (e.g. nonblocking:
+// yes, tcb+types: no)
+FE_InferrableOnCallees = 0x1,
+
+// Language constructs which effects can diagnose as disallowed.
+FE_ExcludeThrow = 0x2,
+FE_ExcludeCatch = 0x4,
+FE_ExcludeObjCMessageSend = 0x8,
+FE_ExcludeStaticLocalVars = 0x10,
+FE_ExcludeThreadLocalVars = 0x20
+ };
+
+ /// Describes the result of effects differing between a base class's virtual
+ /// method and an overriding method in a subclass.
+ enum class OverrideResult {
+Ignore,
+Warn,
+Propagate // Base method's effects are merged with those of the override.
+ };
+
+private:
+ // For uniqueness, currently only Type_ is significant.
+
+ LLVM_PREFERRED_TYPE(Type)
+ unsigned Type_ : 2;
+ Flags Flags_ : 8; // A constant function of Type but cached here.
+
+ // Expansion: for hypothetical TCB+types, there could be one Type for TCB,
+ // then ~16(?) bits "Subtype" to map to a specific named TCB. Subtype would
+ // be considered for uniqueness.
+
+ // Since this struct is serialized as if it were a uint32_t, it's important
+ // to pad and explicitly zero the extra bits.
+ [[maybe_unused]] unsigned Padding : 22;
+
+public:
+ using CalleeDeclOrType =
+ llvm::PointerUnion;
+
+ FunctionEffect() : Type_(unsigned(Type::None)), Flags_(0), Padding(0) {}
+
+ explicit FunctionEffect(Type T);
+
+ /// The type of the effect.
+ Type type() const { return Type(Type_); }
+
+ /// Flags describing behaviors of the effect.
+ Flags flags() const { return Flags_; }
+
+ /// The description printed in diagnostics, e.g. 'nonblocking'.
+ StringRef name() const;
+
+ /// A hashable representation.
+ uint32_t opaqueRepr() const { return Type_ | (Flags_ << 2u); }
+
+ /// Return true if adding or removing the effect as part of a type conversion
+ /// should generate a diagnostic.
+ bool diagnoseConversion(bool Adding, QualType OldType,
+ FunctionEffectSet OldFX, QualType NewType,
+ FunctionEffectSet NewFX) const;
dougsonos wrote:
I think this is a consequence of trying to abstract the concept of an effect,
and hide the differences that make one effect different from another. You might
remember that the original implementation had effects as basically an object
with a vtable.
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 (was: nolock/noalloc) (PR #84983)
@@ -4429,6 +4433,218 @@ class FunctionNoProtoType : public FunctionType, public
llvm::FoldingSetNode {
}
};
+//
--
+
+// TODO: Should FunctionEffect be located elsewhere, where Decl is not
+// forward-declared?
+class Decl;
+class CXXMethodDecl;
+class FunctionEffectSet;
+class TypeSourceInfo;
+
+/// Represents an abstract function effect.
+class FunctionEffect {
+public:
+ /// Identifies the particular type of effect.
+ enum class Type {
+None = 0,
+NonBlocking,
+NonAllocating,
+ };
+
+ /// Flags describing behaviors of the effect.
+ // (Why not a struct with bitfields? There's one function that would like to
+ // test a caller-specified bit. There are some potential optimizations that
+ // would OR together the bits of multiple effects.)
+ using Flags = unsigned;
+ enum FlagBit : unsigned {
+// Can verification inspect callees' implementations? (e.g. nonblocking:
+// yes, tcb+types: no)
+FE_InferrableOnCallees = 0x1,
+
+// Language constructs which effects can diagnose as disallowed.
+FE_ExcludeThrow = 0x2,
+FE_ExcludeCatch = 0x4,
+FE_ExcludeObjCMessageSend = 0x8,
+FE_ExcludeStaticLocalVars = 0x10,
+FE_ExcludeThreadLocalVars = 0x20
+ };
+
+ /// Describes the result of effects differing between a base class's virtual
+ /// method and an overriding method in a subclass.
+ enum class OverrideResult {
+Ignore,
+Warn,
+Propagate // Base method's effects are merged with those of the override.
+ };
+
+private:
+ // For uniqueness, currently only Type_ is significant.
+
+ LLVM_PREFERRED_TYPE(Type)
+ unsigned Type_ : 2;
+ Flags Flags_ : 8; // A constant function of Type but cached here.
+
+ // Expansion: for hypothetical TCB+types, there could be one Type for TCB,
+ // then ~16(?) bits "Subtype" to map to a specific named TCB. Subtype would
+ // be considered for uniqueness.
+
+ // Since this struct is serialized as if it were a uint32_t, it's important
+ // to pad and explicitly zero the extra bits.
+ [[maybe_unused]] unsigned Padding : 22;
+
+public:
+ using CalleeDeclOrType =
+ llvm::PointerUnion;
+
+ FunctionEffect() : Type_(unsigned(Type::None)), Flags_(0), Padding(0) {}
+
+ explicit FunctionEffect(Type T);
+
+ /// The type of the effect.
+ Type type() const { return Type(Type_); }
+
+ /// Flags describing behaviors of the effect.
+ Flags flags() const { return Flags_; }
+
+ /// The description printed in diagnostics, e.g. 'nonblocking'.
+ StringRef name() const;
+
+ /// A hashable representation.
+ uint32_t opaqueRepr() const { return Type_ | (Flags_ << 2u); }
+
+ /// Return true if adding or removing the effect as part of a type conversion
+ /// should generate a diagnostic.
+ bool diagnoseConversion(bool Adding, QualType OldType,
+ FunctionEffectSet OldFX, QualType NewType,
+ FunctionEffectSet NewFX) const;
+
+ /// Return true if adding or removing the effect in a redeclaration should
+ /// generate a diagnostic.
+ bool diagnoseRedeclaration(bool Adding, const FunctionDecl &OldFunction,
+ FunctionEffectSet OldFX,
+ const FunctionDecl &NewFunction,
+ FunctionEffectSet NewFX) const;
+
+ /// Return true if adding or removing the effect in a C++ virtual method
+ /// override should generate a diagnostic.
+ OverrideResult diagnoseMethodOverride(bool Adding,
+const CXXMethodDecl &OldMethod,
+FunctionEffectSet OldFX,
+const CXXMethodDecl &NewMethod,
+FunctionEffectSet NewFX) const;
+
+ /// Return true if the effect is allowed to be inferred on a Decl of the
+ /// specified type (generally a FunctionProtoType but TypeSourceInfo is
+ /// provided so any AttributedType sugar can be examined). TSI can be null
+ /// on an implicit function like a default constructor.
+ ///
+ /// This is only used if the effect has FE_InferrableOnCallees flag set.
+ /// Example: This allows nonblocking(false) to prevent inference for the
+ /// function.
+ bool canInferOnFunction(QualType QT, const TypeSourceInfo *FType) const;
+
+ // Return false for success. When true is returned for a direct call, then
the
+ // FE_InferrableOnCallees flag may trigger inference rather than an immediate
+ // diagnostic. Caller should be assumed to have the effect (it may not have
it
+ // explicitly when inferring).
+ bool diagnoseFunctionCall(bool Direct, FunctionEffectSet CalleeFX) const;
+
+ friend bool operator==(const FunctionEffect &LHS, const FunctionEffect &RHS)
{
+return LHS.Type_ == RHS.Type_;
+ }
+ friend bool operator!=(const FunctionEffect &LHS, const FunctionEffect &RHS)
{
+return LHS.Type_ != RHS.Type_;
+ }
+ fr
[clang] nonblocking/nonallocating attributes (was: nolock/noalloc) (PR #84983)
@@ -10798,6 +10798,95 @@ 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 must not allocate or deallocate memory">, dougsonos wrote: Here, I have removed the function name, and '%0' is the name of the effect. So you'd see 'nonblocking' function must not allocate or deallocate 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] nonblocking/nonallocating attributes (was: nolock/noalloc) (PR #84983)
@@ -3144,6 +3154,9 @@ class Sema final {
QualType T, TypeSourceInfo *TSInfo,
StorageClass SC);
+ /// Potentially add a FunctionDecl or BlockDecl to DeclsWithEffectsToVerify.
+ void CheckAddCallableWithEffects(const Decl *D, FunctionEffectSet FX);
dougsonos wrote:
Yeah this is a hard area for me with names.
What it's trying to convey is that it might, depending on some checks, add the
callable to a list of decls to verify.
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 (was: nolock/noalloc) (PR #84983)
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] nonblocking/nonallocating attributes (was: nolock/noalloc) (PR #84983)
@@ -5016,3 +5024,254 @@ void AutoType::Profile(llvm::FoldingSetNodeID &ID,
const ASTContext &Context) {
Profile(ID, Context, getDeducedType(), getKeyword(), isDependentType(),
getTypeConstraintConcept(), getTypeConstraintArguments());
}
+
+FunctionEffect::FunctionEffect(Type T)
+: Type_(static_cast(T)), Flags_(0), Padding(0) {
+ switch (T) {
+ case Type::NonBlocking:
+Flags_ = FE_InferrableOnCallees | FE_ExcludeThrow | FE_ExcludeCatch |
+ FE_ExcludeObjCMessageSend | FE_ExcludeStaticLocalVars |
+ FE_ExcludeThreadLocalVars;
+break;
+
+ case Type::NonAllocating:
+// Same as NonBlocking, except without FE_ExcludeStaticLocalVars
+Flags_ = FE_InferrableOnCallees | FE_ExcludeThrow | FE_ExcludeCatch |
+ FE_ExcludeObjCMessageSend | FE_ExcludeThreadLocalVars;
+break;
+ default:
+break;
+ }
+}
+
+StringRef FunctionEffect::name() const {
+ switch (type()) {
+ default:
+return "";
+ case Type::NonBlocking:
+return "nonblocking";
+ case Type::NonAllocating:
+return "nonallocating";
+ }
+}
+
+bool FunctionEffect::diagnoseConversion(bool Adding, QualType OldType,
dougsonos wrote:
Thanks, will rename all of the `diagnoseX` methods to `shouldDiagnoseX`.
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 (was: nolock/noalloc) (PR #84983)
@@ -5016,3 +5024,254 @@ void AutoType::Profile(llvm::FoldingSetNodeID &ID,
const ASTContext &Context) {
Profile(ID, Context, getDeducedType(), getKeyword(), isDependentType(),
getTypeConstraintConcept(), getTypeConstraintArguments());
}
+
+FunctionEffect::FunctionEffect(Type T)
+: Type_(static_cast(T)), Flags_(0), Padding(0) {
+ switch (T) {
+ case Type::NonBlocking:
+Flags_ = FE_InferrableOnCallees | FE_ExcludeThrow | FE_ExcludeCatch |
+ FE_ExcludeObjCMessageSend | FE_ExcludeStaticLocalVars |
+ FE_ExcludeThreadLocalVars;
+break;
+
+ case Type::NonAllocating:
+// Same as NonBlocking, except without FE_ExcludeStaticLocalVars
+Flags_ = FE_InferrableOnCallees | FE_ExcludeThrow | FE_ExcludeCatch |
+ FE_ExcludeObjCMessageSend | FE_ExcludeThreadLocalVars;
+break;
+ default:
+break;
+ }
+}
+
+StringRef FunctionEffect::name() const {
+ switch (type()) {
+ default:
+return "";
+ case Type::NonBlocking:
+return "nonblocking";
+ case Type::NonAllocating:
+return "nonallocating";
+ }
+}
+
+bool FunctionEffect::diagnoseConversion(bool Adding, QualType OldType,
+FunctionEffectSet OldFX,
+QualType NewType,
+FunctionEffectSet NewFX) const {
+
+ switch (type()) {
+ case Type::NonAllocating:
+// nonallocating can't be added (spoofed) during a conversion, unless we
+// have nonblocking
+if (Adding) {
+ for (const auto &Effect : OldFX) {
+if (Effect.type() == Type::NonBlocking)
+ return false;
+ }
+}
+[[fallthrough]];
+ case Type::NonBlocking:
+// nonblocking can't be added (spoofed) during a conversion
+return Adding;
+ default:
+break;
+ }
+ return false;
+}
+
+bool FunctionEffect::diagnoseRedeclaration(bool Adding,
+ const FunctionDecl &OldFunction,
+ FunctionEffectSet OldFX,
+ const FunctionDecl &NewFunction,
+ FunctionEffectSet NewFX) const {
+ switch (type()) {
+ case Type::NonAllocating:
+ case Type::NonBlocking:
+// nonblocking/nonallocating can't be removed in a redeclaration
+// adding -> false, removing -> true (diagnose)
+return !Adding;
+ default:
+break;
+ }
+ return false;
+}
+
+FunctionEffect::OverrideResult FunctionEffect::diagnoseMethodOverride(
+bool Adding, const CXXMethodDecl &OldMethod, FunctionEffectSet OldFX,
+const CXXMethodDecl &NewMethod, FunctionEffectSet NewFX) const {
+ switch (type()) {
+ case Type::NonAllocating:
+ case Type::NonBlocking:
+// if added on an override, that's fine and not diagnosed.
+// if missing from an override (removed), propagate from base to derived.
+return Adding ? OverrideResult::Ignore : OverrideResult::Propagate;
+ default:
+break;
+ }
+ return OverrideResult::Ignore;
+}
+
+bool FunctionEffect::canInferOnFunction(QualType QT,
+const TypeSourceInfo *FType) const {
+ switch (type()) {
+ case Type::NonAllocating:
+ case Type::NonBlocking: {
+// Does the sugar have nonblocking(false) / nonallocating(false) ?
+if (QT->hasAttr(type() == Type::NonBlocking ? attr::Kind::Blocking
+: attr::Kind::Allocating)) {
+ return false;
+}
+
+return true;
+ }
+
+ default:
+break;
+ }
+ return false;
+}
+
+bool FunctionEffect::diagnoseFunctionCall(bool Direct,
+ FunctionEffectSet CalleeFX) const {
+ switch (type()) {
+ case Type::NonAllocating:
+ case Type::NonBlocking: {
+const Type CallerType = type();
+for (const auto &Effect : CalleeFX) {
+ const Type ET = Effect.type();
+ // Does callee have same or stronger constraint?
+ if (ET == CallerType ||
+ (CallerType == Type::NonAllocating && ET == Type::NonBlocking)) {
+return false; // no diagnostic
+ }
+}
+return true; // warning
+ }
+ default:
+break;
+ }
+ return false;
+}
+
+// =
+
+MutableFunctionEffectSet::MutableFunctionEffectSet(
+const FunctionEffect &effect) {
+ push_back(effect);
+}
+
+void MutableFunctionEffectSet::insert(const FunctionEffect &Effect) {
+ const auto &Iter = std::lower_bound(begin(), end(), Effect);
+ if (Iter == end() || *Iter != Effect) {
+insert(Iter, Effect);
+ }
+}
+
+MutableFunctionEffectSet &
+MutableFunctionEffectSet::operator|=(FunctionEffectSet RHS) {
+ // TODO: For large RHS sets, use set_union or a custom insert-in-place
+ for (const auto &Effect : RHS) {
+insert(Effect);
+ }
+ return *this;
+}
+
+FunctionEffectSet FunctionEffectSet::getUnion(ASTContext &C,
+ const FunctionEffectSet &LHS,
+
[clang] nonblocking/nonallocating attributes (was: nolock/noalloc) (PR #84983)
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] nonblocking/nonallocating attributes (was: nolock/noalloc) (PR #84983)
@@ -4429,6 +4433,218 @@ class FunctionNoProtoType : public FunctionType, public
llvm::FoldingSetNode {
}
};
+//
--
+
+// TODO: Should FunctionEffect be located elsewhere, where Decl is not
+// forward-declared?
+class Decl;
+class CXXMethodDecl;
+class FunctionEffectSet;
+class TypeSourceInfo;
+
+/// Represents an abstract function effect.
+class FunctionEffect {
+public:
+ /// Identifies the particular type of effect.
+ enum class Type {
+None = 0,
+NonBlocking,
+NonAllocating,
+ };
+
+ /// Flags describing behaviors of the effect.
+ // (Why not a struct with bitfields? There's one function that would like to
+ // test a caller-specified bit. There are some potential optimizations that
+ // would OR together the bits of multiple effects.)
+ using Flags = unsigned;
+ enum FlagBit : unsigned {
+// Can verification inspect callees' implementations? (e.g. nonblocking:
+// yes, tcb+types: no)
+FE_InferrableOnCallees = 0x1,
+
+// Language constructs which effects can diagnose as disallowed.
+FE_ExcludeThrow = 0x2,
+FE_ExcludeCatch = 0x4,
+FE_ExcludeObjCMessageSend = 0x8,
+FE_ExcludeStaticLocalVars = 0x10,
+FE_ExcludeThreadLocalVars = 0x20
+ };
+
+ /// Describes the result of effects differing between a base class's virtual
+ /// method and an overriding method in a subclass.
+ enum class OverrideResult {
+Ignore,
+Warn,
+Propagate // Base method's effects are merged with those of the override.
+ };
+
+private:
+ // For uniqueness, currently only Type_ is significant.
+
+ LLVM_PREFERRED_TYPE(Type)
+ unsigned Type_ : 2;
+ Flags Flags_ : 8; // A constant function of Type but cached here.
+
+ // Expansion: for hypothetical TCB+types, there could be one Type for TCB,
+ // then ~16(?) bits "Subtype" to map to a specific named TCB. Subtype would
+ // be considered for uniqueness.
+
+ // Since this struct is serialized as if it were a uint32_t, it's important
+ // to pad and explicitly zero the extra bits.
+ [[maybe_unused]] unsigned Padding : 22;
+
+public:
+ using CalleeDeclOrType =
+ llvm::PointerUnion;
+
+ FunctionEffect() : Type_(unsigned(Type::None)), Flags_(0), Padding(0) {}
+
+ explicit FunctionEffect(Type T);
+
+ /// The type of the effect.
+ Type type() const { return Type(Type_); }
+
+ /// Flags describing behaviors of the effect.
+ Flags flags() const { return Flags_; }
+
+ /// The description printed in diagnostics, e.g. 'nonblocking'.
+ StringRef name() const;
+
+ /// A hashable representation.
+ uint32_t opaqueRepr() const { return Type_ | (Flags_ << 2u); }
+
+ /// Return true if adding or removing the effect as part of a type conversion
+ /// should generate a diagnostic.
+ bool diagnoseConversion(bool Adding, QualType OldType,
+ FunctionEffectSet OldFX, QualType NewType,
+ FunctionEffectSet NewFX) const;
+
+ /// Return true if adding or removing the effect in a redeclaration should
+ /// generate a diagnostic.
+ bool diagnoseRedeclaration(bool Adding, const FunctionDecl &OldFunction,
+ FunctionEffectSet OldFX,
+ const FunctionDecl &NewFunction,
+ FunctionEffectSet NewFX) const;
+
+ /// Return true if adding or removing the effect in a C++ virtual method
+ /// override should generate a diagnostic.
+ OverrideResult diagnoseMethodOverride(bool Adding,
+const CXXMethodDecl &OldMethod,
+FunctionEffectSet OldFX,
+const CXXMethodDecl &NewMethod,
+FunctionEffectSet NewFX) const;
+
+ /// Return true if the effect is allowed to be inferred on a Decl of the
+ /// specified type (generally a FunctionProtoType but TypeSourceInfo is
+ /// provided so any AttributedType sugar can be examined). TSI can be null
+ /// on an implicit function like a default constructor.
+ ///
+ /// This is only used if the effect has FE_InferrableOnCallees flag set.
+ /// Example: This allows nonblocking(false) to prevent inference for the
+ /// function.
+ bool canInferOnFunction(QualType QT, const TypeSourceInfo *FType) const;
+
+ // Return false for success. When true is returned for a direct call, then
the
+ // FE_InferrableOnCallees flag may trigger inference rather than an immediate
+ // diagnostic. Caller should be assumed to have the effect (it may not have
it
+ // explicitly when inferring).
+ bool diagnoseFunctionCall(bool Direct, FunctionEffectSet CalleeFX) const;
+
+ friend bool operator==(const FunctionEffect &LHS, const FunctionEffect &RHS)
{
+return LHS.Type_ == RHS.Type_;
+ }
+ friend bool operator!=(const FunctionEffect &LHS, const FunctionEffect &RHS)
{
+return LHS.Type_ != RHS.Type_;
+ }
+ fr
[clang] nonblocking/nonallocating attributes (was: nolock/noalloc) (PR #84983)
@@ -1868,6 +1868,28 @@ bool Sema::IsFunctionConversion(QualType FromType,
QualType ToType,
FromFn = QT->getAs();
Changed = true;
}
+
+if (getLangOpts().CPlusPlus) {
+ // For C, when called from checkPointerTypesForAssignment,
+ // we need not to change the type, or else even an innocuous cast
+ // like dropping effects will fail.
dougsonos wrote:
In an earlier review your suggestion was (in effect) to do nothing here for C
while doing the type adjustment for C++. Is there a more correct way to check
for plain C than the absence of C++? Or is this just a matter of the comment
not clearly describing the intent of the code? 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] nonblocking/nonallocating attributes (was: nolock/noalloc) (PR #84983)
@@ -7959,6 +7979,122 @@ static Attr *getCCTypeAttr(ASTContext &Ctx, ParsedAttr
&Attr) {
llvm_unreachable("unexpected attribute kind!");
}
+static bool
+handleNonBlockingNonAllocatingTypeAttr(TypeProcessingState &state,
+ ParsedAttr &PAttr, QualType &type,
+ FunctionTypeUnwrapper &unwrapped) {
+ // Delay if this is not a function type.
+ if (!unwrapped.isFunctionType())
+return false;
+
+ const bool isNonBlocking = PAttr.getKind() == ParsedAttr::AT_NonBlocking ||
+ PAttr.getKind() == ParsedAttr::AT_Blocking;
+ Sema &S = state.getSema();
+
+ // Require FunctionProtoType
+ auto *FPT = unwrapped.get()->getAs();
+ if (FPT == nullptr) {
+// TODO: special diagnostic?
+return false;
+ }
+
+ bool Cond = true; // default
+
+ if (PAttr.getKind() == ParsedAttr::AT_NonBlocking ||
+ PAttr.getKind() == ParsedAttr::AT_NonAllocating) {
+// Parse the conditional expression, if any
+// TODO: There's a better way to do this. See PR feedback.
+// TODO: Handle a type-dependent expression.
+if (PAttr.getNumArgs() > 0) {
+ if (!S.checkBoolExprArgumentAttr(PAttr, 0, Cond)) {
+PAttr.setInvalid();
+return false;
+ }
+}
+ } else {
+Cond = false;
+ }
+
+ FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
+
+ auto incompatible = [&](StringRef attrTrue, StringRef attrFalse) {
+Sema &S = state.getSema();
+S.Diag(PAttr.getLoc(), diag::err_attributes_are_not_compatible)
+<< attrTrue << attrFalse << false;
+// we don't necessarily have the location of the previous attribute,
+// so no note.
+PAttr.setInvalid();
+return true;
+ };
+
+ // check nonblocking(true) against blocking, and same for
+ // nonallocating
+ const BoolAttrState newState =
+ Cond ? BoolAttrState::True : BoolAttrState::False;
+ const BoolAttrState oppositeNewState =
+ Cond ? BoolAttrState::False : BoolAttrState::True;
dougsonos wrote:
Yeah I've already munged these lines...
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 (was: nolock/noalloc) (PR #84983)
@@ -0,0 +1,117 @@
+// RUN: %clang_cc1 %s -ast-dump -fblocks | FileCheck %s
+
+// Make sure that the attribute gets parsed and attached to the correct AST
elements.
+
+#pragma clang diagnostic ignored "-Wunused-variable"
+
+//
=
+// Square brackets, true
+
+namespace square_brackets {
+
+// On the type of the FunctionDecl
+void nl_function() [[clang::nonblocking]];
+// CHECK: FunctionDecl {{.*}} nl_function 'void ()
__attribute__((clang_nonblocking))'
+
+// On the type of the VarDecl holding a function pointer
+void (*nl_func_a)() [[clang::nonblocking]];
+// CHECK: VarDecl {{.*}} nl_func_a 'void (*)()
__attribute__((clang_nonblocking))'
+
+// On the type of the ParmVarDecl of a function parameter
+static void nlReceiver(void (*nl_func)() [[clang::nonblocking]]);
+// CHECK: ParmVarDecl {{.*}} nl_func 'void (*)()
__attribute__((clang_nonblocking))'
+
+// As an AttributedType within the nested types of a typedef
+typedef void (*nl_fp_type)() [[clang::nonblocking]];
+// CHECK: TypedefDecl {{.*}} nl_fp_type 'void (*)()
__attribute__((clang_nonblocking))'
+using nl_fp_talias = void (*)() [[clang::nonblocking]];
+// CHECK: TypeAliasDecl {{.*}} nl_fp_talias 'void (*)()
__attribute__((clang_nonblocking))'
+
+// From a typedef or typealias, on a VarDecl
+nl_fp_type nl_fp_var1;
+// CHECK: VarDecl {{.*}} nl_fp_var1 'nl_fp_type':'void (*)()
__attribute__((clang_nonblocking))'
+nl_fp_talias nl_fp_var2;
+// CHECK: VarDecl {{.*}} nl_fp_var2 'nl_fp_talias':'void (*)()
__attribute__((clang_nonblocking))'
+
+// On type of a FieldDecl
+struct Struct {
+ void (*nl_func_field)() [[clang::nonblocking]];
+// CHECK: FieldDecl {{.*}} nl_func_field 'void (*)()
__attribute__((clang_nonblocking))'
+};
+
+// nonallocating should be subsumed into nonblocking
+void nl1() [[clang::nonblocking]] [[clang::nonallocating]];
+// CHECK: FunctionDecl {{.*}} nl1 'void () __attribute__((clang_nonblocking))'
dougsonos wrote:
Hm. OK. I can see two ways to accomplish this:
- store both `nonblocking` and `nonallocating` `FunctionEffects` in the
`FunctionEffectSet` attached to the `FunctionProtoType`, despite the former
constantly trumping the other.
- store only the `nonblocking` effect in the `FunctionEffectSet` but also
preserve attributes with `AttributedType` sugar.
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 (was: nolock/noalloc) (PR #84983)
dougsonos wrote:
> I understand that you’d want to avoid allocating memory for effects over and
> over again, but at the same time—I think it’s fine to just don’t cache effect
> sets at all.
I agree that this would be simpler and fine.
> Each effect has a set of properties, which are represented as flags.
...
> As I see it, either all of an effect’s properties should be modelled as
> flags, or none should be (mixing the two sounds like the most complicated
> approach), and if we’re doing the former, then I don’t think there is a point
> in storing anything other than just the flags.
An effect is more than its flags: an identity, which can be `nonblocking`,
`nonallocating` and maybe soon `tcb("name")` (In the Discourse thread, there
were concerns about overlap with TCB, and this design really wants to support
an improved TCB that can analyze indirect calls). In the TCB case, identity is
all that matters, and none of the flags will matter. Identity is also the
straightforward way to implement the concept of `nonblocking` being a superset
of `nonallocating` in a number of places that check.
So that rules out the idea of reducing an effect to those flags.
But yes, an effect need not hold anything more than its type/identity (and for
TCB, a "subtype" that mapped to its name/identity). The flags are commented as
being cached in the `FunctionEffect` as a convenience/optimization since there
is space. But they definitely don't **have** to be there and could be simply
returned as constants, mapped from the type. I would agree that that would make
things clearer.
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 (was: nolock/noalloc) (PR #84983)
dougsonos wrote:
> > I understand that you’d want to avoid allocating memory for effects over
> > and over again, but at the same time—I think it’s fine to just don’t cache
> > effect sets at all.
>
> I agree that this would be simpler and fine.
>
> > Each effect has a set of properties, which are represented as flags.
> > ...
> > As I see it, either all of an effect’s properties should be modelled as
> > flags, or none should be (mixing the two sounds like the most complicated
> > approach), and if we’re doing the former, then I don’t think there is a
> > point in storing anything other than just the flags.
>
> An effect is more than its flags: its type is an identity, e.g.
> `nonblocking`, `nonallocating` and maybe soon `tcb("name")` (In the Discourse
> thread, there were concerns about overlap with TCB, and this design really
> wants to support an improved TCB that can analyze indirect calls). In the TCB
> case, identity is all that matters, and none of the flags will matter.
> Identity is also the straightforward way to implement the concept of
> `nonblocking` being a superset of `nonallocating` in a number of places that
> check.
>
> So that rules out the idea of reducing an effect to those flags.
>
> But yes, an effect need not hold anything more than its type/identity (and
> for TCB, a "subtype" that mapped to its name/identity). The flags are
> commented as being cached in the `FunctionEffect` as a
> convenience/optimization since there is space. But they definitely don't
> **have** to be there and could be simply returned as constants, determined by
> the type. I would agree that that would make things clearer.
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 (was: nolock/noalloc) (PR #84983)
@@ -0,0 +1,117 @@
+// RUN: %clang_cc1 %s -ast-dump -fblocks | FileCheck %s
+
+// Make sure that the attribute gets parsed and attached to the correct AST
elements.
+
+#pragma clang diagnostic ignored "-Wunused-variable"
+
+//
=
+// Square brackets, true
+
+namespace square_brackets {
+
+// On the type of the FunctionDecl
+void nl_function() [[clang::nonblocking]];
+// CHECK: FunctionDecl {{.*}} nl_function 'void ()
__attribute__((clang_nonblocking))'
+
+// On the type of the VarDecl holding a function pointer
+void (*nl_func_a)() [[clang::nonblocking]];
+// CHECK: VarDecl {{.*}} nl_func_a 'void (*)()
__attribute__((clang_nonblocking))'
+
+// On the type of the ParmVarDecl of a function parameter
+static void nlReceiver(void (*nl_func)() [[clang::nonblocking]]);
+// CHECK: ParmVarDecl {{.*}} nl_func 'void (*)()
__attribute__((clang_nonblocking))'
+
+// As an AttributedType within the nested types of a typedef
+typedef void (*nl_fp_type)() [[clang::nonblocking]];
+// CHECK: TypedefDecl {{.*}} nl_fp_type 'void (*)()
__attribute__((clang_nonblocking))'
+using nl_fp_talias = void (*)() [[clang::nonblocking]];
+// CHECK: TypeAliasDecl {{.*}} nl_fp_talias 'void (*)()
__attribute__((clang_nonblocking))'
+
+// From a typedef or typealias, on a VarDecl
+nl_fp_type nl_fp_var1;
+// CHECK: VarDecl {{.*}} nl_fp_var1 'nl_fp_type':'void (*)()
__attribute__((clang_nonblocking))'
+nl_fp_talias nl_fp_var2;
+// CHECK: VarDecl {{.*}} nl_fp_var2 'nl_fp_talias':'void (*)()
__attribute__((clang_nonblocking))'
+
+// On type of a FieldDecl
+struct Struct {
+ void (*nl_func_field)() [[clang::nonblocking]];
+// CHECK: FieldDecl {{.*}} nl_func_field 'void (*)()
__attribute__((clang_nonblocking))'
+};
+
+// nonallocating should be subsumed into nonblocking
+void nl1() [[clang::nonblocking]] [[clang::nonallocating]];
+// CHECK: FunctionDecl {{.*}} nl1 'void () __attribute__((clang_nonblocking))'
dougsonos wrote:
Many function attributes (calling conventions) are only represented in the AST
as part of `FunctionProtoType` without any source locations. That seems to
argue for the first option.
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 (was: nolock/noalloc) (PR #84983)
dougsonos wrote: > One thing I noticed you bring up on Discourse and which I agree is probably > for the better: this pr can just be about introducing an effect system; > actually using it can be deferred to a follow-up pr. This pr is already big > enough if you consider just those changes, and there’s already enough to > consider here to the point that _also_ trying to review the analysis part of > this here would probably get too messy... Yeah, when things settle down from this latest round of feedback, I'll back out as much as possible for "phase two". 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 (was: nolock/noalloc) (PR #84983)
@@ -4429,6 +4433,218 @@ class FunctionNoProtoType : public FunctionType, public
llvm::FoldingSetNode {
}
};
+//
--
+
+// TODO: Should FunctionEffect be located elsewhere, where Decl is not
+// forward-declared?
+class Decl;
+class CXXMethodDecl;
+class FunctionEffectSet;
+class TypeSourceInfo;
+
+/// Represents an abstract function effect.
+class FunctionEffect {
+public:
+ /// Identifies the particular type of effect.
+ enum class Type {
+None = 0,
+NonBlocking,
+NonAllocating,
+ };
+
+ /// Flags describing behaviors of the effect.
+ // (Why not a struct with bitfields? There's one function that would like to
+ // test a caller-specified bit. There are some potential optimizations that
+ // would OR together the bits of multiple effects.)
+ using Flags = unsigned;
+ enum FlagBit : unsigned {
+// Can verification inspect callees' implementations? (e.g. nonblocking:
+// yes, tcb+types: no)
+FE_InferrableOnCallees = 0x1,
+
+// Language constructs which effects can diagnose as disallowed.
+FE_ExcludeThrow = 0x2,
+FE_ExcludeCatch = 0x4,
+FE_ExcludeObjCMessageSend = 0x8,
+FE_ExcludeStaticLocalVars = 0x10,
+FE_ExcludeThreadLocalVars = 0x20
+ };
+
+ /// Describes the result of effects differing between a base class's virtual
+ /// method and an overriding method in a subclass.
+ enum class OverrideResult {
+Ignore,
+Warn,
+Propagate // Base method's effects are merged with those of the override.
+ };
+
+private:
+ // For uniqueness, currently only Type_ is significant.
+
+ LLVM_PREFERRED_TYPE(Type)
+ unsigned Type_ : 2;
+ Flags Flags_ : 8; // A constant function of Type but cached here.
+
+ // Expansion: for hypothetical TCB+types, there could be one Type for TCB,
+ // then ~16(?) bits "Subtype" to map to a specific named TCB. Subtype would
+ // be considered for uniqueness.
+
+ // Since this struct is serialized as if it were a uint32_t, it's important
+ // to pad and explicitly zero the extra bits.
+ [[maybe_unused]] unsigned Padding : 22;
+
+public:
+ using CalleeDeclOrType =
+ llvm::PointerUnion;
+
+ FunctionEffect() : Type_(unsigned(Type::None)), Flags_(0), Padding(0) {}
+
+ explicit FunctionEffect(Type T);
+
+ /// The type of the effect.
+ Type type() const { return Type(Type_); }
+
+ /// Flags describing behaviors of the effect.
+ Flags flags() const { return Flags_; }
+
+ /// The description printed in diagnostics, e.g. 'nonblocking'.
+ StringRef name() const;
+
+ /// A hashable representation.
+ uint32_t opaqueRepr() const { return Type_ | (Flags_ << 2u); }
+
+ /// Return true if adding or removing the effect as part of a type conversion
+ /// should generate a diagnostic.
+ bool diagnoseConversion(bool Adding, QualType OldType,
+ FunctionEffectSet OldFX, QualType NewType,
+ FunctionEffectSet NewFX) const;
dougsonos wrote:
One way to address this would be to move all of the behavioral methods of
`FunctionEffect` to be static methods of `Sema` which take `FunctionEffect` as
the first parameter.
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 (was: nolock/noalloc) (PR #84983)
@@ -4429,6 +4433,210 @@ class FunctionNoProtoType : public FunctionType, public
llvm::FoldingSetNode {
}
};
+//
--
+
+class Decl;
+class CXXMethodDecl;
+class FunctionEffectSet;
+
+/// Represents an abstract function effect, using just an enumeration
describing
+/// its type. Encapsulates its semantic behaviors.
+class FunctionEffect {
+public:
+ /// Identifies the particular effect.
+ enum class Kind : uint8_t {
+None,
+NonBlocking,
+NonAllocating,
+ };
+
+ /// Flags describing some behaviors of the effect.
+ using Flags = unsigned;
+ enum FlagBit : Flags {
+// Can verification inspect callees' implementations? (e.g. nonblocking:
+// yes, tcb+types: no)
+FE_InferrableOnCallees = 0x1,
+
+// Language constructs which effects can diagnose as disallowed.
+FE_ExcludeThrow = 0x2,
+FE_ExcludeCatch = 0x4,
+FE_ExcludeObjCMessageSend = 0x8,
+FE_ExcludeStaticLocalVars = 0x10,
+FE_ExcludeThreadLocalVars = 0x20
+ };
+
+ /// Describes the result of effects differing between a base class's virtual
+ /// method and an overriding method in a subclass.
+ enum class OverrideResult {
+Ignore,
+Warn,
+Merge // Base method's effects are merged with those of the override.
+ };
+
+private:
+ Kind FKind;
+
+ // Expansion: for hypothetical TCB+types, there could be one Kind for TCB,
+ // then ~16(?) bits "SubKind" to map to a specific named TCB. SubKind would
+ // be considered for uniqueness.
+
+public:
+ FunctionEffect() : FKind(Kind::None) {}
+
+ explicit FunctionEffect(Kind T) : FKind(T) {}
+
+ /// The kind of the effect.
+ Kind kind() const { return FKind; }
+
+ /// Return an opaque integer, as a serializable representation.
+ uint32_t getAsOpaqueValue() const { return llvm::to_underlying(FKind); }
+
+ /// Construct from a serialized representation.
+ static FunctionEffect getFromOpaqueValue(uint32_t V) {
+return FunctionEffect(static_cast(V));
+ }
+
+ /// Flags describing some behaviors of the effect.
+ Flags flags() const {
+switch (FKind) {
+case Kind::NonBlocking:
+ return FE_InferrableOnCallees | FE_ExcludeThrow | FE_ExcludeCatch |
+ FE_ExcludeObjCMessageSend | FE_ExcludeStaticLocalVars |
+ FE_ExcludeThreadLocalVars;
+case Kind::NonAllocating:
+ // Same as NonBlocking, except without FE_ExcludeStaticLocalVars
+ return FE_InferrableOnCallees | FE_ExcludeThrow | FE_ExcludeCatch |
+ FE_ExcludeObjCMessageSend | FE_ExcludeThreadLocalVars;
+case Kind::None:
+ break;
+}
+llvm_unreachable("unknown effect kind");
+ }
+
+ /// The description printed in diagnostics, e.g. 'nonblocking'.
+ StringRef name() const;
+
+ /// Return true if adding or removing the effect as part of a type conversion
+ /// should generate a diagnostic.
+ bool shouldDiagnoseConversion(bool Adding, QualType OldType,
+const FunctionEffectSet &OldFX,
+QualType NewType,
+const FunctionEffectSet &NewFX) const;
+
+ /// Return true if adding or removing the effect in a redeclaration should
+ /// generate a diagnostic.
+ bool shouldDiagnoseRedeclaration(bool Adding, const FunctionDecl
&OldFunction,
+ const FunctionEffectSet &OldFX,
+ const FunctionDecl &NewFunction,
+ const FunctionEffectSet &NewFX) const;
+
+ /// Return true if adding or removing the effect in a C++ virtual method
+ /// override should generate a diagnostic.
+ OverrideResult
+ shouldDiagnoseMethodOverride(bool Adding, const CXXMethodDecl &OldMethod,
+ const FunctionEffectSet &OldFX,
+ const CXXMethodDecl &NewMethod,
+ const FunctionEffectSet &NewFX) const;
+
+ /// Return true if the effect is allowed to be inferred on the callee,
+ /// which is either a FunctionDecl or BlockDecl.
+ /// This is only used if the effect has FE_InferrableOnCallees flag set.
+ /// Example: This allows nonblocking(false) to prevent inference for the
+ /// function.
+ bool canInferOnFunction(const Decl &Callee) const;
+
+ // Return false for success. When true is returned for a direct call, then
the
+ // FE_InferrableOnCallees flag may trigger inference rather than an immediate
+ // diagnostic. Caller should be assumed to have the effect (it may not have
it
+ // explicitly when inferring).
+ bool shouldDiagnoseFunctionCall(bool Direct,
+ const FunctionEffectSet &CalleeFX) const;
+
+ friend bool operator==(const FunctionEffect &LHS, const FunctionEffect &RHS)
{
+return LHS.FKind == RHS.FKind;
+ }
+ friend bool operator!=(const FunctionEffect &LHS, const FunctionEffect &RHS)
{
+return LHS.FKind != RHS.
[clang] nonblocking/nonallocating attributes (was: nolock/noalloc) (PR #84983)
@@ -3637,6 +3637,15 @@ FunctionProtoType::FunctionProtoType(QualType result,
ArrayRef params,
auto &EllipsisLoc = *getTrailingObjects();
EllipsisLoc = epi.EllipsisLoc;
}
+
+ if (epi.FunctionEffects) {
+auto &ExtraBits = *getTrailingObjects();
+ExtraBits.HasFunctionEffects = true;
+
+// N.B. This is uninitialized storage.
+FunctionEffectSet *PFX = getTrailingObjects();
+new (PFX) FunctionEffectSet(epi.FunctionEffects);
dougsonos wrote:
It would also appear that neither `FunctionProtoType` nor `TrailingObjects` has
any provision for invoking the destructors of the trailing objects. Is it okay
that they are leaked?
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 (was: nolock/noalloc) (PR #84983)
@@ -0,0 +1,190 @@ +// RUN: %clang_cc1 -fsyntax-only -fblocks -fcxx-exceptions -verify %s +// These are in a separate file because errors (e.g. incompatible attributes) currently prevent dougsonos wrote: Split the file. 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 (was: nolock/noalloc) (PR #84983)
@@ -1868,6 +1868,28 @@ bool Sema::IsFunctionConversion(QualType FromType,
QualType ToType,
FromFn = QT->getAs();
Changed = true;
}
+
+if (getLangOpts().CPlusPlus) {
+ // For C, when called from checkPointerTypesForAssignment,
+ // we need not to change the type, or else even an innocuous cast
+ // like dropping effects will fail.
dougsonos wrote:
Cleaned this up
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 (was: nolock/noalloc) (PR #84983)
@@ -18324,6 +18324,47 @@ bool Sema::CheckOverridingFunctionAttributes(const
CXXMethodDecl *New,
return true;
}
+ // Virtual overrides: check for matching effects.
+ const auto OldFX = Old->getFunctionEffects();
+ const auto NewFX = New->getFunctionEffects();
+
+ if (OldFX != NewFX) {
+const auto Diffs = FunctionEffectSet::differences(OldFX, NewFX);
+bool AnyDiags = false;
+
+for (const auto &Item : Diffs) {
+ const FunctionEffect &Effect = Item.first;
+ const bool Adding = Item.second;
+ switch (Effect.diagnoseMethodOverride(Adding, *Old, OldFX, *New, NewFX))
{
+ case FunctionEffect::OverrideResult::Ignore:
+break;
+ case FunctionEffect::OverrideResult::Warn:
+Diag(New->getLocation(), diag::warn_mismatched_func_effect_override)
+<< Effect.name();
+Diag(Old->getLocation(), diag::note_overridden_virtual_function);
+// TODO: It would be nice to have a FIXIT here!
+AnyDiags = true;
+break;
+ case FunctionEffect::OverrideResult::Propagate: {
+auto MergedFX = FunctionEffectSet::getUnion(Context, OldFX, NewFX);
+
+FunctionProtoType::ExtProtoInfo EPI = NewFT->getExtProtoInfo();
+EPI.FunctionEffects = MergedFX;
+QualType ModQT = Context.getFunctionType(NewFT->getReturnType(),
+ NewFT->getParamTypes(), EPI);
+
+// TODO: It's ugly to be mutating the incoming const method. It is
+// mutable in the calling function, though. There is also the
+// possibility here that we are discarding some other sort of sugar on
+// the method's type.
+const_cast(New)->setType(ModQT);
dougsonos wrote:
Did that
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 (was: nolock/noalloc) (PR #84983)
@@ -0,0 +1,117 @@
+// RUN: %clang_cc1 %s -ast-dump -fblocks | FileCheck %s
+
+// Make sure that the attribute gets parsed and attached to the correct AST
elements.
+
+#pragma clang diagnostic ignored "-Wunused-variable"
+
+//
=
+// Square brackets, true
+
+namespace square_brackets {
+
+// On the type of the FunctionDecl
+void nl_function() [[clang::nonblocking]];
+// CHECK: FunctionDecl {{.*}} nl_function 'void ()
__attribute__((clang_nonblocking))'
+
+// On the type of the VarDecl holding a function pointer
+void (*nl_func_a)() [[clang::nonblocking]];
+// CHECK: VarDecl {{.*}} nl_func_a 'void (*)()
__attribute__((clang_nonblocking))'
+
+// On the type of the ParmVarDecl of a function parameter
+static void nlReceiver(void (*nl_func)() [[clang::nonblocking]]);
+// CHECK: ParmVarDecl {{.*}} nl_func 'void (*)()
__attribute__((clang_nonblocking))'
+
+// As an AttributedType within the nested types of a typedef
+typedef void (*nl_fp_type)() [[clang::nonblocking]];
+// CHECK: TypedefDecl {{.*}} nl_fp_type 'void (*)()
__attribute__((clang_nonblocking))'
+using nl_fp_talias = void (*)() [[clang::nonblocking]];
+// CHECK: TypeAliasDecl {{.*}} nl_fp_talias 'void (*)()
__attribute__((clang_nonblocking))'
+
+// From a typedef or typealias, on a VarDecl
+nl_fp_type nl_fp_var1;
+// CHECK: VarDecl {{.*}} nl_fp_var1 'nl_fp_type':'void (*)()
__attribute__((clang_nonblocking))'
+nl_fp_talias nl_fp_var2;
+// CHECK: VarDecl {{.*}} nl_fp_var2 'nl_fp_talias':'void (*)()
__attribute__((clang_nonblocking))'
+
+// On type of a FieldDecl
+struct Struct {
+ void (*nl_func_field)() [[clang::nonblocking]];
+// CHECK: FieldDecl {{.*}} nl_func_field 'void (*)()
__attribute__((clang_nonblocking))'
+};
+
+// nonallocating should be subsumed into nonblocking
+void nl1() [[clang::nonblocking]] [[clang::nonallocating]];
+// CHECK: FunctionDecl {{.*}} nl1 'void () __attribute__((clang_nonblocking))'
dougsonos wrote:
Now both effects remain present in the `FunctionProtoType`
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 (was: nolock/noalloc) (PR #84983)
@@ -4429,6 +4433,210 @@ class FunctionNoProtoType : public FunctionType, public
llvm::FoldingSetNode {
}
};
+//
--
+
+class Decl;
+class CXXMethodDecl;
+class FunctionEffectSet;
+
+/// Represents an abstract function effect, using just an enumeration
describing
+/// its type. Encapsulates its semantic behaviors.
+class FunctionEffect {
+public:
+ /// Identifies the particular effect.
+ enum class Kind : uint8_t {
+None,
+NonBlocking,
+NonAllocating,
+ };
+
+ /// Flags describing some behaviors of the effect.
+ using Flags = unsigned;
+ enum FlagBit : Flags {
+// Can verification inspect callees' implementations? (e.g. nonblocking:
+// yes, tcb+types: no)
+FE_InferrableOnCallees = 0x1,
+
+// Language constructs which effects can diagnose as disallowed.
+FE_ExcludeThrow = 0x2,
+FE_ExcludeCatch = 0x4,
+FE_ExcludeObjCMessageSend = 0x8,
+FE_ExcludeStaticLocalVars = 0x10,
+FE_ExcludeThreadLocalVars = 0x20
+ };
+
+ /// Describes the result of effects differing between a base class's virtual
+ /// method and an overriding method in a subclass.
+ enum class OverrideResult {
+Ignore,
+Warn,
+Merge // Base method's effects are merged with those of the override.
+ };
+
+private:
+ Kind FKind;
+
+ // Expansion: for hypothetical TCB+types, there could be one Kind for TCB,
+ // then ~16(?) bits "SubKind" to map to a specific named TCB. SubKind would
+ // be considered for uniqueness.
+
+public:
+ FunctionEffect() : FKind(Kind::None) {}
+
+ explicit FunctionEffect(Kind T) : FKind(T) {}
+
+ /// The kind of the effect.
+ Kind kind() const { return FKind; }
+
+ /// Return an opaque integer, as a serializable representation.
+ uint32_t getAsOpaqueValue() const { return llvm::to_underlying(FKind); }
+
+ /// Construct from a serialized representation.
+ static FunctionEffect getFromOpaqueValue(uint32_t V) {
+return FunctionEffect(static_cast(V));
+ }
+
+ /// Flags describing some behaviors of the effect.
+ Flags flags() const {
+switch (FKind) {
+case Kind::NonBlocking:
+ return FE_InferrableOnCallees | FE_ExcludeThrow | FE_ExcludeCatch |
+ FE_ExcludeObjCMessageSend | FE_ExcludeStaticLocalVars |
+ FE_ExcludeThreadLocalVars;
+case Kind::NonAllocating:
+ // Same as NonBlocking, except without FE_ExcludeStaticLocalVars
+ return FE_InferrableOnCallees | FE_ExcludeThrow | FE_ExcludeCatch |
+ FE_ExcludeObjCMessageSend | FE_ExcludeThreadLocalVars;
+case Kind::None:
+ break;
+}
+llvm_unreachable("unknown effect kind");
+ }
+
+ /// The description printed in diagnostics, e.g. 'nonblocking'.
+ StringRef name() const;
+
+ /// Return true if adding or removing the effect as part of a type conversion
+ /// should generate a diagnostic.
+ bool shouldDiagnoseConversion(bool Adding, QualType OldType,
+const FunctionEffectSet &OldFX,
+QualType NewType,
+const FunctionEffectSet &NewFX) const;
+
+ /// Return true if adding or removing the effect in a redeclaration should
+ /// generate a diagnostic.
+ bool shouldDiagnoseRedeclaration(bool Adding, const FunctionDecl
&OldFunction,
+ const FunctionEffectSet &OldFX,
+ const FunctionDecl &NewFunction,
+ const FunctionEffectSet &NewFX) const;
+
+ /// Return true if adding or removing the effect in a C++ virtual method
+ /// override should generate a diagnostic.
+ OverrideResult
+ shouldDiagnoseMethodOverride(bool Adding, const CXXMethodDecl &OldMethod,
+ const FunctionEffectSet &OldFX,
+ const CXXMethodDecl &NewMethod,
+ const FunctionEffectSet &NewFX) const;
+
+ /// Return true if the effect is allowed to be inferred on the callee,
+ /// which is either a FunctionDecl or BlockDecl.
+ /// This is only used if the effect has FE_InferrableOnCallees flag set.
+ /// Example: This allows nonblocking(false) to prevent inference for the
+ /// function.
+ bool canInferOnFunction(const Decl &Callee) const;
+
+ // Return false for success. When true is returned for a direct call, then
the
+ // FE_InferrableOnCallees flag may trigger inference rather than an immediate
+ // diagnostic. Caller should be assumed to have the effect (it may not have
it
+ // explicitly when inferring).
+ bool shouldDiagnoseFunctionCall(bool Direct,
+ const FunctionEffectSet &CalleeFX) const;
+
+ friend bool operator==(const FunctionEffect &LHS, const FunctionEffect &RHS)
{
+return LHS.FKind == RHS.FKind;
+ }
+ friend bool operator!=(const FunctionEffect &LHS, const FunctionEffect &RHS)
{
+return LHS.FKind != RHS.
[clang] nonblocking/nonallocating attributes (was: nolock/noalloc) (PR #84983)
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] nonblocking/nonallocating attributes (was: nolock/noalloc) (PR #84983)
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] nonblocking/nonallocating attributes (was: nolock/noalloc) (PR #84983)
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] nonblocking/nonallocating attributes (was: nolock/noalloc) (PR #84983)
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] [llvm] [clang backend] In AArch64's DataLayout, specify a minimum function alignment of 4. (PR #90702)
https://github.com/dougsonos created
https://github.com/llvm/llvm-project/pull/90702
This addresses an issue where the explicit alignment of 2 (for C++ ABI reasons)
was being propagated to the back end and causing under-aligned functions (in
special sections). (#90358)
This is an alternate approach suggested by @efriedma-quic in PR #90415.
(There are a few failing tests. One I looked at was checking for the explicit
alignment of 2, which no longer appears in AArch64 due to this change.)
>From 4c312af7af5d0e419269c5b2304b0f898363bb85 Mon Sep 17 00:00:00 2001
From: Doug Wyatt
Date: Tue, 30 Apr 2024 21:43:16 -0700
Subject: [PATCH] In AArch64's DataLayout, specify a minimum function alignment
of 4. This addresses an issue where the explicit alignment of 2 (for C++ ABI
reasons) was being propagated to the back end and causing under-aligned
functions (in special sections). (#90358)
---
clang/lib/Basic/Targets/AArch64.cpp | 12 ++--
llvm/lib/IR/AutoUpgrade.cpp | 8
llvm/lib/Target/AArch64/AArch64TargetMachine.cpp | 8
3 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/clang/lib/Basic/Targets/AArch64.cpp
b/clang/lib/Basic/Targets/AArch64.cpp
index c8d243a8fb7aea..a5dd803f636b90 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -1480,11 +1480,11 @@ AArch64leTargetInfo::AArch64leTargetInfo(const
llvm::Triple &Triple,
void AArch64leTargetInfo::setDataLayout() {
if (getTriple().isOSBinFormatMachO()) {
if(getTriple().isArch32Bit())
- resetDataLayout("e-m:o-p:32:32-i64:64-i128:128-n32:64-S128", "_");
+ resetDataLayout("e-m:o-p:32:32-Fn32-i64:64-i128:128-n32:64-S128", "_");
else
- resetDataLayout("e-m:o-i64:64-i128:128-n32:64-S128", "_");
+ resetDataLayout("e-m:o-Fn32-i64:64-i128:128-n32:64-S128", "_");
} else
-resetDataLayout("e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128");
+
resetDataLayout("e-m:e-Fn32-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128");
}
void AArch64leTargetInfo::getTargetDefines(const LangOptions &Opts,
@@ -1507,7 +1507,7 @@ void AArch64beTargetInfo::getTargetDefines(const
LangOptions &Opts,
void AArch64beTargetInfo::setDataLayout() {
assert(!getTriple().isOSBinFormatMachO());
- resetDataLayout("E-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128");
+ resetDataLayout("E-m:e-Fn32-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128");
}
WindowsARM64TargetInfo::WindowsARM64TargetInfo(const llvm::Triple &Triple,
@@ -1530,8 +1530,8 @@ WindowsARM64TargetInfo::WindowsARM64TargetInfo(const
llvm::Triple &Triple,
void WindowsARM64TargetInfo::setDataLayout() {
resetDataLayout(Triple.isOSBinFormatMachO()
- ? "e-m:o-i64:64-i128:128-n32:64-S128"
- : "e-m:w-p:64:64-i32:32-i64:64-i128:128-n32:64-S128",
+ ? "e-m:o-Fn32-i64:64-i128:128-n32:64-S128"
+ :
"e-m:w-p:64:64-Fn32-i32:32-i64:64-i128:128-n32:64-S128",
Triple.isOSBinFormatMachO() ? "_" : "");
}
diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index 634b2dd5119e8d..eed946dc36580e 100644
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/lib/IR/AutoUpgrade.cpp
@@ -5387,6 +5387,14 @@ std::string llvm::UpgradeDataLayoutString(StringRef DL,
StringRef TT) {
return Res;
}
+ // AArch64 data layout upgrades.
+ if (T.isAArch64()) {
+// Add "-Fn32"
+if (!DL.contains("-Fn32"))
+ Res.append("-Fn32");
+return Res;
+ }
+
if (!T.isX86())
return Res;
diff --git a/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
b/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
index 7ef78cbba352a5..4ff5fb94162a93 100644
--- a/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
+++ b/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
@@ -275,15 +275,15 @@ static std::string computeDataLayout(const Triple &TT,
bool LittleEndian) {
if (TT.isOSBinFormatMachO()) {
if (TT.getArch() == Triple::aarch64_32)
- return "e-m:o-p:32:32-i64:64-i128:128-n32:64-S128";
-return "e-m:o-i64:64-i128:128-n32:64-S128";
+ return "e-m:o-p:32:32-Fn32-i64:64-i128:128-n32:64-S128";
+return "e-m:o-Fn32-i64:64-i128:128-n32:64-S128";
}
if (TT.isOSBinFormatCOFF())
-return "e-m:w-p:64:64-i32:32-i64:64-i128:128-n32:64-S128";
+return "e-m:w-p:64:64-Fn32-i32:32-i64:64-i128:128-n32:64-S128";
std::string Endian = LittleEndian ? "e" : "E";
std::string Ptr32 = TT.getEnvironment() == Triple::GNUILP32 ? "-p:32:32" :
"";
return Endian + "-m:e" + Ptr32 +
- "-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128";
+ "-Fn32-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128";
}
static StringRef computeDefaultCPU(const Triple &TT, StringRef CPU) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listi
[clang] [llvm] [clang backend] In AArch64's DataLayout, specify a minimum function alignment of 4. (PR #90702)
@@ -5387,6 +5387,14 @@ std::string llvm::UpgradeDataLayoutString(StringRef DL,
StringRef TT) {
return Res;
}
+ // AArch64 data layout upgrades.
+ if (T.isAArch64()) {
+// Add "-Fn32"
+if (!DL.contains("-Fn32"))
+ Res.append("-Fn32");
dougsonos wrote:
OK, thanks. I was afraid of that. I was copying two patterns - in ARM, `Fn8`
seemed to be after any `p` and before `i`. In this file, the modifications are
usually (always?) appending.
https://github.com/llvm/llvm-project/pull/90702
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang backend] In AArch64's DataLayout, specify a minimum function alignment of 4. (PR #90702)
https://github.com/dougsonos edited https://github.com/llvm/llvm-project/pull/90702 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang backend] In AArch64's DataLayout, specify a minimum function alignment of 4. (PR #90702)
https://github.com/dougsonos updated
https://github.com/llvm/llvm-project/pull/90702
>From 4c312af7af5d0e419269c5b2304b0f898363bb85 Mon Sep 17 00:00:00 2001
From: Doug Wyatt
Date: Tue, 30 Apr 2024 21:43:16 -0700
Subject: [PATCH 1/3] In AArch64's DataLayout, specify a minimum function
alignment of 4. This addresses an issue where the explicit alignment of 2
(for C++ ABI reasons) was being propagated to the back end and causing
under-aligned functions (in special sections). (#90358)
---
clang/lib/Basic/Targets/AArch64.cpp | 12 ++--
llvm/lib/IR/AutoUpgrade.cpp | 8
llvm/lib/Target/AArch64/AArch64TargetMachine.cpp | 8
3 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/clang/lib/Basic/Targets/AArch64.cpp
b/clang/lib/Basic/Targets/AArch64.cpp
index c8d243a8fb7aea..a5dd803f636b90 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -1480,11 +1480,11 @@ AArch64leTargetInfo::AArch64leTargetInfo(const
llvm::Triple &Triple,
void AArch64leTargetInfo::setDataLayout() {
if (getTriple().isOSBinFormatMachO()) {
if(getTriple().isArch32Bit())
- resetDataLayout("e-m:o-p:32:32-i64:64-i128:128-n32:64-S128", "_");
+ resetDataLayout("e-m:o-p:32:32-Fn32-i64:64-i128:128-n32:64-S128", "_");
else
- resetDataLayout("e-m:o-i64:64-i128:128-n32:64-S128", "_");
+ resetDataLayout("e-m:o-Fn32-i64:64-i128:128-n32:64-S128", "_");
} else
-resetDataLayout("e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128");
+
resetDataLayout("e-m:e-Fn32-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128");
}
void AArch64leTargetInfo::getTargetDefines(const LangOptions &Opts,
@@ -1507,7 +1507,7 @@ void AArch64beTargetInfo::getTargetDefines(const
LangOptions &Opts,
void AArch64beTargetInfo::setDataLayout() {
assert(!getTriple().isOSBinFormatMachO());
- resetDataLayout("E-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128");
+ resetDataLayout("E-m:e-Fn32-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128");
}
WindowsARM64TargetInfo::WindowsARM64TargetInfo(const llvm::Triple &Triple,
@@ -1530,8 +1530,8 @@ WindowsARM64TargetInfo::WindowsARM64TargetInfo(const
llvm::Triple &Triple,
void WindowsARM64TargetInfo::setDataLayout() {
resetDataLayout(Triple.isOSBinFormatMachO()
- ? "e-m:o-i64:64-i128:128-n32:64-S128"
- : "e-m:w-p:64:64-i32:32-i64:64-i128:128-n32:64-S128",
+ ? "e-m:o-Fn32-i64:64-i128:128-n32:64-S128"
+ :
"e-m:w-p:64:64-Fn32-i32:32-i64:64-i128:128-n32:64-S128",
Triple.isOSBinFormatMachO() ? "_" : "");
}
diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index 634b2dd5119e8d..eed946dc36580e 100644
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/lib/IR/AutoUpgrade.cpp
@@ -5387,6 +5387,14 @@ std::string llvm::UpgradeDataLayoutString(StringRef DL,
StringRef TT) {
return Res;
}
+ // AArch64 data layout upgrades.
+ if (T.isAArch64()) {
+// Add "-Fn32"
+if (!DL.contains("-Fn32"))
+ Res.append("-Fn32");
+return Res;
+ }
+
if (!T.isX86())
return Res;
diff --git a/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
b/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
index 7ef78cbba352a5..4ff5fb94162a93 100644
--- a/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
+++ b/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
@@ -275,15 +275,15 @@ static std::string computeDataLayout(const Triple &TT,
bool LittleEndian) {
if (TT.isOSBinFormatMachO()) {
if (TT.getArch() == Triple::aarch64_32)
- return "e-m:o-p:32:32-i64:64-i128:128-n32:64-S128";
-return "e-m:o-i64:64-i128:128-n32:64-S128";
+ return "e-m:o-p:32:32-Fn32-i64:64-i128:128-n32:64-S128";
+return "e-m:o-Fn32-i64:64-i128:128-n32:64-S128";
}
if (TT.isOSBinFormatCOFF())
-return "e-m:w-p:64:64-i32:32-i64:64-i128:128-n32:64-S128";
+return "e-m:w-p:64:64-Fn32-i32:32-i64:64-i128:128-n32:64-S128";
std::string Endian = LittleEndian ? "e" : "E";
std::string Ptr32 = TT.getEnvironment() == Triple::GNUILP32 ? "-p:32:32" :
"";
return Endian + "-m:e" + Ptr32 +
- "-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128";
+ "-Fn32-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128";
}
static StringRef computeDefaultCPU(const Triple &TT, StringRef CPU) {
>From 5a2b9e1b0444c52cd7f7cf9a3ddbb573403101a5 Mon Sep 17 00:00:00 2001
From: Doug Wyatt
Date: Wed, 1 May 2024 07:27:11 -0700
Subject: [PATCH 2/3] Move "-Fn32" to the end of the data layout strings, to
make the auto-upgrade simplest.
---
clang/lib/Basic/Targets/AArch64.cpp | 12 ++--
llvm/lib/Target/AArch64/AArch64TargetMachine.cpp | 8
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/clang/lib/Basic/Targets/AArch64.cpp
b/clang/lib/Basic/Targets/AArch64.cpp
index a5dd803f636b90..1a02520d7bd1f8 100
[clang] [llvm] [clang backend] In AArch64's DataLayout, specify a minimum function alignment of 4. (PR #90702)
dougsonos wrote: > I'd expect regression tests for the autoupgrade, and a test that clang isn't > generating the alignment markings anymore. I just pushed: - the `-Fn32` is at the end of the data layout strings - the 6 tests that failed for me locally (testing only AArch64) are fixed That includes a change to CodeGen/CodeGenCXX/member-alignment.cpp, which checks for alignment / no alignment on C++ methods, depending on target. I'm not clear on when the autoupgrade runs and how to test it. Is there an example I can follow? Thanks! https://github.com/llvm/llvm-project/pull/90702 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang backend] In AArch64's DataLayout, specify a minimum function alignment of 4. (PR #90702)
https://github.com/dougsonos updated
https://github.com/llvm/llvm-project/pull/90702
>From 4c312af7af5d0e419269c5b2304b0f898363bb85 Mon Sep 17 00:00:00 2001
From: Doug Wyatt
Date: Tue, 30 Apr 2024 21:43:16 -0700
Subject: [PATCH 1/4] In AArch64's DataLayout, specify a minimum function
alignment of 4. This addresses an issue where the explicit alignment of 2
(for C++ ABI reasons) was being propagated to the back end and causing
under-aligned functions (in special sections). (#90358)
---
clang/lib/Basic/Targets/AArch64.cpp | 12 ++--
llvm/lib/IR/AutoUpgrade.cpp | 8
llvm/lib/Target/AArch64/AArch64TargetMachine.cpp | 8
3 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/clang/lib/Basic/Targets/AArch64.cpp
b/clang/lib/Basic/Targets/AArch64.cpp
index c8d243a8fb7aea..a5dd803f636b90 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -1480,11 +1480,11 @@ AArch64leTargetInfo::AArch64leTargetInfo(const
llvm::Triple &Triple,
void AArch64leTargetInfo::setDataLayout() {
if (getTriple().isOSBinFormatMachO()) {
if(getTriple().isArch32Bit())
- resetDataLayout("e-m:o-p:32:32-i64:64-i128:128-n32:64-S128", "_");
+ resetDataLayout("e-m:o-p:32:32-Fn32-i64:64-i128:128-n32:64-S128", "_");
else
- resetDataLayout("e-m:o-i64:64-i128:128-n32:64-S128", "_");
+ resetDataLayout("e-m:o-Fn32-i64:64-i128:128-n32:64-S128", "_");
} else
-resetDataLayout("e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128");
+
resetDataLayout("e-m:e-Fn32-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128");
}
void AArch64leTargetInfo::getTargetDefines(const LangOptions &Opts,
@@ -1507,7 +1507,7 @@ void AArch64beTargetInfo::getTargetDefines(const
LangOptions &Opts,
void AArch64beTargetInfo::setDataLayout() {
assert(!getTriple().isOSBinFormatMachO());
- resetDataLayout("E-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128");
+ resetDataLayout("E-m:e-Fn32-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128");
}
WindowsARM64TargetInfo::WindowsARM64TargetInfo(const llvm::Triple &Triple,
@@ -1530,8 +1530,8 @@ WindowsARM64TargetInfo::WindowsARM64TargetInfo(const
llvm::Triple &Triple,
void WindowsARM64TargetInfo::setDataLayout() {
resetDataLayout(Triple.isOSBinFormatMachO()
- ? "e-m:o-i64:64-i128:128-n32:64-S128"
- : "e-m:w-p:64:64-i32:32-i64:64-i128:128-n32:64-S128",
+ ? "e-m:o-Fn32-i64:64-i128:128-n32:64-S128"
+ :
"e-m:w-p:64:64-Fn32-i32:32-i64:64-i128:128-n32:64-S128",
Triple.isOSBinFormatMachO() ? "_" : "");
}
diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index 634b2dd5119e8d..eed946dc36580e 100644
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/lib/IR/AutoUpgrade.cpp
@@ -5387,6 +5387,14 @@ std::string llvm::UpgradeDataLayoutString(StringRef DL,
StringRef TT) {
return Res;
}
+ // AArch64 data layout upgrades.
+ if (T.isAArch64()) {
+// Add "-Fn32"
+if (!DL.contains("-Fn32"))
+ Res.append("-Fn32");
+return Res;
+ }
+
if (!T.isX86())
return Res;
diff --git a/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
b/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
index 7ef78cbba352a5..4ff5fb94162a93 100644
--- a/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
+++ b/llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
@@ -275,15 +275,15 @@ static std::string computeDataLayout(const Triple &TT,
bool LittleEndian) {
if (TT.isOSBinFormatMachO()) {
if (TT.getArch() == Triple::aarch64_32)
- return "e-m:o-p:32:32-i64:64-i128:128-n32:64-S128";
-return "e-m:o-i64:64-i128:128-n32:64-S128";
+ return "e-m:o-p:32:32-Fn32-i64:64-i128:128-n32:64-S128";
+return "e-m:o-Fn32-i64:64-i128:128-n32:64-S128";
}
if (TT.isOSBinFormatCOFF())
-return "e-m:w-p:64:64-i32:32-i64:64-i128:128-n32:64-S128";
+return "e-m:w-p:64:64-Fn32-i32:32-i64:64-i128:128-n32:64-S128";
std::string Endian = LittleEndian ? "e" : "E";
std::string Ptr32 = TT.getEnvironment() == Triple::GNUILP32 ? "-p:32:32" :
"";
return Endian + "-m:e" + Ptr32 +
- "-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128";
+ "-Fn32-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128";
}
static StringRef computeDefaultCPU(const Triple &TT, StringRef CPU) {
>From 5a2b9e1b0444c52cd7f7cf9a3ddbb573403101a5 Mon Sep 17 00:00:00 2001
From: Doug Wyatt
Date: Wed, 1 May 2024 07:27:11 -0700
Subject: [PATCH 2/4] Move "-Fn32" to the end of the data layout strings, to
make the auto-upgrade simplest.
---
clang/lib/Basic/Targets/AArch64.cpp | 12 ++--
llvm/lib/Target/AArch64/AArch64TargetMachine.cpp | 8
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/clang/lib/Basic/Targets/AArch64.cpp
b/clang/lib/Basic/Targets/AArch64.cpp
index a5dd803f636b90..1a02520d7bd1f8 100
