Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport gdb patch to fix integer value -1 outside valid range error in clang 16+ #188

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yanyir
Copy link

@yanyir yanyir commented Aug 31, 2024

When building crash with clang 17, I encountered numerous error: integer value -1 is outside the valid range of values errors, such as:

./../gdbsupport/enum-flags.h:85:52: error: integer value -1 is outside the valid range of values [0, 15] for the enumeration type 'ui_out_flag' [-Wenum-constexpr-conversion]
   85 |     integer_for_size<sizeof (T), static_cast<bool>(T (-1) < T (0))>::type
      |                                                    ^
In file included from cp-valprint.c:20:
In file included from ./defs.h:65:
./../gdbsupport/enum-flags.h:85:52: error: integer value -1 is outside the valid range of values [0, 15] for the enumeration type 'ui_out_flag' [-Wenum-constexpr-conversion]
   85 |     integer_for_size<sizeof (T), static_cast<bool>(T (-1) < T (0))>::type
      |                                                    ^
In file included from d-lang.c:20:
In file included from ./defs.h:65:
./../gdbsupport/enum-flags.h:85:52: error: integer value -1 is outside the valid range of values [0, 15] for the enumeration type 'ui_out_flag' [-Wenum-constexpr-conversion]
   85 |     integer_for_size<sizeof (T), static_cast<bool>(T (-1) < T (0))>::type
      |                                                    ^
In file included from cp-namespace.c:21:
In file included from ./defs.h:65:
./../gdbsupport/enum-flags.h:85:52: error: integer value -1 is outside the valid range of values [0, 15] for the enumeration type 'ui_out_flag' [-Wenum-constexpr-conversion]
   85 |     integer_for_size<sizeof (T), static_cast<bool>(T (-1) < T (0))>::type
      |                                                    ^
In file included from d-namespace.c:20:
In file included from ./defs.h:65:
./../gdbsupport/enum-flags.h:85:52: error: integer value -1 is outside the valid range of values [0, 15] for the enumeration type 'ui_out_flag' [-Wenum-constexpr-conversion]
   85 |     integer_for_size<sizeof (T), static_cast<bool>(T (-1) < T (0))>::type
      |                                                    ^
In file included from ctfread.c:78:
In file included from ./defs.h:65:
./../gdbsupport/enum-flags.h:85:52: error: integer value -1 is outside the valid range of values [0, 15] for the enumeration type 'ui_out_flag' [-Wenum-constexpr-conversion]
   85 |     integer_for_size<sizeof (T), static_cast<bool>(T (-1) < T (0))>::type
      |                                                    ^
In file included from d-valprint.c:20:
In file included from ./defs.h:65:
./../gdbsupport/enum-flags.h:85:52: error: integer value -1 is outside the valid range of values [0, 15] for the enumeration type 'ui_out_flag' [-Wenum-constexpr-conversion]
   85 |     integer_for_size<sizeof (T), static_cast<bool>(T (-1) < T (0))>::type
      |                                                    ^
./../gdbsupport/enum-flags.h:85:52: error: integer value -1 is outside the valid range of values [0, 3] for the enumeration type 'innermost_block_tracker_type' [-Wenum-constexpr-conversion]
./../gdbsupport/enum-flags.h:85:52: error: integer value -1 is outside the valid range of values [0, 3] for the enumeration type 'innermost_block_tracker_type' [-Wenum-constexpr-conversion]
./../gdbsupport/enum-flags.h:85:52: error: integer value -1 is outside the valid range of values [0, 1] for the enumeration type 'btrace_insn_flag' [-Wenum-constexpr-conversion]
./../gdbsupport/enum-flags.h:85:52: error: integer value -1 is outside the valid range of values [0, 3] for the enumeration type 'btrace_function_flag' [-Wenum-constexpr-conversion]
./../gdbsupport/enum-flags.h:85:52: error: integer value -1 is outside the valid range of values [0, 1] for the enumeration type 'btrace_insn_flag' [-Wenum-constexpr-conversion]
./../gdbsupport/enum-flags.h:85:52: error: integer value -1 is outside the valid range of values [0, 3] for the enumeration type 'btrace_function_flag' [-Wenum-constexpr-conversion]
In file included from dbxread.c:33:
In file included from ./defs.h:65:
./../gdbsupport/enum-flags.h:85:52: error: integer value -1 is outside the valid range of values [0, 15] for the enumeration type 'ui_out_flag' [-Wenum-constexpr-conversion]
   85 |     integer_for_size<sizeof (T), static_cast<bool>(T (-1) < T (0))>::type
      |                                                    ^
...

GDB ignore -Wenum-constexpr-conversion around enum-flags.h to solve the problem.

After applying this patch form GDB, my build succeeded.

Related GDB commit:
8cbde735e40551a694d4ee5cfd4d525d3d77ff7e ("gdbsupport: ignore -Wenum-constexpr-conversion in enum-flags.h")

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(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_flagsselftests::enum_flags_tests::RE, selftests::enum_flags_tests::RE, enum_flagsselftests::enum_flags_tests::RE2, selftests::enum_flags_tests::RE2, enum_flagsselftests::enum_flags_tests::URE, selftests::enum_fla
gs_tests::URE, int>, selftests::enum_flags_tests::check_valid_expr254::archetype<[enum_flagsselftests::enum_flags_tests::RE](https://github.com/crash-utility/crash/compare/enum_flags%3Cselftests::enum_flags_tests::RE), selftests::enum_flags_tests::RE, enum_flagsselftests::enum_flags_tests::RE2, selfte
sts::enum_flags_tests::RE2, enum_flagsselftests::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

Backport gdb patch to fix the "integer value -1 is outside the valid range of values" errors when building with clang 16+.

Without the patch, when using clang 16 to build, we get some errors like:
    In file included from cp-valprint.c:20:
    In file included from ./defs.h:65:
    ./../gdbsupport/enum-flags.h:85:52: error: integer value -1 is outside the valid range of values [0, 15] for the enumeration type 'ui_out_flag' [-Wenum-constexpr-conversion]
       85 |     integer_for_size<sizeof (T), static_cast<bool>(T (-1) < T (0))>::type
          |                                                    ^                                           ^

GDB ignore -Wenum-constexpr-conversion around enum-flags.h to solve the problem.

Related GDB commit:
8cbde735e405 "gdbsupport: ignore -Wenum-constexpr-conversion in enum-flags.h"
@yanyir
Copy link
Author

yanyir commented Sep 6, 2024

Sorry, I happened to not backport the full patch of commit before. Now it should be complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant