[Clang] Introduce OverflowBehaviorType for fine-grained overflow control #148914
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-lldb @llvm/pr-subscribers-clang-driver Author: Justin Stitt (JustinStitt) ChangesIntroduce The new
A key aspect of this feature is its interaction with existing mechanisms. This implementation also includes specific promotion rules to preserve the intended overflow behavior within expressions and new diagnostics to warn against accidentally discarding the attribute. See the docs and tests for examples and more info. RFCsCCs (because of your participation in RFCs or previous implementations)@kees @AaronBallman @vitalybuka @melver @efriedma-quic Patch is 138.97 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/148914.diff 66 Files Affected:
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]
|
JustinStitt
force-pushed
the
overflow-behavior-types-pr
branch
from
fb45ed3 to
c301e29
Compare
jyknight
left a comment
jyknight
left a comment
There was a problem hiding this comment.
A couple comments from a quick glance.
JustinStitt
force-pushed
the
overflow-behavior-types-pr
branch
from
645069d to
a5d8ad5
Compare
|
gentle ping. |
JustinStitt
force-pushed
the
overflow-behavior-types-pr
branch
from
fad8d73 to
579a796
Compare
|
ping. I rebased due to some small conflicts. question to reviewers: Should I squash these ~10 commits into one for ease of review? |
mizvekov
left a comment
mizvekov
left a comment
There was a problem hiding this comment.
Some comments, I will try a more through review later.
mizvekov
left a comment
mizvekov
left a comment
There was a problem hiding this comment.
How about inner qualifiers? The patch doesn't address the situation.
For a test like:
using ConstInt = const int;
using WrapConstInt1 = ConstInt __attribute__((overflow_behavior(wrap)));
using WrapConstInt2 = const int __attribute__((overflow_behavior(wrap)));WrapConstInt1 and WrapConstInt2 should be the same type, as this is what makes most sense, I think.
This would be the same rules as qualifiers on array types, the inner and outer qualifiers mean the same thing and are added together for canonicalization.
|
thanks @mizvekov for the initial review. I've tried to address all your concerns with commit As for your comment:
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 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"; // falseI am not sure what you mean by "inner qualifiers" from your example. My understanding of inner qualifiers involves pointer types like |
I think this is a simple enough change, it's just about moving where the function is implemented. I think Type.h would be a better fit, but I'd wait a little bit for other opinions before changing anything.
So there are two positions qualifiers can appear in this type.
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. You can implement this by changing your canonicalization for |
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? |
OK, that sounds fine to me. It addresses the issue of developers slapping a random explicit cast just to silence warnings. Following your approach, they must specify overflow behavior. I'm going to hold off updating this PRs implementation until we have a more concrete direction. Perhaps I am just confusing reviewers by changing conversion semantics weekly. |
|
Summarizing the round table w/ @ojhunt @rjmccall @mizvekov @efriedma-quic (and others) it seems the current design (summarized by John in the rfc) is on the right track. The main talking point from the OBT discussion was about implicitly casting operands to functions whose parameter is OBT. Consider the following example: void foo(__ob_trap int);
void bar() {
s64 a = INT_MAX + 1337;
foo(a);
}In such an example, I believe @ojhunt and @rjmccall argued that we shouldn't try to trap when |
|
The suggestion was specifically limited to parameters: basically, when expressions are used to initialize parameters, the final conversion to the parameter type should work as if the parameter did not have an OBT.
Meanwhile, there are very significant incremental-adoption benefits for annotations having purely local impact. Being able to add We'd also talked about some kind of parameter attribute that would add overflow checking to the argument expression. That is, of course, inherently non-local, and therefore has unavoidable incremental-adoption problems. It also has a serious safety problem in that it doesn't recursively apply: it would add checking to void *ptr = malloc(n * sizeof(widget_entry_t));but not to size_t alloc_size = n * sizeof(widget_entry_t);
void *ptr = malloc(alloc_size);An alternative design would be to add a warning for all implicit conversions of non-constant values to A warning like this would, in a way, end up enforcing the same properties I know you're looking at strong typedefs for, but without even more changes to the basic mechanics of C. (Of course, it wouldn't work for e.g. providing type-safe dimensions! But I think you'll find more resistance to that than you might expect.) There was a secondary discussion about whether OBTs should be part of the signature of the function for the purposes of overloading, mangling, and so on, and I think the consensus there was "definitely not". Again, you want adopting |
|
Pinging @kees as he might have concerns with the first 80% of your comment but for me this part was interesting:
I think a goal of OBT's is that they are their own type and should overload and mangle like Back to void foo(_Atomic int b) {}
void foo(int b) {}
void ambiguous (int b) { foo(b); }
void also_ambiguous(_Atomic int b) { foo(b); }Meanwhile, void foo(_BitInt(12) b) {}
void foo(int b) {}
void picks_exact_match(_BitInt(12) b) { foo(b); }
void also_picks_exact_match(int b) { foo(b); }
void ambiguous_no_exact_match(_BitInt(13) b) { foo(b); }
void ambiguous_same_conversion_ranks_for_both(long b) { foo(b); }... And both are still mangled. So this suggests to me we can go a similar route with OBTs, mangle (and break ABI) but don't allow overloading with them. Thoughts on this? I'm no expert on what sorts of types should constitute ABI changes introduced by different mangled signatures but OBTs feel like they carry enough semantic weight to break ABI, I think this is what user's would expect as well. I am happy to learn more about this so please correct me if I am wrong. |
I agree, just void handler(__ob_trap u16 input);
...
int big = INT_MAX;
...
handler(big); // traps when truncated to u16 parameterThis isn't different from: void handler(__ob_trap u16 input);
...
int big = INT_MAX;
__ob_trap u16 small;
...
small = big; // traps when truncated to u16
handler(small);and isn't different from: void handler(__ob_trap u16 input);
...
u16 medium = U16_MAX;
...
handler(medium + 1); // promoted to int, traps when truncated to u16 parameter
Just to second @JustinStitt 's comments on this: I absolutely want mangling to see a different type for |
Just to be clear, that's going to impact things like function overloading (which we support in C as well as C++), type inspection via |
To chime in here, my current implementation supports overloading, templating + specialization and |
I understand that this doesn't have to be seen as different. I am telling you that is practically useful to make it different in order to enable incremental adoption of |
I am not suggesting that template argument deduction should ignore OBTs. Overloading solely by whether an argument is |
So it would be part of the type sometimes (argument deduction, mangling) and not part of the type otherwise (overloading)? That seems like a bad idea too, to me. My mental model for this is still that it's like |
I agree that this helps with adoption and as such I am OK with that approach. Later on, we can get the behavior with some strong typedef thing. The other stuff regarding overloading and templates should match probably |
I really don't understand why In contrast, The overloading behavior of |
I can explain my reasoning. I think it would be confusing for template specializations to notice |
Oh, it's definitely a type specifier. However, because the intended design is to drop this in while minimizing ABI incompatibility (to which I think should be added: without directly impacting callers), I think there are strong design arguments for applying certain qualifier-like behaviors to it, like ignoring it at the top level of a parameter type when computing the signature of a function. It is important to distinguish something requiring slightly more effort to specify in a standard, which does not matter, and something being confusing for users, which matters a lot. Your template vs. redeclaration example is exactly what happens with qualifiers, and I do not think it has ever been a problem for users. Actually allowing overloading based on a parameter being |
|
FYI, I have since that round table discussed this feature with a few other people, and a good justification for overloading has come up. These overloads would be useful for implementing math functions. For example, it is incongruous that the builtin arithmetic operators would be allowed to depend on the OBT specifier of the parameters, but a user created operator would not. The trivial example would be implementing an This could be also useful in the implementation of other mathematical functions, an example would be a polynomial evaluator. |
|
It is my understanding that we cannot have both 1) overloading on OBT and 2) no mangling on OBT. FWIW, our first user of this may very well be the Linux kernel and I don't think they care much about the overloading semantics of OBTs. Personally, I also don't care which way we go with this. My only goal is to deliver reliable type-level overflow behavior (something our users want). There is no rush to landing this feature but I do want to resolve ambiguity in design/implementation because now I am getting confused on the semantics folks want out of OBTs. What's the path forward here? |
I mean they're C right? so no overloading, and while there is
I think the mangling + the C interop issues may make this defacto chosen for us, but I am curious if @rjmccall would know how this interacts with template constraints? Two paths: the above template specialization match, but also if there were a hypothetical trait query that could describe obt-ness, which would have the effect of permitting overload on obt state?
I think there are two core questions: combining behavior and parameter/mangling behavior. I am of the impression that we had a moderate consensus on size, sightedness, and obt mode mismatches, and a lack of strong clarity on exactly how parameters should behave. |
|
The identity of a function template depends on its template arguments and dependent signature and does not factor in what the specialization of the dependent signature ends up looking like. There is no problem with having both a The argument about allowing concrete overloads for math functions is interesting, although it's very hard for me to imagine someone actually writing out overloads like this rather than using a template. Covering every type in |
|
Okay, I'm trying to isolate subtopics here. AFAICT these are resolved:
Open, with my proposed outcomes with rationale:
|
|
Can you elaborate about why Linux has expectations about C++ symbol mangling? |
They're C, but there is overloading: https://godbolt.org/z/4s5fc9v8s |
Well that's certainly a new horror I learned about today. I think that should have been guarded by a spoiler guard with a "horror" tag :D |
I can't be counted on for much of value, but I can be counted on to know about coding horrors. ;-) |
Linux uses |
|
Gah, sorry, literal cat on keyboard |
|
Okay, but is that good in this case? Are you sure the kernel does not have separate-compilation boundaries where you're going to want to e.g. freely interchange calls to |
Even if we write them out as templates, we still need the OBT specifier on argument types to be part of the function type, so that would rule out the option where we adjust them away entirely from the function type. That still leaves the options of either allowing overloading or not. For user code, it seems plausible that in some situations they would only care about a few overloads. Regarding the option of accepting no overloads, but without any function type adjustment: C++, since 17, already has precedent with the noexcept specifier, where that became part of the function type, but it's still not overloadable. |
12 participants
Introduce
OverflowBehaviorType(OBT), a new type attribute in Clang that provides developers with fine-grained control over the overflow behavior of integer types. This feature allows for a more nuanced approach to integer safety, achieving better granularity than global compiler flags like-fwrapvand-ftrapv. Type specifiers are also available via__ob_wrapand__ob_trap.These can be applied to integer types (both signed and unsigned) as well as typedef declarations, where the behavior is one of the following:
wrap: Guarantees that arithmetic operations on the type will wrap on overflow, similar to-fwrapv. This suppresses UBSan's integer overflow checks for the attributed type and prevents eager compiler optimizations.trap: Enforces overflow checking for the type, even when global flags like-fwrapvwould otherwise suppress it.A key aspect of this feature is its interaction with existing mechanisms.
OverflowBehaviorTypetakes precedence over global flags and, notably, over entries in the Sanitizer Special Case List (SSCL). This allows developers to "allowlist" critical types for overflow instrumentation, even if they are disabled by a broad rule in an SSCL.See the docs and tests for examples and more info.
RFCs
RFC v2
RFC v1
CCs (because of your participation in RFCs or previous implementations)
@kees @AaronBallman @vitalybuka @melver @efriedma-quic