@llvm/pr-subscribers-lldb
@llvm/pr-subscribers-debuginfo
@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-clang-modules
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Justin Stitt (JustinStitt)

diff --git a/clang/docs/OverflowBehaviorTypes.rst b/clang/docs/OverflowBehaviorTypes.rst
new file mode 100644
index 0000000000000..18e337e181e8a
--- /dev/null
+++ b/clang/docs/OverflowBehaviorTypes.rst
@@ -0,0 +1,269 @@
+=====================
+OverflowBehaviorTypes
+=====================
+
+.. contents::
+   :local:
+
+Introduction
+============
+
+Clang provides a type attribute that allows developers to have fine-grained control
+over the overflow behavior of integer types. The ``overflow_behavior``
+attribute can be used to specify how arithmetic operations on a given integer
+type should behave upon overflow. This is particularly useful for projects that
+need to balance performance and safety, allowing developers to enable or
+disable overflow checks for specific types.
+
+The attribute can be enabled using the compiler option
+``-foverflow-behavior-types``.
+
+The attribute syntax is as follows:
+
+.. code-block:: c++
+
+  __attribute__((overflow_behavior(behavior)))
+
+Where ``behavior`` can be one of the following:
+
+* ``wrap``: Specifies that arithmetic operations on the integer type should
+  wrap on overflow. This is equivalent to the behavior of ``-fwrapv``, but it
+  applies only to the attributed type and may be used with both signed and
+  unsigned types. When this is enabled, UBSan's integer overflow and integer
+  truncation checks (``signed-integer-overflow``,
+  ``unsigned-integer-overflow``, ``implicit-signed-integer-truncation``, and
+  ``implicit-unsigned-integer-truncation``) are suppressed for the attributed
+  type.
+
+* ``no_wrap``: Specifies that arithmetic operations on the integer type should
+  be checked for overflow. When using the ``signed-integer-overflow`` sanitizer
+  or when using ``-ftrapv`` alongside a signed type, this is the default
+  behavior. Using this, one may enforce overflow checks for a type even when
+  ``-fwrapv`` is enabled globally.
+
+This attribute can be applied to ``typedef`` declarations and to integer types directly.
+
+Examples
+========
+
+Here is an example of how to use the ``overflow_behavior`` attribute with a ``typedef``:
+
+.. code-block:: c++
+
+  typedef unsigned int __attribute__((overflow_behavior(no_wrap))) non_wrapping_uint;
+
+  non_wrapping_uint add_one(non_wrapping_uint a) {
+    return a + 1; // Overflow is checked for this operation.
+  }
+
+Here is an example of how to use the ``overflow_behavior`` attribute with a type directly:
+
+.. code-block:: c++
+
+  int mul_alot(int n) {
+    int __attribute__((overflow_behavior(wrap))) a = n;
+    return a * 1337; // Potential overflow is not checked and is well-defined
+  }
+
+"Well-defined" overflow is consistent with two's complement wrap-around
+semantics and won't be removed via eager compiler optimizations (like some
+undefined behavior might).
+
+Overflow behavior types are implicitly convertible to and from built-in
+integral types.
+
+Note that C++ overload set formation rules treat promotions to and from
+overflow behavior types the same as normal integral promotions and conversions.
+
+Interaction with Command-Line Flags and Sanitizer Special Case Lists
+====================================================================
+
+The ``overflow_behavior`` attribute interacts with sanitizers, ``-ftrapv``,
+``-fwrapv``, and Sanitizer Special Case Lists (SSCL) by wholly overriding these
+global flags. The following table summarizes the interactions:
+
+.. list-table:: Overflow Behavior Precedence
+   :widths: 15 15 15 15 20 15
+   :header-rows: 1
+
+   * - Behavior
+     - Default(No Flags)
+     - -ftrapv
+     - -fwrapv
+     - Sanitizers
+     - SSCL
+   * - ``overflow_behavior(wrap)``
+     - Wraps
+     - No trap
+     - Wraps
+     - No report
+     - Overrides SSCL
+   * - ``overflow_behavior(no_wrap)``
+     - Traps
+     - Traps
+     - Traps
+     - Reports
+     - Overrides SSCL
+
+It is important to note the distinction between signed and unsigned types. For
+unsigned integers, which wrap on overflow by default, ``overflow_behavior(no_wrap)``
+is particularly useful for enabling overflow checks. For signed integers, whose
+overflow behavior is undefined by default, ``overflow_behavior(wrap)`` provides
+a guaranteed wrapping behavior.
+
+The ``overflow_behavior`` attribute can be used to override the behavior of
+entries from a :doc:`SanitizerSpecialCaseList`. This is useful for allowlisting
+specific types into overflow instrumentation.
+
+Promotion Rules
+===============
+
+The promotion rules for overflow behavior types are designed to preserve the
+specified overflow behavior throughout an arithmetic expression. They differ
+from standard C/C++ integer promotions but in a predictable way, similar to
+how ``_Complex`` and ``_BitInt`` have their own promotion rules.
+
+* **OBT and Standard Integer Type**: In an operation involving an overflow
+  behavior type (OBT) and a standard integer type, the result will have the
+  type of the OBT, including its overflow behavior, sign, and bit-width. The
+  standard integer type is implicitly converted to match the OBT.
+
+  .. code-block:: c++
+
+    typedef char __attribute__((overflow_behavior(no_wrap))) no_wrap_char;
+    // The result of this expression is no_wrap_char.
+    no_wrap_char c;
+    unsigned long ul;
+    auto result = c + ul;
+
+* **Two OBTs of the Same Kind**: When an operation involves two OBTs of the
+  same kind (e.g., both ``wrap``), the result will have the larger of the two
+  bit-widths. If the bit-widths are the same, an unsigned type is favored over
+  a signed one.
+
+  .. code-block:: c++
+
+    typedef unsigned char __attribute__((overflow_behavior(wrap))) u8_wrap;
+    typedef unsigned short __attribute__((overflow_behavior(wrap))) u16_wrap;
+    // The result of this expression is u16_wrap.
+    u8_wrap a;
+    u16_wrap b;
+    auto result = a + b;
+
+* **Two OBTs of Different Kinds**: In an operation between a ``wrap`` and a
+  ``no_wrap`` type, a ``no_wrap`` is produced. It is recommended to avoid such
+  operations, as Clang may emit a warning for such cases in the future.
+  Regardless, the resulting type matches the bit-width, sign and behavior of
+  the ``no_wrap`` type.
+
+Diagnostics
+===========
+
+Clang provides diagnostics to help developers manage overflow behavior types.
+
+-Wimplicitly-discarded-overflow-behavior
+----------------------------------------
+
+This warning is issued when an overflow behavior type is implicitly converted
+to a standard integer type, which may lead to the loss of the specified
+overflow behavior.
+
+.. code-block:: c++
+
+  typedef int __attribute__((overflow_behavior(wrap))) wrapping_int;
+
+  void some_function(int);
+
+  void another_function(wrapping_int w) {
+    some_function(w); // warning: implicit conversion from 'wrapping_int' to
+                      // 'int' discards overflow behavior
+  }
+
+To fix this, you can explicitly cast the overflow behavior type to a standard
+integer type.
+
+.. code-block:: c++
+
+  typedef int __attribute__((overflow_behavior(wrap))) wrapping_int;
+
+  void some_function(int);
+
+  void another_function(wrapping_int w) {
+    some_function(static_cast<int>(w)); // OK
+  }
+
+This warning acts as a group that includes
+``-Wimplicitly-discarded-overflow-behavior-pedantic`` and
+``-Wimplicitly-discarded-overflow-behavior-assignment``.
+
+-Wimplicitly-discarded-overflow-behavior-pedantic
+-------------------------------------------------
+
+A less severe version of the warning, ``-Wimplicitly-discarded-overflow-behavior-pedantic``,
+is issued for implicit conversions from an unsigned wrapping type to a standard
+unsigned integer type. This is considered less problematic because both types
+have well-defined wrapping behavior, but the conversion still discards the
+explicit ``overflow_behavior`` attribute.
+
+.. code-block:: c++
+
+  typedef unsigned int __attribute__((overflow_behavior(wrap))) wrapping_uint;
+
+  void some_function(unsigned int);
+
+  void another_function(wrapping_uint w) {
+    some_function(w); // warning: implicit conversion from 'wrapping_uint' to
+                      // 'unsigned int' discards overflow behavior
+                      // [-Wimplicitly-discarded-overflow-behavior-pedantic]
+  }
+
+-Wimplicitly-discarded-overflow-behavior-assignment
+---------------------------------------------------
+
+This warning is issued when an overflow behavior type is implicitly converted
+to a standard integer type as part of an assignment, which may lead to the
+loss of the specified overflow behavior. This is a more specific version of
+the ``-Wimplicitly-discarded-overflow-behavior`` warning, and it is off by
+default.
+
+.. code-block:: c++
+
+  typedef int __attribute__((overflow_behavior(wrap))) wrapping_int;
+
+  void some_function() {
+    wrapping_int w = 1;
+    int i = w; // warning: implicit conversion from 'wrapping_int' to 'int'
+               // discards overflow behavior
+               // [-Wimplicitly-discarded-overflow-behavior-assignment]
+  }
+
+To fix this, you can explicitly cast the overflow behavior type to a standard
+integer type.
+
+.. code-block:: c++
+
+  typedef int __attribute__((overflow_behavior(wrap))) wrapping_int;
+
+  void some_function() {
+    wrapping_int w = 1;
+    int i = static_cast<int>(w); // OK
+    int j = (int)w; // C-style OK
+  }
+
+
+-Woverflow-behavior-attribute-ignored
+-------------------------------------
+
+This warning is issued when the ``overflow_behavior`` attribute is applied to
+a type that is not an integer type.
+
+.. code-block:: c++
+
+  typedef float __attribute__((overflow_behavior(wrap))) wrapping_float;
+  // warning: 'overflow_behavior' attribute only applies to integer types;
+  // attribute is ignored [-Woverflow-behavior-attribute-ignored]
+
+  typedef struct S { int i; } __attribute__((overflow_behavior(wrap))) S_t;
+  // warning: 'overflow_behavior' attribute only applies to integer types;
+  // attribute is ignored [-Woverflow-behavior-attribute-ignored]
+
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 970825c98fec1..0c9139cb252d2 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -378,6 +378,8 @@ New Compiler Flags
 
 - New options ``-g[no-]key-instructions`` added, disabled by default. Reduces jumpiness of debug stepping for optimized code in some debuggers (not LLDB at this time). Not recommended for use without optimizations. DWARF only. Note both the positive and negative flags imply ``-g``.
 
+- New option ``-foverflow-behavior-types`` added to enable parsing of the ``overflow_behavior`` type attribute.
+
 Deprecated Compiler Flags
 -------------------------
 
@@ -478,6 +480,10 @@ related warnings within the method body.
 - Clang will print the "reason" string argument passed on to
   ``[[clang::warn_unused_result("reason")]]`` as part of the warning diagnostic.
 
+- Introduced a new type attribute ``__attribute__((overflow_behavior))`` which
+  currently accepts either ``wrap`` or ``no_wrap`` as an argument, enabling
+  type-level control over overflow behavior.
+
 Improvements to Clang's diagnostics
 -----------------------------------
 
diff --git a/clang/docs/SanitizerSpecialCaseList.rst b/clang/docs/SanitizerSpecialCaseList.rst
index 2c50778d0f491..f4c9274b89b68 100644
--- a/clang/docs/SanitizerSpecialCaseList.rst
+++ b/clang/docs/SanitizerSpecialCaseList.rst
@@ -133,6 +133,40 @@ precedence. Here are a few examples.
   fun:*bar
   fun:bad_bar=sanitize
 
+Interaction with Overflow Behavior Types
+----------------------------------------
+
+The ``overflow_behavior`` attribute provides a more granular, source-level
+control that takes precedence over the Sanitizer Special Case List. If a type
+is given an ``overflow_behavior`` attribute, it will override any matching
+``type:`` entry in a special case list.
+
+This allows developers to enforce a specific overflow behavior for a critical
+type, even if a broader rule in the special case list would otherwise disable
+instrumentation for it.
+
+.. code-block:: bash
+
+  $ cat ignorelist.txt
+  # Disable signed overflow checks for all types by default.
+  [signed-integer-overflow]
+  type:*
+
+  $ cat foo.c
+  // Force 'critical_type' to always have overflow checks,
+  // overriding the ignorelist.
+  typedef int __attribute__((overflow_behavior(no_wrap))) critical_type;
+
+  void foo(int x) {
+    critical_type a = x;
+    a++; // Overflow is checked here due to the 'no_wrap' attribute.
+
+    int b = x;
+    b++; // Overflow is NOT checked here due to the ignorelist.
+  }
+
+For more details on overflow behavior types, see :doc:`OverflowBehaviorTypes`.
+
 Format
 ======
 
diff --git a/clang/docs/UndefinedBehaviorSanitizer.rst b/clang/docs/UndefinedBehaviorSanitizer.rst
index 0a2d833783e57..f98eda1e9399c 100644
--- a/clang/docs/UndefinedBehaviorSanitizer.rst
+++ b/clang/docs/UndefinedBehaviorSanitizer.rst
@@ -380,6 +380,28 @@ This attribute may not be
 supported by other compilers, so consider using it together with
 ``#if defined(__clang__)``.
 
+Disabling Overflow Instrumentation with ``__attribute__((overflow_behavior(wrap)))``
+------------------------------------------------------------------------------------
+
+For more fine-grained control over how integer overflow is handled, you can use
+the ``__attribute__((overflow_behavior(wrap)))`` attribute. This attribute can
+be applied to ``typedef`` declarations and integer types to specify that
+arithmetic operations on that type should wrap on overflow. This can be used to
+disable overflow sanitization for specific types, while leaving it enabled for
+all other types.
+
+For more information, see :doc:`OverflowBehaviorTypes`.
+
+Enforcing Overflow Instrumentation with ``__attribute__((overflow_behavior(no_wrap)))``
+---------------------------------------------------------------------------------------
+
+Conversely, you can use ``__attribute__((overflow_behavior(no_wrap)))`` to
+enforce overflow checks for a specific type, even when ``-fwrapv`` is enabled
+globally. This is useful for ensuring that critical calculations are always
+checked for overflow, regardless of the global compiler settings.
+
+For more information, see :doc:`OverflowBehaviorTypes`.
+
 Suppressing Errors in Recompiled Code (Ignorelist)
 --------------------------------------------------
 
diff --git a/clang/docs/index.rst b/clang/docs/index.rst
index 6c792af66a62c..3205709aa395a 100644
--- a/clang/docs/index.rst
+++ b/clang/docs/index.rst
@@ -40,6 +40,7 @@ Using Clang as a Compiler
    SanitizerCoverage
    SanitizerStats
    SanitizerSpecialCaseList
+   OverflowBehaviorTypes
    BoundsSafety
    BoundsSafetyAdoptionGuide
    BoundsSafetyImplPlans
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 2b9cd035623cc..dbfc8df6fba5e 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -14,6 +14,7 @@
 #ifndef LLVM_CLANG_AST_ASTCONTEXT_H
 #define LLVM_CLANG_AST_ASTCONTEXT_H
 
+#include "Type.h"
 #include "clang/AST/ASTFwd.h"
 #include "clang/AST/CanonicalType.h"
 #include "clang/AST/CommentCommandTraits.h"
