mirror of
https://git.yoctoproject.org/poky
synced 2026-09-19 09:49:33 +02:00
gdb: Fix build with latest clang
This patch is already proposed upstream and perhaps landing soon in gdb master. (From OE-Core rev: 6721de5a049b245f274081b9b474e81761ea40fd) Signed-off-by: Khem Raj <raj.khem@gmail.com> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
This commit is contained in:
@@ -12,5 +12,6 @@ SRC_URI = "${GNU_MIRROR}/gdb/gdb-${PV}.tar.xz \
|
|||||||
file://0005-Change-order-of-CFLAGS.patch \
|
file://0005-Change-order-of-CFLAGS.patch \
|
||||||
file://0006-Fix-invalid-sigprocmask-call.patch \
|
file://0006-Fix-invalid-sigprocmask-call.patch \
|
||||||
file://0007-Define-alignof-using-_Alignof-when-using-C11-or-newe.patch \
|
file://0007-Define-alignof-using-_Alignof-when-using-C11-or-newe.patch \
|
||||||
|
file://0001-Fix-Wenum-constexpr-conversion-in-enum-flags.h.patch \
|
||||||
"
|
"
|
||||||
SRC_URI[sha256sum] = "38254eacd4572134bca9c5a5aa4d4ca564cbbd30c369d881f733fb6b903354f2"
|
SRC_URI[sha256sum] = "38254eacd4572134bca9c5a5aa4d4ca564cbbd30c369d881f733fb6b903354f2"
|
||||||
|
|||||||
@@ -0,0 +1,313 @@
|
|||||||
|
From dd22f64329c46797b3a3de2605ad1fa14cf77dd4 Mon Sep 17 00:00:00 2001
|
||||||
|
From: Carlos Galvez <carlosgalvezp@gmail.com>
|
||||||
|
Date: Sun, 30 Jun 2024 21:36:24 +0200
|
||||||
|
Subject: [PATCH] Fix -Wenum-constexpr-conversion in enum-flags.h
|
||||||
|
|
||||||
|
This fixes PR 31331:
|
||||||
|
https://sourceware.org/bugzilla/show_bug.cgi?id=31331
|
||||||
|
|
||||||
|
Currently, enum-flags.h is suppressing the warning
|
||||||
|
-Wenum-constexpr-conversion coming from recent versions of Clang.
|
||||||
|
This warning is intended to be made a compiler error
|
||||||
|
(non-downgradeable) in future versions of Clang:
|
||||||
|
|
||||||
|
https://github.com/llvm/llvm-project/issues/59036
|
||||||
|
|
||||||
|
The rationale is that casting a value of an integral type into an
|
||||||
|
enumeration is Undefined Behavior if the value does not fit in the
|
||||||
|
range of values of the enum:
|
||||||
|
https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1766
|
||||||
|
|
||||||
|
Undefined Behavior is not allowed in constant expressions, leading to
|
||||||
|
an ill-formed program.
|
||||||
|
|
||||||
|
In this case, in enum-flags.h, we are casting the value -1 to an enum
|
||||||
|
of a positive range only, which is UB as per the Standard and thus not
|
||||||
|
allowed in a constexpr context.
|
||||||
|
|
||||||
|
The purpose of doing this instead of using std::underlying_type is
|
||||||
|
because, for C-style enums, std::underlying_type typically returns
|
||||||
|
"unsigned int". However, when operating with it arithmetically, the
|
||||||
|
enum is promoted to *signed* int, which is what we want to avoid.
|
||||||
|
|
||||||
|
This patch solves this issue as follows:
|
||||||
|
|
||||||
|
* Use std::underlying_type and remove the custom enum_underlying_type.
|
||||||
|
|
||||||
|
* Ensure that operator~ is called always on an unsigned integer. We do
|
||||||
|
this by casting the input enum into std::size_t, which can fit any
|
||||||
|
unsigned integer. We have the guarantee that the cast is safe,
|
||||||
|
because we have checked that the underlying type is unsigned. If the
|
||||||
|
enum had negative values, the underlying type would be signed.
|
||||||
|
|
||||||
|
This solves the issue with C-style enums, but also solves a hidden
|
||||||
|
issue: enums with underlying type of std::uint8_t or std::uint16_t are
|
||||||
|
*also* promoted to signed int. Now they are all explicitly casted
|
||||||
|
to the largest unsigned int type and operator~ is safe to use.
|
||||||
|
|
||||||
|
* There is one more thing that needs fix. Currently, operator~ is
|
||||||
|
implemented as follows:
|
||||||
|
|
||||||
|
return (enum_type) ~underlying(e);
|
||||||
|
|
||||||
|
After applying ~underlying(e), the result is a very large value,
|
||||||
|
which we then cast to "enum_type". This cast is Undefined Behavior
|
||||||
|
if the large value does not fit in the range of the enum. For
|
||||||
|
C++ enums (scoped and/or with explicit underlying type), the range
|
||||||
|
of the enum is the entire range of the underlying type, so the cast
|
||||||
|
is safe. However, for C-style enums, the range is the smallest
|
||||||
|
bit-field that can hold all the values of the enumeration. So the
|
||||||
|
range is a lot smaller and casting a large value to the enum would
|
||||||
|
invoke Undefined Behavior.
|
||||||
|
|
||||||
|
To solve this problem, we create a new trait
|
||||||
|
EnumHasFixedUnderlyingType, to ensure operator~ may only be called
|
||||||
|
on C++-style enums. This behavior is roughly the same as what we
|
||||||
|
had on trunk, but relying on different properties of the enums.
|
||||||
|
|
||||||
|
* Once this is implemented, the following tests fail to compile:
|
||||||
|
|
||||||
|
CHECK_VALID (true, int, true ? EF () : EF2 ())
|
||||||
|
|
||||||
|
This is because it expects the enums to be promoted to signed int,
|
||||||
|
instead of unsigned int (which is the true underlying type).
|
||||||
|
|
||||||
|
I propose to remove these tests altogether, because:
|
||||||
|
|
||||||
|
- The comment nearby say they are not very important.
|
||||||
|
- Comparing 2 enums of different type like that is strange, relies
|
||||||
|
on integer promotions and thus hurts readability. As per comments
|
||||||
|
in the related PR, we likely don't want this type of code in gdb
|
||||||
|
code anyway, so there's no point in testing it.
|
||||||
|
- Most importantly, this type of comparison will be ill-formed in
|
||||||
|
C++26 for regular enums, so enum_flags does not need to emulate
|
||||||
|
that.
|
||||||
|
|
||||||
|
Since this is the only place where the warning was suppressed, remove
|
||||||
|
also the corresponding macro in include/diagnostics.h.
|
||||||
|
|
||||||
|
The change has been tested by running the entire gdb test suite
|
||||||
|
(make check) and comparing the results (testsuite/gdb.sum) against
|
||||||
|
trunk. No noticeable differences have been observed.
|
||||||
|
Tested-by: Keith Seitz <keiths@redhat.com>
|
||||||
|
|
||||||
|
Signed-off-by: Khem Raj <raj.khem@gmail.com>
|
||||||
|
Upstream-Status: Submitted [https://patchwork.sourceware.org/project/gdb/patch/20240630193624.2906762-1-carlosgalvezp@gmail.com/]
|
||||||
|
---
|
||||||
|
gdb/unittests/enum-flags-selftests.c | 27 -------
|
||||||
|
gdbsupport/enum-flags.h | 104 ++++++++++++++++++---------
|
||||||
|
include/diagnostics.h | 5 --
|
||||||
|
3 files changed, 70 insertions(+), 66 deletions(-)
|
||||||
|
|
||||||
|
diff --git a/gdb/unittests/enum-flags-selftests.c b/gdb/unittests/enum-flags-selftests.c
|
||||||
|
index b55d8c3..02563e5 100644
|
||||||
|
--- a/gdb/unittests/enum-flags-selftests.c
|
||||||
|
+++ b/gdb/unittests/enum-flags-selftests.c
|
||||||
|
@@ -233,33 +233,6 @@ CHECK_VALID (true, UEF, ~UEF ())
|
||||||
|
CHECK_VALID (true, EF, true ? EF () : RE ())
|
||||||
|
CHECK_VALID (true, EF, true ? RE () : EF ())
|
||||||
|
|
||||||
|
-/* These are valid, but it's not a big deal since you won't be able to
|
||||||
|
- assign the resulting integer to an enum or an enum_flags without a
|
||||||
|
- cast.
|
||||||
|
-
|
||||||
|
- The latter two tests are disabled on older GCCs because they
|
||||||
|
- incorrectly fail with gcc 4.8 and 4.9 at least. Running the test
|
||||||
|
- outside a SFINAE context shows:
|
||||||
|
-
|
||||||
|
- invalid user-defined conversion from ‘EF’ to ‘RE2’
|
||||||
|
-
|
||||||
|
- They've been confirmed to compile/pass with gcc 5.3, gcc 7.1 and
|
||||||
|
- clang 3.7. */
|
||||||
|
-
|
||||||
|
-CHECK_VALID (true, int, true ? EF () : EF2 ())
|
||||||
|
-CHECK_VALID (true, int, true ? EF2 () : EF ())
|
||||||
|
-CHECK_VALID (true, int, true ? EF () : RE2 ())
|
||||||
|
-CHECK_VALID (true, int, true ? RE2 () : EF ())
|
||||||
|
-
|
||||||
|
-/* Same, but with an unsigned enum. */
|
||||||
|
-
|
||||||
|
-typedef unsigned int uns;
|
||||||
|
-
|
||||||
|
-CHECK_VALID (true, uns, true ? EF () : UEF ())
|
||||||
|
-CHECK_VALID (true, uns, true ? UEF () : EF ())
|
||||||
|
-CHECK_VALID (true, uns, true ? EF () : URE ())
|
||||||
|
-CHECK_VALID (true, uns, true ? URE () : EF ())
|
||||||
|
-
|
||||||
|
/* Unfortunately this can't work due to the way C++ computes the
|
||||||
|
return type of the ternary conditional operator. int isn't
|
||||||
|
implicitly convertible to the raw enum type, so the type of the
|
||||||
|
diff --git a/gdbsupport/enum-flags.h b/gdbsupport/enum-flags.h
|
||||||
|
index 5078004..acec203 100644
|
||||||
|
--- a/gdbsupport/enum-flags.h
|
||||||
|
+++ b/gdbsupport/enum-flags.h
|
||||||
|
@@ -75,30 +75,6 @@
|
||||||
|
namespace. The compiler finds the corresponding
|
||||||
|
is_enum_flags_enum_type function via ADL. */
|
||||||
|
|
||||||
|
-/* Note that std::underlying_type<enum_type> is not what we want here,
|
||||||
|
- since that returns unsigned int even when the enum decays to signed
|
||||||
|
- int. */
|
||||||
|
-template<int size, bool sign> class integer_for_size { typedef void type; };
|
||||||
|
-template<> struct integer_for_size<1, 0> { typedef uint8_t type; };
|
||||||
|
-template<> struct integer_for_size<2, 0> { typedef uint16_t type; };
|
||||||
|
-template<> struct integer_for_size<4, 0> { typedef uint32_t type; };
|
||||||
|
-template<> struct integer_for_size<8, 0> { typedef uint64_t type; };
|
||||||
|
-template<> struct integer_for_size<1, 1> { typedef int8_t type; };
|
||||||
|
-template<> struct integer_for_size<2, 1> { typedef int16_t type; };
|
||||||
|
-template<> struct integer_for_size<4, 1> { typedef int32_t type; };
|
||||||
|
-template<> struct integer_for_size<8, 1> { typedef int64_t type; };
|
||||||
|
-
|
||||||
|
-template<typename T>
|
||||||
|
-struct enum_underlying_type
|
||||||
|
-{
|
||||||
|
- DIAGNOSTIC_PUSH
|
||||||
|
- DIAGNOSTIC_IGNORE_ENUM_CONSTEXPR_CONVERSION
|
||||||
|
- typedef typename
|
||||||
|
- integer_for_size<sizeof (T), static_cast<bool>(T (-1) < T (0))>::type
|
||||||
|
- type;
|
||||||
|
- DIAGNOSTIC_POP
|
||||||
|
-};
|
||||||
|
-
|
||||||
|
namespace enum_flags_detail
|
||||||
|
{
|
||||||
|
|
||||||
|
@@ -119,10 +95,62 @@ struct zero_type;
|
||||||
|
/* gdb::Requires trait helpers. */
|
||||||
|
template <typename enum_type>
|
||||||
|
using EnumIsUnsigned
|
||||||
|
- = std::is_unsigned<typename enum_underlying_type<enum_type>::type>;
|
||||||
|
+ = std::is_unsigned<typename std::underlying_type<enum_type>::type>;
|
||||||
|
+
|
||||||
|
+/* Helper to detect whether an enum has a fixed underlying type. This can be
|
||||||
|
+ achieved by using a scoped enum (in which case the type is "int") or
|
||||||
|
+ an explicit underlying type. C-style enums that are unscoped or do not
|
||||||
|
+ have an explicit underlying type have an implementation-defined underlying
|
||||||
|
+ type.
|
||||||
|
+
|
||||||
|
+ https://timsong-cpp.github.io/cppwp/n4659/dcl.enum#5
|
||||||
|
+
|
||||||
|
+ We need this trait in order to ensure that operator~ below does NOT
|
||||||
|
+ operate on old-style enums. This is because we apply operator~ on
|
||||||
|
+ the value and then cast the result to the enum_type. This is however
|
||||||
|
+ Undefined Behavior if the result does not fit in the range of possible
|
||||||
|
+ values for the enum. For enums with fixed underlying type, the entire
|
||||||
|
+ range of the integer is available. However, for old-style enums, the range
|
||||||
|
+ is only the smallest bit-field that can hold all the values of the
|
||||||
|
+ enumeration, typically much smaller than the underlying integer:
|
||||||
|
+
|
||||||
|
+ https://timsong-cpp.github.io/cppwp/n4659/expr.static.cast#10
|
||||||
|
+ https://timsong-cpp.github.io/cppwp/n4659/dcl.enum#8
|
||||||
|
+
|
||||||
|
+ To implement this, we leverage the fact that, since C++17, enums with
|
||||||
|
+ fixed underlying type can be list-initialized from an integer:
|
||||||
|
+ https://timsong-cpp.github.io/cppwp/n4659/dcl.init.list#3.7
|
||||||
|
+
|
||||||
|
+ Old-style enums cannot be initialized like that, leading to ill-formed
|
||||||
|
+ code.
|
||||||
|
+
|
||||||
|
+ We then use this together with SFINAE to create the desired trait.
|
||||||
|
+
|
||||||
|
+*/
|
||||||
|
+// Primary template
|
||||||
|
+template <typename enum_type, typename = void>
|
||||||
|
+struct EnumHasFixedUnderlyingType : std::false_type
|
||||||
|
+{
|
||||||
|
+ static_assert(std::is_enum<enum_type>::value);
|
||||||
|
+};
|
||||||
|
+
|
||||||
|
+// Specialization that is active only if enum_type can be list-initialized
|
||||||
|
+// from an integer (0). Only enums with fixed underlying type satisfy this
|
||||||
|
+// property in C++17.
|
||||||
|
+template <typename enum_type>
|
||||||
|
+struct EnumHasFixedUnderlyingType<enum_type, std::void_t<decltype(enum_type{0})>> : std::true_type
|
||||||
|
+{
|
||||||
|
+ static_assert(std::is_enum<enum_type>::value);
|
||||||
|
+};
|
||||||
|
+
|
||||||
|
+template <typename enum_type>
|
||||||
|
+using EnumIsSafeForBitwiseComplement = std::conjunction<
|
||||||
|
+ EnumIsUnsigned<enum_type>,
|
||||||
|
+ EnumHasFixedUnderlyingType<enum_type>
|
||||||
|
+>;
|
||||||
|
+
|
||||||
|
template <typename enum_type>
|
||||||
|
-using EnumIsSigned
|
||||||
|
- = std::is_signed<typename enum_underlying_type<enum_type>::type>;
|
||||||
|
+using EnumIsUnsafeForBitwiseComplement = std::negation<EnumIsSafeForBitwiseComplement<enum_type>>;
|
||||||
|
|
||||||
|
}
|
||||||
|
|
||||||
|
@@ -131,7 +159,7 @@ class enum_flags
|
||||||
|
{
|
||||||
|
public:
|
||||||
|
typedef E enum_type;
|
||||||
|
- typedef typename enum_underlying_type<enum_type>::type underlying_type;
|
||||||
|
+ typedef typename std::underlying_type<enum_type>::type underlying_type;
|
||||||
|
|
||||||
|
/* For to_string. Maps one enumerator of E to a string. */
|
||||||
|
struct string_mapping
|
||||||
|
@@ -394,33 +422,41 @@ ENUM_FLAGS_GEN_COMP (operator!=, !=)
|
||||||
|
template <typename enum_type,
|
||||||
|
typename = is_enum_flags_enum_type_t<enum_type>,
|
||||||
|
typename
|
||||||
|
- = gdb::Requires<enum_flags_detail::EnumIsUnsigned<enum_type>>>
|
||||||
|
+ = gdb::Requires<enum_flags_detail::EnumIsSafeForBitwiseComplement<enum_type>>>
|
||||||
|
constexpr enum_type
|
||||||
|
operator~ (enum_type e)
|
||||||
|
{
|
||||||
|
using underlying = typename enum_flags<enum_type>::underlying_type;
|
||||||
|
- return (enum_type) ~underlying (e);
|
||||||
|
+ // Cast to std::size_t first, to prevent integer promotions from
|
||||||
|
+ // enums with fixed underlying type std::uint8_t or std::uint16_t
|
||||||
|
+ // to signed int.
|
||||||
|
+ // This ensures we apply the bitwise complement on an unsigned type.
|
||||||
|
+ return (enum_type)(underlying) ~std::size_t (e);
|
||||||
|
}
|
||||||
|
|
||||||
|
template <typename enum_type,
|
||||||
|
typename = is_enum_flags_enum_type_t<enum_type>,
|
||||||
|
- typename = gdb::Requires<enum_flags_detail::EnumIsSigned<enum_type>>>
|
||||||
|
+ typename = gdb::Requires<enum_flags_detail::EnumIsUnsafeForBitwiseComplement<enum_type>>>
|
||||||
|
constexpr void operator~ (enum_type e) = delete;
|
||||||
|
|
||||||
|
template <typename enum_type,
|
||||||
|
typename = is_enum_flags_enum_type_t<enum_type>,
|
||||||
|
typename
|
||||||
|
- = gdb::Requires<enum_flags_detail::EnumIsUnsigned<enum_type>>>
|
||||||
|
+ = gdb::Requires<enum_flags_detail::EnumIsSafeForBitwiseComplement<enum_type>>>
|
||||||
|
constexpr enum_flags<enum_type>
|
||||||
|
operator~ (enum_flags<enum_type> e)
|
||||||
|
{
|
||||||
|
using underlying = typename enum_flags<enum_type>::underlying_type;
|
||||||
|
- return (enum_type) ~underlying (e);
|
||||||
|
+ // Cast to std::size_t first, to prevent integer promotions from
|
||||||
|
+ // enums with fixed underlying type std::uint8_t or std::uint16_t
|
||||||
|
+ // to signed int.
|
||||||
|
+ // This ensures we apply the bitwise complement on an unsigned type.
|
||||||
|
+ return (enum_type)(underlying) ~std::size_t (e);
|
||||||
|
}
|
||||||
|
|
||||||
|
template <typename enum_type,
|
||||||
|
typename = is_enum_flags_enum_type_t<enum_type>,
|
||||||
|
- typename = gdb::Requires<enum_flags_detail::EnumIsSigned<enum_type>>>
|
||||||
|
+ typename = gdb::Requires<enum_flags_detail::EnumIsUnsafeForBitwiseComplement<enum_type>>>
|
||||||
|
constexpr void operator~ (enum_flags<enum_type> e) = delete;
|
||||||
|
|
||||||
|
/* Delete operator<< and operator>>. */
|
||||||
|
diff --git a/include/diagnostics.h b/include/diagnostics.h
|
||||||
|
index 97e30ab..14575e2 100644
|
||||||
|
--- a/include/diagnostics.h
|
||||||
|
+++ b/include/diagnostics.h
|
||||||
|
@@ -76,11 +76,6 @@
|
||||||
|
# define DIAGNOSTIC_ERROR_SWITCH \
|
||||||
|
DIAGNOSTIC_ERROR ("-Wswitch")
|
||||||
|
|
||||||
|
-# if __has_warning ("-Wenum-constexpr-conversion")
|
||||||
|
-# define DIAGNOSTIC_IGNORE_ENUM_CONSTEXPR_CONVERSION \
|
||||||
|
- DIAGNOSTIC_IGNORE ("-Wenum-constexpr-conversion")
|
||||||
|
-# endif
|
||||||
|
-
|
||||||
|
#elif defined (__GNUC__) /* GCC */
|
||||||
|
|
||||||
|
# define DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS \
|
||||||
Reference in New Issue
Block a user