https://github.com/bjope updated https://github.com/llvm/llvm-project/pull/68590
From 44eedec9dae39e72d5474426a15a07d9be4144fc Mon Sep 17 00:00:00 2001 From: Bjorn Pettersson <bjorn.a.petters...@ericsson.com> Date: Mon, 9 Oct 2023 16:13:39 +0200 Subject: [PATCH 1/2] [Sema] Handle large shift counts in GetExprRange GetExprRange did not expect that very large shift counts when narrowing the range based on logical right shifts. So with inputs such as *a >> 123456789012345678901uwb it would hit assertions about trying to convert a too large APInt into uint64_t. This patch fixes that by using the APInt value when determining if we should reduce the range by the shift count or not. --- clang/lib/Sema/SemaChecking.cpp | 5 ++--- clang/test/Sema/c2x-expr-range.c | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) create mode 100644 clang/test/Sema/c2x-expr-range.c diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 3932d9cd07d9864..eb4d8d2e2806bcb 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -13554,11 +13554,10 @@ static IntRange GetExprRange(ASTContext &C, const Expr *E, unsigned MaxWidth, if (std::optional<llvm::APSInt> shift = BO->getRHS()->getIntegerConstantExpr(C)) { if (shift->isNonNegative()) { - unsigned zext = shift->getZExtValue(); - if (zext >= L.Width) + if (shift->uge(L.Width)) L.Width = (L.NonNegative ? 0 : 1); else - L.Width -= zext; + L.Width -= shift->getZExtValue(); } } diff --git a/clang/test/Sema/c2x-expr-range.c b/clang/test/Sema/c2x-expr-range.c new file mode 100644 index 000000000000000..1690a6280386bd5 --- /dev/null +++ b/clang/test/Sema/c2x-expr-range.c @@ -0,0 +1,20 @@ +// RUN: %clang_cc1 -verify -fsyntax-only -std=c2x -triple=x86_64-unknown-linux %s + +// test1 used to hit an assertion failure like this during parsing: +// clang: ../include/llvm/ADT/APInt.h:1488: uint64_t llvm::APInt::getZExtValue() const: Assertion `getActiveBits() <= 64 && "Too many bits for uint64_t"' failed. +// With a stack trace like this leading up to the assert: +// +// #9 0x00005645bca16518 GetExprRange(clang::ASTContext&, clang::Expr const*, unsigned int, bool, bool) SemaChecking.cpp:0:0 +// #10 0x00005645bca048a8 AnalyzeImplicitConversions(clang::Sema&, clang::Expr*, clang::SourceLocation, bool) SemaChecking.cpp:0:0 +// #11 0x00005645bca06af1 clang::Sema::CheckCompletedExpr(clang::Expr*, clang::SourceLocation, bool) +// +void test1(int *a) +{ + (void)(*a >> 123456789012345678901uwb <= 0); // expected-warning {{shift count >= width of type}} +} + +// Same as above but using __uint128_t instead of __BitInt. +void test2(__uint128_t *a) +{ + (void)(*a >> ((__uint128_t)__UINT64_MAX__ + (__uint128_t)1) <= 0); // expected-warning {{shift count >= width of type}} +} From f8881531c6b5f8a7f71554c01c4e7df8e24e7f72 Mon Sep 17 00:00:00 2001 From: Bjorn Pettersson <bjorn.a.petters...@ericsson.com> Date: Mon, 9 Oct 2023 17:34:41 +0200 Subject: [PATCH 2/2] [Sema] Handle large shift counts in GetExprRange GetExprRange did not expect that very large shift counts when narrowing the range based on logical right shifts. So with inputs such as *a >> 123456789012345678901uwb it would hit assertions about trying to convert a too large APInt into uint64_t. This patch fixes that by using the APInt value when determining if we should reduce the range by the shift count or not. --- clang/docs/ReleaseNotes.rst | 4 ++++ clang/test/Sema/c2x-expr-range.c | 14 +++++--------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 2d0302c399fb6f3..45ee6db4112b978 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -219,6 +219,10 @@ Bug Fixes in This Version - Clang no longer considers the loss of ``__unaligned`` qualifier from objects as an invalid conversion during method function overload resolution. +- Fixed an issue when a shift count larger than ``__INT64_MAX__``, in a right + shift operation, could result in missing warnings about + ``shift count >= width of type`` or internal compiler error. + Bug Fixes to Compiler Builtins ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/test/Sema/c2x-expr-range.c b/clang/test/Sema/c2x-expr-range.c index 1690a6280386bd5..0fe1bc547bbd353 100644 --- a/clang/test/Sema/c2x-expr-range.c +++ b/clang/test/Sema/c2x-expr-range.c @@ -1,19 +1,15 @@ // RUN: %clang_cc1 -verify -fsyntax-only -std=c2x -triple=x86_64-unknown-linux %s -// test1 used to hit an assertion failure like this during parsing: -// clang: ../include/llvm/ADT/APInt.h:1488: uint64_t llvm::APInt::getZExtValue() const: Assertion `getActiveBits() <= 64 && "Too many bits for uint64_t"' failed. -// With a stack trace like this leading up to the assert: -// -// #9 0x00005645bca16518 GetExprRange(clang::ASTContext&, clang::Expr const*, unsigned int, bool, bool) SemaChecking.cpp:0:0 -// #10 0x00005645bca048a8 AnalyzeImplicitConversions(clang::Sema&, clang::Expr*, clang::SourceLocation, bool) SemaChecking.cpp:0:0 -// #11 0x00005645bca06af1 clang::Sema::CheckCompletedExpr(clang::Expr*, clang::SourceLocation, bool) -// +// Regression test for bug where we used to hit assertion due to shift amount +// being larger than 64 bits. We want to see a warning about too large shift +// amount. void test1(int *a) { (void)(*a >> 123456789012345678901uwb <= 0); // expected-warning {{shift count >= width of type}} } -// Same as above but using __uint128_t instead of __BitInt. +// Similar to test1 above, but using __uint128_t instead of __BitInt. +// We want to see a warning about too large shift amount. void test2(__uint128_t *a) { (void)(*a >> ((__uint128_t)__UINT64_MAX__ + (__uint128_t)1) <= 0); // 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