@@ -259,6 +260,7 @@ class ASTContext : public RefCountedBase<ASTContext> {
   mutable llvm::ContextualFoldingSet<DependentBitIntType, ASTContext &>
       DependentBitIntTypes;
   mutable llvm::FoldingSet<BTFTagAttributedType> BTFTagAttributedTypes;
+  mutable llvm::FoldingSet<OverflowBehaviorType> OverflowBehaviorTypes;
   llvm::FoldingSet<HLSLAttributedResourceType> HLSLAttributedResourceTypes;
   llvm::FoldingSet<HLSLInlineSpirvType> HLSLInlineSpirvTypes;
 
@@ -899,6 +901,8 @@ class ASTContext : public RefCountedBase<ASTContext> {
   bool isTypeIgnoredBySanitizer(const SanitizerMask &Mask,
                                 const QualType &Ty) const;
 
+  bool isUnaryOverflowPatternExcluded(const UnaryOperator *UO);
+
   const XRayFunctionFilter &getXRayFilter() const {
     return *XRayFilter;
   }
@@ -999,6 +1003,15 @@ class ASTContext : public RefCountedBase<ASTContext> {
   comments::FullComment *getCommentForDecl(const Decl *D,
                                            const Preprocessor *PP) const;
 
+  /// Attempts to merge two types that may be OverflowBehaviorTypes.
+  ///
+  /// \returns A QualType if the types were handled, std::nullopt otherwise.
+  /// A null QualType indicates an incompatible merge.
+  std::optional<QualType>
+  tryMergeOverflowBehaviorTypes(QualType LHS, QualType RHS, bool OfBlockPointer,
+                                bool Unqualified, bool BlockReturnType,
+                                bool IsConditionalOperator);
+
   /// Return parsed documentation comment attached to a given declaration.
   /// Returns nullptr if no comment is attached. Does not look at any
   /// redeclarations of the declaration.
@@ -1843,6 +1856,13 @@ class ASTContext : public RefCountedBase<ASTContext> {
   QualType getBTFTagAttributedType(const BTFTypeTagAttr *BTFAttr,
                                    QualType Wrapped) const;
 
+  QualType getOverflowBehaviorType(const OverflowBehaviorAttr *Attr,
+                                   QualType Wrapped) const;
+
+  QualType
+  getOverflowBehaviorType(OverflowBehaviorType::OverflowBehaviorKind Kind,
+                          QualType Wrapped) const;
+
   QualType getHLSLAttributedResourceType(
       QualType Wrapped, QualType Contained,
       const HLSLAttributedResourceType::Attributes &Attrs);
diff --git a/clang/include/clang/AST/ASTNodeTraverser.h b/clang/include/clang/AST/ASTNodeTraverser.h
index 8ebabb2bde10d..e684dcd5c27e8 100644
--- a/clang/include/clang/AST/ASTNodeTraverser.h
+++ b/clang/include/clang/AST/ASTNodeTraverser.h
@@ -445,6 +445,9 @@ class ASTNodeTraverser
   void VisitBTFTagAttributedType(const BTFTagAttributedType *T) {
     Visit(T->getWrappedType());
   }
+  void VisitOverflowBehaviorType(const OverflowBehaviorType *T) {
+    Visit(T->getUnderlyingType());
+  }
   void VisitHLSLAttributedResourceType(const HLSLAttributedResourceType *T) {
     QualType Contained = T->getContainedType();
     if (!Contained.isNull())
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 523c0326d47ef..4fbea7272d004 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -1477,6 +1477,14 @@ class DeclRefExpr final
     return DeclRefExprBits.IsImmediateEscalating;
   }
 
+  bool isOverflowBehaviorDiscarded() const {
+    return DeclRefExprBits.IsOverflwBehaviorDiscarded;
+  }
+
+  void setOverflowBehaviorDiscarded(bool Set) {
+    DeclRefExprBits.IsOverflwBehaviorDiscarded = Set;
+  }
+
   void setIsImmediateEscalating(bool Set) {
     DeclRefExprBits.IsImmediateEscalating = Set;
   }
diff --git a/clang/include/clang/AST/PropertiesBase.td b/clang/include/clang/AST/PropertiesBase.td
index 1215056ffde1b..26dd53760c3a2 100644
--- a/clang/include/clang/AST/PropertiesBase.td
+++ b/clang/include/clang/AST/PropertiesBase.td
@@ -81,6 +81,8 @@ def AutoTypeKeyword : EnumPropertyType;
 def Bool : PropertyType<"bool">;
 def BuiltinTypeKind : EnumPropertyType<"BuiltinType::Kind">;
 def BTFTypeTagAttr : PropertyType<"const BTFTypeTagAttr *">;
+def OverflowBehaviorKind
+    : EnumPropertyType<"OverflowBehaviorType::OverflowBehaviorKind">;
 def CallingConv : EnumPropertyType;
 def DeclarationName : PropertyType;
 def DeclarationNameKind : EnumPropertyType<"DeclarationName::NameKind">;
diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h
index 5cb2f57edffe4..4a681a58b1cc9 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -1151,6 +1151,9 @@ DEF_TRAVERSE_TYPE(CountAttributedType, {
 DEF_TRAVERSE_TYPE(BTFTagAttributedType,
                   { TRY_TO(TraverseType(T->getWrappedType())); })
 
+DEF_TRAVERSE_TYPE(OverflowBehaviorType,
+                  { TRY_TO(TraverseType(T->getUnderlyingType())); })
+
 DEF_TRAVERSE_TYPE(HLSLAttributedResourceType,
                   { TRY_TO(TraverseType(T->getWrappedType())); })
 
@@ -1462,6 +1465,9 @@ DEF_TRAVERSE_TYPELOC(CountAttributedType,
 DEF_TRAVERSE_TYPELOC(BTFTagAttributedType,
                      { TRY_TO(TraverseTypeLoc(TL.getWrappedLoc())); })
 
+DEF_TRAVERSE_TYPELOC(OverflowBehaviorType,
+                     { TRY_TO(TraverseTypeLoc(TL.getWrappedLoc())); })
+
 DEF_TRAVERSE_TYPELOC(HLSLAttributedResourceType,
   ...
[truncated]

gentle ping.

ping.

I rebased due to some small conflicts.

question to reviewers: Should I squash these ~10 commits into one for ease of review?

thanks @mizvekov for the initial review. I've tried to address all your concerns with commit 04af8bf. Let me know if anything there doesn't quite hit the mark - I'm happy to iterate.

As for your comment:

I am more concerned that these functions don't seem like a good fit for being part of ASTContext, maybe we can find a better home for them.

Do you think maybe that should be another PR that I can send later on? Should it come before this PR? Where do you think a better home is for that overflow pattern elision stuff?

And for your inner qualifiers question:

How about inner qualifiers? The patch doesn't address the situation.

How should we address these? FWIW, currently, the two types in your example do not compare equal:

  std::cout << std::boolalpha
            << std::is_same<WrapConstInt1, WrapConstInt2>::value << "\n"; // false

I am not sure what you mean by "inner qualifiers" from your example. My understanding of inner qualifiers involves pointer types like const int *p vs int * const p.

Do you think maybe that should be another PR that I can send later on? Should it come before this PR? Where do you think a better home is for that overflow pattern elision stuff?

I think this is a simple enough change, it's just about moving where the function is implemented.
We can do it in this PR. This is not a blocking concern from me, it just seems to me like this function depends on just
too little from ASTContext in order to be part of it, and it doesn't quite fit the overall theme.

I think Type.h would be a better fit, but I'd wait a little bit for other opinions before changing anything.

How should we address these? FWIW, currently, the two types in your example do not compare equal:

  std::cout << std::boolalpha
            << std::is_same<WrapConstInt1, WrapConstInt2>::value << "\n"; // false

I am not sure what you mean by "inner qualifiers" from your example. My understanding of inner qualifiers involves pointer types like const int *p vs int * const p.

So there are two positions qualifiers can appear in this type.

1. Qualifiers over the OverflowBheaviourType itself. Lets call those the outer qualifiers.
2. Qualifiers over the underlying type of OverflowBheaviourType. Lets call those the inner qualifiers.

It doesn't seem plausible to me that these qualifiers have different meanings, that is to say, that the two types in my example should be different types, instead of two spellings of the same type.

One solution here would be to adopt the same rules as ArrayTypes for this situation.
An array of const int is the same as a const array of ints, which is also the same as a const array of const ints.

You can implement this by changing your canonicalization for OverflowBheaviourType, making it remove the qualifiers from the underlying type, and add those as qualifiers over the OverflowBeaviourType.

It doesn't seem plausible to me that these qualifiers have different meanings, that is to say, that the two types in my example should be different types, instead of two spellings of the same type.

I'm getting conflicting ideas from this sentence. Do you mean to say that the two types from your example should be the same type?

In looking at what's here, I suddenly realize that I think we accidentally deviated a bit on naming, in that the trap behavior got named "no_wrap" which is ambiguous. For example, a future overflow resolution behavior can be added like "saturate", which isn't wrapping either, and suddenly "no_wrap" isn't clear what it's doing. The "no_wrap" runtime overflow resolution is to trap (which is correct here), so I think it should be explicitly named "trap" instead of "no_wrap". (If there was a reason I've forgotten for not naming it "trap", though, please ignore me there -- I don't want to stall this with bikeshedding.)

Additionally, just for some namespace sanity (especially from the perspective of Linux's use of OBTs), can we have the type specifier named "_ob$behavior": __ob_wrap, __ob_trap ? That would make things a bit cleaner for Linux's resulting macros for OBT usage to de-conflict possible namespace collisions there...

Additionally, just for some namespace sanity (especially from the perspective of Linux's use of OBTs), can we have the type specifier named "_ob$behavior": __ob_wrap, __ob_trap ? That would make things a bit cleaner for Linux's resulting macros for OBT usage to de-conflict possible namespace collisions there...

The type specifier is optional, you can still use the attribute syntax here, so if you are hiding this behind macros anyway, might as well just use that spelling instead? Unless I have misunderstood.

Additionally, just for some namespace sanity (especially from the perspective of Linux's use of OBTs), can we have the type specifier named "_ob$behavior": __ob_wrap, __ob_trap ? That would make things a bit cleaner for Linux's resulting macros for OBT usage to de-conflict possible namespace collisions there...

The type specifier is optional, you can still use the attribute syntax here, so if you are hiding this behind macros anyway, might as well just use that spelling instead? Unless I have misunderstood.

I guess I'm thinking about 2 aspects: 1) "__wrap" is a very short name, and it feels like it is begging to collide or at least be confusing. 2) Linux has a common way to detect and enable/disable compiler features via macros that may expand to a given attribute (e.g. even if a compiler feature is available, Linux may want to disable it at the source level). I'd like to avoid confusion there.

I guess I'm thinking about 2 aspects: 1) "__wrap" is a very short name, and it feels like it is begging to collide or at least be confusing. 2) Linux has a common way to detect and enable/disable compiler features via macros that may expand to a given attribute (e.g. even if a compiler feature is available, Linux may want to disable it at the source level). I'd like to avoid confusion there.

I see. Well on one hand, we already have a bunch of implementation specific keywords that are very small like that. The latest one which is __ptrauth.

The thinking here is that the language spec formally reserves these identifiers for the implementation, so in theory this shouldn't be a problem, and compiler implementors will often take this liberty without a second thought.

@ojhunt @AaronBallman and @erichkeane have more or less recently added new such keywords, so I ask them, what's our criteria here, what sort of precedent we have?

Certainly we don't seem to be consistent whether we pick a _Capital or a __snake style spelling, would that make any difference for the linux kernel, if they were spelled _Wrap and _Trap instead?

Formally, I don't think there's any concern; all reserved identifiers are equally reserved. In practice, libc implementations and other libraries tend to stomp over the reserved namespace for common words, so it's probably worth doing a GitHub search or something.

I think naming collisions are a secondary issue - mentioned in Kees' comments. I think the main issue is that "no_wrap" ought to be called "trap" everywhere.

I am fine with the renaming... I already have a patch to do just that too. I am curious what others think.

I think naming collisions are a secondary issue - mentioned in Kees' comments. I think the main issue is that "no_wrap" ought to be called "trap" everywhere.

I am fine with the renaming... I already have a patch to do just that too. I am curious what others think.

Yeah renaming no_wrap to trap is good, thanks for catching that.

Yeah, my primary review note was generally no_wrap -> trap. The other aspect of the __wrap and __trap being short was more of a "this feels like we might get into weird places in some nebulous future". So yeah, if it's the right spelling, no objection from me.

@mizvekov Any objections to __ob_wrap and __ob_trap for the keyword spellings? I guess the precedence is one you already mentioned: __ptrauth which sort of prefixes/namespaces the keyword within itself.

I'm getting test failures because __trap is used in cuda code? or at the very least some math code, so to avoid name collisions and to appease our most immediate user (linux kernel) we should probably go with __ob_trap but I'll wait for final confirmation on this.

llvm-project/build-overflow-behavior-types-rel/lib/clang/22/include/__clan
g_cuda_device_functions.h:531:24: error: expected ')'
llvm-project/build-overflow-behavior-types-rel/lib/clang/22/include/__clan
g_cuda_device_functions.h:531:23: note: to match this '('
  531 | __DEVICE__ void __trap(void) { __asm__ __volatile__("trap;"); }
      |                       ^

@mizvekov Any objections to __ob_wrap and __ob_trap for the keyword spellings? I guess the precedence is one you already mentioned: __ptrauth which sort of prefixes/namespaces the keyword within itself.

Yeah, that looks fine to me.

I don't believe this warrants a keyword, and I don't believe the RFC supports the addition of such. As far as I can make out all the RFC discussion is in terms of an attribute.

I don't think the RFC is for bike shedding discussions about the spelling of the type, this can be decided between the reviewers.

This has been discussed here, please read existing comments and let's continue in the appropriate thread, so we don't have to restart and repeat ourselves.

See: #148914 (comment)

This comment became quite long, so I just want to add a preamble. First off, I'm sorry that I missed the RFC for this or I would have raised these objections then - If I did see it I would have blindly assumed that it was solely removing the UB nature of overflow, with no other semantic changes, and so probably not paid it any heed. This is a feature I have wanted for a long time, but the narrowing behavior is just not ok - I would have to essentially make this a banned feature in any code base I have influence over, for the reasons I'll be discussing below.

I’m also not sure I understand the semantics of narrowing when __nowrap is used, I’ve tried to - given

uint64_t a = ...
uint32_t __no_wrap b = ...
auto result = a + b

The documentation makes it clear that smaller_type __no_wrap a = larger_type is an error, but what happens in the case of (smaller_type __no_wrap) + larger_type - is that an error?

I could dump these issues into the RFC thread but I don’t think that’s necessarily appropriate - @AaronBallman or

Anyway, on to the wall of text comment (as people who are on WG21 can attest I have an unerring ability to do this, despite my best efforts):

The description of this PR states that this feature is fine grained control of fwrapv, etc that make wrapping have defined semantics rather than undefined. But the change in semantics here is not that, it is a significantly different behaviour, and has the effect of making previously correct code to incorrect code. I.e code that is correct, and has no UB (e.g. no overflow), is changed to be incorrect - introducing both overflow and truncation into code that previously had none.

A common refrain in the replies to my comments has been this is a new feature that is opt in, but the problem is there are many cases developers would want to opt in to these in ways that would be relatively global. It's super easy to imagine a project doing:

typedef uint8_t __nowrap uint8_safe_t;
typedef uint16_t __nowrap uint16_safe_t;
typedef uint32_t __nowrap uint32_safe_t;
typedef uint63_t __nowrap uint63_safe_t;
typedef uint8_t __wrap uint8_wrap_t;
typedef uint16_t __wrap uint16_wrap_t;
typedef uint32_t __wrap uint32_wrap_t;
typedef uint63_t __wrap uint63_wrap_t;

Then adding a linter rule that requires all new code to use one these instead of the existing types. In codebases I have working in, this is exactly what they would want to do. But the narrowing change makes such a change impossible - interaction between new and existing code, or system APIs, means the narrowing can happen anywhere, and similarly literals like 1ull + somevalue can now break as well.

These changes in semantics can introduce security vulnerabilities where previously none existed. Take the format string example in the RFC, which was called out as a result of this change in behavior, which is the reason I assumed it had be removed:

unsigned long long a;
size_t __wrap b;

printf("%llx", a + b);

on systems where sizeof(int) != sizeof(unsigned long long) this previously correct code is now a security vulnerability. In principle this could happen on a 64bit system if varargs aren’t required to be at least 8 byte aligned, though I’m not sure if such systems exist.

You also get silent truncation in previously correct code:

struct S32 {
  uint32_t __wrap a; // I want defined semantics for arithmetic on this field
                     // e.g. no UB -> compiler created security vulnerability
};
struct S64 {
  uint64_t __wrap a;
};

uint64_t doSomething();
uint64_t foo(S32 s) {
   return doSomething() + s.a; // silently truncate to uint32_t, and silently expand out to uint64_t;
}

You can imagine code this also impacting code like:

auto /*uint32_t*/ base = getBase();
auto /*uint<less than 32>_t __wrap*/ offset = ...; // so many formats and use cases have bytes here
if (offset >= size)
  return;
buffer[base + offset] = ...; // you're now writing out of expected bounds

I also suspect these semantics mean that (assuming the above uint64_t doSomething(); exists)

uint64_t foo(uint32_t __wrap a, uint64_t wrap b) {
   return doSomething() + a + b;
}

could now produce a different result (it's already different from what existing correct code does) from

uint64_t foo(uint32_t __wrap a, uint64_t wrap b) {
   return doSomething() + b + a;
}

Due to the narrowing of doSomething() + a where previously the developer had (presumably) ensured a correct result. Note that this is distinct from int32 + int32 + int64 vs int64 + int32 + int32 which is a real and existing problem, because we’re starting with correct code.

Similar to this, previously correct C++ becomes wrong, and changes substantially:

// just some nonsense to demonstrate the viral nature of this change in narrowing
template <class T> struct S {
   S(T a) : a(a) {}
   T a;
}

template <class A, class B> auto doSomething(S<A> s1, S<B> s2) -> S<decltype(s1.a + s2.b)> {
   return S(s1.a + s2.a);
}
 

doSomething now loses data if one of A or B is a smaller __wrap type (e.g. doSomething(S<uint64_t>(...), S<uint32_t __wrap>(...))), and while historically this might have been caught by something like

S<uint64_t> s = doSomething(...)

Modern C++ has auto and S s = doSomething(...) which will silently accept the change from S<uint64_t> to S<uint32_t __wrap>, and the viral nature will continue to silently propagate the uint32_t __wrap through everything.

I think _Auto or whatever arbitrarily different spelling C choose for auto has similar hazards.

The reality is that given the change to narrowing behavior this attribute does not actually solve the real world problems I’ve seen, or that I have had developers express a desire for support with, and it makes existing correct code incorrect if adopted, to the extent that it can create new vulnerabilities rather than removing them. Given the above issues, not only would this not be a solution to what developers have asked for, it would likely be necessary to forbid the adoption of the __wrap portion of the feature entirely, and __no_wrap would at least be challenging due to the potential introduction of crashes in code that is error (which includes UB) free.

That said, these are just the issues that I see from my point of view - the RFC says that this is intended to support linux kernel use cases, so I’d like to know whether these issues and hazards seem like a problem to them.

These changes in semantics can introduce security vulnerabilities where previously none existed. Take the format string example in the RFC, which was called out as a result of this change in behavior, which is the reason I assumed it had be removed:

Sorry, I didn't include the most obvious security vulnerability:

   uint16_t __wrap count = ...;
   malloc(sizeof(SomeType) * count);

The narrowing behavior truncates the allocation size.

Just to be clear though, the developer may not have explicitly state __wrap here: because of the way types are merged and propagated that uint16_t __wrap may have originally been a uint64_t, but every operation involving the uint16_t __wrap has resulted in the types being constantly narrowed. That would mean even if the code was

   uint64_t count = ...;
   malloc(sizeof(SomeType) * count);

The RHS expression may have already been truncated - it might directly have the type uint16_t __wrap or that may have been lost earlier on, making it even harder to track down.

@rnk The Clang Area team might want to be aware of the above comments, it's probably worth a discussion at a future meeting.

FYI, I'm meeting with @kees off-thread today to analyze kernel use cases and digest @ojhunt's review comments.

FYI, I'm meeting with @kees off-thread today to analyze kernel use cases and digest @ojhunt's review comments.

Sorry for the wall of text, and again sorry for not seeing the RFC :(

Sorry for the wall of text, and again sorry for not seeing the RFC :(

All good, your review is vital. I really have no personal attachments to any particular design decisions made -- I want to make sure this feature is useful to as many developers as possible.

Quick question before I go meet with Kees: How do you propose we handle overflow semantics with less-than-int stuff.

The core reason all of this narrowing and promotion magic exists is to solve the problem of:

u8 __ob_wrap a = 255;

if (a+1 == 0){...} // 1 is `int`, therefore whole expression promoted to `int`.

// Whoops! We no longer wrap-around at the 8-bit boundary because we are in `int` type boundaries now.

Not truncating things back down to the wrap type renders the __ob_wrap essentially useless for less-than-int types. We need to persist bitwidth information somehow. The current approach just narrows everything to the OBT type. Another approach mentioned in the RFC is to store the widths in some data structure. But I can see some issues with that approach too.

So, @ojhunt how do you ideally want all these narrowing semantics to work?

Sorry for the wall of text, and again sorry for not seeing the RFC :(

All good, your review is vital. I really have no personal attachments to any particular design decisions made -- I want to make sure this feature is useful to as many developers as possible.

Quick question before I go meet with Kees: How do you propose we handle overflow semantics with less-than-int stuff.

The core reason all of this narrowing and promotion magic exists is to solve the problem of:

u8 __ob_wrap a = 255;

if (a+1 == 0){...} // 1 is `int`, therefore whole expression promoted to `int`.

// Whoops! We no longer wrap-around at the 8-bit boundary because we are in `int` type boundaries now.

You would need to give me some indication that this is a behavior developers want or expect.

I think it's useful to think about how this attribute will be used in practice, including the linux kernel:

#if __has_attribute(...) // or has keyword or whatever
#define __wrap_attr __obt_wrap
#else
#define __wrap_attr
#endif

Given that usage - which is how it would have to be set up - how would your example of

uint8_t __wrap_attr a = 255;
if (a + 1 == 0) { ... }

To work in any real use case? It will not work on any compiler that does not have this attribute.

But this code is also fundamentally not realistic - a real version of this code would look like:

uint8_t __wrap_attr a = 255;
if (a + 1 > 0xff) { ... }

Except with the __wrap_attr this will no longer ever be true.

In fact any type bounds check like this does not work:

if (some_int + some_intN > __INTN_MAX__)

The only case that the truncation of large values does is your above if (a + 1 == 0) which is simply not a realistic scenario.

Not truncating things back down to the wrap type renders the __ob_wrap essentially useless for less-than-int types. We need to persist bitwidth information somehow. The current approach just narrows everything to the OBT type. Another approach mentioned in the RFC is to store the widths in some data structure. But I can see some issues with that approach too.

Why do you think you need to do this? The problem with overflow is not "the value is larger than I expected" it is "overflow leads to UB allowing compiler introduced vulnerabilities", and "ftrapv and fwrapv are global, but we want different behavior in different contexts".

This change to narrowing does not have anything to do with that.

So, @ojhunt how do you ideally want all these narrowing semantics to work?

The exact same way promotion works today: there should not be any narrowing. There is no reason to change this behavior to narrow: it makes the feature confusing, introduces security vulnerabilities, makes adoption unusable, and literally discards data in ways that produce warnings in many default configuration. Implicit narrowing is widely regarded as a mistake nowadays, and this not only adds a new version of implicit narrowing, it removes the warnings about that narrowing.

Returning to your example:

if (a + 1 == 0) { .. }

this is not something I believe anyone would expect, and more to the point is not something anyone could actually use. Removing narrowing does not make the wrapping behavior meaningless:

int f(bool b) {
  int8_t __obt_trap a = 1;
  int32_t i = __INT_MAX__ ;
  if (b)
    i = i + a; // int32_t + uint8_t __obt_trap => int32_t + int32_t __obt_trap => trap due to the overflow
  return i;
}

But more to the point, consider this:

int64_t a = ...;
int32_t __obt_wrap /* or __obt_trap */ b = ...;
int64_t result = a + b;

Would any developer would expect a truncated 32bit result? or an overflow trap? That doesn't match -ftrapv or -fwrapv, or any hypothetical future "erroneous behavior" specification for overflow.

We can also consider an unsigned version of the above:

uint64_t a = ...;
uint32_t __obt_wrap /* or __obt_trap */ b = ...;
uint64_t result = a + b;

Overflow of a + b is well defined, so for unsigned __obt_wrap would reasonably be expected to be a no op (imagine an template that uses __obt_wrap in an attempt to have defined overflow for signed type arguments, knowing that unsigned already has wrapping behavior), and now that is (1) not true, and (2) truncates a previously full sized type.

Basically, rather that asking "how would if (a + 1 == 0) work?" when that''s simply not semantics that I believe any developer would expect. Do you believe that there are developers that would expect or want to not be pass for any combination of (non-obt qualified) integer types:

template <class A, class B> void f(A a, B b) {
  static_assert(same_type_v<decltype((A)0 + (B __wrap)0),  decltype((A)0 + (B)0) __obt_wrap>);
  static_assert(same_type_v<decltype((A __wrap)0 + (B)0),  decltype((A)0 + (A)0) __obt_wrap>);
  static_assert(same_type_v<decltype((A)0 + (B __obt_trap)0),  decltype((A)0 + (B)0) __obt_trap>);
  static_assert(same_type_v<decltype((A __obt_trap)0 + (B)0),  decltype((A)0 + (A)0) __obt_trap>);
}

Because these qualifiers introduce implicit exposure that is not directly exposed in local source, e.g that types below could be auto, the result of direct field access (e.g. no local declared), function returns (again no local), means you can easily get code that does not appear to involve these qualifiers, but that is semantically equivalent to:

uint64_t u64 = 1ull<<32;
uint32_t __wrap u32 = 1;
u64 = u64 + u32;

Which from what I can tell results in u64 being equal to 1, despite that all the existing language semantics say that the arithmetic should be 64bit.

Let's imagine a warning about this then all of the existing correct code now needs to have casts everywhere, and those casts can easily drop the qualifier, ie. the above would need to be "corrected" with

uint64_t u64 = 1ull<<32;
uint32_t __obt_wrap u32 = 1;
u64 = u64 + (some_type)u32;

But what should some_type be? if some_type is uint32_t you drop wrapping rules where you may not actually need to (e.g auto, etc), or should it be u64 __obt_wrap or u64 __obt_trap? if this is generic code that may not have a single answer - you'd likely end up needing to create a pile of templates to get the correct type (according to normal integer operations), and I don't know if it's even addressable in C, but it doesn't have templates so maybe less relevant.