Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 7 May 2023 18:39:39 GMT
From:      Dimitry Andric <dim@FreeBSD.org>
To:        ports-committers@FreeBSD.org, dev-commits-ports-all@FreeBSD.org, dev-commits-ports-main@FreeBSD.org
Subject:   git: 9e765fe77dab - main - devel/gdb: fix build with clang 16
Message-ID:  <202305071839.347IddHU074697@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by dim:

URL: https://cgit.FreeBSD.org/ports/commit/?id=9e765fe77dabf9e37b052510072b0f41a2ab3ae6

commit 9e765fe77dabf9e37b052510072b0f41a2ab3ae6
Author:     Dimitry Andric <dim@FreeBSD.org>
AuthorDate: 2023-04-24 15:00:37 +0000
Commit:     Dimitry Andric <dim@FreeBSD.org>
CommitDate: 2023-05-07 18:38:47 +0000

    devel/gdb: fix build with clang 16
    
    Clang 16 has a new error about integer values being outside the valid
    range for enum types, which shows up when building gdb:
    
      In file included from /wrkdirs/usr/ports/devel/gdb/work-py39/gdb-13.1/gdb/x86-fbsd-nat.c:20:
      In file included from /wrkdirs/usr/ports/devel/gdb/work-py39/gdb-13.1/gdb/defs.h:65:
      /wrkdirs/usr/ports/devel/gdb/work-py39/gdb-13.1/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
                                                         ^
      In file included from /wrkdirs/usr/ports/devel/gdb/work-py39/gdb-13.1/gdb/x86-bsd-nat.c:20:
      In file included from /wrkdirs/usr/ports/devel/gdb/work-py39/gdb-13.1/gdb/defs.h:65:
      /wrkdirs/usr/ports/devel/gdb/work-py39/gdb-13.1/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
                                                         ^
      In file included from /wrkdirs/usr/ports/devel/gdb/work-py39/gdb-13.1/gdb/x86-nat.c:20:
      In file included from /wrkdirs/usr/ports/devel/gdb/work-py39/gdb-13.1/gdb/defs.h:65:
      /wrkdirs/usr/ports/devel/gdb/work-py39/gdb-13.1/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
                                                         ^
      In file included from /wrkdirs/usr/ports/devel/gdb/work-py39/gdb-13.1/gdb/windows-tdep.c:18:
      In file included from /wrkdirs/usr/ports/devel/gdb/work-py39/gdb-13.1/gdb/defs.h:65:
      /wrkdirs/usr/ports/devel/gdb/work-py39/gdb-13.1/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
                                                         ^
    
    Upstream already noticed this, and committed
    https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=ae61525fcf4 as
    a workaround, so add this as an additional patch, until gdb 13.2 is
    released.
    
    PR:             271045
    Approved by:    maintainer timeout (2 weeks)
    MFH:            2023Q2
---
 devel/gdb/Makefile                 |   3 +-
 devel/gdb/files/commit-ae61525fcf4 | 128 +++++++++++++++++++++++++++++++++++++
 2 files changed, 130 insertions(+), 1 deletion(-)

diff --git a/devel/gdb/Makefile b/devel/gdb/Makefile
index d11470e63b41..d0158a6c6b56 100644
--- a/devel/gdb/Makefile
+++ b/devel/gdb/Makefile
@@ -1,6 +1,6 @@
 PORTNAME=	gdb
 DISTVERSION=	13.1
-PORTREVISION=	1
+PORTREVISION=	2
 CATEGORIES=	devel
 MASTER_SITES=	GNU
 
@@ -36,6 +36,7 @@ CFLAGS+=	-DRL_NO_COMPAT
 EXCLUDE=	dejagnu expect sim texinfo intl
 EXTRACT_AFTER_ARGS=	${EXCLUDE:S/^/--exclude /}
 EXTRA_PATCHES=	${FILESDIR}/commit-a980a7d24b9
+EXTRA_PATCHES+=	${FILESDIR}/commit-ae61525fcf4
 
 VER=		${DISTVERSION:S/.//g}
 PLIST_SUB=	VER=${VER}
diff --git a/devel/gdb/files/commit-ae61525fcf4 b/devel/gdb/files/commit-ae61525fcf4
new file mode 100644
index 000000000000..202f695b654c
--- /dev/null
+++ b/devel/gdb/files/commit-ae61525fcf4
@@ -0,0 +1,128 @@
+commit ae61525fcf456ab395d55c45492a106d1275873a
+Author: Simon Marchi <simon.marchi@efficios.com>
+Date:   2023-02-23 12:35:40 -0500
+
+    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
+
+diff --git gdbsupport/enum-flags.h gdbsupport/enum-flags.h
+index 700037f6126..41ac7838f06 100644
+--- gdbsupport/enum-flags.h
++++ 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 include/diagnostics.h include/diagnostics.h
+index d3ff27bc008..41e6db65391 100644
+--- include/diagnostics.h
++++ 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 */



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202305071839.347IddHU074697>