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 \ | ||