Merge pull request #9822 from hnrklssn/bounds-safety-apinotes

This adds support for annotating function parameters with __counted_by, __sized_by, __counted_by_or_null, __sized_by_or_null, and __ended_by, using API notes. The main content of handlePtrCountedByEndedByAttr is extracted to applyPtrCountedByEndedByAttr and decoupled from ParsedAttr. The helper function ParseBoundsAttributeArgFromString is added to make it possible to parse count expressions from SemaAPINotes.

The current implementation of __terminated_by/__null_terminated makes it harder to extract from the iterative type processing, but since it doesn't require any extra context to parse the attribute, it can be applied using the normal Type override instead.

rdar://139830881

Author: hnrklssn

clang/include/clang/APINotes/Types.h

@@ -300,6 +300,72 @@ inline bool operator!=(const ContextInfo &LHS, const ContextInfo &RHS) {
   return !(LHS == RHS);
 }
 
+/* TO_UPSTREAM(BoundsSafety) ON */
+class BoundsSafetyInfo {
+public:
+  enum class BoundsSafetyKind {
+    CountedBy = 0,
+    CountedByOrNull,
+    SizedBy,
+    SizedByOrNull,
+    EndedBy,
+  };
+
+private:
+  /// Whether this property has been audited for nullability.
+  LLVM_PREFERRED_TYPE(bool)
+  unsigned KindAudited : 1;
+
+  /// The kind of nullability for this property. Only valid if the nullability
+  /// has been audited.
+  LLVM_PREFERRED_TYPE(BoundsSafetyKind)
+  unsigned Kind : 3;
+
+  LLVM_PREFERRED_TYPE(bool)
+  unsigned LevelAudited : 1;
+
+  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;
+}
+/* TO_UPSTREAM(BoundsSafety) OFF */
+
 /// API notes for a variable/property.
 class VariableInfo : public CommonEntityInfo {
   /// Whether this property has been audited for nullability.
@@ -439,10 +505,14 @@ class ParamInfo : public VariableInfo {
   unsigned RawRetainCountConvention : 3;
 
 public:
+  /* TO_UPSTREAM(BoundsSafety) ON */
+  std::optional<BoundsSafetyInfo> BoundsSafety;
+  /* TO_UPSTREAM(BoundsSafety) OFF */
+
   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;
@@ -488,6 +558,11 @@ class ParamInfo : public VariableInfo {
     if (!RawRetainCountConvention)
       RawRetainCountConvention = RHS.RawRetainCountConvention;
 
+    /* TO_UPSTREAM(BoundsSafety) ON */
+    if (!BoundsSafety)
+      BoundsSafety = RHS.BoundsSafety;
+    /* TO_UPSTREAM(BoundsSafety) OFF */
+
     return *this;
   }
 
@@ -502,7 +577,10 @@ 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 &&
+         /* TO_UPSTREAM(BoundsSafety) ON */
+         LHS.BoundsSafety == RHS.BoundsSafety;
+  /* TO_UPSTREAM(BoundsSafety) OFF */
 }
 
 inline bool operator!=(const ParamInfo &LHS, const ParamInfo &RHS) {

clang/include/clang/Parse/Parser.h

@@ -2021,7 +2021,7 @@ class Parser : public CodeCompletionHandler {
   /// Parse a __builtin_bit_cast(T, E), used to implement C++2a std::bit_cast.
   ExprResult ParseBuiltinBitCast();
 
-  /* TO_UPSTREAM(BoundsSafety) ON*/
+  /* TO_UPSTREAM(BoundsSafety) ON */
   //===--------------------------------------------------------------------===//
   /// BoundsSafety: __builtin_unsafe_forge_bidi_indexable(expr, size)
   ExprResult ParseUnsafeForgeBidiIndexable();
@@ -3924,6 +3924,25 @@ class Parser : public CodeCompletionHandler {
   /// \param IncludeLoc The location at which this parse was triggered.
   TypeResult ParseTypeFromString(StringRef TypeStr, StringRef Context,
                                  SourceLocation IncludeLoc);
+  /* TO_UPSTREAM(BoundsSafety) ON */
+  /// Parse the given string as an expression in the argument position for a
+  /// bounds safety attribute.
+  ///
+  /// This is a dangerous utility function currently employed only by API notes.
+  /// It is not a general entry-point for safely parsing expressions from
+  /// strings.
+  ///
+  /// \param ExprStr The string to be parsed as an expression.
+  /// \param Context The name of the context in which this string is being
+  /// parsed, which will be used in diagnostics.
+  /// \param ParentDecl If a function or method is provided, the parameters are
+  ///   added to the current parsing context.
+  /// \param IncludeLoc The location at which this parse was triggered.
+  ExprResult ParseBoundsAttributeArgFromString(StringRef ExprStr,
+                                               StringRef Context,
+                                               Decl *ParentDecl,
+                                               SourceLocation IncludeLoc);
+  /* TO_UPSTREAM(BoundsSafety) OFF */
 
   //===--------------------------------------------------------------------===//
   // Modules

clang/include/clang/Sema/Sema.h

@@ -1131,6 +1131,12 @@ class Sema final : public SemaBase {
   std::function<TypeResult(StringRef, StringRef, SourceLocation)>
       ParseTypeFromStringCallback;
 
+  /* TO_UPSTREAM(BoundsSafety) ON */
+  /// Callback to the parser to parse a type expressed as a string.
+  std::function<ExprResult(StringRef, StringRef, Decl *, SourceLocation)>
+      ParseBoundsAttributeArgFromStringCallback;
+  /* TO_UPSTREAM(BoundsSafety) OFF */
+
   /// VAListTagName - The declaration name corresponding to __va_list_tag.
   /// This is used as part of a hack to omit that class from ADL results.
   DeclarationName VAListTagName;
@@ -15498,7 +15504,13 @@ class Sema final : public SemaBase {
       llvm::SmallVectorImpl<TypeCoupledDeclRefInfo> &Decls, bool CountInBytes,
       bool OrNull);
 
-  /* TO_UPSTREAM(BoundsSafety) ON*/
+  /* TO_UPSTREAM(BoundsSafety) ON */
+  void applyPtrCountedByEndedByAttr(Decl *D, unsigned Level,
+                                    AttributeCommonInfo::Kind Kind,
+                                    Expr *AttrArg, SourceLocation Loc,
+                                    SourceRange Range, StringRef DiagName,
+                                    bool OriginatesInAPINotes = false);
+
   /// Perform Bounds Safety Semantic checks for assigning to a `__counted_by` or
   /// `__counted_by_or_null` pointer type \param LHSTy.
   ///

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 = 34; // SwiftReturnOwnership
+const uint16_t VERSION_MINOR = 35; // BoundsSafety
 
 const uint8_t kSwiftConforms = 1;
 const uint8_t kSwiftDoesNotConform = 2;

clang/lib/APINotes/APINotesReader.cpp

@@ -322,6 +322,31 @@ class FieldTableInfo
   }
 };
 
+/* TO_UPSTREAM(BoundsSafety) ON */
+/// 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 >= (uint8_t)BoundsSafetyInfo::BoundsSafetyKind::CountedBy);
+    assert(Kind <= (uint8_t)BoundsSafetyInfo::BoundsSafetyKind::EndedBy);
+    Info.setKindAudited((BoundsSafetyInfo::BoundsSafetyKind)Kind);
+  }
+
+  uint16_t ExternalBoundsLen =
+      endian::readNext<uint16_t, llvm::endianness::little>(Data);
+  Info.ExternalBounds = std::string(Data, Data + ExternalBoundsLen);
+  Data += ExternalBoundsLen;
+}
+/* TO_UPSTREAM(BoundsSafety) OFF */
+
 /// Read serialized ParamInfo.
 void ReadParamInfo(const uint8_t *&Data, ParamInfo &Info) {
   ReadVariableInfo(Data, Info);
@@ -338,7 +363,13 @@ void ReadParamInfo(const uint8_t *&Data, ParamInfo &Info) {
   if (Payload & 0x01)
     Info.setNoEscape(Payload & 0x02);
   Payload >>= 2;
-  assert(Payload == 0 && "Bad API notes");
+  /* TO_UPSTREAM(BoundsSafety) ON */
+  if (Payload & 0x01) {
+    BoundsSafetyInfo BSI;
+    ReadBoundsSafetyInfo(Data, BSI);
+    Info.BoundsSafety = BSI;
+  }
+  /* TO_UPSTREAM(BoundsSafety) OFF */
 }
 
 /// Read serialized FunctionInfo.

clang/lib/APINotes/APINotesTypes.cpp

@@ -61,6 +61,34 @@ 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((BoundsSafetyKind)Kind >= BoundsSafetyKind::CountedBy);
+    assert((BoundsSafetyKind)Kind <= BoundsSafetyKind::EndedBy);
+    switch ((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)
@@ -69,6 +97,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 {

clang/lib/APINotes/APINotesWriter.cpp

@@ -1074,15 +1074,48 @@ void APINotesWriter::Implementation::writeGlobalVariableBlock(
   }
 }
 
+/* TO_UPSTREAM(BoundsSafety) ON */
 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()) {
+    flags |= 0x01;                    // 1 bit
+    flags |= (uint8_t)*kind << 1;     // 3 bits
+  }
+  flags <<= 4;
+  if (auto level = BSI.getLevel()) {
+    flags |= 0x01;        // 1 bit
+    flags |= *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();
+}
+/* TO_UPSTREAM(BoundsSafety) OFF */
+
 unsigned getParamInfoSize(const ParamInfo &PI) {
-  return getVariableInfoSize(PI) + 1;
+  unsigned BSISize = 0;
+  /* TO_UPSTREAM(BoundsSafety) ON */
+  if (auto BSI = PI.BoundsSafety)
+    BSISize = getBoundsSafetyInfoSize(*BSI);
+  /* TO_UPSTREAM(BoundsSafety) OFF */
+  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)
@@ -1100,6 +1133,10 @@ void emitParamInfo(raw_ostream &OS, const ParamInfo &PI) {
 
   llvm::support::endian::Writer writer(OS, llvm::endianness::little);
   writer.write<uint8_t>(flags);
+  /* TO_UPSTREAM(BoundsSafety) ON */
+  if (auto BSI = PI.BoundsSafety)
+    emitBoundsSafetyInfo(OS, *PI.BoundsSafety);
+  /* TO_UPSTREAM(BoundsSafety) OFF */
 }
 
 /// Retrieve the serialized size of the given FunctionInfo, for use in on-disk