diff options
| author | Khem Raj <raj.khem@gmail.com> | 2024-08-28 12:41:32 -0700 |
|---|---|---|
| committer | Richard Purdie <richard.purdie@linuxfoundation.org> | 2024-08-29 21:58:19 +0100 |
| commit | 088d9fe6b7086802367d7a080da0c1465862c24f (patch) | |
| tree | 161d15fcb262877feb6b77bdc3ac3039a648a950 | |
| parent | 5aeabd3217da2a7ce9f789d5d224f6682b4555a1 (diff) | |
| download | poky-088d9fe6b7086802367d7a080da0c1465862c24f.tar.gz | |
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>
| -rw-r--r-- | meta/recipes-devtools/gdb/gdb.inc | 1 | ||||
| -rw-r--r-- | meta/recipes-devtools/gdb/gdb/0001-Fix-Wenum-constexpr-conversion-in-enum-flags.h.patch | 313 |
2 files changed, 314 insertions, 0 deletions
diff --git a/meta/recipes-devtools/gdb/gdb.inc b/meta/recipes-devtools/gdb/gdb.inc index 6fdf11d394..6aa9dced86 100644 --- a/meta/recipes-devtools/gdb/gdb.inc +++ b/meta/recipes-devtools/gdb/gdb.inc | |||
| @@ -12,5 +12,6 @@ SRC_URI = "${GNU_MIRROR}/gdb/gdb-${PV}.tar.xz \ | |||
| 12 | file://0005-Change-order-of-CFLAGS.patch \ | 12 | file://0005-Change-order-of-CFLAGS.patch \ |
| 13 | file://0006-Fix-invalid-sigprocmask-call.patch \ | 13 | file://0006-Fix-invalid-sigprocmask-call.patch \ |
| 14 | file://0007-Define-alignof-using-_Alignof-when-using-C11-or-newe.patch \ | 14 | file://0007-Define-alignof-using-_Alignof-when-using-C11-or-newe.patch \ |
| 15 | file://0001-Fix-Wenum-constexpr-conversion-in-enum-flags.h.patch \ | ||
| 15 | " | 16 | " |
| 16 | SRC_URI[sha256sum] = "38254eacd4572134bca9c5a5aa4d4ca564cbbd30c369d881f733fb6b903354f2" | 17 | SRC_URI[sha256sum] = "38254eacd4572134bca9c5a5aa4d4ca564cbbd30c369d881f733fb6b903354f2" |
diff --git a/meta/recipes-devtools/gdb/gdb/0001-Fix-Wenum-constexpr-conversion-in-enum-flags.h.patch b/meta/recipes-devtools/gdb/gdb/0001-Fix-Wenum-constexpr-conversion-in-enum-flags.h.patch new file mode 100644 index 0000000000..8866db2e93 --- /dev/null +++ b/meta/recipes-devtools/gdb/gdb/0001-Fix-Wenum-constexpr-conversion-in-enum-flags.h.patch | |||
| @@ -0,0 +1,313 @@ | |||
| 1 | From dd22f64329c46797b3a3de2605ad1fa14cf77dd4 Mon Sep 17 00:00:00 2001 | ||
| 2 | From: Carlos Galvez <carlosgalvezp@gmail.com> | ||
| 3 | Date: Sun, 30 Jun 2024 21:36:24 +0200 | ||
| 4 | Subject: [PATCH] Fix -Wenum-constexpr-conversion in enum-flags.h | ||
| 5 | |||
| 6 | This fixes PR 31331: | ||
| 7 | https://sourceware.org/bugzilla/show_bug.cgi?id=31331 | ||
| 8 | |||
| 9 | Currently, enum-flags.h is suppressing the warning | ||
| 10 | -Wenum-constexpr-conversion coming from recent versions of Clang. | ||
| 11 | This warning is intended to be made a compiler error | ||
| 12 | (non-downgradeable) in future versions of Clang: | ||
| 13 | |||
| 14 | https://github.com/llvm/llvm-project/issues/59036 | ||
| 15 | |||
| 16 | The rationale is that casting a value of an integral type into an | ||
| 17 | enumeration is Undefined Behavior if the value does not fit in the | ||
| 18 | range of values of the enum: | ||
| 19 | https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1766 | ||
| 20 | |||
| 21 | Undefined Behavior is not allowed in constant expressions, leading to | ||
| 22 | an ill-formed program. | ||
| 23 | |||
| 24 | In this case, in enum-flags.h, we are casting the value -1 to an enum | ||
| 25 | of a positive range only, which is UB as per the Standard and thus not | ||
| 26 | allowed in a constexpr context. | ||
| 27 | |||
| 28 | The purpose of doing this instead of using std::underlying_type is | ||
| 29 | because, for C-style enums, std::underlying_type typically returns | ||
| 30 | "unsigned int". However, when operating with it arithmetically, the | ||
| 31 | enum is promoted to *signed* int, which is what we want to avoid. | ||
| 32 | |||
| 33 | This patch solves this issue as follows: | ||
| 34 | |||
| 35 | * Use std::underlying_type and remove the custom enum_underlying_type. | ||
| 36 | |||
| 37 | * Ensure that operator~ is called always on an unsigned integer. We do | ||
| 38 | this by casting the input enum into std::size_t, which can fit any | ||
| 39 | unsigned integer. We have the guarantee that the cast is safe, | ||
| 40 | because we have checked that the underlying type is unsigned. If the | ||
| 41 | enum had negative values, the underlying type would be signed. | ||
| 42 | |||
| 43 | This solves the issue with C-style enums, but also solves a hidden | ||
| 44 | issue: enums with underlying type of std::uint8_t or std::uint16_t are | ||
| 45 | *also* promoted to signed int. Now they are all explicitly casted | ||
| 46 | to the largest unsigned int type and operator~ is safe to use. | ||
| 47 | |||
| 48 | * There is one more thing that needs fix. Currently, operator~ is | ||
| 49 | implemented as follows: | ||
| 50 | |||
| 51 | return (enum_type) ~underlying(e); | ||
| 52 | |||
| 53 | After applying ~underlying(e), the result is a very large value, | ||
| 54 | which we then cast to "enum_type". This cast is Undefined Behavior | ||
| 55 | if the large value does not fit in the range of the enum. For | ||
| 56 | C++ enums (scoped and/or with explicit underlying type), the range | ||
| 57 | of the enum is the entire range of the underlying type, so the cast | ||
| 58 | is safe. However, for C-style enums, the range is the smallest | ||
| 59 | bit-field that can hold all the values of the enumeration. So the | ||
| 60 | range is a lot smaller and casting a large value to the enum would | ||
| 61 | invoke Undefined Behavior. | ||
| 62 | |||
| 63 | To solve this problem, we create a new trait | ||
| 64 | EnumHasFixedUnderlyingType, to ensure operator~ may only be called | ||
| 65 | on C++-style enums. This behavior is roughly the same as what we | ||
| 66 | had on trunk, but relying on different properties of the enums. | ||
| 67 | |||
| 68 | * Once this is implemented, the following tests fail to compile: | ||
| 69 | |||
| 70 | CHECK_VALID (true, int, true ? EF () : EF2 ()) | ||
| 71 | |||
| 72 | This is because it expects the enums to be promoted to signed int, | ||
| 73 | instead of unsigned int (which is the true underlying type). | ||
| 74 | |||
| 75 | I propose to remove these tests altogether, because: | ||
| 76 | |||
| 77 | - The comment nearby say they are not very important. | ||
| 78 | - Comparing 2 enums of different type like that is strange, relies | ||
| 79 | on integer promotions and thus hurts readability. As per comments | ||
| 80 | in the related PR, we likely don't want this type of code in gdb | ||
| 81 | code anyway, so there's no point in testing it. | ||
| 82 | - Most importantly, this type of comparison will be ill-formed in | ||
| 83 | C++26 for regular enums, so enum_flags does not need to emulate | ||
| 84 | that. | ||
| 85 | |||
| 86 | Since this is the only place where the warning was suppressed, remove | ||
| 87 | also the corresponding macro in include/diagnostics.h. | ||
| 88 | |||
| 89 | The change has been tested by running the entire gdb test suite | ||
| 90 | (make check) and comparing the results (testsuite/gdb.sum) against | ||
| 91 | trunk. No noticeable differences have been observed. | ||
| 92 | Tested-by: Keith Seitz <keiths@redhat.com> | ||
| 93 | |||
| 94 | Signed-off-by: Khem Raj <raj.khem@gmail.com> | ||
| 95 | Upstream-Status: Submitted [https://patchwork.sourceware.org/project/gdb/patch/20240630193624.2906762-1-carlosgalvezp@gmail.com/] | ||
| 96 | --- | ||
| 97 | gdb/unittests/enum-flags-selftests.c | 27 ------- | ||
| 98 | gdbsupport/enum-flags.h | 104 ++++++++++++++++++--------- | ||
| 99 | include/diagnostics.h | 5 -- | ||
| 100 | 3 files changed, 70 insertions(+), 66 deletions(-) | ||
| 101 | |||
| 102 | diff --git a/gdb/unittests/enum-flags-selftests.c b/gdb/unittests/enum-flags-selftests.c | ||
| 103 | index b55d8c3..02563e5 100644 | ||
| 104 | --- a/gdb/unittests/enum-flags-selftests.c | ||
| 105 | +++ b/gdb/unittests/enum-flags-selftests.c | ||
| 106 | @@ -233,33 +233,6 @@ CHECK_VALID (true, UEF, ~UEF ()) | ||
| 107 | CHECK_VALID (true, EF, true ? EF () : RE ()) | ||
| 108 | CHECK_VALID (true, EF, true ? RE () : EF ()) | ||
| 109 | |||
| 110 | -/* These are valid, but it's not a big deal since you won't be able to | ||
| 111 | - assign the resulting integer to an enum or an enum_flags without a | ||
| 112 | - cast. | ||
| 113 | - | ||
| 114 | - The latter two tests are disabled on older GCCs because they | ||
| 115 | - incorrectly fail with gcc 4.8 and 4.9 at least. Running the test | ||
| 116 | - outside a SFINAE context shows: | ||
| 117 | - | ||
| 118 | - invalid user-defined conversion from ‘EF’ to ‘RE2’ | ||
| 119 | - | ||
| 120 | - They've been confirmed to compile/pass with gcc 5.3, gcc 7.1 and | ||
| 121 | - clang 3.7. */ | ||
| 122 | - | ||
| 123 | -CHECK_VALID (true, int, true ? EF () : EF2 ()) | ||
| 124 | -CHECK_VALID (true, int, true ? EF2 () : EF ()) | ||
| 125 | -CHECK_VALID (true, int, true ? EF () : RE2 ()) | ||
| 126 | -CHECK_VALID (true, int, true ? RE2 () : EF ()) | ||
| 127 | - | ||
| 128 | -/* Same, but with an unsigned enum. */ | ||
| 129 | - | ||
| 130 | -typedef unsigned int uns; | ||
| 131 | - | ||
| 132 | -CHECK_VALID (true, uns, true ? EF () : UEF ()) | ||
| 133 | -CHECK_VALID (true, uns, true ? UEF () : EF ()) | ||
| 134 | -CHECK_VALID (true, uns, true ? EF () : URE ()) | ||
| 135 | -CHECK_VALID (true, uns, true ? URE () : EF ()) | ||
| 136 | - | ||
| 137 | /* Unfortunately this can't work due to the way C++ computes the | ||
| 138 | return type of the ternary conditional operator. int isn't | ||
| 139 | implicitly convertible to the raw enum type, so the type of the | ||
| 140 | diff --git a/gdbsupport/enum-flags.h b/gdbsupport/enum-flags.h | ||
| 141 | index 5078004..acec203 100644 | ||
| 142 | --- a/gdbsupport/enum-flags.h | ||
| 143 | +++ b/gdbsupport/enum-flags.h | ||
| 144 | @@ -75,30 +75,6 @@ | ||
| 145 | namespace. The compiler finds the corresponding | ||
| 146 | is_enum_flags_enum_type function via ADL. */ | ||
| 147 | |||
| 148 | -/* Note that std::underlying_type<enum_type> is not what we want here, | ||
| 149 | - since that returns unsigned int even when the enum decays to signed | ||
| 150 | - int. */ | ||
| 151 | -template<int size, bool sign> class integer_for_size { typedef void type; }; | ||
| 152 | -template<> struct integer_for_size<1, 0> { typedef uint8_t type; }; | ||
| 153 | -template<> struct integer_for_size<2, 0> { typedef uint16_t type; }; | ||
| 154 | -template<> struct integer_for_size<4, 0> { typedef uint32_t type; }; | ||
| 155 | -template<> struct integer_for_size<8, 0> { typedef uint64_t type; }; | ||
| 156 | -template<> struct integer_for_size<1, 1> { typedef int8_t type; }; | ||
| 157 | -template<> struct integer_for_size<2, 1> { typedef int16_t type; }; | ||
| 158 | -template<> struct integer_for_size<4, 1> { typedef int32_t type; }; | ||
| 159 | -template<> struct integer_for_size<8, 1> { typedef int64_t type; }; | ||
| 160 | - | ||
| 161 | -template<typename T> | ||
| 162 | -struct enum_underlying_type | ||
| 163 | -{ | ||
| 164 | - DIAGNOSTIC_PUSH | ||
| 165 | - DIAGNOSTIC_IGNORE_ENUM_CONSTEXPR_CONVERSION | ||
| 166 | - typedef typename | ||
| 167 | - integer_for_size<sizeof (T), static_cast<bool>(T (-1) < T (0))>::type | ||
| 168 | - type; | ||
| 169 | - DIAGNOSTIC_POP | ||
| 170 | -}; | ||
| 171 | - | ||
| 172 | namespace enum_flags_detail | ||
| 173 | { | ||
| 174 | |||
| 175 | @@ -119,10 +95,62 @@ struct zero_type; | ||
| 176 | /* gdb::Requires trait helpers. */ | ||
| 177 | template <typename enum_type> | ||
| 178 | using EnumIsUnsigned | ||
| 179 | - = std::is_unsigned<typename enum_underlying_type<enum_type>::type>; | ||
| 180 | + = std::is_unsigned<typename std::underlying_type<enum_type>::type>; | ||
| 181 | + | ||
| 182 | +/* Helper to detect whether an enum has a fixed underlying type. This can be | ||
| 183 | + achieved by using a scoped enum (in which case the type is "int") or | ||
| 184 | + an explicit underlying type. C-style enums that are unscoped or do not | ||
| 185 | + have an explicit underlying type have an implementation-defined underlying | ||
| 186 | + type. | ||
| 187 | + | ||
| 188 | + https://timsong-cpp.github.io/cppwp/n4659/dcl.enum#5 | ||
| 189 | + | ||
| 190 | + We need this trait in order to ensure that operator~ below does NOT | ||
| 191 | + operate on old-style enums. This is because we apply operator~ on | ||
| 192 | + the value and then cast the result to the enum_type. This is however | ||
| 193 | + Undefined Behavior if the result does not fit in the range of possible | ||
| 194 | + values for the enum. For enums with fixed underlying type, the entire | ||
| 195 | + range of the integer is available. However, for old-style enums, the range | ||
| 196 | + is only the smallest bit-field that can hold all the values of the | ||
| 197 | + enumeration, typically much smaller than the underlying integer: | ||
| 198 | + | ||
| 199 | + https://timsong-cpp.github.io/cppwp/n4659/expr.static.cast#10 | ||
| 200 | + https://timsong-cpp.github.io/cppwp/n4659/dcl.enum#8 | ||
| 201 | + | ||
| 202 | + To implement this, we leverage the fact that, since C++17, enums with | ||
| 203 | + fixed underlying type can be list-initialized from an integer: | ||
| 204 | + https://timsong-cpp.github.io/cppwp/n4659/dcl.init.list#3.7 | ||
| 205 | + | ||
| 206 | + Old-style enums cannot be initialized like that, leading to ill-formed | ||
| 207 | + code. | ||
| 208 | + | ||
| 209 | + We then use this together with SFINAE to create the desired trait. | ||
| 210 | + | ||
| 211 | +*/ | ||
| 212 | +// Primary template | ||
| 213 | +template <typename enum_type, typename = void> | ||
| 214 | +struct EnumHasFixedUnderlyingType : std::false_type | ||
| 215 | +{ | ||
| 216 | + static_assert(std::is_enum<enum_type>::value); | ||
| 217 | +}; | ||
| 218 | + | ||
| 219 | +// Specialization that is active only if enum_type can be list-initialized | ||
| 220 | +// from an integer (0). Only enums with fixed underlying type satisfy this | ||
| 221 | +// property in C++17. | ||
| 222 | +template <typename enum_type> | ||
| 223 | +struct EnumHasFixedUnderlyingType<enum_type, std::void_t<decltype(enum_type{0})>> : std::true_type | ||
| 224 | +{ | ||
| 225 | + static_assert(std::is_enum<enum_type>::value); | ||
| 226 | +}; | ||
| 227 | + | ||
| 228 | +template <typename enum_type> | ||
| 229 | +using EnumIsSafeForBitwiseComplement = std::conjunction< | ||
| 230 | + EnumIsUnsigned<enum_type>, | ||
| 231 | + EnumHasFixedUnderlyingType<enum_type> | ||
| 232 | +>; | ||
| 233 | + | ||
| 234 | template <typename enum_type> | ||
| 235 | -using EnumIsSigned | ||
| 236 | - = std::is_signed<typename enum_underlying_type<enum_type>::type>; | ||
| 237 | +using EnumIsUnsafeForBitwiseComplement = std::negation<EnumIsSafeForBitwiseComplement<enum_type>>; | ||
| 238 | |||
| 239 | } | ||
| 240 | |||
| 241 | @@ -131,7 +159,7 @@ class enum_flags | ||
| 242 | { | ||
| 243 | public: | ||
| 244 | typedef E enum_type; | ||
| 245 | - typedef typename enum_underlying_type<enum_type>::type underlying_type; | ||
| 246 | + typedef typename std::underlying_type<enum_type>::type underlying_type; | ||
| 247 | |||
| 248 | /* For to_string. Maps one enumerator of E to a string. */ | ||
| 249 | struct string_mapping | ||
| 250 | @@ -394,33 +422,41 @@ ENUM_FLAGS_GEN_COMP (operator!=, !=) | ||
| 251 | template <typename enum_type, | ||
| 252 | typename = is_enum_flags_enum_type_t<enum_type>, | ||
| 253 | typename | ||
| 254 | - = gdb::Requires<enum_flags_detail::EnumIsUnsigned<enum_type>>> | ||
| 255 | + = gdb::Requires<enum_flags_detail::EnumIsSafeForBitwiseComplement<enum_type>>> | ||
| 256 | constexpr enum_type | ||
| 257 | operator~ (enum_type e) | ||
| 258 | { | ||
| 259 | using underlying = typename enum_flags<enum_type>::underlying_type; | ||
| 260 | - return (enum_type) ~underlying (e); | ||
| 261 | + // Cast to std::size_t first, to prevent integer promotions from | ||
| 262 | + // enums with fixed underlying type std::uint8_t or std::uint16_t | ||
| 263 | + // to signed int. | ||
| 264 | + // This ensures we apply the bitwise complement on an unsigned type. | ||
| 265 | + return (enum_type)(underlying) ~std::size_t (e); | ||
| 266 | } | ||
| 267 | |||
| 268 | template <typename enum_type, | ||
| 269 | typename = is_enum_flags_enum_type_t<enum_type>, | ||
| 270 | - typename = gdb::Requires<enum_flags_detail::EnumIsSigned<enum_type>>> | ||
| 271 | + typename = gdb::Requires<enum_flags_detail::EnumIsUnsafeForBitwiseComplement<enum_type>>> | ||
| 272 | constexpr void operator~ (enum_type e) = delete; | ||
| 273 | |||
| 274 | template <typename enum_type, | ||
| 275 | typename = is_enum_flags_enum_type_t<enum_type>, | ||
| 276 | typename | ||
| 277 | - = gdb::Requires<enum_flags_detail::EnumIsUnsigned<enum_type>>> | ||
| 278 | + = gdb::Requires<enum_flags_detail::EnumIsSafeForBitwiseComplement<enum_type>>> | ||
| 279 | constexpr enum_flags<enum_type> | ||
| 280 | operator~ (enum_flags<enum_type> e) | ||
| 281 | { | ||
| 282 | using underlying = typename enum_flags<enum_type>::underlying_type; | ||
| 283 | - return (enum_type) ~underlying (e); | ||
| 284 | + // Cast to std::size_t first, to prevent integer promotions from | ||
| 285 | + // enums with fixed underlying type std::uint8_t or std::uint16_t | ||
| 286 | + // to signed int. | ||
| 287 | + // This ensures we apply the bitwise complement on an unsigned type. | ||
| 288 | + return (enum_type)(underlying) ~std::size_t (e); | ||
| 289 | } | ||
| 290 | |||
| 291 | template <typename enum_type, | ||
| 292 | typename = is_enum_flags_enum_type_t<enum_type>, | ||
| 293 | - typename = gdb::Requires<enum_flags_detail::EnumIsSigned<enum_type>>> | ||
| 294 | + typename = gdb::Requires<enum_flags_detail::EnumIsUnsafeForBitwiseComplement<enum_type>>> | ||
| 295 | constexpr void operator~ (enum_flags<enum_type> e) = delete; | ||
| 296 | |||
| 297 | /* Delete operator<< and operator>>. */ | ||
| 298 | diff --git a/include/diagnostics.h b/include/diagnostics.h | ||
| 299 | index 97e30ab..14575e2 100644 | ||
| 300 | --- a/include/diagnostics.h | ||
| 301 | +++ b/include/diagnostics.h | ||
| 302 | @@ -76,11 +76,6 @@ | ||
| 303 | # define DIAGNOSTIC_ERROR_SWITCH \ | ||
| 304 | DIAGNOSTIC_ERROR ("-Wswitch") | ||
| 305 | |||
| 306 | -# if __has_warning ("-Wenum-constexpr-conversion") | ||
| 307 | -# define DIAGNOSTIC_IGNORE_ENUM_CONSTEXPR_CONVERSION \ | ||
| 308 | - DIAGNOSTIC_IGNORE ("-Wenum-constexpr-conversion") | ||
| 309 | -# endif | ||
| 310 | - | ||
| 311 | #elif defined (__GNUC__) /* GCC */ | ||
| 312 | |||
| 313 | # define DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS \ | ||
