From e33231d3ec6d2461371350bca932bce4e4c4e096 Mon Sep 17 00:00:00 2001 From: v01dxyz Date: Tue, 12 Nov 2024 13:28:47 +0100 Subject: [PATCH 1/9] cheri_compartment: Warn if return type is void or return value is unused The current implementation has a little shortcoming: the warning message for void return type is shown twice because cheri_compartment is a function type attribute (cf SemaType.cpp) that is added to both the attribute list of the declarator and the one of the function type. --- clang/lib/Sema/SemaDeclAttr.cpp | 9 +++++++++ ...heri-compartment-warn-if-return-void-or-unused.c | 13 +++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 clang/test/Sema/cheri/cheri-compartment-warn-if-return-void-or-unused.c diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 93a224ff46d02..4a8c67e33b9e4 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -2393,6 +2393,15 @@ static void handleCHERICompartmentName(Sema &S, Decl *D, const ParsedAttr &Attr) SourceLocation LiteralLoc; if (!S.checkStringLiteralArgumentAttr(Attr, 0, Str, &LiteralLoc)) return; + + if (D->getFunctionType() && + D->getFunctionType()->getReturnType()->isVoidType()) + S.Diag(Attr.getLoc(), diag::warn_attribute_void_function_method) + << Attr << 0; + else + D->addAttr(::new (S.Context) WarnUnusedResultAttr( + S.Context, Attr, "CHERI compartment call")); + D->addAttr(::new (S.Context) CHERICompartmentNameAttr(S.Context, Attr, Str)); } 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..dd4ea5b2443d9 --- /dev/null +++ b/clang/test/Sema/cheri/cheri-compartment-warn-if-return-void-or-unused.c @@ -0,0 +1,13 @@ +// 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 -fsyntax-only -verify + +__attribute__((cheri_compartment("example"))) void void_return_type_f() // expected-warning{{attribute 'cheri_compartment' cannot be applied to functions without return value}} expected-warning{{attribute 'cheri_compartment' cannot be applied to functions without return value}} +{ +} + +__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}} +} From fe06a9f1306fa609b596eb51d0b66ea591e79b77 Mon Sep 17 00:00:00 2001 From: v01dxyz Date: Fri, 15 Nov 2024 20:53:16 +0100 Subject: [PATCH 2/9] Update tests --- clang/test/CodeGen/cheri/cheri-mcu-interrupts.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) 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]] From 5ca20c897bd2944427b6776d4d12c880685b99e6 Mon Sep 17 00:00:00 2001 From: v01dxyz Date: Sat, 30 Nov 2024 04:24:46 +0100 Subject: [PATCH 3/9] Add FixItHints, Improve wording of Diagnosis messages --- .../clang/Basic/DiagnosticSemaKinds.td | 5 ++ clang/include/clang/Sema/Sema.h | 8 +++ clang/lib/Sema/AnalysisBasedWarnings.cpp | 26 +++++++-- clang/lib/Sema/SemaDeclAttr.cpp | 56 +++++++++++++++---- clang/lib/Sema/SemaStmt.cpp | 6 ++ ...ompartment-warn-if-return-void-or-unused.c | 15 ++++- 6 files changed, 99 insertions(+), 17 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 89070c050bb2c..0a49706c9934c 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -2336,6 +2336,11 @@ 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 make it impossible for callers to detect failure">; +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">; 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/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 4a8c67e33b9e4..8edbb33865d39 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -2388,17 +2388,49 @@ 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; - if (D->getFunctionType() && - D->getFunctionType()->getReturnType()->isVoidType()) - S.Diag(Attr.getLoc(), diag::warn_attribute_void_function_method) - << Attr << 0; - else + // 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); + + const TypeSourceInfo *TSI = FD->getTypeSourceInfo(); + TypeLoc TL = TSI->getTypeLoc().IgnoreParens(); + + // ignore function type attributes + while (auto ATL = TL.getAs()) + TL = ATL.getModifiedLoc(); + + if (auto FTL = TL.getAs()) { + SourceRange SR = FTL.getReturnLoc().getSourceRange(); + S.Diag(SR.getBegin(), diag::note_cheri_compartment_void_return_type) + << FixItHint::CreateReplacement(SR, "int"); + } + // FunctionTypeLoc FTL = FD->getFunctionTypeLoc(); + + // TypeLoc RL = FTL.getReturnLoc(); + // SourceRange SR = FD->getReturnTypeSourceRange(); + + } else D->addAttr(::new (S.Context) WarnUnusedResultAttr( S.Context, Attr, "CHERI compartment call")); @@ -8941,7 +8973,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; @@ -9450,7 +9483,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); @@ -9764,12 +9797,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 @@ -10020,6 +10054,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)); @@ -10031,13 +10066,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/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 index dd4ea5b2443d9..696fdbe0c39b6 100644 --- 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 @@ -1,8 +1,17 @@ -// 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 -fsyntax-only -verify +// 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() // expected-warning{{attribute 'cheri_compartment' cannot be applied to functions without return value}} expected-warning{{attribute 'cheri_compartment' cannot be applied to functions without return value}} +// -fdiagnostics-parseable-fixits -fixit +// + +// -fdiagnostics-fixit-info +__attribute__((cheri_compartment("example"))) void void_return_type_f(int a) // expected-warning{{void return on a cross-compartment call make 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; From f7dd4cee26331b0a14e0b4c6a6dbfeffbe0e1c85 Mon Sep 17 00:00:00 2001 From: v01dxyz Date: Thu, 12 Dec 2024 01:59:15 +0100 Subject: [PATCH 4/9] Make getFunctionTypeLoc support AttributedType --- clang/include/clang/AST/TypeLoc.h | 4 ++++ clang/lib/AST/Decl.cpp | 19 +++++++++++++++++-- clang/lib/Sema/SemaDeclAttr.cpp | 15 +-------------- 3 files changed, 22 insertions(+), 16 deletions(-) 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/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 85fe7d63775d3..e897b9946e729 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -3745,8 +3745,23 @@ bool FunctionDecl::doesDeclarationForceExternallyVisibleDefinition() const { FunctionTypeLoc FunctionDecl::getFunctionTypeLoc() const { const TypeSourceInfo *TSI = getTypeSourceInfo(); - return TSI ? TSI->getTypeLoc().IgnoreParens().getAs() - : FunctionTypeLoc(); + + 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/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 8edbb33865d39..247287199173d 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -2413,23 +2413,10 @@ static void handleCHERICompartmentName(Sema &S, Decl *D, const ParsedAttr &Attr, if (FD && FD->getReturnType()->isVoidType()) { S.Diag(Attr.getLoc(), diag::warn_cheri_compartment_void_return_type); - const TypeSourceInfo *TSI = FD->getTypeSourceInfo(); - TypeLoc TL = TSI->getTypeLoc().IgnoreParens(); - - // ignore function type attributes - while (auto ATL = TL.getAs()) - TL = ATL.getModifiedLoc(); - - if (auto FTL = TL.getAs()) { - SourceRange SR = FTL.getReturnLoc().getSourceRange(); + if (SourceRange SR = FD->getReturnTypeSourceRange(); SR.isValid()) { S.Diag(SR.getBegin(), diag::note_cheri_compartment_void_return_type) << FixItHint::CreateReplacement(SR, "int"); } - // FunctionTypeLoc FTL = FD->getFunctionTypeLoc(); - - // TypeLoc RL = FTL.getReturnLoc(); - // SourceRange SR = FD->getReturnTypeSourceRange(); - } else D->addAttr(::new (S.Context) WarnUnusedResultAttr( S.Context, Attr, "CHERI compartment call")); From 19f0b3c83c3450030be36e4a6224db7b17a1d6f9 Mon Sep 17 00:00:00 2001 From: v01dxyz Date: Thu, 12 Dec 2024 11:58:34 +0100 Subject: [PATCH 5/9] Add warnings for a compartment call returning void to a DiagGroup --- clang/include/clang/Basic/DiagnosticGroups.td | 3 ++- clang/include/clang/Basic/DiagnosticSemaKinds.td | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) 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 0a49706c9934c..c21aa9d465e75 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -2337,10 +2337,12 @@ def note_in_reference_temporary_list_initializer : Note< 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 make it impossible for callers to detect failure">; + "void return on a cross-compartment call make 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">; + "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">; From 4c34086b6b99d4523e6f9edf9181ef3dc81a97d6 Mon Sep 17 00:00:00 2001 From: v01dxyz Date: Thu, 12 Dec 2024 19:31:08 +0100 Subject: [PATCH 6/9] Relaunch CI From 237c4c4f8d4b795083c27bbb02dabe9419573b57 Mon Sep 17 00:00:00 2001 From: v01dxyz Date: Sat, 14 Dec 2024 08:07:31 +0100 Subject: [PATCH 7/9] Fix typos for diagnosis --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 ++-- .../cheri/cheri-compartment-warn-if-return-void-or-unused.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index c21aa9d465e75..6eb1454812a60 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -2337,11 +2337,11 @@ def note_in_reference_temporary_list_initializer : Note< 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 make it impossible for callers to detect failure">, + "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">, + "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 " 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 index 696fdbe0c39b6..35cb22b50a6f2 100644 --- 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 @@ -5,13 +5,13 @@ // // -fdiagnostics-fixit-info -__attribute__((cheri_compartment("example"))) void void_return_type_f(int a) // expected-warning{{void return on a cross-compartment call make it impossible for callers to detect failure}} expected-note{{replace void return type with int}} +__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}} + 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}} +} // expected-warning{{cross-compartement calls that always succeed should return 0 instead}} __attribute__((cheri_compartment("example"))) int int_return_type_f() { return 0; From b8c007fa98493e80f5985fa075a1c09f0ce4b987 Mon Sep 17 00:00:00 2001 From: v01dxyz Date: Sat, 14 Dec 2024 09:27:07 +0100 Subject: [PATCH 8/9] Add a comment explaining divergence with upstream --- clang/lib/AST/Decl.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index e897b9946e729..7c0f2f96ab26f 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -3745,7 +3745,13 @@ bool FunctionDecl::doesDeclarationForceExternallyVisibleDefinition() const { FunctionTypeLoc FunctionDecl::getFunctionTypeLoc() const { const TypeSourceInfo *TSI = getTypeSourceInfo(); - + // 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(); From 55f97ff70a53c07c80a8ce7039659bbd67b56e40 Mon Sep 17 00:00:00 2001 From: v01dxyz Date: Mon, 16 Dec 2024 06:48:46 +0100 Subject: [PATCH 9/9] Remove useless comments --- .../cheri/cheri-compartment-warn-if-return-void-or-unused.c | 4 ---- 1 file changed, 4 deletions(-) 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 index 35cb22b50a6f2..20d7d683fd3af 100644 --- 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 @@ -1,10 +1,6 @@ // 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 -// -fdiagnostics-parseable-fixits -fixit -// - -// -fdiagnostics-fixit-info __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) {