diff --git a/clang/include/clang/AST/TypeLoc.h b/clang/include/clang/AST/TypeLoc.h index d2d4f1b6e2e06..54cc13f02385a 100644 --- a/clang/include/clang/AST/TypeLoc.h +++ b/clang/include/clang/AST/TypeLoc.h @@ -883,6 +883,10 @@ class AttributedTypeLoc : public ConcreteTypeLocgetEquivalentType(), getNonLocalData()); + } + /// The type attribute. const Attr *getAttr() const { return getLocalData()->TypeAttr; diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 8be758c873247..48fe078ee4005 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -158,10 +158,11 @@ def CHERIPrototypesStrict: DiagGroup<"cheri-prototypes-strict">; // Remarks about setting/not setting subobject bounds def CheriSubobjectBoundsSuspicous : DiagGroup<"cheri-subobject-bounds-suspicious">; def CheriSubobjectBoundsRemarks : DiagGroup<"cheri-subobject-bounds">; +def CHERICompartmentReturnVoid : DiagGroup<"cheri-compartment-return-void">; def CheriAll : DiagGroup<"cheri", [CHERICaps, CHERIBitwiseOps, CHERIMisaligned, CHERIImplicitConversion, CheriSubobjectBoundsSuspicous, - CHERIProvenance, CHERIImplicitConversionSign, CHERIPrototypes]>; + CHERIProvenance, CHERIImplicitConversionSign, CHERIPrototypes, CHERICompartmentReturnVoid]>; // CHERI warnings that are too noisy to turn on by default def CHERICapabilityToIntegerCast : DiagGroup<"capability-to-integer-cast">; def CheriPedantic : DiagGroup<"cheri-pedantic", [CHERICapabilityToIntegerCast, CHERIPrototypesStrict, CHERIProvenancePedantic]>; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 89070c050bb2c..6eb1454812a60 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -2336,6 +2336,13 @@ def note_in_reference_temporary_list_initializer : Note< "list-initialize this reference">; def note_var_fixit_add_initialization : Note< "initialize the variable %0 to silence this warning">; +def warn_cheri_compartment_void_return_type : Warning < + "void return on a cross-compartment call makes it impossible for callers to detect failure">, + InGroup; +def note_cheri_compartment_void_return_type : Note<"replace void return type with int">; +def warn_cheri_compartment_return_void_or_falloff : Warning < + "cross-compartement calls that always succeed should return 0 instead">, + InGroup; def note_uninit_fixit_remove_cond : Note< "remove the %select{'%1' if its condition|condition if it}0 " "is always %select{false|true}2">; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 7240a8f3f04f3..178f00eb4489f 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -4717,8 +4717,16 @@ class Sema final { bool IgnoreTypeAttributes; }; + enum DeclAttributeLocation { + DAL_Unspecified, + DAL_DeclSpec, + DAL_DeclChunk, + DAL_Decl, + }; + void ProcessDeclAttributeList(Scope *S, Decl *D, const ParsedAttributesView &AttrList, + DeclAttributeLocation DAL = DAL_Unspecified, const ProcessDeclAttributeOptions &Options = ProcessDeclAttributeOptions()); bool ProcessAccessDeclAttributeList(AccessSpecDecl *ASDecl, diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 85fe7d63775d3..7c0f2f96ab26f 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -3745,8 +3745,29 @@ bool FunctionDecl::doesDeclarationForceExternallyVisibleDefinition() const { FunctionTypeLoc FunctionDecl::getFunctionTypeLoc() const { const TypeSourceInfo *TSI = getTypeSourceInfo(); - return TSI ? TSI->getTypeLoc().IgnoreParens().getAs() - : FunctionTypeLoc(); + // CHERIOT: The following code diverges from upstream. The previous + // code returned a null FunctionTypeLoc when the function was + // annotated (impacting getReturnTypeSourceRange too). It is + // necessary to provide hints to replace the return type of a + // compartment entry point which returns void instead of int. + // + // https://github.com/llvm/llvm-project/pull/118420 + if (!TSI) + return FunctionTypeLoc(); + + TypeLoc TL = TSI->getTypeLoc(); + FunctionTypeLoc FTL; + + while (!(FTL = TL.getAs())) { + if (auto PTL = TL.getAs()) + TL = PTL.getInnerLoc(); + else if (auto ATL = TL.getAs()) + TL = ATL.getEquivalentTypeLoc(); + else + break; + } + + return FTL; } SourceRange FunctionDecl::getReturnTypeSourceRange() const { diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 43b13e0ec4d24..88576595c6a6c 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -621,7 +621,7 @@ struct CheckFallThroughDiagnostics { } bool checkDiagnostics(DiagnosticsEngine &D, bool ReturnsVoid, - bool HasNoReturn) const { + bool HasNoReturn, bool HasCHERICompartmentName) const { if (funMode == Function) { return (ReturnsVoid || D.isIgnored(diag::warn_maybe_falloff_nonvoid_function, @@ -630,7 +630,8 @@ struct CheckFallThroughDiagnostics { D.isIgnored(diag::warn_noreturn_function_has_return_expr, FuncLoc)) && (!ReturnsVoid || - D.isIgnored(diag::warn_suggest_noreturn_block, FuncLoc)); + D.isIgnored(diag::warn_suggest_noreturn_block, FuncLoc)) && + !HasCHERICompartmentName; } if (funMode == Coroutine) { return (ReturnsVoid || @@ -658,6 +659,7 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body, bool ReturnsVoid = false; bool HasNoReturn = false; + bool HasCHERICompartmentName = false; bool IsCoroutine = FSI->isCoroutine(); if (const auto *FD = dyn_cast(D)) { @@ -666,6 +668,7 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body, else ReturnsVoid = FD->getReturnType()->isVoidType(); HasNoReturn = FD->isNoReturn(); + HasCHERICompartmentName = FD->hasAttr(); } else if (const auto *MD = dyn_cast(D)) { ReturnsVoid = MD->getReturnType()->isVoidType(); @@ -684,8 +687,9 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body, DiagnosticsEngine &Diags = S.getDiagnostics(); // Short circuit for compilation speed. - if (CD.checkDiagnostics(Diags, ReturnsVoid, HasNoReturn)) - return; + if (CD.checkDiagnostics(Diags, ReturnsVoid, HasNoReturn, + HasCHERICompartmentName)) + return; SourceLocation LBrace = Body->getBeginLoc(), RBrace = Body->getEndLoc(); auto EmitDiag = [&](SourceLocation Loc, unsigned DiagID) { if (IsCoroutine) @@ -708,12 +712,26 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body, EmitDiag(RBrace, CD.diag_MaybeFallThrough_HasNoReturn); else if (!ReturnsVoid) EmitDiag(RBrace, CD.diag_MaybeFallThrough_ReturnsNonVoid); + + if (HasCHERICompartmentName) + if (!ReturnsVoid) + S.Diag(RBrace, diag::warn_cheri_compartment_return_void_or_falloff); + else + S.Diag(RBrace, diag::warn_cheri_compartment_return_void_or_falloff) + << FixItHint::CreateInsertion(RBrace, "return 0;"); break; case AlwaysFallThrough: if (HasNoReturn) EmitDiag(RBrace, CD.diag_AlwaysFallThrough_HasNoReturn); else if (!ReturnsVoid) EmitDiag(RBrace, CD.diag_AlwaysFallThrough_ReturnsNonVoid); + + if (HasCHERICompartmentName) + if (!ReturnsVoid) + S.Diag(RBrace, diag::warn_cheri_compartment_return_void_or_falloff); + else + S.Diag(RBrace, diag::warn_cheri_compartment_return_void_or_falloff) + << FixItHint::CreateInsertion(RBrace, "return 0;"); break; case NeverFallThroughOrReturn: if (ReturnsVoid && !HasNoReturn && CD.diag_NeverFallThroughOrReturn) { diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 93a224ff46d02..247287199173d 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -2388,11 +2388,39 @@ static void handleCHERIMethodSuffix(Sema &S, Decl *D, const ParsedAttr &Attr) { D->addAttr(::new (S.Context) CHERIMethodSuffixAttr(S.Context, Attr, Str)); } -static void handleCHERICompartmentName(Sema &S, Decl *D, const ParsedAttr &Attr) { +static void handleCHERICompartmentName(Sema &S, Decl *D, const ParsedAttr &Attr, + Sema::DeclAttributeLocation DAL) { + if (DAL != Sema::DAL_DeclSpec && DAL != Sema::DAL_Unspecified) + return; + + // cheri_compartment is both: + // + // * a Declaration attribute: marks the function as a compartment entry point + // * a Function Type attribute: affects the calling convention + // + // That's the reason why we don't short-circuit with hasDeclarator + // (as other handlers do). + StringRef Str; SourceLocation LiteralLoc; if (!S.checkStringLiteralArgumentAttr(Attr, 0, Str, &LiteralLoc)) return; + + // cheri_compartment is considered as function type attribute + + const auto *FD = dyn_cast(D); + + if (FD && FD->getReturnType()->isVoidType()) { + S.Diag(Attr.getLoc(), diag::warn_cheri_compartment_void_return_type); + + if (SourceRange SR = FD->getReturnTypeSourceRange(); SR.isValid()) { + S.Diag(SR.getBegin(), diag::note_cheri_compartment_void_return_type) + << FixItHint::CreateReplacement(SR, "int"); + } + } else + D->addAttr(::new (S.Context) WarnUnusedResultAttr( + S.Context, Attr, "CHERI compartment call")); + D->addAttr(::new (S.Context) CHERICompartmentNameAttr(S.Context, Attr, Str)); } @@ -8932,7 +8960,8 @@ static bool MustDelayAttributeArguments(const ParsedAttr &AL) { /// silently ignore it if a GNU attribute. static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, - const Sema::ProcessDeclAttributeOptions &Options) { + const Sema::ProcessDeclAttributeOptions &Options, + Sema::DeclAttributeLocation DAL = Sema::DAL_Unspecified) { if (AL.isInvalid() || AL.getKind() == ParsedAttr::IgnoredAttribute) return; @@ -9441,7 +9470,7 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, handleCHERIMethodSuffix(S, D, AL); break; case ParsedAttr::AT_CHERICompartmentName: - handleCHERICompartmentName(S, D, AL); + handleCHERICompartmentName(S, D, AL, DAL); break; case ParsedAttr::AT_InterruptState: handleInterruptState(S, D, AL); @@ -9755,12 +9784,13 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, /// attribute list to the specified decl, ignoring any type attributes. void Sema::ProcessDeclAttributeList( Scope *S, Decl *D, const ParsedAttributesView &AttrList, + Sema::DeclAttributeLocation DAL, const ProcessDeclAttributeOptions &Options) { if (AttrList.empty()) return; for (const ParsedAttr &AL : AttrList) - ProcessDeclAttribute(*this, S, D, AL, Options); + ProcessDeclAttribute(*this, S, D, AL, Options, DAL); // FIXME: We should be able to handle these cases in TableGen. // GCC accepts @@ -10011,6 +10041,7 @@ void Sema::ProcessDeclAttributes(Scope *S, Decl *D, const Declarator &PD) { // Apply decl attributes from the DeclSpec if present. if (!PD.getDeclSpec().getAttributes().empty()) { ProcessDeclAttributeList(S, D, PD.getDeclSpec().getAttributes(), + DAL_DeclSpec, ProcessDeclAttributeOptions() .WithIncludeCXX11Attributes(false) .WithIgnoreTypeAttributes(true)); @@ -10022,13 +10053,14 @@ void Sema::ProcessDeclAttributes(Scope *S, Decl *D, const Declarator &PD) { // when X is a decl attribute. for (unsigned i = 0, e = PD.getNumTypeObjects(); i != e; ++i) { ProcessDeclAttributeList(S, D, PD.getTypeObject(i).getAttrs(), + DAL_DeclChunk, ProcessDeclAttributeOptions() .WithIncludeCXX11Attributes(false) .WithIgnoreTypeAttributes(true)); } // Finally, apply any attributes on the decl itself. - ProcessDeclAttributeList(S, D, PD.getAttributes()); + ProcessDeclAttributeList(S, D, PD.getAttributes(), DAL_Decl); // Apply additional attributes specified by '#pragma clang attribute'. AddPragmaAttributes(S, D); diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index 3589abb004a78..3b0391caafe69 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -4120,6 +4120,12 @@ StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp, return StmtError(); RetValExp = ER.get(); } + } else if (getCurFunctionOrMethodDecl() + ->hasAttr()) { + SourceLocation AfterReturnLoc = getLocForEndOfToken(ReturnLoc); + /* Compartment call */ + Diag(ReturnLoc, diag::warn_cheri_compartment_return_void_or_falloff) + << FixItHint::CreateInsertion(AfterReturnLoc, " 0"); } Result = ReturnStmt::Create(Context, ReturnLoc, RetValExp, diff --git a/clang/test/CodeGen/cheri/cheri-mcu-interrupts.c b/clang/test/CodeGen/cheri/cheri-mcu-interrupts.c index 79f8acf8e4f4f..a5cc6f13d7650 100644 --- a/clang/test/CodeGen/cheri/cheri-mcu-interrupts.c +++ b/clang/test/CodeGen/cheri/cheri-mcu-interrupts.c @@ -23,10 +23,11 @@ int inherit(void) // The default for exported functions should be interrupts enabled // -// CHECK: define dso_local chericcallcce void @_Z21default_enable_calleev() local_unnamed_addr addrspace(200) #[[DEFEN:[0-9]]] +// CHECK: define dso_local chericcallcce i32 @_Z21default_enable_calleev() local_unnamed_addr addrspace(200) #[[DEFEN:[0-9]]] __attribute__((cheri_compartment("example"))) -void default_enable_callee(void) +int default_enable_callee(void) { + return 0; } // CHECK: define dso_local chericcallcc void @default_enable_callback() local_unnamed_addr addrspace(200) #[[DEFEN]] @@ -37,11 +38,12 @@ void default_enable_callback(void) // Explicitly setting interrupt status should override the default -// CHECK: define dso_local chericcallcce void @_Z23explicit_disable_calleev() local_unnamed_addr addrspace(200) #[[EXPDIS:[0-9]]] +// CHECK: define dso_local chericcallcce i32 @_Z23explicit_disable_calleev() local_unnamed_addr addrspace(200) #[[EXPDIS:[0-9]]] __attribute__((cheri_interrupt_state(disabled))) __attribute__((cheri_compartment("example"))) -void explicit_disable_callee(void) +int explicit_disable_callee(void) { + return 0; } // CHECK: define dso_local chericcallcc void @explicit_disable_callback() local_unnamed_addr addrspace(200) #[[EXPDIS]] diff --git a/clang/test/Sema/cheri/cheri-compartment-warn-if-return-void-or-unused.c b/clang/test/Sema/cheri/cheri-compartment-warn-if-return-void-or-unused.c new file mode 100644 index 0000000000000..20d7d683fd3af --- /dev/null +++ b/clang/test/Sema/cheri/cheri-compartment-warn-if-return-void-or-unused.c @@ -0,0 +1,18 @@ +// RUN: %clang_cc1 %s -o - -triple riscv32-unknown-unknown -emit-llvm -mframe-pointer=none -mcmodel=small -target-cpu cheriot -target-feature +xcheri -target-feature -64bit -target-feature -relax -target-feature -xcheri-rvc -target-feature -save-restore -target-abi cheriot -Oz -cheri-compartment=example -verify -fsyntax-only +// RUN: %clang_cc1 %s -o - -triple riscv32-unknown-unknown -emit-llvm -mframe-pointer=none -mcmodel=small -target-cpu cheriot -target-feature +xcheri -target-feature -64bit -target-feature -relax -target-feature -xcheri-rvc -target-feature -save-restore -target-abi cheriot -Oz -cheri-compartment=example -fdiagnostics-parseable-fixits -fsyntax-only 2>&1 | FileCheck %s + +__attribute__((cheri_compartment("example"))) void void_return_type_f(int a) // expected-warning{{void return on a cross-compartment call makes it impossible for callers to detect failure}} expected-note{{replace void return type with int}} +{ + if (a) { + /// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:[[COL:[0-9]+]]-[[@LINE+1]]:[[COL]]}:" 0" + return; // expected-warning{{cross-compartement calls that always succeed should return 0 instead}} + } +} // expected-warning{{cross-compartement calls that always succeed should return 0 instead}} + +__attribute__((cheri_compartment("example"))) int int_return_type_f() { + return 0; +} + +void unused_int_return_type_f() { + int_return_type_f(); // expected-warning{{ignoring return value of function declared with 'nodiscard' attribute: CHERI compartment call}} +}