Repository navigation
[Clang] Introduce OverflowBehaviorType for fine-grained overflow control - #148914
Conversation
|
@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]
|
fb45ed3 to
c301e29
Compare
jyknight
left a comment
There was a problem hiding this comment.
A couple comments from a quick glance.
645069d to
a5d8ad5
Compare
|
gentle ping. |
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
There was a problem hiding this comment.
Some comments, I will try a more through review later.
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? |
Hard to summarize because WG14's mailing lists are very broken (not all of the emails make it to all of the members and the archives seem to have stopped updating sometime mid last year). I'll manually forward you the emails I have on the topic. |
e4dfa20 to
3d8aa21
Compare
|
squashed. rebased. |
|
Ping, @AaronBallman have you had a chance to take a look at this? |
3d8aa21 to
3e06143
Compare
|
rebased |
AaronBallman
left a comment
There was a problem hiding this comment.
I was able to complete my final pass through the PR, I have some suggestions on additional tests to add and perhaps some diagnostic wording that could be updated, but I don't think anything is holding up landing the changes (we can address my nits post-commit assuming I've not uncovered something you think is critical to address now).
I'm giving my LG but I'd still appreciate at least one more LG from someone more familiar with codegen before we land. CC @efriedma-quic @asl
|
@mizvekov any lingering concerns before we try to merge this in? |
mizvekov
left a comment
There was a problem hiding this comment.
LGTM, let's land this.
I am pretty comfortable with this since it's still an experimental cc1 enabled feature, so we can still improve the design as needed.
3cf14e1 to
146982a
Compare
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
Signed-off-by: Justin Stitt <justinstitt@google.com>
146982a to
d470042
Compare
|
rebasing to convince the CI to run again as I believe the test failure is unrelated to this PR. |
This PR adds OverflowBehavior case to CIR CodeGen. Fixes CI failures in clangIR introduced by #148914
This PR adds OverflowBehavior case to CIR CodeGen. Fixes CI failures in clangIR introduced by llvm/llvm-project#148914
|
It appears that this patch is causing for failures on ppc64le: https://lab.llvm.org/buildbot/#/builders/145/builds/12863 Would you be able to help take a look or revert this PR if a fix is not possible in the near future? |
I'll take a look on Monday! Thanks for spotting this |
| let Args = [IdentifierArgument<"BehaviorKind">]; | ||
| let Subjects = SubjectList<[Var, TypedefName, Field], WarnDiag, | ||
| "variables, typedefs, and data members">; | ||
| let Documentation = [Undocumented]; |
There was a problem hiding this comment.
Sorry for missing this during review, but this attribute should come with documentation. @JustinStitt would you like me to file an issue for that?
There was a problem hiding this comment.
Ah, yeah go ahead and file something.
I'll document this later today.
There was a problem hiding this comment.
Thank you! I filed #203322 and assigned it to you.
This PR adds OverflowBehavior case to CIR CodeGen. Fixes CI failures in clangIR introduced by llvm/llvm-project#148914
…fine-grained overflow control (#148914)" Handle the new type conservatively. See llvm/llvm-project@7f631bb (llvm/llvm-project#148914).
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