https://github.com/flash1729 created https://github.com/llvm/llvm-project/pull/186960
Related: #183340 Upstream from swiftlang/llvm-project@721e6a3 with some modifications. Since __counted_by and related attributes aren't supported upstream yet in function signatures, this only upstreams the format part — YAML parsing, binary serialization, and deserialization. Semantic application is a follow-up. Modifications from downstream: - Renamed local YAML struct from `BoundsSafety` to `BoundsSafetyNotes` to avoid name collision with `Param::BoundsSafety` field (downstream fixed this in a subsequent commit) - `Level` in `BoundsSafetyNotes` is now `std::optional<unsigned>` so we can actually tell apart "user didn't write Level:" from "Level: 0" - `asdf_sized` and `asdf_sized_n` use `void *buf` — makes more sense for sized_by - Added `asdf_counted_indirect` with `int **` and `Level: 1` to test the indirection level path - Removed a vacuous assert (`CountedBy == 0`, always true for unsigned) - Added `operator!=` for `BoundsSafetyInfo` to match other types in `Types.h` - Added doc comments for `LevelAudited` and `Level` >From 4bc6d2b105e1393a4a71a530673d455046d5a248 Mon Sep 17 00:00:00 2001 From: flash1729 <[email protected]> Date: Tue, 17 Mar 2026 11:10:31 +0530 Subject: [PATCH] [APINotes][BoundsSafety] Upstream API notes format for bounds-safety function parameters Upstreams format-level support for bounds-safety annotations on function parameters from the swiftlang downstream (swiftlang/llvm-project@721e6a3), addressing issue #183340. Since __counted_by and related attributes are not yet supported in upstream function signatures, this patch upstreams only the format portion: YAML parsing, binary serialization, and deserialization. Semantic application in SemaAPINotes.cpp is deferred to a follow-up once attribute support lands. Adds BoundsSafetyInfo to Types.h with a BoundsSafetyKind enum covering counted_by, counted_by_or_null, sized_by, sized_by_or_null, and ended_by. Also encodes an optional pointer indirection level and an external bounds expression string. BoundsSafety is added as an optional field on ParamInfo with merge and equality support. Bumps VERSION_MINOR to 39. --- clang/include/clang/APINotes/Types.h | 82 ++++++++++++++++++- clang/lib/APINotes/APINotesFormat.h | 2 +- clang/lib/APINotes/APINotesReader.cpp | 29 ++++++- clang/lib/APINotes/APINotesTypes.cpp | 29 +++++++ clang/lib/APINotes/APINotesWriter.cpp | 35 +++++++- clang/lib/APINotes/APINotesYAMLCompiler.cpp | 43 ++++++++++ .../Inputs/Headers/BoundsUnsafe.apinotes | 39 +++++++++ .../APINotes/Inputs/Headers/BoundsUnsafe.h | 6 ++ .../APINotes/Inputs/Headers/module.modulemap | 4 + clang/test/APINotes/bounds-safety.c | 28 +++++++ 10 files changed, 292 insertions(+), 5 deletions(-) create mode 100644 clang/test/APINotes/Inputs/Headers/BoundsUnsafe.apinotes create mode 100644 clang/test/APINotes/Inputs/Headers/BoundsUnsafe.h create mode 100644 clang/test/APINotes/bounds-safety.c diff --git a/clang/include/clang/APINotes/Types.h b/clang/include/clang/APINotes/Types.h index fb2b91a3e1750..ed6737672b378 100644 --- a/clang/include/clang/APINotes/Types.h +++ b/clang/include/clang/APINotes/Types.h @@ -338,6 +338,78 @@ inline bool operator!=(const ContextInfo &LHS, const ContextInfo &RHS) { return !(LHS == RHS); } +class BoundsSafetyInfo { +public: + enum class BoundsSafetyKind { + CountedBy = 0, + CountedByOrNull, + SizedBy, + SizedByOrNull, + EndedBy, + }; + +private: + /// Whether the bounds safety kind has been audited. + LLVM_PREFERRED_TYPE(bool) + unsigned KindAudited : 1; + + /// The kind of bounds safety for this property. Only valid if the bounds + /// safety has been audited. + LLVM_PREFERRED_TYPE(BoundsSafetyKind) + unsigned Kind : 3; + + /// Whether the pointer indirection level has been specified. + LLVM_PREFERRED_TYPE(bool) + unsigned LevelAudited : 1; + + /// The pointer indirection level at which the bounds annotation applies. + /// Only valid if LevelAudited is set. + unsigned Level : 3; + +public: + std::string ExternalBounds; + + BoundsSafetyInfo() + : KindAudited(false), Kind(0), LevelAudited(false), Level(0), + ExternalBounds("") {} + + std::optional<BoundsSafetyKind> getKind() const { + return KindAudited ? std::optional<BoundsSafetyKind>( + static_cast<BoundsSafetyKind>(Kind)) + : std::nullopt; + } + + void setKindAudited(BoundsSafetyKind kind) { + KindAudited = true; + Kind = static_cast<unsigned>(kind); + } + + std::optional<unsigned> getLevel() const { + return LevelAudited ? std::optional<unsigned>(Level) : std::nullopt; + } + + void setLevelAudited(unsigned level) { + LevelAudited = true; + Level = level; + } + + friend bool operator==(const BoundsSafetyInfo &, const BoundsSafetyInfo &); + + LLVM_DUMP_METHOD void dump(llvm::raw_ostream &OS) const; +}; + +inline bool operator==(const BoundsSafetyInfo &LHS, + const BoundsSafetyInfo &RHS) { + return LHS.KindAudited == RHS.KindAudited && LHS.Kind == RHS.Kind && + LHS.LevelAudited == RHS.LevelAudited && LHS.Level == RHS.Level && + LHS.ExternalBounds == RHS.ExternalBounds; +} + +inline bool operator!=(const BoundsSafetyInfo &LHS, + const BoundsSafetyInfo &RHS) { + return !(LHS == RHS); +} + /// API notes for a variable/property. class VariableInfo : public CommonEntityInfo { /// Whether this property has been audited for nullability. @@ -477,10 +549,12 @@ class ParamInfo : public VariableInfo { unsigned RawRetainCountConvention : 3; public: + std::optional<BoundsSafetyInfo> BoundsSafety; + ParamInfo() : NoEscapeSpecified(false), NoEscape(false), LifetimeboundSpecified(false), Lifetimebound(false), - RawRetainCountConvention() {} + RawRetainCountConvention(), BoundsSafety(std::nullopt) {} std::optional<bool> isNoEscape() const { return NoEscapeSpecified ? std::optional<bool>(NoEscape) : std::nullopt; @@ -526,6 +600,9 @@ class ParamInfo : public VariableInfo { if (!RawRetainCountConvention) RawRetainCountConvention = RHS.RawRetainCountConvention; + if (!BoundsSafety) + BoundsSafety = RHS.BoundsSafety; + return *this; } @@ -540,7 +617,8 @@ inline bool operator==(const ParamInfo &LHS, const ParamInfo &RHS) { LHS.NoEscape == RHS.NoEscape && LHS.LifetimeboundSpecified == RHS.LifetimeboundSpecified && LHS.Lifetimebound == RHS.Lifetimebound && - LHS.RawRetainCountConvention == RHS.RawRetainCountConvention; + LHS.RawRetainCountConvention == RHS.RawRetainCountConvention && + LHS.BoundsSafety == RHS.BoundsSafety; } inline bool operator!=(const ParamInfo &LHS, const ParamInfo &RHS) { diff --git a/clang/lib/APINotes/APINotesFormat.h b/clang/lib/APINotes/APINotesFormat.h index bb423ccb2bfaf..fe485327b8130 100644 --- a/clang/lib/APINotes/APINotesFormat.h +++ b/clang/lib/APINotes/APINotesFormat.h @@ -24,7 +24,7 @@ const uint16_t VERSION_MAJOR = 0; /// API notes file minor version number. /// /// When the format changes IN ANY WAY, this number should be incremented. -const uint16_t VERSION_MINOR = 38; // SwiftSafety +const uint16_t VERSION_MINOR = 39; // BoundsSafety const uint8_t kSwiftConforms = 1; const uint8_t kSwiftDoesNotConform = 2; diff --git a/clang/lib/APINotes/APINotesReader.cpp b/clang/lib/APINotes/APINotesReader.cpp index 7f9bb5f12cda7..086c30ea47ec2 100644 --- a/clang/lib/APINotes/APINotesReader.cpp +++ b/clang/lib/APINotes/APINotesReader.cpp @@ -330,6 +330,29 @@ class FieldTableInfo } }; +/// Read serialized BoundsSafetyInfo. +void ReadBoundsSafetyInfo(const uint8_t *&Data, BoundsSafetyInfo &Info) { + uint8_t Payload = endian::readNext<uint8_t, llvm::endianness::little>(Data); + + if (Payload & 0x01) { + uint8_t Level = (Payload >> 1) & 0x7; + Info.setLevelAudited(Level); + } + Payload >>= 4; + + if (Payload & 0x01) { + uint8_t Kind = (Payload >> 1) & 0x7; + assert(Kind <= + static_cast<uint8_t>(BoundsSafetyInfo::BoundsSafetyKind::EndedBy)); + Info.setKindAudited(static_cast<BoundsSafetyInfo::BoundsSafetyKind>(Kind)); + } + + uint16_t ExternalBoundsLen = + endian::readNext<uint16_t, llvm::endianness::little>(Data); + Info.ExternalBounds = std::string(Data, Data + ExternalBoundsLen); + Data += ExternalBoundsLen; +} + /// Read serialized ParamInfo. void ReadParamInfo(const uint8_t *&Data, ParamInfo &Info) { ReadVariableInfo(Data, Info); @@ -346,7 +369,11 @@ void ReadParamInfo(const uint8_t *&Data, ParamInfo &Info) { if (Payload & 0x01) Info.setNoEscape(Payload & 0x02); Payload >>= 2; - assert(Payload == 0 && "Bad API notes"); + if (Payload & 0x01) { + BoundsSafetyInfo BSI; + ReadBoundsSafetyInfo(Data, BSI); + Info.BoundsSafety = BSI; + } } /// Read serialized FunctionInfo. diff --git a/clang/lib/APINotes/APINotesTypes.cpp b/clang/lib/APINotes/APINotesTypes.cpp index bff4be104c6c8..9baf06efde6ba 100644 --- a/clang/lib/APINotes/APINotesTypes.cpp +++ b/clang/lib/APINotes/APINotesTypes.cpp @@ -76,6 +76,33 @@ LLVM_DUMP_METHOD void ObjCPropertyInfo::dump(llvm::raw_ostream &OS) const { OS << '\n'; } +LLVM_DUMP_METHOD void BoundsSafetyInfo::dump(llvm::raw_ostream &OS) const { + if (KindAudited) { + assert(static_cast<BoundsSafetyKind>(Kind) <= BoundsSafetyKind::EndedBy); + switch (static_cast<BoundsSafetyKind>(Kind)) { + case BoundsSafetyKind::CountedBy: + OS << "[counted_by] "; + break; + case BoundsSafetyKind::CountedByOrNull: + OS << "[counted_by_or_null] "; + break; + case BoundsSafetyKind::SizedBy: + OS << "[sized_by] "; + break; + case BoundsSafetyKind::SizedByOrNull: + OS << "[sized_by_or_null] "; + break; + case BoundsSafetyKind::EndedBy: + OS << "[ended_by] "; + break; + } + } + if (LevelAudited) + OS << "Level: " << Level << " "; + OS << "ExternalBounds: " + << (ExternalBounds.empty() ? "<missing>" : ExternalBounds) << '\n'; +} + LLVM_DUMP_METHOD void ParamInfo::dump(llvm::raw_ostream &OS) const { static_cast<const VariableInfo &>(*this).dump(OS); if (NoEscapeSpecified) @@ -84,6 +111,8 @@ LLVM_DUMP_METHOD void ParamInfo::dump(llvm::raw_ostream &OS) const { OS << (Lifetimebound ? "[Lifetimebound] " : ""); OS << "RawRetainCountConvention: " << RawRetainCountConvention << ' '; OS << '\n'; + if (BoundsSafety) + BoundsSafety->dump(OS); } LLVM_DUMP_METHOD void FunctionInfo::dump(llvm::raw_ostream &OS) const { diff --git a/clang/lib/APINotes/APINotesWriter.cpp b/clang/lib/APINotes/APINotesWriter.cpp index 47ed93a567c0e..daf7903c9d9ad 100644 --- a/clang/lib/APINotes/APINotesWriter.cpp +++ b/clang/lib/APINotes/APINotesWriter.cpp @@ -1071,14 +1071,45 @@ void APINotesWriter::Implementation::writeGlobalVariableBlock( } namespace { +void emitBoundsSafetyInfo(raw_ostream &OS, const BoundsSafetyInfo &BSI) { + llvm::support::endian::Writer writer(OS, llvm::endianness::little); + uint8_t flags = 0; + if (auto kind = BSI.getKind()) { + assert((uint8_t)*kind < (1 << 3)); + flags |= 0x01; // 1 bit + flags |= (uint8_t)*kind << 1; // 3 bits + } + flags <<= 4; + if (auto level = BSI.getLevel()) { + assert((uint8_t)*level < (1 << 3)); + flags |= 0x01; // 1 bit + flags |= (uint8_t)*level << 1; // 3 bits + } + + writer.write<uint8_t>(flags); + writer.write<uint16_t>(BSI.ExternalBounds.size()); + writer.write( + ArrayRef<char>{BSI.ExternalBounds.data(), BSI.ExternalBounds.size()}); +} + +unsigned getBoundsSafetyInfoSize(const BoundsSafetyInfo &BSI) { + return 1 + sizeof(uint16_t) + BSI.ExternalBounds.size(); +} + unsigned getParamInfoSize(const ParamInfo &PI) { - return getVariableInfoSize(PI) + 1; + unsigned BSISize = 0; + if (auto BSI = PI.BoundsSafety) + BSISize = getBoundsSafetyInfoSize(*BSI); + return getVariableInfoSize(PI) + 1 + BSISize; } void emitParamInfo(raw_ostream &OS, const ParamInfo &PI) { emitVariableInfo(OS, PI); uint8_t flags = 0; + if (PI.BoundsSafety) + flags |= 0x01; + flags <<= 2; if (auto noescape = PI.isNoEscape()) { flags |= 0x01; if (*noescape) @@ -1096,6 +1127,8 @@ void emitParamInfo(raw_ostream &OS, const ParamInfo &PI) { llvm::support::endian::Writer writer(OS, llvm::endianness::little); writer.write<uint8_t>(flags); + if (auto BSI = PI.BoundsSafety) + emitBoundsSafetyInfo(OS, *BSI); } /// Retrieve the serialized size of the given FunctionInfo, for use in on-disk diff --git a/clang/lib/APINotes/APINotesYAMLCompiler.cpp b/clang/lib/APINotes/APINotesYAMLCompiler.cpp index 3be528feb325e..fdffbbac00c4c 100644 --- a/clang/lib/APINotes/APINotesYAMLCompiler.cpp +++ b/clang/lib/APINotes/APINotesYAMLCompiler.cpp @@ -49,6 +49,31 @@ enum class APIAvailability { }; } // namespace +namespace { +struct BoundsSafetyNotes { + BoundsSafetyInfo::BoundsSafetyKind Kind; + std::optional<unsigned> Level; + StringRef BoundsExpr = ""; +}; +} // namespace + +namespace llvm { +namespace yaml { +template <> struct ScalarEnumerationTraits<BoundsSafetyInfo::BoundsSafetyKind> { + static void enumeration(IO &IO, BoundsSafetyInfo::BoundsSafetyKind &AA) { + IO.enumCase(AA, "counted_by", + BoundsSafetyInfo::BoundsSafetyKind::CountedBy); + IO.enumCase(AA, "counted_by_or_null", + BoundsSafetyInfo::BoundsSafetyKind::CountedByOrNull); + IO.enumCase(AA, "sized_by", BoundsSafetyInfo::BoundsSafetyKind::SizedBy); + IO.enumCase(AA, "sized_by_or_null", + BoundsSafetyInfo::BoundsSafetyKind::SizedByOrNull); + IO.enumCase(AA, "ended_by", BoundsSafetyInfo::BoundsSafetyKind::EndedBy); + } +}; +} // namespace yaml +} // namespace llvm + namespace llvm { namespace yaml { template <> struct ScalarEnumerationTraits<APIAvailability> { @@ -86,6 +111,7 @@ struct Param { std::optional<bool> Lifetimebound = false; std::optional<NullabilityKind> Nullability; std::optional<RetainCountConventionKind> RetainCountConvention; + std::optional<BoundsSafetyNotes> BoundsSafety; StringRef Type; }; @@ -137,6 +163,15 @@ template <> struct MappingTraits<Param> { IO.mapOptional("NoEscape", P.NoEscape); IO.mapOptional("Lifetimebound", P.Lifetimebound); IO.mapOptional("Type", P.Type, StringRef("")); + IO.mapOptional("BoundsSafety", P.BoundsSafety); + } +}; + +template <> struct MappingTraits<BoundsSafetyNotes> { + static void mapping(IO &IO, BoundsSafetyNotes &BS) { + IO.mapRequired("Kind", BS.Kind); + IO.mapRequired("BoundedBy", BS.BoundsExpr); + IO.mapOptional("Level", BS.Level); } }; } // namespace yaml @@ -787,6 +822,14 @@ class YAMLConverter { PI.setLifetimebound(P.Lifetimebound); PI.setType(std::string(P.Type)); PI.setRetainCountConvention(P.RetainCountConvention); + if (P.BoundsSafety) { + BoundsSafetyInfo BSI; + BSI.setKindAudited(P.BoundsSafety->Kind); + if (P.BoundsSafety->Level) + BSI.setLevelAudited(*P.BoundsSafety->Level); + BSI.ExternalBounds = P.BoundsSafety->BoundsExpr.str(); + PI.BoundsSafety = BSI; + } if (static_cast<int>(OutInfo.Params.size()) <= P.Position) OutInfo.Params.resize(P.Position + 1); if (P.Position == -1) diff --git a/clang/test/APINotes/Inputs/Headers/BoundsUnsafe.apinotes b/clang/test/APINotes/Inputs/Headers/BoundsUnsafe.apinotes new file mode 100644 index 0000000000000..27b44dae0466d --- /dev/null +++ b/clang/test/APINotes/Inputs/Headers/BoundsUnsafe.apinotes @@ -0,0 +1,39 @@ +Name: BoundsUnsafe +Functions: + - Name: asdf_counted + Parameters: + - Position: 0 + BoundsSafety: + Kind: counted_by + BoundedBy: len + - Name: asdf_sized + Parameters: + - Position: 0 + BoundsSafety: + Kind: sized_by + BoundedBy: size + - Name: asdf_counted_n + Parameters: + - Position: 0 + BoundsSafety: + Kind: counted_by_or_null + BoundedBy: len + - Name: asdf_sized_n + Parameters: + - Position: 0 + BoundsSafety: + Kind: sized_by_or_null + BoundedBy: size + - Name: asdf_ended + Parameters: + - Position: 0 + BoundsSafety: + Kind: ended_by + BoundedBy: end + - Name: asdf_counted_indirect + Parameters: + - Position: 0 + BoundsSafety: + Kind: counted_by + BoundedBy: len + Level: 1 diff --git a/clang/test/APINotes/Inputs/Headers/BoundsUnsafe.h b/clang/test/APINotes/Inputs/Headers/BoundsUnsafe.h new file mode 100644 index 0000000000000..2122b5c9757b2 --- /dev/null +++ b/clang/test/APINotes/Inputs/Headers/BoundsUnsafe.h @@ -0,0 +1,6 @@ +void asdf_counted(int *buf, int len); +void asdf_sized(void *buf, int size); +void asdf_counted_n(int *buf, int len); +void asdf_sized_n(void *buf, int size); +void asdf_ended(int *buf, int *end); +void asdf_counted_indirect(int **buf, int len); diff --git a/clang/test/APINotes/Inputs/Headers/module.modulemap b/clang/test/APINotes/Inputs/Headers/module.modulemap index bedb7d505f794..a83844c117fcf 100644 --- a/clang/test/APINotes/Inputs/Headers/module.modulemap +++ b/clang/test/APINotes/Inputs/Headers/module.modulemap @@ -1,3 +1,7 @@ +module BoundsUnsafe { + header "BoundsUnsafe.h" +} + module ExternCtx { header "ExternCtx.h" } diff --git a/clang/test/APINotes/bounds-safety.c b/clang/test/APINotes/bounds-safety.c new file mode 100644 index 0000000000000..d9c6032aeb568 --- /dev/null +++ b/clang/test/APINotes/bounds-safety.c @@ -0,0 +1,28 @@ +// RUN: rm -rf %t && mkdir -p %t +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/ModulesCache -fapinotes-modules -fsyntax-only -I %S/Inputs/Headers %s -ast-dump -ast-dump-filter asdf | FileCheck %s + +// FIXME: Bounds safety annotations are parsed and stored in the APINotes +// format, but are not yet applied as attributes in the AST upstream since +// __counted_by and related attributes are not yet supported in function +// signatures. Once that support is added, update this test to verify the +// annotations appear on the relevant declarations. + +#include "BoundsUnsafe.h" + +// CHECK: imported in BoundsUnsafe asdf_counted 'void (int *, int)' +// CHECK: imported in BoundsUnsafe buf 'int *' + +// CHECK: imported in BoundsUnsafe asdf_sized 'void (void *, int)' +// CHECK: imported in BoundsUnsafe buf 'void *' + +// CHECK: imported in BoundsUnsafe asdf_counted_n 'void (int *, int)' +// CHECK: imported in BoundsUnsafe buf 'int *' + +// CHECK: imported in BoundsUnsafe asdf_sized_n 'void (void *, int)' +// CHECK: imported in BoundsUnsafe buf 'void *' + +// CHECK: imported in BoundsUnsafe asdf_ended 'void (int *, int *)' +// CHECK: imported in BoundsUnsafe buf 'int *' + +// CHECK: imported in BoundsUnsafe asdf_counted_indirect 'void (int **, int)' +// CHECK: imported in BoundsUnsafe buf 'int **' _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
