summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKhem Raj <raj.khem@gmail.com>2024-08-28 12:41:32 -0700
committerRichard Purdie <richard.purdie@linuxfoundation.org>2024-08-29 21:58:19 +0100
commit088d9fe6b7086802367d7a080da0c1465862c24f (patch)
tree161d15fcb262877feb6b77bdc3ac3039a648a950
parent5aeabd3217da2a7ce9f789d5d224f6682b4555a1 (diff)
downloadpoky-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.inc1
-rw-r--r--meta/recipes-devtools/gdb/gdb/0001-Fix-Wenum-constexpr-conversion-in-enum-flags.h.patch313
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 "
16SRC_URI[sha256sum] = "38254eacd4572134bca9c5a5aa4d4ca564cbbd30c369d881f733fb6b903354f2" 17SRC_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 @@
1From dd22f64329c46797b3a3de2605ad1fa14cf77dd4 Mon Sep 17 00:00:00 2001
2From: Carlos Galvez <carlosgalvezp@gmail.com>
3Date: Sun, 30 Jun 2024 21:36:24 +0200
4Subject: [PATCH] Fix -Wenum-constexpr-conversion in enum-flags.h
5
6This fixes PR 31331:
7https://sourceware.org/bugzilla/show_bug.cgi?id=31331
8
9Currently, enum-flags.h is suppressing the warning
10-Wenum-constexpr-conversion coming from recent versions of Clang.
11This warning is intended to be made a compiler error
12(non-downgradeable) in future versions of Clang:
13
14https://github.com/llvm/llvm-project/issues/59036
15
16The rationale is that casting a value of an integral type into an
17enumeration is Undefined Behavior if the value does not fit in the
18range of values of the enum:
19https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1766
20
21Undefined Behavior is not allowed in constant expressions, leading to
22an ill-formed program.
23
24In this case, in enum-flags.h, we are casting the value -1 to an enum
25of a positive range only, which is UB as per the Standard and thus not
26allowed in a constexpr context.
27
28The purpose of doing this instead of using std::underlying_type is
29because, for C-style enums, std::underlying_type typically returns
30"unsigned int". However, when operating with it arithmetically, the
31enum is promoted to *signed* int, which is what we want to avoid.
32
33This 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
86Since this is the only place where the warning was suppressed, remove
87also the corresponding macro in include/diagnostics.h.
88
89The change has been tested by running the entire gdb test suite
90(make check) and comparing the results (testsuite/gdb.sum) against
91trunk. No noticeable differences have been observed.
92Tested-by: Keith Seitz <keiths@redhat.com>
93
94Signed-off-by: Khem Raj <raj.khem@gmail.com>
95Upstream-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
102diff --git a/gdb/unittests/enum-flags-selftests.c b/gdb/unittests/enum-flags-selftests.c
103index 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
140diff --git a/gdbsupport/enum-flags.h b/gdbsupport/enum-flags.h
141index 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>>. */
298diff --git a/include/diagnostics.h b/include/diagnostics.h
299index 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 \