https://github.com/jpjepko updated https://github.com/llvm/llvm-project/pull/188128
>From f9d6311c09a4c1b52d3212c9ff5da49bd349117e Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Mon, 23 Mar 2026 16:21:05 +0100 Subject: [PATCH 1/3] Add attr and clangsa check --- clang/docs/ReleaseNotes.rst | 11 ++ clang/include/clang/Basic/Attr.td | 6 + clang/include/clang/Basic/AttrDocs.td | 20 ++ .../clang/StaticAnalyzer/Checkers/Checkers.td | 11 ++ clang/lib/Sema/SemaDeclAttr.cpp | 25 +++ .../StaticAnalyzer/Checkers/CMakeLists.txt | 1 + .../Checkers/NullTerminatedChecker.cpp | 177 ++++++++++++++++++ clang/test/Analysis/analyzer-config.c | 1 + clang/test/Analysis/null-terminated.c | 157 ++++++++++++++++ clang/test/Analysis/null-terminated.cpp | 54 ++++++ ...a-attribute-supported-attributes-list.test | 1 + clang/test/Sema/attr-null-terminated.c | 16 ++ 12 files changed, 480 insertions(+) create mode 100644 clang/lib/StaticAnalyzer/Checkers/NullTerminatedChecker.cpp create mode 100644 clang/test/Analysis/null-terminated.c create mode 100644 clang/test/Analysis/null-terminated.cpp create mode 100644 clang/test/Sema/attr-null-terminated.c diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index af7d09c0c23d2..16e7576b65db2 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -516,6 +516,10 @@ Attribute Changes in Clang * The ``modular_format`` attribute now supports the ``fixed`` aspect for C ISO 18037 fixed-point ``printf`` specifiers. +- Added the ``null_terminated`` attribute, which can be applied to function + parameters of pointer or array type (of scalar type) to indicate that the + function expects a null-terminated buffer. + Improvements to Clang's diagnostics ----------------------------------- - Fixed bug in ``-Wdocumentation`` so that it correctly handles explicit @@ -1050,6 +1054,13 @@ Moved checkers - The checker ``unix.cstring.UninitializedRead`` is now out of alpha. +New checkers or options +^^^^^^^^^^^^^^^^^^^^^^^ + +- Introduced the ``alpha.core.NullTerminated`` checker to detect arrays missing + a null terminator passed as a parameter marked with the ``null_terminated`` + attribute. + .. _release-notes-sanitizers: Sanitizers diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index a222092cd42cf..63ce0c0543c08 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -5470,6 +5470,12 @@ def NonString : InheritableAttr { let Documentation = [NonStringDocs]; } +def NullTerminated : InheritableAttr { + let Spellings = [Clang<"null_terminated">]; + let Subjects = SubjectList<[ParmVar]>; + let Documentation = [NullTerminatedDocs]; +} + def ModularFormat : InheritableAttr { let Spellings = [Clang<"modular_format">]; let Args = [IdentifierArgument<"ModularImplFn">, StringArgument<"ImplName">, diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 04362de2d5be2..7f4fb2a33ef41 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -10038,6 +10038,26 @@ silence diagnostics with code like: }]; } +def NullTerminatedDocs : Documentation { + let Category = DocCatDecl; + let Content = [{ +The ``null_terminated`` attribute can be applied to a function parameter whose +type is a pointer or array to indicate that the function expects the buffer to +be null-terminated (with a zero-valued element). This enables the checker +``alpha.core.NullTerminated`` to detect buffers missing a null terminator passed +as a parameter marked with this attribute, which may cause undefined behavior. + +.. code-block:: c + + void receive(__attribute__((null_terminated)) const int signals[]); + + void caller(void) { + int sigs[] = {1, 2, 3}; // Missing null terminator + receive(sigs); + } + }]; +} + def ModularFormatDocs : Documentation { let Category = DocCatFunction; let Content = [{ diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index d02c3195069f3..9076bd7e42086 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -289,6 +289,17 @@ def StdVariantChecker : Checker<"StdVariant">, HelpText<"Check for bad type access for std::variant.">, Documentation<HasDocumentation>; +def NullTerminatedChecker + : Checker<"NullTerminated">, + HelpText<"Check for arrays not null-terminated passed to functions whose " + "parameters are marked with the 'null_terminated' attribute">, + CheckerOptions<[CmdLineOption< + Integer, "MaxArraySize", + "Maximum number of array elements to scan for a null terminator. " + "Arrays larger than this are skipped.", + "1024", InAlpha>]>, + Documentation<HasDocumentation>; + } // end "alpha.core" //===----------------------------------------------------------------------===// diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 2159c586e5738..386d100532b53 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -5204,6 +5204,28 @@ static void handleNonStringAttr(Sema &S, Decl *D, const ParsedAttr &AL) { D->addAttr(::new (S.Context) NonStringAttr(S.Context, AL)); } +static void handleNullTerminatedAttr(Sema &S, Decl *D, const ParsedAttr &AL) { + // Only makes sense on function params that are buffers. + QualType QT = cast<ValueDecl>(D)->getType().getNonReferenceType(); + if (!QT->isArrayType() && !QT->isPointerType()) { + S.Diag(D->getBeginLoc(), diag::warn_attribute_wrong_decl_type_str) + << AL << AL.isRegularKeywordAttribute() + << "parameters of pointer or array type"; + return; + } + // Null terminators only make sense for scalar element types. + QualType EltTy = QT->isArrayType() + ? S.Context.getAsArrayType(QT)->getElementType() + : QT->getPointeeType(); + if (!EltTy->isScalarType()) { + S.Diag(D->getBeginLoc(), diag::warn_attribute_wrong_decl_type_str) + << AL << AL.isRegularKeywordAttribute() + << "parameters of pointer or array of scalar type"; + return; + } + D->addAttr(::new (S.Context) NullTerminatedAttr(S.Context, AL)); +} + static void handleNoDebugAttr(Sema &S, Decl *D, const ParsedAttr &AL) { D->addAttr(::new (S.Context) NoDebugAttr(S.Context, AL)); } @@ -7828,6 +7850,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, case ParsedAttr::AT_NonString: handleNonStringAttr(S, D, AL); break; + case ParsedAttr::AT_NullTerminated: + handleNullTerminatedAttr(S, D, AL); + break; case ParsedAttr::AT_NonNull: if (auto *PVD = dyn_cast<ParmVarDecl>(D)) handleNonNullAttrParameter(S, PVD, AL); diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt index 8a0621077b977..5ce7e8ad9c6df 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -73,6 +73,7 @@ add_clang_library(clangStaticAnalyzerCheckers NoReturnFunctionChecker.cpp NonNullParamChecker.cpp NonnullGlobalConstantsChecker.cpp + NullTerminatedChecker.cpp NoOwnershipChangeVisitor.cpp NullabilityChecker.cpp NumberObjectConversionChecker.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/NullTerminatedChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NullTerminatedChecker.cpp new file mode 100644 index 0000000000000..5517d3ff69920 --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/NullTerminatedChecker.cpp @@ -0,0 +1,177 @@ +//===- NullTerminatedChecker.cpp - Check null_terminated params -*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This defines NullTerminatedChecker, which checks for arguments treated as +// buffers that are expected to be null-terminated (ends with zero-valued +// element). For constant-size arrays, the checker scans all elements and +// considers the array null-terminated if any element is constrained to zero. +// +//===----------------------------------------------------------------------===// + +#include "clang/AST/Attr.h" +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" + +using namespace clang; +using namespace ento; + +namespace { +class NullTerminatedChecker : public Checker<check::PreCall> { +public: + int MaxArraySize = 1024; + + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; + +private: + const BugType BT{this, "Array not null-terminated", "API"}; + + /// Return true if any element in [0, \param ArraySize) can be zero. + bool hasAnyZeroElement(ProgramStateRef State, SValBuilder &SVB, + QualType EltTy, uint64_t ArraySize, + const TypedValueRegion *TVR) const; + + /// Return true if the element at \param Idx can be zero. + bool canBeZero(ProgramStateRef State, SValBuilder &SVB, QualType EltTy, + SVal Idx, const TypedValueRegion *TVR) const; +}; +} // namespace + +bool NullTerminatedChecker::canBeZero(ProgramStateRef State, SValBuilder &SVB, + QualType EltTy, SVal Idx, + const TypedValueRegion *TVR) const { + // Resolve element at Idx to a defined SVal. + auto IdxNL = Idx.getAs<NonLoc>(); + if (!IdxNL) + return true; + SVal EltAddr = State->getLValue(EltTy, *IdxNL, loc::MemRegionVal(TVR)); + auto EltLocOpt = EltAddr.getAs<Loc>(); + if (!EltLocOpt) + return true; + SVal Val = State->getSVal(*EltLocOpt); // Load from addr + auto DV = Val.getAs<DefinedSVal>(); + if (!DV) + return true; + + // Is the element possibly zero on this path? + SVal EqZero = SVB.evalEQ(State, *DV, SVB.makeZeroVal(EltTy)); + auto EqZeroDV = EqZero.getAs<DefinedSVal>(); + if (!EqZeroDV) + return true; + ProgramStateRef Zero, NonZero; + std::tie(Zero, NonZero) = State->assume(*EqZeroDV); + return static_cast<bool>(Zero); +} + +bool NullTerminatedChecker::hasAnyZeroElement( + ProgramStateRef State, SValBuilder &SVB, QualType EltTy, uint64_t ArraySize, + const TypedValueRegion *TVR) const { + QualType IdxTy = SVB.getArrayIndexType(); + for (uint64_t I = 0; I < ArraySize; ++I) { + SVal Idx = SVB.makeIntVal(I, IdxTy); + if (canBeZero(State, SVB, EltTy, Idx, TVR)) + return true; + } + return false; +} + +void NullTerminatedChecker::checkPreCall(const CallEvent &Call, + CheckerContext &C) const { + const auto *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl()); + if (!FD) + return; + + ProgramStateRef State = C.getState(); + SValBuilder &SVB = C.getSValBuilder(); + ASTContext &Ctx = C.getASTContext(); + + unsigned NumParams = FD->getNumParams(); + unsigned NumArgs = Call.getNumArgs(); + + // The call to min handles the case when |NumParams| != |NumArgs|. + for (unsigned I = 0, N = std::min(NumParams, NumArgs); I < N; ++I) { + const ParmVarDecl *Param = FD->getParamDecl(I); + if (!Param->hasAttr<NullTerminatedAttr>()) + continue; + + SVal ArgVal = Call.getArgSVal(I); + const MemRegion *R = ArgVal.getAsRegion(); + if (!R) + continue; + + // Strip ElementRegion wrappers (array-to-pointer decay produces + // &Element{Array, 0}). + R = R->StripCasts(); + if (const auto *ER = dyn_cast<ElementRegion>(R)) + R = ER->getSuperRegion(); + + const auto *TVR = dyn_cast<TypedValueRegion>(R); + if (!TVR) + continue; + + QualType ElemTy; + bool HasNullTerm = false; + + // Constant-size array (skips C99 FAMs). + if (const auto *CAT = Ctx.getAsConstantArrayType(TVR->getValueType())) { + uint64_t ArraySize = CAT->getSize().getZExtValue(); + + // C89-style FAMs are dynamically allocated to larger sizes than as + // declared so the analyzer cannot reason about them. Skip zero-length + // arrays altogether, and one-length arrays if they are at the end of a + // struct. + if (ArraySize == 0 || ArraySize > static_cast<uint64_t>(MaxArraySize)) + continue; + if (ArraySize == 1) + if (const auto *FR = dyn_cast<FieldRegion>(TVR)) + if (FR->getDecl()->getFieldIndex() == + FR->getDecl()->getParent()->getNumFields() - 1) + continue; + ElemTy = CAT->getElementType(); + HasNullTerm = hasAnyZeroElement(State, SVB, ElemTy, ArraySize, TVR); + } else { + continue; + } + + // TODO: Handle VLAs. + + if (HasNullTerm) + continue; + + // Only warn when all elements constrained to non-zero values. + if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) { + SmallString<128> Msg; + llvm::raw_svector_ostream OS(Msg); + OS << "array argument is not null-terminated; parameter " + << Param->getName() << " expects a null-terminated array"; + auto Report = std::make_unique<PathSensitiveBugReport>(BT, Msg, N); + Report->addRange(Call.getArgSourceRange(I)); + if (const Expr *ArgE = Call.getArgExpr(I)) + bugreporter::trackExpressionValue(N, ArgE, *Report); + C.emitReport(std::move(Report)); + } + } +} + +void ento::registerNullTerminatedChecker(CheckerManager &Mgr) { + auto *Checker = Mgr.registerChecker<NullTerminatedChecker>(); + Checker->MaxArraySize = + Mgr.getAnalyzerOptions().getCheckerIntegerOption(Checker, "MaxArraySize"); + if (Checker->MaxArraySize < 0) { + Mgr.reportInvalidCheckerOptionValue(Checker, "MaxArraySize", + "a non-negative value"); + Checker->MaxArraySize = 0; + } +} + +bool ento::shouldRegisterNullTerminatedChecker(const CheckerManager &Mgr) { + return true; +} diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c index 04dc8c24421bc..4d7b1a733b632 100644 --- a/clang/test/Analysis/analyzer-config.c +++ b/clang/test/Analysis/analyzer-config.c @@ -7,6 +7,7 @@ // CHECK-NEXT: alpha.clone.CloneChecker:IgnoredFilesPattern = "" // CHECK-NEXT: alpha.clone.CloneChecker:MinimumCloneComplexity = 50 // CHECK-NEXT: alpha.clone.CloneChecker:ReportNormalClones = true +// CHECK-NEXT: alpha.core.NullTerminated:MaxArraySize = 1024 // CHECK-NEXT: alpha.cplusplus.STLAlgorithmModeling:AggressiveStdFindModeling = false // CHECK-NEXT: alpha.osx.cocoa.DirectIvarAssignment:AnnotatedFunctions = false // CHECK-NEXT: apply-fixits = false diff --git a/clang/test/Analysis/null-terminated.c b/clang/test/Analysis/null-terminated.c new file mode 100644 index 0000000000000..b1b9c47e7b1f4 --- /dev/null +++ b/clang/test/Analysis/null-terminated.c @@ -0,0 +1,157 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.NullTerminated -DDEFAULT -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.NullTerminated \ +// RUN: -analyzer-config alpha.core.NullTerminated:MaxArraySize=3 \ +// RUN: -DMAX_ARR -verify %s + +void receive(__attribute__((null_terminated)) const int signals[]); + +#ifdef DEFAULT +void test_static_bad(void) { + int sigs[] = {1, 2, 3}; + receive(sigs); // expected-warning{{array argument is not null-terminated}} +} + +void test_static_good(void) { + int sigs[] = {1, 2, 0}; + receive(sigs); +} + +void test_imperative_bad(void) { + int sigs[3]; + sigs[0] = 1; + sigs[1] = 2; + sigs[2] = 3; + receive(sigs); // expected-warning{{array argument is not null-terminated}} +} + +void test_imperative_good(void) { + int sigs[3]; + sigs[0] = 1; + sigs[1] = 2; + sigs[2] = 0; + receive(sigs); +} + +void test_modified_bad(void) { + int sigs[] = {1, 2, 0}; + sigs[2] = 3; + receive(sigs); // expected-warning{{array argument is not null-terminated}} +} + +void test_modified_good(void) { + int sigs[] = {0, 2, 3}; + sigs[1] = 0; + receive(sigs); + sigs[2] = 0; + receive(sigs); +} + +// Early-terminated arrays +void test_early_term_middle(void) { + int sigs[] = {1, 0, 3}; + receive(sigs); +} + +void test_early_term_first(void) { + int sigs[] = {0, 1, 2}; + receive(sigs); +} + +// Single zero element +void test_only_null_term(void) { + int sigs[] = {0}; + receive(sigs); +} + +// Conditional path +void test_conditional(int cond) { + int sigs[] = {1, 2, 0}; + if (cond) + sigs[2] = 3; + receive(sigs); // expected-warning{{array argument is not null-terminated}} +} + +// Zero-length array +struct flex { + int n; + int data[0]; +}; +void test_zero_length(struct flex *f) { + receive(f->data); +} + +void receive_char(__attribute__((null_terminated)) const char buf[]); + +void test_char_bad(void) { + char buf[] = {'a', 'b', 'c'}; + receive_char(buf); // expected-warning{{array argument is not null-terminated}} +} + +void test_char_good(void) { + char buf[] = {'a', '\0', 'c'}; + receive_char(buf); + receive_char("hello"); + receive_char(""); +} + +// Compound literals +void test_compound_literal_bad(void) { + const int *p = (int[]){1, 2, 3}; + receive(p); // expected-warning{{array argument is not null-terminated}} + receive((int[]){1, 2, 3}); // expected-warning{{array argument is not null-terminated}} +} +void test_compound_literal_direct_good(void) { + const int *p = (int[]){1, 2, 0}; + receive(p); + receive((int[]){1, 2, 0}); +} + +// Global arrays: CSA limitation. +int global_bad[] = {1, 2, 3}; +int global_good[] = {1, 2, 0}; + +void test_global(void) { + receive(global_bad); + receive(global_good); +} + +// Static local arrays +void test_static_local_bad(void) { + static int sigs[] = {1, 2, 3}; + receive(sigs); // expected-warning{{array argument is not null-terminated}} +} + +void test_static_local_good(void) { + static const int sigs[] = {1, 2, 0}; + receive(sigs); +} + +// CSA cannot reason about memset - we test here for false positives. +void *memset(void *, int, unsigned long); + +void test_memset_zero_bad(void) { + int sigs[4] = {0, 0, 0, 0}; + memset(sigs, -1, sizeof(sigs)); // set all bits to 1 + receive(sigs); +} + +void test_memset_zero_good(void) { + int sigs[4] = {1, 2, 3, 4}; + memset(sigs, 0, sizeof(sigs)); + receive(sigs); +} + +#endif // DEFAULT + +#ifdef MAX_ARR +void test_maxarraysize_at_limit(void) { + int sigs[3] = {1, 2, 3}; + receive(sigs); // expected-warning{{array argument is not null-terminated}} +} + +void test_maxarraysize_over_limit(void) { + int sigs[4] = {1, 2, 3, 4}; + receive(sigs); // no-warning +} + +#endif // MAX_ARR diff --git a/clang/test/Analysis/null-terminated.cpp b/clang/test/Analysis/null-terminated.cpp new file mode 100644 index 0000000000000..170d9f1ea7bf2 --- /dev/null +++ b/clang/test/Analysis/null-terminated.cpp @@ -0,0 +1,54 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.NullTerminated -verify %s + +void receive(__attribute__((null_terminated)) const int signals[]); + +void test_constexpr_bad() { + constexpr int sigs[] = {1, 2, 3}; + receive(sigs); // expected-warning{{array argument is not null-terminated}} +} + +void test_constexpr_good() { + constexpr int sigs[] = {1, 2, 0}; + receive(sigs); +} + +void test_constexpr_early_term() { + constexpr int sigs[] = {1, 0, 3}; + receive(sigs); +} + +void receive_ref(__attribute__((null_terminated)) const int (&signals)[3]); + +void test_ref_bad() { + const int sigs[3] = {1, 2, 3}; + receive_ref(sigs); // expected-warning{{array argument is not null-terminated}} +} + +void test_ref_good() { + const int sigs[3] = {1, 2, 0}; + receive_ref(sigs); +} + +// CSA limitation - CSA cannot see through default member initializers. +struct S { + int bad[3] = {1, 2, 3}; + int good[3] = {1, 2, 0}; + +}; +void test_inclass_bad() { + S s; + receive(s.bad); +} + +void test_inclass_good() { + S s; + receive(s.good); +} + +// Test C++ attribute syntax +void receive_cpp([[clang::null_terminated]] const int signals[]); + +void test_cpp_spelling() { + int sigs[] = {1, 2, 3}; + receive_cpp(sigs); // expected-warning{{array argument is not null-terminated}} +} diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test index 03b9a77ec1814..90077fcb990c5 100644 --- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test +++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test @@ -146,6 +146,7 @@ // CHECK-NEXT: NoUwtable (SubjectMatchRule_hasType_functionType) // CHECK-NEXT: NonString (SubjectMatchRule_variable, SubjectMatchRule_field) // CHECK-NEXT: NotTailCalled (SubjectMatchRule_function) +// CHECK-NEXT: NullTerminated (SubjectMatchRule_variable_is_parameter) // CHECK-NEXT: OMPAssume (SubjectMatchRule_function, SubjectMatchRule_objc_method) // CHECK-NEXT: OSConsumed (SubjectMatchRule_variable_is_parameter) // CHECK-NEXT: OSReturnsNotRetained (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_variable_is_parameter) diff --git a/clang/test/Sema/attr-null-terminated.c b/clang/test/Sema/attr-null-terminated.c new file mode 100644 index 0000000000000..314418350519e --- /dev/null +++ b/clang/test/Sema/attr-null-terminated.c @@ -0,0 +1,16 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +void good_ptr(__attribute__((null_terminated)) const int *p); +void good_arr(__attribute__((null_terminated)) const int p[]); +void good_char(__attribute__((null_terminated)) const char *s); + +// Not a pointer or array type +void bad_int(__attribute__((null_terminated)) int x); // expected-warning {{'null_terminated' attribute only applies to parameters of pointer or array type}} + +// Not a parameter +__attribute__((null_terminated)) int global; // expected-warning {{'null_terminated' attribute only applies to parameters}} +struct S { __attribute__((null_terminated)) int *field; }; // expected-warning {{'null_terminated' attribute only applies to parameters}} + +// Takes no arguments +void bad_args0(__attribute__((null_terminated("test"))) const int *p); // expected-error {{'null_terminated' attribute takes no arguments}} +void bad_args1(__attribute__((null_terminated(123))) const int *p); // expected-error {{'null_terminated' attribute takes no arguments}} >From b94009095d95edc8f2f21feab321bb98d388ae2d Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Fri, 19 Jun 2026 18:20:27 +0200 Subject: [PATCH 2/3] Remove explicit null_terminated attribute Use annotate attribute instead to avoid introducing a new attribute. --- clang/docs/ReleaseNotes.rst | 8 ++---- clang/include/clang/Basic/Attr.td | 6 ----- clang/include/clang/Basic/AttrDocs.td | 20 --------------- .../clang/StaticAnalyzer/Checkers/Checkers.td | 2 +- clang/lib/Sema/SemaDeclAttr.cpp | 25 ------------------- .../Checkers/NullTerminatedChecker.cpp | 15 ++++++++++- clang/test/Analysis/null-terminated.c | 8 +++--- clang/test/Analysis/null-terminated.cpp | 10 +++++--- ...a-attribute-supported-attributes-list.test | 1 - clang/test/Sema/attr-null-terminated.c | 16 ------------ 10 files changed, 28 insertions(+), 83 deletions(-) delete mode 100644 clang/test/Sema/attr-null-terminated.c diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 16e7576b65db2..8d242ba9cc3aa 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -516,10 +516,6 @@ Attribute Changes in Clang * The ``modular_format`` attribute now supports the ``fixed`` aspect for C ISO 18037 fixed-point ``printf`` specifiers. -- Added the ``null_terminated`` attribute, which can be applied to function - parameters of pointer or array type (of scalar type) to indicate that the - function expects a null-terminated buffer. - Improvements to Clang's diagnostics ----------------------------------- - Fixed bug in ``-Wdocumentation`` so that it correctly handles explicit @@ -1058,8 +1054,8 @@ New checkers or options ^^^^^^^^^^^^^^^^^^^^^^^ - Introduced the ``alpha.core.NullTerminated`` checker to detect arrays missing - a null terminator passed as a parameter marked with the ``null_terminated`` - attribute. + a null terminator passed as a parameter annotated with + ``__attribute__((annotate("null_terminated")))``. .. _release-notes-sanitizers: diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 63ce0c0543c08..a222092cd42cf 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -5470,12 +5470,6 @@ def NonString : InheritableAttr { let Documentation = [NonStringDocs]; } -def NullTerminated : InheritableAttr { - let Spellings = [Clang<"null_terminated">]; - let Subjects = SubjectList<[ParmVar]>; - let Documentation = [NullTerminatedDocs]; -} - def ModularFormat : InheritableAttr { let Spellings = [Clang<"modular_format">]; let Args = [IdentifierArgument<"ModularImplFn">, StringArgument<"ImplName">, diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 7f4fb2a33ef41..04362de2d5be2 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -10038,26 +10038,6 @@ silence diagnostics with code like: }]; } -def NullTerminatedDocs : Documentation { - let Category = DocCatDecl; - let Content = [{ -The ``null_terminated`` attribute can be applied to a function parameter whose -type is a pointer or array to indicate that the function expects the buffer to -be null-terminated (with a zero-valued element). This enables the checker -``alpha.core.NullTerminated`` to detect buffers missing a null terminator passed -as a parameter marked with this attribute, which may cause undefined behavior. - -.. code-block:: c - - void receive(__attribute__((null_terminated)) const int signals[]); - - void caller(void) { - int sigs[] = {1, 2, 3}; // Missing null terminator - receive(sigs); - } - }]; -} - def ModularFormatDocs : Documentation { let Category = DocCatFunction; let Content = [{ diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td index 9076bd7e42086..28d8bf07425de 100644 --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -292,7 +292,7 @@ def StdVariantChecker : Checker<"StdVariant">, def NullTerminatedChecker : Checker<"NullTerminated">, HelpText<"Check for arrays not null-terminated passed to functions whose " - "parameters are marked with the 'null_terminated' attribute">, + "parameters are annotated with annotate(\"null_terminated\")">, CheckerOptions<[CmdLineOption< Integer, "MaxArraySize", "Maximum number of array elements to scan for a null terminator. " diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 386d100532b53..2159c586e5738 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -5204,28 +5204,6 @@ static void handleNonStringAttr(Sema &S, Decl *D, const ParsedAttr &AL) { D->addAttr(::new (S.Context) NonStringAttr(S.Context, AL)); } -static void handleNullTerminatedAttr(Sema &S, Decl *D, const ParsedAttr &AL) { - // Only makes sense on function params that are buffers. - QualType QT = cast<ValueDecl>(D)->getType().getNonReferenceType(); - if (!QT->isArrayType() && !QT->isPointerType()) { - S.Diag(D->getBeginLoc(), diag::warn_attribute_wrong_decl_type_str) - << AL << AL.isRegularKeywordAttribute() - << "parameters of pointer or array type"; - return; - } - // Null terminators only make sense for scalar element types. - QualType EltTy = QT->isArrayType() - ? S.Context.getAsArrayType(QT)->getElementType() - : QT->getPointeeType(); - if (!EltTy->isScalarType()) { - S.Diag(D->getBeginLoc(), diag::warn_attribute_wrong_decl_type_str) - << AL << AL.isRegularKeywordAttribute() - << "parameters of pointer or array of scalar type"; - return; - } - D->addAttr(::new (S.Context) NullTerminatedAttr(S.Context, AL)); -} - static void handleNoDebugAttr(Sema &S, Decl *D, const ParsedAttr &AL) { D->addAttr(::new (S.Context) NoDebugAttr(S.Context, AL)); } @@ -7850,9 +7828,6 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, case ParsedAttr::AT_NonString: handleNonStringAttr(S, D, AL); break; - case ParsedAttr::AT_NullTerminated: - handleNullTerminatedAttr(S, D, AL); - break; case ParsedAttr::AT_NonNull: if (auto *PVD = dyn_cast<ParmVarDecl>(D)) handleNonNullAttrParameter(S, PVD, AL); diff --git a/clang/lib/StaticAnalyzer/Checkers/NullTerminatedChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NullTerminatedChecker.cpp index 5517d3ff69920..04a594b524a33 100644 --- a/clang/lib/StaticAnalyzer/Checkers/NullTerminatedChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/NullTerminatedChecker.cpp @@ -11,6 +11,9 @@ // element). For constant-size arrays, the checker scans all elements and // considers the array null-terminated if any element is constrained to zero. // +// Parameters are marked as expecting null-terminated buffers using: +// __attribute__((annotate("null_terminated"))) +// //===----------------------------------------------------------------------===// #include "clang/AST/Attr.h" @@ -34,6 +37,9 @@ class NullTerminatedChecker : public Checker<check::PreCall> { private: const BugType BT{this, "Array not null-terminated", "API"}; + /// Return true if the parameter has annotate("null_terminated"). + static bool isNullTerminatedParam(const ParmVarDecl *Param); + /// Return true if any element in [0, \param ArraySize) can be zero. bool hasAnyZeroElement(ProgramStateRef State, SValBuilder &SVB, QualType EltTy, uint64_t ArraySize, @@ -45,6 +51,13 @@ class NullTerminatedChecker : public Checker<check::PreCall> { }; } // namespace +bool NullTerminatedChecker::isNullTerminatedParam(const ParmVarDecl *Param) { + return llvm::any_of(Param->specific_attrs<AnnotateAttr>(), + [](const AnnotateAttr *Ann) { + return Ann->getAnnotation() == "null_terminated"; + }); +} + bool NullTerminatedChecker::canBeZero(ProgramStateRef State, SValBuilder &SVB, QualType EltTy, SVal Idx, const TypedValueRegion *TVR) const { @@ -99,7 +112,7 @@ void NullTerminatedChecker::checkPreCall(const CallEvent &Call, // The call to min handles the case when |NumParams| != |NumArgs|. for (unsigned I = 0, N = std::min(NumParams, NumArgs); I < N; ++I) { const ParmVarDecl *Param = FD->getParamDecl(I); - if (!Param->hasAttr<NullTerminatedAttr>()) + if (!isNullTerminatedParam(Param)) continue; SVal ArgVal = Call.getArgSVal(I); diff --git a/clang/test/Analysis/null-terminated.c b/clang/test/Analysis/null-terminated.c index b1b9c47e7b1f4..043ac449c1a41 100644 --- a/clang/test/Analysis/null-terminated.c +++ b/clang/test/Analysis/null-terminated.c @@ -3,7 +3,9 @@ // RUN: -analyzer-config alpha.core.NullTerminated:MaxArraySize=3 \ // RUN: -DMAX_ARR -verify %s -void receive(__attribute__((null_terminated)) const int signals[]); +#define NULL_TERMINATED __attribute__((annotate("null_terminated"))) + +void receive(NULL_TERMINATED const int signals[]); #ifdef DEFAULT void test_static_bad(void) { @@ -80,7 +82,7 @@ void test_zero_length(struct flex *f) { receive(f->data); } -void receive_char(__attribute__((null_terminated)) const char buf[]); +void receive_char(NULL_TERMINATED const char buf[]); void test_char_bad(void) { char buf[] = {'a', 'b', 'c'}; @@ -127,7 +129,7 @@ void test_static_local_good(void) { } // CSA cannot reason about memset - we test here for false positives. -void *memset(void *, int, unsigned long); +void *memset(void *, int, __SIZE_TYPE__); void test_memset_zero_bad(void) { int sigs[4] = {0, 0, 0, 0}; diff --git a/clang/test/Analysis/null-terminated.cpp b/clang/test/Analysis/null-terminated.cpp index 170d9f1ea7bf2..84bc02b009865 100644 --- a/clang/test/Analysis/null-terminated.cpp +++ b/clang/test/Analysis/null-terminated.cpp @@ -1,6 +1,8 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.NullTerminated -verify %s -void receive(__attribute__((null_terminated)) const int signals[]); +#define NULL_TERMINATED __attribute__((annotate("null_terminated"))) + +void receive(NULL_TERMINATED const int signals[]); void test_constexpr_bad() { constexpr int sigs[] = {1, 2, 3}; @@ -17,7 +19,7 @@ void test_constexpr_early_term() { receive(sigs); } -void receive_ref(__attribute__((null_terminated)) const int (&signals)[3]); +void receive_ref(NULL_TERMINATED const int (&signals)[3]); void test_ref_bad() { const int sigs[3] = {1, 2, 3}; @@ -45,8 +47,8 @@ void test_inclass_good() { receive(s.good); } -// Test C++ attribute syntax -void receive_cpp([[clang::null_terminated]] const int signals[]); +// Test C++11 attribute syntax +void receive_cpp([[clang::annotate("null_terminated")]] const int signals[]); void test_cpp_spelling() { int sigs[] = {1, 2, 3}; diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test index 90077fcb990c5..03b9a77ec1814 100644 --- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test +++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test @@ -146,7 +146,6 @@ // CHECK-NEXT: NoUwtable (SubjectMatchRule_hasType_functionType) // CHECK-NEXT: NonString (SubjectMatchRule_variable, SubjectMatchRule_field) // CHECK-NEXT: NotTailCalled (SubjectMatchRule_function) -// CHECK-NEXT: NullTerminated (SubjectMatchRule_variable_is_parameter) // CHECK-NEXT: OMPAssume (SubjectMatchRule_function, SubjectMatchRule_objc_method) // CHECK-NEXT: OSConsumed (SubjectMatchRule_variable_is_parameter) // CHECK-NEXT: OSReturnsNotRetained (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_variable_is_parameter) diff --git a/clang/test/Sema/attr-null-terminated.c b/clang/test/Sema/attr-null-terminated.c deleted file mode 100644 index 314418350519e..0000000000000 --- a/clang/test/Sema/attr-null-terminated.c +++ /dev/null @@ -1,16 +0,0 @@ -// RUN: %clang_cc1 -fsyntax-only -verify %s - -void good_ptr(__attribute__((null_terminated)) const int *p); -void good_arr(__attribute__((null_terminated)) const int p[]); -void good_char(__attribute__((null_terminated)) const char *s); - -// Not a pointer or array type -void bad_int(__attribute__((null_terminated)) int x); // expected-warning {{'null_terminated' attribute only applies to parameters of pointer or array type}} - -// Not a parameter -__attribute__((null_terminated)) int global; // expected-warning {{'null_terminated' attribute only applies to parameters}} -struct S { __attribute__((null_terminated)) int *field; }; // expected-warning {{'null_terminated' attribute only applies to parameters}} - -// Takes no arguments -void bad_args0(__attribute__((null_terminated("test"))) const int *p); // expected-error {{'null_terminated' attribute takes no arguments}} -void bad_args1(__attribute__((null_terminated(123))) const int *p); // expected-error {{'null_terminated' attribute takes no arguments}} >From 89f1bd24e24b60656b0b178f6707e9db631ff689 Mon Sep 17 00:00:00 2001 From: John Jepko <[email protected]> Date: Fri, 19 Jun 2026 21:08:14 +0200 Subject: [PATCH 3/3] Add docs for null term checker, extra test case New test case ensures annotation works on both sides of the parameter. --- clang/docs/analyzer/checkers.rst | 18 ++++++++++++++++++ clang/test/Analysis/null-terminated.c | 6 ++++++ 2 files changed, 24 insertions(+) diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index f4cda9ec4b0c1..adf7310c65186 100644 --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -3238,6 +3238,24 @@ Check for cases where the dynamic and the static type of an object are unrelated NSNumber *number = date; [number doubleValue]; +.. _alpha-core-NullTerminated: + +alpha.core.NullTerminated (C, C++) +"""""""""""""""""""""""""""""""""" +Check for arrays not null-terminated passed to functions whose parameters are +annotated with ``__attribute__((annotate("null_terminated")))``. + +.. code-block:: c + + #define NULL_TERMINATED __attribute__((annotate("null_terminated"))) + + void receive(NULL_TERMINATED const int signals[]); + + void caller(void) { + int sigs[] = {1, 2, 3}; // Missing null terminator + receive(sigs); // warn: array argument is not null-terminated + } + .. _alpha-core-PointerArithm: alpha.core.PointerArithm (C) diff --git a/clang/test/Analysis/null-terminated.c b/clang/test/Analysis/null-terminated.c index 043ac449c1a41..d077c70b84d7e 100644 --- a/clang/test/Analysis/null-terminated.c +++ b/clang/test/Analysis/null-terminated.c @@ -6,6 +6,7 @@ #define NULL_TERMINATED __attribute__((annotate("null_terminated"))) void receive(NULL_TERMINATED const int signals[]); +void receive_after(const int signals[] NULL_TERMINATED); #ifdef DEFAULT void test_static_bad(void) { @@ -18,6 +19,11 @@ void test_static_good(void) { receive(sigs); } +void test_attr_after_param(void) { + int sigs[] = {1, 2, 3}; + receive_after(sigs); // expected-warning{{array argument is not null-terminated}} +} + void test_imperative_bad(void) { int sigs[3]; sigs[0] = 1; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
