mirror of
https://github.com/bminor/binutils-gdb.git
synced 2025-12-05 15:15:42 +00:00
4a0b2cb721
This fixes PR 31331: https://sourceware.org/bugzilla/show_bug.cgi?id=31331 Currently, enum-flags.h is suppressing the warning -Wenum-constexpr-conversion coming from recent versions of Clang. This warning is intended to be made a compiler error (non-downgradeable) in future versions of Clang: https://github.com/llvm/llvm-project/issues/59036 The rationale is that casting a value of an integral type into an enumeration is Undefined Behavior if the value does not fit in the range of values of the enum: https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1766 Undefined Behavior is not allowed in constant expressions, leading to an ill-formed program. In this case, in enum-flags.h, we are casting the value -1 to an enum of a positive range only, which is UB as per the Standard and thus not allowed in a constexpr context. The purpose of doing this instead of using std::underlying_type is because, for C-style enums, std::underlying_type typically returns "unsigned int". However, when operating with it arithmetically, the enum is promoted to *signed* int, which is what we want to avoid. This patch solves this issue as follows: * Use std::underlying_type and remove the custom enum_underlying_type. * Ensure that operator~ is called always on an unsigned integer. We do this by casting the input enum into std::size_t, which can fit any unsigned integer. We have the guarantee that the cast is safe, because we have checked that the underlying type is unsigned. If the enum had negative values, the underlying type would be signed. This solves the issue with C-style enums, but also solves a hidden issue: enums with underlying type of std::uint8_t or std::uint16_t are *also* promoted to signed int. Now they are all explicitly casted to the largest unsigned int type and operator~ is safe to use. * There is one more thing that needs fix. Currently, operator~ is implemented as follows: return (enum_type) ~underlying(e); After applying ~underlying(e), the result is a very large value, which we then cast to "enum_type". This cast is Undefined Behavior if the large value does not fit in the range of the enum. For C++ enums (scoped and/or with explicit underlying type), the range of the enum is the entire range of the underlying type, so the cast is safe. However, for C-style enums, the range is the smallest bit-field that can hold all the values of the enumeration. So the range is a lot smaller and casting a large value to the enum would invoke Undefined Behavior. To solve this problem, we create a new trait EnumHasFixedUnderlyingType, to ensure operator~ may only be called on C++-style enums. This behavior is roughly the same as what we had on trunk, but relying on different properties of the enums. * Once this is implemented, the following tests fail to compile: CHECK_VALID (true, int, true ? EF () : EF2 ()) This is because it expects the enums to be promoted to signed int, instead of unsigned int (which is the true underlying type). I propose to remove these tests altogether, because: - The comment nearby say they are not very important. - Comparing 2 enums of different type like that is strange, relies on integer promotions and thus hurts readability. As per comments in the related PR, we likely don't want this type of code in gdb code anyway, so there's no point in testing it. - Most importantly, this type of comparison will be ill-formed in C++26 for regular enums, so enum_flags does not need to emulate that. Since this is the only place where the warning was suppressed, remove also the corresponding macro in include/diagnostics.h. The change has been tested by running the entire gdb test suite (make check) and comparing the results (testsuite/gdb.sum) against trunk. No noticeable differences have been observed. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31331 Tested-by: Keith Seitz <keiths@redhat.com> Approved-By: Tom Tromey <tom@tromey.com>
158 lines
4.9 KiB
C
158 lines
4.9 KiB
C
/* Copyright (C) 2017-2024 Free Software Foundation, Inc.
|
|
|
|
This program is free software; you can redistribute it and/or modify
|
|
it under the terms of the GNU General Public License as published by
|
|
the Free Software Foundation; either version 3 of the License, or
|
|
(at your option) any later version.
|
|
|
|
This program is distributed in the hope that it will be useful,
|
|
but WITHOUT ANY WARRANTY; without even the implied warranty of
|
|
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
|
GNU General Public License for more details.
|
|
|
|
You should have received a copy of the GNU General Public License
|
|
along with this program. If not, see <http://www.gnu.org/licenses/>. */
|
|
|
|
#ifndef DIAGNOSTICS_H
|
|
#define DIAGNOSTICS_H
|
|
|
|
/* If at all possible, fix the source rather than using these macros
|
|
to silence warnings. If you do use these macros be aware that
|
|
you'll need to condition their use on particular compiler versions,
|
|
which can be done for gcc using ansidecl.h's GCC_VERSION macro.
|
|
|
|
gcc versions between 4.2 and 4.6 do not allow pragma control of
|
|
diagnostics inside functions, giving a hard error if you try to use
|
|
the finer control available with later versions.
|
|
gcc prior to 4.2 warns about diagnostic push and pop.
|
|
|
|
The other macros have restrictions too, for example gcc-5, gcc-6
|
|
and gcc-7 warn that -Wstringop-truncation is unknown, unless you
|
|
also add DIAGNOSTIC_IGNORE ("-Wpragma"). */
|
|
|
|
#ifdef __GNUC__
|
|
# define DIAGNOSTIC_PUSH _Pragma ("GCC diagnostic push")
|
|
# define DIAGNOSTIC_POP _Pragma ("GCC diagnostic pop")
|
|
|
|
/* Stringification. */
|
|
# define DIAGNOSTIC_STRINGIFY_1(x) #x
|
|
# define DIAGNOSTIC_STRINGIFY(x) DIAGNOSTIC_STRINGIFY_1 (x)
|
|
|
|
# define DIAGNOSTIC_IGNORE(option) \
|
|
_Pragma (DIAGNOSTIC_STRINGIFY (GCC diagnostic ignored option))
|
|
# define DIAGNOSTIC_ERROR(option) \
|
|
_Pragma (DIAGNOSTIC_STRINGIFY (GCC diagnostic error option))
|
|
#else
|
|
# define DIAGNOSTIC_PUSH
|
|
# define DIAGNOSTIC_POP
|
|
# define DIAGNOSTIC_IGNORE(option)
|
|
#endif
|
|
|
|
#if defined (__clang__) /* clang */
|
|
|
|
# define DIAGNOSTIC_IGNORE_SELF_MOVE DIAGNOSTIC_IGNORE ("-Wself-move")
|
|
# define DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS \
|
|
DIAGNOSTIC_IGNORE ("-Wdeprecated-declarations")
|
|
# define DIAGNOSTIC_IGNORE_REGISTER DIAGNOSTIC_IGNORE ("-Wregister")
|
|
|
|
# if __has_warning ("-Wenum-compare-switch")
|
|
# define DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES \
|
|
DIAGNOSTIC_IGNORE ("-Wenum-compare-switch")
|
|
# endif
|
|
|
|
# define DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL \
|
|
DIAGNOSTIC_IGNORE ("-Wformat-nonliteral")
|
|
|
|
# if __has_warning ("-Wuser-defined-warnings")
|
|
# define DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS \
|
|
DIAGNOSTIC_IGNORE ("-Wuser-defined-warnings")
|
|
# endif
|
|
|
|
# if __has_warning ("-Wunused-but-set-variable")
|
|
# define DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE \
|
|
DIAGNOSTIC_IGNORE ("-Wunused-but-set-variable")
|
|
# endif
|
|
|
|
# define DIAGNOSTIC_ERROR_SWITCH \
|
|
DIAGNOSTIC_ERROR ("-Wswitch")
|
|
|
|
#elif defined (__GNUC__) /* GCC */
|
|
|
|
# define DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS \
|
|
DIAGNOSTIC_IGNORE ("-Wdeprecated-declarations")
|
|
|
|
# if __GNUC__ >= 7
|
|
# define DIAGNOSTIC_IGNORE_REGISTER DIAGNOSTIC_IGNORE ("-Wregister")
|
|
# endif
|
|
|
|
# define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION \
|
|
DIAGNOSTIC_IGNORE ("-Wstringop-truncation")
|
|
|
|
# if __GNUC__ >= 11
|
|
# define DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD \
|
|
DIAGNOSTIC_IGNORE ("-Wstringop-overread")
|
|
#endif
|
|
|
|
# define DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL \
|
|
DIAGNOSTIC_IGNORE ("-Wformat-nonliteral")
|
|
|
|
# if __GNUC__ >= 5
|
|
# define DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE \
|
|
DIAGNOSTIC_IGNORE ("-Wunused-but-set-variable")
|
|
# endif
|
|
|
|
# if __GNUC__ >= 13
|
|
# define DIAGNOSTIC_IGNORE_SELF_MOVE DIAGNOSTIC_IGNORE ("-Wself-move")
|
|
# endif
|
|
|
|
/* GCC 4.8's "diagnostic push/pop" seems broken when using this, -Wswitch
|
|
remains enabled at the error level even after a pop. Therefore, don't
|
|
use it for GCC < 5. */
|
|
# if __GNUC__ >= 5
|
|
# define DIAGNOSTIC_ERROR_SWITCH DIAGNOSTIC_ERROR ("-Wswitch")
|
|
# endif
|
|
|
|
#endif
|
|
|
|
#ifndef DIAGNOSTIC_IGNORE_SELF_MOVE
|
|
# define DIAGNOSTIC_IGNORE_SELF_MOVE
|
|
#endif
|
|
|
|
#ifndef DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS
|
|
# define DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS
|
|
#endif
|
|
|
|
#ifndef DIAGNOSTIC_IGNORE_REGISTER
|
|
# define DIAGNOSTIC_IGNORE_REGISTER
|
|
#endif
|
|
|
|
#ifndef DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES
|
|
# define DIAGNOSTIC_IGNORE_SWITCH_DIFFERENT_ENUM_TYPES
|
|
#endif
|
|
|
|
#ifndef DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION
|
|
# define DIAGNOSTIC_IGNORE_STRINGOP_TRUNCATION
|
|
#endif
|
|
|
|
#ifndef DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD
|
|
# define DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD
|
|
#endif
|
|
|
|
#ifndef DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
|
|
# define DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
|
|
#endif
|
|
|
|
#ifndef DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS
|
|
# define DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS
|
|
#endif
|
|
|
|
#ifndef DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE
|
|
# define DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE
|
|
#endif
|
|
|
|
#ifndef DIAGNOSTIC_ERROR_SWITCH
|
|
# define DIAGNOSTIC_ERROR_SWITCH
|
|
#endif
|
|
|
|
#endif /* DIAGNOSTICS_H */
|