https://github.com/efriedma-quic approved this pull request.
LGTM
https://github.com/llvm/llvm-project/pull/75481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
zygoloid wrote:
I don't think we've established an explicit policy, and we've made ABI-breaking
changes previously. I think we should avoid breaks within a major release
version (don't backport this) to avoid giving packagers headaches: the ubsan
runtime is installed in a versioned directory,
AaronBallman wrote:
> > Also, I don't have commit access (first PR here), someone has to commit on
> > my behalf!
>
> @AaronBallman
Thank you for mentioning this! I'll commit on your behalf, but I'd like to hear
from @zygoloid on the ABI question. It would also be nice if @efriedma-quic
@@ -147,6 +147,7 @@ struct ImplicitConversionData {
const TypeDescriptor
const TypeDescriptor
/* ImplicitConversionCheckKind */ unsigned char Kind;
+ unsigned int BitfieldBits;
AaronBallman wrote:
Ping @zygoloid -- I'm not certain if we have an ABI
Zonotora wrote:
> Also, I don't have commit access (first PR here), someone has to commit on my
> behalf!
@AaronBallman
https://github.com/llvm/llvm-project/pull/75481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
Zonotora wrote:
Also, I don't have commit access (first PR here), someone has to commit on my
behalf!
https://github.com/llvm/llvm-project/pull/75481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://github.com/Zonotora updated
https://github.com/llvm/llvm-project/pull/75481
>From 42207b6369b22db7e8400f290be8c6ee75a5278a Mon Sep 17 00:00:00 2001
From: Zonotora
Date: Sat, 16 Dec 2023 19:33:21 +0100
Subject: [PATCH 1/3] [clang] Extract negativity check lambda to function
---
Zonotora wrote:
@LebedevRI LGTM?
https://github.com/llvm/llvm-project/pull/75481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -0,0 +1,61 @@
+// RUN: %clang -fsanitize=implicit-bitfield-conversion -target x86_64-linux -S
-emit-llvm -o - %s | FileCheck %s
--check-prefixes=CHECK,CHECK-BITFIELD-CONVERSION
Zonotora wrote:
Added!
https://github.com/llvm/llvm-project/pull/75481
https://github.com/Zonotora updated
https://github.com/llvm/llvm-project/pull/75481
>From 3213a5d3c8230cc941d026475624b754bc87a41e Mon Sep 17 00:00:00 2001
From: Zonotora
Date: Sat, 16 Dec 2023 19:33:21 +0100
Subject: [PATCH 1/3] [clang] Extract negativity check lambda to function
---
@@ -147,6 +147,7 @@ struct ImplicitConversionData {
const TypeDescriptor
const TypeDescriptor
/* ImplicitConversionCheckKind */ unsigned char Kind;
+ unsigned int BitfieldBits;
Zonotora wrote:
@zygoloid
@@ -0,0 +1,61 @@
+// RUN: %clang -fsanitize=implicit-bitfield-conversion -target x86_64-linux -S
-emit-llvm -o - %s | FileCheck %s
--check-prefixes=CHECK,CHECK-BITFIELD-CONVERSION
Zonotora wrote:
I will add some testcases later today!
@@ -0,0 +1,61 @@
+// RUN: %clang -fsanitize=implicit-bitfield-conversion -target x86_64-linux -S
-emit-llvm -o - %s | FileCheck %s
--check-prefixes=CHECK,CHECK-BITFIELD-CONVERSION
efriedma-quic wrote:
I'd like to see some testcases that run in C++ mode.
@@ -147,6 +147,7 @@ struct ImplicitConversionData {
const TypeDescriptor
const TypeDescriptor
/* ImplicitConversionCheckKind */ unsigned char Kind;
+ unsigned int BitfieldBits;
efriedma-quic wrote:
Do we have an ABI stability policy for UBsan? Is
https://github.com/vitalybuka approved this pull request.
https://github.com/llvm/llvm-project/pull/75481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1094,6 +1114,27 @@ void ScalarExprEmitter::EmitIntegerTruncationCheck(Value
*Src, QualType SrcType,
{Src, Dst});
}
+static llvm::Value *EmitIsNegativeTestHelper(Value *V, QualType VType,
vitalybuka wrote:
I still see it here, does it
https://github.com/Zonotora updated
https://github.com/llvm/llvm-project/pull/75481
>From f0f02479736548a72c8a35ca3fea067b02e66bfd Mon Sep 17 00:00:00 2001
From: Zonotora
Date: Sat, 16 Dec 2023 19:33:21 +0100
Subject: [PATCH 1/3] [clang] Extract negativity check lambda to function
---
https://github.com/Zonotora updated
https://github.com/llvm/llvm-project/pull/75481
>From cd64144a4cf18adba1494871bfd9da1b0bf0a489 Mon Sep 17 00:00:00 2001
From: Zonotora
Date: Sat, 16 Dec 2023 19:33:21 +0100
Subject: [PATCH 1/3] [clang] Extract negativity check lambda to function
---
@@ -5571,11 +5571,52 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const
BinaryOperator *E) {
break;
}
-RValue RV = EmitAnyExpr(E->getRHS());
+llvm::Value *Previous = nullptr;
efriedma-quic wrote:
Sure, if you want to leave the
@@ -5571,11 +5571,52 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const
BinaryOperator *E) {
break;
}
-RValue RV = EmitAnyExpr(E->getRHS());
+llvm::Value *Previous = nullptr;
+RValue RV;
+QualType SrcType = E->getRHS()->getType();
+//
@@ -5571,11 +5571,52 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const
BinaryOperator *E) {
break;
}
-RValue RV = EmitAnyExpr(E->getRHS());
+llvm::Value *Previous = nullptr;
Zonotora wrote:
I would also like to have it work similar
@@ -5571,11 +5571,52 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const
BinaryOperator *E) {
break;
}
-RValue RV = EmitAnyExpr(E->getRHS());
+llvm::Value *Previous = nullptr;
efriedma-quic wrote:
Can we de-duplicate this code with
https://github.com/efriedma-quic commented:
I'm a bit concerned about the lack of C++ testcases, since you're making
changes to some C++-only codepaths.
https://github.com/llvm/llvm-project/pull/75481
___
cfe-commits mailing list
https://github.com/efriedma-quic edited
https://github.com/llvm/llvm-project/pull/75481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/AaronBallman approved this pull request.
LGTM, so long as @rjmccall or @efriedma-quic don't have concerns about codegen
changes. Please give them a few days to chime in before landing.
https://github.com/llvm/llvm-project/pull/75481
@@ -5571,11 +5571,52 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const
BinaryOperator *E) {
break;
}
-RValue RV = EmitAnyExpr(E->getRHS());
+llvm::Value *Previous = nullptr;
+RValue RV;
+QualType SrcType = E->getRHS()->getType();
+//
https://github.com/zygoloid edited
https://github.com/llvm/llvm-project/pull/75481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/zygoloid commented:
Looks good to me. @AaronBallman Did you have remaining concerns here?
https://github.com/llvm/llvm-project/pull/75481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
@@ -555,13 +555,11 @@ static void
handleImplicitConversion(ImplicitConversionData *Data,
ReportOptions Opts, ValueHandle Src,
ValueHandle Dst) {
SourceLocation Loc = Data->Loc.acquire();
- ErrorType
Zonotora wrote:
@zygoloid Thanks for your feedback! Fixed all comments except the last one. I
totally agree with you about the distinction of UB checks, and that should be
part of a different future PR!
https://github.com/llvm/llvm-project/pull/75481
@@ -555,13 +555,11 @@ static void
handleImplicitConversion(ImplicitConversionData *Data,
ReportOptions Opts, ValueHandle Src,
ValueHandle Dst) {
SourceLocation Loc = Data->Loc.acquire();
- ErrorType
https://github.com/Zonotora updated
https://github.com/llvm/llvm-project/pull/75481
>From cd64144a4cf18adba1494871bfd9da1b0bf0a489 Mon Sep 17 00:00:00 2001
From: Zonotora
Date: Sat, 16 Dec 2023 19:33:21 +0100
Subject: [PATCH 1/3] [clang] Extract negativity check lambda to function
---
zygoloid wrote:
> I'm not opposed to the changes, though I do find it somewhat strange to add
> them to UBSan given that they're not undefined behavior (even though there's
> precedent).
This is adding checks to `-fsanitize=implicit-conversion`, which is not part of
`-fsanitize=undefined`.
@@ -555,13 +555,11 @@ static void
handleImplicitConversion(ImplicitConversionData *Data,
ReportOptions Opts, ValueHandle Src,
ValueHandle Dst) {
SourceLocation Loc = Data->Loc.acquire();
- ErrorType
@@ -5571,11 +5571,50 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const
BinaryOperator *E) {
break;
}
-RValue RV = EmitAnyExpr(E->getRHS());
+llvm::Value *Previous = nullptr;
+RValue RV;
+QualType SrcType = E->getRHS()->getType();
+//
@@ -5571,11 +5571,50 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const
BinaryOperator *E) {
break;
}
-RValue RV = EmitAnyExpr(E->getRHS());
+llvm::Value *Previous = nullptr;
+RValue RV;
+QualType SrcType = E->getRHS()->getType();
+//
@@ -5571,11 +5571,50 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const
BinaryOperator *E) {
break;
}
-RValue RV = EmitAnyExpr(E->getRHS());
+llvm::Value *Previous = nullptr;
+RValue RV;
+QualType SrcType = E->getRHS()->getType();
+//
@@ -5571,11 +5571,50 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const
BinaryOperator *E) {
break;
}
-RValue RV = EmitAnyExpr(E->getRHS());
+llvm::Value *Previous = nullptr;
+RValue RV;
+QualType SrcType = E->getRHS()->getType();
+//
@@ -4564,15 +4737,41 @@ Value *ScalarExprEmitter::VisitBinAssign(const
BinaryOperator *E) {
case Qualifiers::OCL_None:
// __block variables need to have the rhs evaluated first, plus
// this should improve codegen just a little.
-RHS = Visit(E->getRHS());
+
https://github.com/zygoloid commented:
Generally this looks good to me, thanks.
https://github.com/llvm/llvm-project/pull/75481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/zygoloid edited
https://github.com/llvm/llvm-project/pull/75481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Zonotora wrote:
@zygoloid @AaronBallman any updates?
https://github.com/llvm/llvm-project/pull/75481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Zonotora wrote:
> Precommit CI seems to have found some related failures that should be
> investigated. I'd love to hear opinions from the codegen code owners on this,
> as well as @zygoloid as UBSan code owner. I'm not opposed to the changes,
> though I do find it somewhat strange to add
Zonotora wrote:
@zygoloid what do you think?
https://github.com/llvm/llvm-project/pull/75481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/Zonotora updated
https://github.com/llvm/llvm-project/pull/75481
>From a3de13f8356cd615534c759b6e24893d361e62e9 Mon Sep 17 00:00:00 2001
From: Zonotora
Date: Sat, 16 Dec 2023 19:33:21 +0100
Subject: [PATCH 1/3] [clang] Extract negativity check lambda to function
---
rjmccall wrote:
> @rjmccall fixed feedback
>
> > You should handle compound assignments.
>
> I believe I already support compound assignments (and PrePostIncDec), if I am
> not missing something?
Ah, maybe you are, sorry. I was expecting it in `CGExpr.cpp` and forgot that
that code was
Zonotora wrote:
@rjmccall fixed feedback
> You should handle compound assignments.
I believe I already support compound assignments (and PrePostIncDec), if I am
not missing something? :smiley:
https://github.com/llvm/llvm-project/pull/75481
___
https://github.com/Zonotora updated
https://github.com/llvm/llvm-project/pull/75481
>From 5c85a13cbd5fb1d861f2b5a4cbb53abc5736674e Mon Sep 17 00:00:00 2001
From: Zonotora
Date: Sat, 16 Dec 2023 19:33:21 +0100
Subject: [PATCH 1/3] [clang] Extract negativity check lambda to function
---
https://github.com/Zonotora updated
https://github.com/llvm/llvm-project/pull/75481
>From 5c85a13cbd5fb1d861f2b5a4cbb53abc5736674e Mon Sep 17 00:00:00 2001
From: Zonotora
Date: Sat, 16 Dec 2023 19:33:21 +0100
Subject: [PATCH 1/3] [clang] Extract negativity check lambda to function
---
@@ -324,6 +326,19 @@ class ScalarExprEmitter
void EmitIntegerSignChangeCheck(Value *Src, QualType SrcType, Value *Dst,
QualType DstType, SourceLocation Loc);
+ /// Emit a check that an [implicit] truncation of a bitfield does not
+ ///
@@ -1035,7 +1050,7 @@ EmitIntegerTruncationCheckHelper(Value *Src, QualType
SrcType, Value *Dst,
}
llvm::Value *Check = nullptr;
- // 1. Extend the truncated value back to the same width as the Src.
+ // 1. Convert the Dst back to the same type as Src.
@@ -1097,6 +1112,27 @@ void ScalarExprEmitter::EmitIntegerTruncationCheck(Value
*Src, QualType SrcType,
{Src, Dst});
}
+static llvm::Value *EmitIsNegativeTestHelper(Value *V, QualType VType,
+ const char *Name,
+
@@ -1097,6 +1112,27 @@ void ScalarExprEmitter::EmitIntegerTruncationCheck(Value
*Src, QualType SrcType,
{Src, Dst});
}
+static llvm::Value *EmitIsNegativeTestHelper(Value *V, QualType VType,
+ const char *Name,
+
@@ -5570,11 +5570,44 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const
BinaryOperator *E) {
break;
}
-RValue RV = EmitAnyExpr(E->getRHS());
+llvm::Value *Previous = nullptr;
+RValue RV;
+QualType SrcType = E->getRHS()->getType();
+// If
@@ -122,6 +122,26 @@ Non-comprehensive list of changes in this release
New Compiler Flags
--
+- ``-fsanitize=implicit-unsigned-bitfield-truncation`` catches implicit
+ unsigned conversions involving bitfields.
+-
@@ -122,6 +122,26 @@ Non-comprehensive list of changes in this release
New Compiler Flags
--
+- ``-fsanitize=implicit-unsigned-bitfield-truncation`` catches implicit
+ unsigned conversions involving bitfields.
+-
https://github.com/rjmccall commented:
You should handle compound assignments.
https://github.com/llvm/llvm-project/pull/75481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -5570,11 +5570,44 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const
BinaryOperator *E) {
break;
}
-RValue RV = EmitAnyExpr(E->getRHS());
+llvm::Value *Previous = nullptr;
+RValue RV;
+QualType SrcType = E->getRHS()->getType();
+// If
@@ -1097,6 +1112,27 @@ void ScalarExprEmitter::EmitIntegerTruncationCheck(Value
*Src, QualType SrcType,
{Src, Dst});
}
+static llvm::Value *EmitIsNegativeTestHelper(Value *V, QualType VType,
+ const char *Name,
+
@@ -324,6 +326,19 @@ class ScalarExprEmitter
void EmitIntegerSignChangeCheck(Value *Src, QualType SrcType, Value *Dst,
QualType DstType, SourceLocation Loc);
+ /// Emit a check that an [implicit] truncation of a bitfield does not
+ ///
@@ -5570,11 +5570,44 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const
BinaryOperator *E) {
break;
}
-RValue RV = EmitAnyExpr(E->getRHS());
+llvm::Value *Previous = nullptr;
+RValue RV;
+QualType SrcType = E->getRHS()->getType();
+// If
@@ -1097,6 +1112,27 @@ void ScalarExprEmitter::EmitIntegerTruncationCheck(Value
*Src, QualType SrcType,
{Src, Dst});
}
+static llvm::Value *EmitIsNegativeTestHelper(Value *V, QualType VType,
+ const char *Name,
+
@@ -1239,6 +1257,228 @@ void
ScalarExprEmitter::EmitIntegerSignChangeCheck(Value *Src, QualType SrcType,
{Src, Dst});
}
+// Should be called within CodeGenFunction::SanitizerScope RAII scope.
+// Returns 'i1 false' when the truncation Src -> Dst was lossy.
@@ -1035,7 +1050,7 @@ EmitIntegerTruncationCheckHelper(Value *Src, QualType
SrcType, Value *Dst,
}
llvm::Value *Check = nullptr;
- // 1. Extend the truncated value back to the same width as the Src.
+ // 1. Convert the Dst back to the same type as Src.
https://github.com/rjmccall edited
https://github.com/llvm/llvm-project/pull/75481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/Zonotora updated
https://github.com/llvm/llvm-project/pull/75481
>From b23f9855820196f7ee5741d81f7c408a641a96fd Mon Sep 17 00:00:00 2001
From: Zonotora
Date: Sat, 16 Dec 2023 19:33:21 +0100
Subject: [PATCH 1/3] [clang] Extract negativity check lambda to function
---
AaronBallman wrote:
Precommit CI seems to have found some related failures that should be
investigated. I'd love to hear opinions from the codegen code owners on this,
as well as @zygoloid as UBSan code owner. I'm not opposed to the changes,
though I do find it somewhat strange to add them to
vabridgers wrote:
LGTM, but someone else must approve. Thanks for doing this work, Axel!
https://github.com/llvm/llvm-project/pull/75481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
Zonotora wrote:
Hi again, I have now finally gotten time and updated the patch so that the
unnecessary emits I mentioned in the initial commit are avoided. The current
patch introduces a number of new fsanitizer flags to separate integer
conversions from bitfield conversions. E.g.,
-
https://github.com/Zonotora edited
https://github.com/llvm/llvm-project/pull/75481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/Zonotora updated
https://github.com/llvm/llvm-project/pull/75481
>From 3b0af99a0e13ed9d6b8c0365a9fa7ea68ec5 Mon Sep 17 00:00:00 2001
From: Zonotora
Date: Sat, 16 Dec 2023 19:33:21 +0100
Subject: [PATCH 1/2] [clang] Extract negativity check lambda to function
---
@@ -1094,6 +1114,27 @@ void ScalarExprEmitter::EmitIntegerTruncationCheck(Value
*Src, QualType SrcType,
{Src, Dst});
}
+static llvm::Value *EmitIsNegativeTestHelper(Value *V, QualType VType,
vitalybuka wrote:
Please extract lambda to
vitalybuka wrote:
Is is UB?
https://github.com/llvm/llvm-project/pull/75481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Zonotora wrote:
Hi, @vitalybuka @zygoloid @AaronBallman you might have something to say about
this commit
https://github.com/llvm/llvm-project/pull/75481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
llvmbot wrote:
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-codegen
Author: Axel Lundberg (Zonotora)
Changes
This patch implements the implicit truncation and implicit sign change checks
for bitfields using UBSan. E.g.,
`-fsanitize=implicit-integer-truncation` and
github-actions[bot] wrote:
Thank you for submitting a Pull Request (PR) to the LLVM Project!
This PR will be automatically labeled and the relevant teams will be
notified.
If you wish to, you can add reviewers by using the "Reviewers" section on this
page.
If this is not working for you, it
76 matches
Mail list logo
