[clang] [Sema] Fixed faulty shift count warning (PR #69521)

[Shafik Yaghmour via cfe-commits]

@@ -12143,8 +12143,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
 auto FXSema = S.Context.getFixedPointSemantics(LHSExprType);
 LeftSize = FXSema.getWidth() - (unsigned)FXSema.hasUnsignedPadding();
   }
-  llvm::APInt LeftBits(Right.getBitWidth(), LeftSize);
-  if (Right.uge(LeftBits)) {
+  if (Right.uge(LeftSize)) {

shafik wrote:

I feel like `LeftSizeInBits` would be a more appropriate name but the changes 
looks right.

https://github.com/llvm/llvm-project/pull/69521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Fixed faulty shift count warning (PR #69521)

[Karl-Johan Karlsson via cfe-commits]

https://github.com/karka228 closed 
https://github.com/llvm/llvm-project/pull/69521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Fixed faulty shift count warning (PR #69521)

[Karl-Johan Karlsson via cfe-commits]

https://github.com/karka228 updated 
https://github.com/llvm/llvm-project/pull/69521

>From db4a254b721eddacf592a647d3de4fd10a8032e0 Mon Sep 17 00:00:00 2001
From: Karl-Johan Karlsson 
Date: Wed, 18 Oct 2023 16:40:53 +0200
Subject: [PATCH] [Sema] Fixed faulty shift count warning

Constant values of _BitInt have the bitwith to exactly fit the constant
number. This patch fix a problem in Sema when building an APInt where the
supplied bitwidth can become too small and simply truncate the value leading to
a faulty warning.
---
 clang/docs/ReleaseNotes.rst  |  3 +++
 clang/lib/Sema/SemaExpr.cpp  |  7 +++
 clang/test/Sema/c2x-expr-range.c | 12 
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a063733e96cb74c..4cc148905d4e130 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -488,6 +488,9 @@ Bug Fixes in This Version
 - Clang no longer permits using the `_BitInt` types as an underlying type for 
an
   enumeration as specified in the C23 Standard.
   Fixes (`#69619 `_)
+- Fixed an issue when a shift count specified by a small constant 
``_BitInt()``,
+  in a left shift operation, could result in a faulty warnings about
+  ``shift count >= width of type``.
 - Clang now accepts anonymous members initialized with designated initializers
   inside templates.
   Fixes (`#65143 `_)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 895b22805860d98..c2772bfb71c77e9 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -12143,8 +12143,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
 auto FXSema = S.Context.getFixedPointSemantics(LHSExprType);
 LeftSize = FXSema.getWidth() - (unsigned)FXSema.hasUnsignedPadding();
   }
-  llvm::APInt LeftBits(Right.getBitWidth(), LeftSize);
-  if (Right.uge(LeftBits)) {
+  if (Right.uge(LeftSize)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
 << RHS.get()->getSourceRange());
@@ -12186,7 +12185,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
 
   llvm::APInt ResultBits =
   static_cast(Right) + Left.getSignificantBits();
-  if (LeftBits.uge(ResultBits))
+  if (ResultBits.ule(LeftSize))
 return;
   llvm::APSInt Result = Left.extend(ResultBits.getLimitedValue());
   Result = Result.shl(Right);
@@ -12200,7 +12199,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   // bugs -- if the result is cast back to an unsigned type, it will have the
   // expected value. Thus we place this behind a different warning that can be
   // turned off separately if needed.
-  if (LeftBits == ResultBits - 1) {
+  if (ResultBits - 1 == LeftSize) {
 S.Diag(Loc, diag::warn_shift_result_sets_sign_bit)
 << HexResult << LHSType
 << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
diff --git a/clang/test/Sema/c2x-expr-range.c b/clang/test/Sema/c2x-expr-range.c
index 73683e6bfe684aa..eff0b7cd4d9f546 100644
--- a/clang/test/Sema/c2x-expr-range.c
+++ b/clang/test/Sema/c2x-expr-range.c
@@ -12,3 +12,15 @@ void test1(int *a) {
 void test2(__uint128_t *a) {
   (void)(*a >> ((__uint128_t)__UINT64_MAX__ + 1) <= 0); // expected-warning 
{{shift count >= width of type}}
 }
+
+// Regression test for bug where a faulty warning was given. We don't expect to
+// see any warning in this case.
+_BitInt(128) test3(_BitInt(128) a) {
+  return a << 12wb;
+}
+
+// Similar to test3 above, but with a too large shift count. We expect to see a
+// warning in this case.
+_BitInt(128) test4(_BitInt(128) a) {
+  return a << 129wb; // expected-warning {{shift count >= width of type}}
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Fixed faulty shift count warning (PR #69521)

[Karl-Johan Karlsson via cfe-commits]

https://github.com/karka228 updated 
https://github.com/llvm/llvm-project/pull/69521

>From a4a9fb73de9319acd26290ef31d1e17b127c0c3b Mon Sep 17 00:00:00 2001
From: Karl-Johan Karlsson 
Date: Wed, 18 Oct 2023 16:40:53 +0200
Subject: [PATCH] [Sema] Fixed faulty shift count warning

Constant values of _BitInt have the bitwith to exactly fit the constant
number. This patch fix a problem in Sema when building an APInt where the
supplied bitwidth can become too small and simply truncate the value leading to
a faulty warning.
---
 clang/docs/ReleaseNotes.rst  |  3 +++
 clang/lib/Sema/SemaExpr.cpp  |  7 +++
 clang/test/Sema/c2x-expr-range.c | 12 
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index cdef43f2011bc98..05bb069fe709700 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -403,6 +403,9 @@ Bug Fixes in This Version
   operator in C. No longer issuing a confusing diagnostic along the lines of
   "incompatible operand types ('foo' and 'foo')" with extensions such as matrix
   types. Fixes (`#69008 `_)
+- Fixed an issue when a shift count specified by a small constant 
``_BitInt()``,
+  in a left shift operation, could result in a faulty warnings about
+  ``shift count >= width of type``.
 
 Bug Fixes to Compiler Builtins
 ^^
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index aa30a3a03887558..c00178ee1835e00 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -12120,8 +12120,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
 auto FXSema = S.Context.getFixedPointSemantics(LHSExprType);
 LeftSize = FXSema.getWidth() - (unsigned)FXSema.hasUnsignedPadding();
   }
-  llvm::APInt LeftBits(Right.getBitWidth(), LeftSize);
-  if (Right.uge(LeftBits)) {
+  if (Right.uge(LeftSize)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
 << RHS.get()->getSourceRange());
@@ -12163,7 +12162,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
 
   llvm::APInt ResultBits =
   static_cast(Right) + Left.getSignificantBits();
-  if (LeftBits.uge(ResultBits))
+  if (ResultBits.ule(LeftSize))
 return;
   llvm::APSInt Result = Left.extend(ResultBits.getLimitedValue());
   Result = Result.shl(Right);
@@ -12177,7 +12176,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   // bugs -- if the result is cast back to an unsigned type, it will have the
   // expected value. Thus we place this behind a different warning that can be
   // turned off separately if needed.
-  if (LeftBits == ResultBits - 1) {
+  if (ResultBits - 1 == LeftSize) {
 S.Diag(Loc, diag::warn_shift_result_sets_sign_bit)
 << HexResult << LHSType
 << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
diff --git a/clang/test/Sema/c2x-expr-range.c b/clang/test/Sema/c2x-expr-range.c
index 73683e6bfe684aa..eff0b7cd4d9f546 100644
--- a/clang/test/Sema/c2x-expr-range.c
+++ b/clang/test/Sema/c2x-expr-range.c
@@ -12,3 +12,15 @@ void test1(int *a) {
 void test2(__uint128_t *a) {
   (void)(*a >> ((__uint128_t)__UINT64_MAX__ + 1) <= 0); // expected-warning 
{{shift count >= width of type}}
 }
+
+// Regression test for bug where a faulty warning was given. We don't expect to
+// see any warning in this case.
+_BitInt(128) test3(_BitInt(128) a) {
+  return a << 12wb;
+}
+
+// Similar to test3 above, but with a too large shift count. We expect to see a
+// warning in this case.
+_BitInt(128) test4(_BitInt(128) a) {
+  return a << 129wb; // expected-warning {{shift count >= width of type}}
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Fixed faulty shift count warning (PR #69521)

[Aaron Ballman via cfe-commits]

AaronBallman wrote:

> Alright, looks like @shafik isn't going to review this, so I'll just have to 
> accept it. LGTM.

FWIW, he was out of town all last week and a bit of the week before that. I 
think he's still digging out from under the mountain of email.

Changes LGTM as well!

https://github.com/llvm/llvm-project/pull/69521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Fixed faulty shift count warning (PR #69521)

[Erich Keane via cfe-commits]

https://github.com/erichkeane approved this pull request.

Alright, looks like @shafik isn't going to review this, so I'll just have to 
accept it.  LGTM.

https://github.com/llvm/llvm-project/pull/69521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Fixed faulty shift count warning (PR #69521)

[Karl-Johan Karlsson via cfe-commits]

karka228 wrote:

Ping

https://github.com/llvm/llvm-project/pull/69521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Fixed faulty shift count warning (PR #69521)

[Karl-Johan Karlsson via cfe-commits]

https://github.com/karka228 updated 
https://github.com/llvm/llvm-project/pull/69521

>From a9bfc65f6d6b0953cd762f851fb6044e1c4b8ab8 Mon Sep 17 00:00:00 2001
From: Karl-Johan Karlsson 
Date: Wed, 18 Oct 2023 16:40:53 +0200
Subject: [PATCH 1/2] [Sema] Fixed faulty shift count warning

Constant values of _BitInt have the bitwith to exactly fit the constant
number. This patch fix a problem in Sema when building an APInt where the
supplied bitwidth can become too small and simply truncate the value leading to
a faulty warning.
---
 clang/lib/Sema/SemaExpr.cpp  |  7 +++
 clang/test/Sema/c2x-expr-range.c | 12 
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index aa30a3a03887558..c00178ee1835e00 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -12120,8 +12120,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
 auto FXSema = S.Context.getFixedPointSemantics(LHSExprType);
 LeftSize = FXSema.getWidth() - (unsigned)FXSema.hasUnsignedPadding();
   }
-  llvm::APInt LeftBits(Right.getBitWidth(), LeftSize);
-  if (Right.uge(LeftBits)) {
+  if (Right.uge(LeftSize)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
 << RHS.get()->getSourceRange());
@@ -12163,7 +12162,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
 
   llvm::APInt ResultBits =
   static_cast(Right) + Left.getSignificantBits();
-  if (LeftBits.uge(ResultBits))
+  if (ResultBits.ule(LeftSize))
 return;
   llvm::APSInt Result = Left.extend(ResultBits.getLimitedValue());
   Result = Result.shl(Right);
@@ -12177,7 +12176,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   // bugs -- if the result is cast back to an unsigned type, it will have the
   // expected value. Thus we place this behind a different warning that can be
   // turned off separately if needed.
-  if (LeftBits == ResultBits - 1) {
+  if (ResultBits - 1 == LeftSize) {
 S.Diag(Loc, diag::warn_shift_result_sets_sign_bit)
 << HexResult << LHSType
 << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
diff --git a/clang/test/Sema/c2x-expr-range.c b/clang/test/Sema/c2x-expr-range.c
index 73683e6bfe684aa..eff0b7cd4d9f546 100644
--- a/clang/test/Sema/c2x-expr-range.c
+++ b/clang/test/Sema/c2x-expr-range.c
@@ -12,3 +12,15 @@ void test1(int *a) {
 void test2(__uint128_t *a) {
   (void)(*a >> ((__uint128_t)__UINT64_MAX__ + 1) <= 0); // expected-warning 
{{shift count >= width of type}}
 }
+
+// Regression test for bug where a faulty warning was given. We don't expect to
+// see any warning in this case.
+_BitInt(128) test3(_BitInt(128) a) {
+  return a << 12wb;
+}
+
+// Similar to test3 above, but with a too large shift count. We expect to see a
+// warning in this case.
+_BitInt(128) test4(_BitInt(128) a) {
+  return a << 129wb; // expected-warning {{shift count >= width of type}}
+}

>From a57f7401a691c865bf12740dd5b85e330bb6a0ca Mon Sep 17 00:00:00 2001
From: Karl-Johan Karlsson 
Date: Fri, 20 Oct 2023 05:57:10 +0200
Subject: [PATCH 2/2] [Sema] Fixed faulty shift count warning

Added a line about this in the release note.
---
 clang/docs/ReleaseNotes.rst | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 81cbfd90155fe0b..0d2ac4e29f6f41e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -400,6 +400,9 @@ Bug Fixes in This Version
   operator in C. No longer issuing a confusing diagnostic along the lines of
   "incompatible operand types ('foo' and 'foo')" with extensions such as matrix
   types. Fixes (`#69008 `_)
+- Fixed an issue when a shift count specified by a small constant 
``_BitInt()``,
+  in a left shift operation, could result in a faulty warnings about
+  ``shift count >= width of type``.
 
 Bug Fixes to Compiler Builtins
 ^^

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Fixed faulty shift count warning (PR #69521)

[Erich Keane via cfe-commits]

https://github.com/erichkeane commented:

I think this looks right, but if @shafik would take an additional look, I'd 
appreciate it.

Additionally, this needs a release note.

https://github.com/llvm/llvm-project/pull/69521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Fixed faulty shift count warning (PR #69521)

[Björn Pettersson via cfe-commits]

@@ -12120,8 +12120,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
 auto FXSema = S.Context.getFixedPointSemantics(LHSExprType);
 LeftSize = FXSema.getWidth() - (unsigned)FXSema.hasUnsignedPadding();
   }
-  llvm::APInt LeftBits(Right.getBitWidth(), LeftSize);
-  if (Right.uge(LeftBits)) {
+  if (Right.uge(LeftSize)) {

bjope wrote:

Oops, sorry. Now I think my previous comment was wrong. I missed the 
`getActiveBits()` check in APInt::ult. So it should be fine without 
complicating it that way.

https://github.com/llvm/llvm-project/pull/69521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Fixed faulty shift count warning (PR #69521)

[Björn Pettersson via cfe-commits]

@@ -12120,8 +12120,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
 auto FXSema = S.Context.getFixedPointSemantics(LHSExprType);
 LeftSize = FXSema.getWidth() - (unsigned)FXSema.hasUnsignedPadding();
   }
-  llvm::APInt LeftBits(Right.getBitWidth(), LeftSize);
-  if (Right.uge(LeftBits)) {
+  if (Right.uge(LeftSize)) {

bjope wrote:

I tried to understand why the old code used an APInt here.
What if you have something like ` x >> larger_than_64_bits_value`? I suspect 
that then the uge helper here will do a 64-bit unsigned compare. So it will 
truncate `Right` to 64 bits instead, right?

So I think the easiest way would be to compare to APInt:s. But make sure those 
are zero-extended to have a common size.
Maybe something like this:
```
  unsigned CompareBits = std::max(Right.getBitWidth(), 64);
  llvm::APInt LeftBits(CompareBits, LeftSize);
  if (Right.zext(CompareBits).uge(LeftBits)) {
```

https://github.com/llvm/llvm-project/pull/69521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Fixed faulty shift count warning (PR #69521)

[via cfe-commits]

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Karl-Johan Karlsson (karka228)


Changes

Constant values of _BitInt have the bitwith to exactly fit the constant number. 
This patch fix a problem in Sema when building an APInt where the supplied 
bitwidth can become too small and simply truncate the value leading to a faulty 
warning.


---
Full diff: https://github.com/llvm/llvm-project/pull/69521.diff


2 Files Affected:

- (modified) clang/lib/Sema/SemaExpr.cpp (+3-4) 
- (modified) clang/test/Sema/c2x-expr-range.c (+12) 


``diff
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index aa30a3a03887558..c00178ee1835e00 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -12120,8 +12120,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
 auto FXSema = S.Context.getFixedPointSemantics(LHSExprType);
 LeftSize = FXSema.getWidth() - (unsigned)FXSema.hasUnsignedPadding();
   }
-  llvm::APInt LeftBits(Right.getBitWidth(), LeftSize);
-  if (Right.uge(LeftBits)) {
+  if (Right.uge(LeftSize)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
 << RHS.get()->getSourceRange());
@@ -12163,7 +12162,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
 
   llvm::APInt ResultBits =
   static_cast(Right) + Left.getSignificantBits();
-  if (LeftBits.uge(ResultBits))
+  if (ResultBits.ule(LeftSize))
 return;
   llvm::APSInt Result = Left.extend(ResultBits.getLimitedValue());
   Result = Result.shl(Right);
@@ -12177,7 +12176,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   // bugs -- if the result is cast back to an unsigned type, it will have the
   // expected value. Thus we place this behind a different warning that can be
   // turned off separately if needed.
-  if (LeftBits == ResultBits - 1) {
+  if (ResultBits - 1 == LeftSize) {
 S.Diag(Loc, diag::warn_shift_result_sets_sign_bit)
 << HexResult << LHSType
 << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
diff --git a/clang/test/Sema/c2x-expr-range.c b/clang/test/Sema/c2x-expr-range.c
index 73683e6bfe684aa..eff0b7cd4d9f546 100644
--- a/clang/test/Sema/c2x-expr-range.c
+++ b/clang/test/Sema/c2x-expr-range.c
@@ -12,3 +12,15 @@ void test1(int *a) {
 void test2(__uint128_t *a) {
   (void)(*a >> ((__uint128_t)__UINT64_MAX__ + 1) <= 0); // expected-warning 
{{shift count >= width of type}}
 }
+
+// Regression test for bug where a faulty warning was given. We don't expect to
+// see any warning in this case.
+_BitInt(128) test3(_BitInt(128) a) {
+  return a << 12wb;
+}
+
+// Similar to test3 above, but with a too large shift count. We expect to see a
+// warning in this case.
+_BitInt(128) test4(_BitInt(128) a) {
+  return a << 129wb; // expected-warning {{shift count >= width of type}}
+}

``




https://github.com/llvm/llvm-project/pull/69521
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Sema] Fixed faulty shift count warning (PR #69521)

[Karl-Johan Karlsson via cfe-commits]

https://github.com/karka228 created 
https://github.com/llvm/llvm-project/pull/69521

Constant values of _BitInt have the bitwith to exactly fit the constant number. 
This patch fix a problem in Sema when building an APInt where the supplied 
bitwidth can become too small and simply truncate the value leading to a faulty 
warning.


>From a9bfc65f6d6b0953cd762f851fb6044e1c4b8ab8 Mon Sep 17 00:00:00 2001
From: Karl-Johan Karlsson 
Date: Wed, 18 Oct 2023 16:40:53 +0200
Subject: [PATCH] [Sema] Fixed faulty shift count warning

Constant values of _BitInt have the bitwith to exactly fit the constant
number. This patch fix a problem in Sema when building an APInt where the
supplied bitwidth can become too small and simply truncate the value leading to
a faulty warning.
---
 clang/lib/Sema/SemaExpr.cpp  |  7 +++
 clang/test/Sema/c2x-expr-range.c | 12 
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index aa30a3a03887558..c00178ee1835e00 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -12120,8 +12120,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
 auto FXSema = S.Context.getFixedPointSemantics(LHSExprType);
 LeftSize = FXSema.getWidth() - (unsigned)FXSema.hasUnsignedPadding();
   }
-  llvm::APInt LeftBits(Right.getBitWidth(), LeftSize);
-  if (Right.uge(LeftBits)) {
+  if (Right.uge(LeftSize)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
 << RHS.get()->getSourceRange());
@@ -12163,7 +12162,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
 
   llvm::APInt ResultBits =
   static_cast(Right) + Left.getSignificantBits();
-  if (LeftBits.uge(ResultBits))
+  if (ResultBits.ule(LeftSize))
 return;
   llvm::APSInt Result = Left.extend(ResultBits.getLimitedValue());
   Result = Result.shl(Right);
@@ -12177,7 +12176,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   // bugs -- if the result is cast back to an unsigned type, it will have the
   // expected value. Thus we place this behind a different warning that can be
   // turned off separately if needed.
-  if (LeftBits == ResultBits - 1) {
+  if (ResultBits - 1 == LeftSize) {
 S.Diag(Loc, diag::warn_shift_result_sets_sign_bit)
 << HexResult << LHSType
 << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
diff --git a/clang/test/Sema/c2x-expr-range.c b/clang/test/Sema/c2x-expr-range.c
index 73683e6bfe684aa..eff0b7cd4d9f546 100644
--- a/clang/test/Sema/c2x-expr-range.c
+++ b/clang/test/Sema/c2x-expr-range.c
@@ -12,3 +12,15 @@ void test1(int *a) {
 void test2(__uint128_t *a) {
   (void)(*a >> ((__uint128_t)__UINT64_MAX__ + 1) <= 0); // expected-warning 
{{shift count >= width of type}}
 }
+
+// Regression test for bug where a faulty warning was given. We don't expect to
+// see any warning in this case.
+_BitInt(128) test3(_BitInt(128) a) {
+  return a << 12wb;
+}
+
+// Similar to test3 above, but with a too large shift count. We expect to see a
+// warning in this case.
+_BitInt(128) test4(_BitInt(128) a) {
+  return a << 129wb; // expected-warning {{shift count >= width of type}}
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits