summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndrew Kaster <akaster@serenityos.org>2023-03-31 18:16:14 -0600
committerAndreas Kling <kling@serenityos.org>2023-04-02 10:52:28 +0200
commitd74fa5e2832874dea5f721762c80dcae3a53c834 (patch)
treee0b1717a330759c000a8a662ffa525d3a4db7b54
parentaf0120373335b9f3f6f1a589b385f7d81788c33e (diff)
downloadserenity-d74fa5e2832874dea5f721762c80dcae3a53c834.zip
Toolchain: Allow many patches for gdb and add clang workaround
This ports https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=ae61525fcf456ab395d55c45492a106d1275873a from upstream binutils, which enables building with the newest host clang versions.
-rwxr-xr-xToolchain/BuildIt.sh8
-rw-r--r--Toolchain/Patches/gdb/clang-constexpr-conversion-warning.patch134
-rw-r--r--Toolchain/Patches/gdb/gdb.patch (renamed from Toolchain/Patches/gdb.patch)0
3 files changed, 139 insertions, 3 deletions
diff --git a/Toolchain/BuildIt.sh b/Toolchain/BuildIt.sh
index 1147422e66..3a0473c030 100755
--- a/Toolchain/BuildIt.sh
+++ b/Toolchain/BuildIt.sh
@@ -234,11 +234,13 @@ pushd "$DIR/Tarballs"
git init > /dev/null
git add . > /dev/null
git commit -am "BASE" > /dev/null
- git apply "$DIR"/Patches/gdb.patch > /dev/null
+ git am "${DIR}"/Patches/gdb/*.patch > /dev/null
else
- patch -p1 < "$DIR"/Patches/gdb.patch > /dev/null
+ for patch in "${DIR}"/Patches/gdb/*.patch; do
+ patch -p1 < "${patch}" > /dev/null
+ done
fi
- $MD5SUM "$DIR"/Patches/gdb.patch > .patch.applied
+ $MD5SUM "$DIR"/Patches/gdb/*.patch > .patch.applied
popd
fi
diff --git a/Toolchain/Patches/gdb/clang-constexpr-conversion-warning.patch b/Toolchain/Patches/gdb/clang-constexpr-conversion-warning.patch
new file mode 100644
index 0000000000..a2458913b7
--- /dev/null
+++ b/Toolchain/Patches/gdb/clang-constexpr-conversion-warning.patch
@@ -0,0 +1,134 @@
+From ae61525fcf456ab395d55c45492a106d1275873a Mon Sep 17 00:00:00 2001
+From: Simon Marchi <simon.marchi@efficios.com>
+Date: Thu, 23 Feb 2023 12:35:40 -0500
+Subject: [PATCH] gdbsupport: ignore -Wenum-constexpr-conversion in
+ enum-flags.h
+
+When building with clang 16, we get:
+
+ CXX gdb.o
+ In file included from /home/smarchi/src/binutils-gdb/gdb/gdb.c:19:
+ In file included from /home/smarchi/src/binutils-gdb/gdb/defs.h:65:
+ /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/enum-flags.h:95:52: error: integer value -1 is outside the valid range of values [0, 15] for this enumeration type [-Wenum-constexpr-conversion]
+ integer_for_size<sizeof (T), static_cast<bool>(T (-1) < T (0))>::type
+ ^
+
+The error message does not make it clear in the context of which enum
+flag this fails (i.e. what is T in this context), but it doesn't really
+matter, we have similar warning/errors for many of them, if we let the
+build go through.
+
+clang is right that the value -1 is invalid for the enum type we cast -1
+to. However, we do need this expression in order to select an integer
+type with the appropriate signedness. That is, with the same signedness
+as the underlying type of the enum.
+
+I first wondered if that was really needed, if we couldn't use
+std::underlying_type for that. It turns out that the comment just above
+says:
+
+ /* 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. */
+
+I was surprised, because std::is_signed<std::underlying_type<enum_type>>
+returns the right thing. So I tried replacing all this with
+std::underlying_type, see if that would work. Doing so causes some
+build failures in unittests/enum-flags-selftests.c:
+
+ CXX unittests/enum-flags-selftests.o
+ /home/smarchi/src/binutils-gdb/gdb/unittests/enum-flags-selftests.c:254:1: error: static assertion failed due to requirement 'gdb::is_same<selftests::enum_flags_tests::check_valid_expr254::archetype<enum_flags<s
+ elftests::enum_flags_tests::RE>, selftests::enum_flags_tests::RE, enum_flags<selftests::enum_flags_tests::RE2>, selftests::enum_flags_tests::RE2, enum_flags<selftests::enum_flags_tests::URE>, selftests::enum_fla
+ gs_tests::URE, int>, selftests::enum_flags_tests::check_valid_expr254::archetype<enum_flags<selftests::enum_flags_tests::RE>, selftests::enum_flags_tests::RE, enum_flags<selftests::enum_flags_tests::RE2>, selfte
+ sts::enum_flags_tests::RE2, enum_flags<selftests::enum_flags_tests::URE>, selftests::enum_flags_tests::URE, unsigned int>>::value == true':
+ CHECK_VALID (true, int, true ? EF () : EF2 ())
+ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ /home/smarchi/src/binutils-gdb/gdb/unittests/enum-flags-selftests.c:91:3: note: expanded from macro 'CHECK_VALID'
+ CHECK_VALID_EXPR_6 (EF, RE, EF2, RE2, UEF, URE, VALID, EXPR_TYPE, EXPR)
+ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/valid-expr.h:105:3: note: expanded from macro 'CHECK_VALID_EXPR_6'
+ CHECK_VALID_EXPR_INT (ESC_PARENS (typename T1, typename T2, \
+ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/valid-expr.h:66:3: note: expanded from macro 'CHECK_VALID_EXPR_INT'
+ static_assert (gdb::is_detected_exact<archetype<TYPES, EXPR_TYPE>, \
+ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+This is a bit hard to decode, but basically enumerations have the
+following funny property that they decay into a signed int, even if
+their implicit underlying type is unsigned. This code:
+
+ enum A {};
+ enum B {};
+
+ int main() {
+ std::cout << std::is_signed<std::underlying_type<A>::type>::value
+ << std::endl;
+ std::cout << std::is_signed<std::underlying_type<B>::type>::value
+ << std::endl;
+ auto result = true ? A() : B();
+ std::cout << std::is_signed<decltype(result)>::value << std::endl;
+ }
+
+produces:
+
+ 0
+ 0
+ 1
+
+So, the "CHECK_VALID" above checks that this property works for enum flags the
+same way as it would if you were using their underlying enum types. And
+somehow, changing integer_for_size to use std::underlying_type breaks that.
+
+Since the current code does what we want, and I don't see any way of doing it
+differently, ignore -Wenum-constexpr-conversion around it.
+
+Change-Id: Ibc82ae7bbdb812102ae3f1dd099fc859dc6f3cc2
+---
+ gdbsupport/enum-flags.h | 3 +++
+ include/diagnostics.h | 9 +++++++++
+ 2 files changed, 12 insertions(+)
+
+diff --git a/gdbsupport/enum-flags.h b/gdbsupport/enum-flags.h
+index 700037f6126..41ac7838f06 100644
+--- a/gdbsupport/enum-flags.h
++++ b/gdbsupport/enum-flags.h
+@@ -91,9 +91,12 @@ 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
+diff --git a/include/diagnostics.h b/include/diagnostics.h
+index d3ff27bc008..41e6db65391 100644
+--- a/include/diagnostics.h
++++ b/include/diagnostics.h
+@@ -76,6 +76,11 @@
+ # 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 \
+@@ -155,4 +160,8 @@
+ # define DIAGNOSTIC_ERROR_SWITCH
+ #endif
+
++#ifndef DIAGNOSTIC_IGNORE_ENUM_CONSTEXPR_CONVERSION
++# define DIAGNOSTIC_IGNORE_ENUM_CONSTEXPR_CONVERSION
++#endif
++
+ #endif /* DIAGNOSTICS_H */
+--
+2.31.1
diff --git a/Toolchain/Patches/gdb.patch b/Toolchain/Patches/gdb/gdb.patch
index 1784fab165..1784fab165 100644
--- a/Toolchain/Patches/gdb.patch
+++ b/Toolchain/Patches/gdb/gdb.patch