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>
When building GDB on Centos 7 (which has flex 2.5.37) and Clang, I get:
$ make ada-exp.o
YACC ada-exp.c
LEX ada-lex.c
CXX ada-exp.o
In file included from /home/smarchi/src/binutils-gdb/gdb/ada-exp.y:1179:
<stdout>:1106:2: error: ISO C++17 does not allow 'register' storage class specifier [-Wregister]
1106 | register yy_state_type yy_current_state;
| ^~~~~~~~
In ada-lex.l, we already use `DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER`,
which for Clang translates to ignoring `-Wdeprecated-register` [1]. I think
that was produced when compiling as C++11, but now that we always compile as
C++17, Clang produces a `-Wregister` error [2].
For GCC, `DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER` already translates to
ignoring `-Wregister`. So, rename
`DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER` to `DIAGNOSTIC_IGNORE_REGISTER`
and ignore `-Wregister` for Clang too.
[1] https://releases.llvm.org/17.0.1/tools/clang/docs/DiagnosticsReference.html#wdeprecated-register
[2] https://releases.llvm.org/17.0.1/tools/clang/docs/DiagnosticsReference.html#wregister
include/ChangeLog:
* diagnostics.h (DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER): Rename
to...
(DIAGNOSTIC_IGNORE_REGISTER): ... this. Ignore `-Wregister`
instead of `-Wdeprecated-register`.
Change-Id: I8a4a51c7222c68577fa22ecacdddfcba32d9dbc5
Adds two new external authors to etc/update-copyright.py to cover
bfd/ax_tls.m4, and adds gprofng to dirs handled automatically, then
updates copyright messages as follows:
1) Update cgen/utils.scm emitted copyrights.
2) Run "etc/update-copyright.py --this-year" with an extra external
author I haven't committed, 'Kalray SA.', to cover gas testsuite
files (which should have their copyright message removed).
3) Build with --enable-maintainer-mode --enable-cgen-maint=yes.
4) Check out */po/*.pot which we don't update frequently.
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
The newer update-copyright.py fixes file encoding too, removing cr/lf
on binutils/bfdtest2.c and ld/testsuite/ld-cygwin/exe-export.exp, and
embedded cr in binutils/testsuite/binutils-all/ar.exp string match.
"-Wdeprecated-declarations" warning option can be helpful to track
deprecated function delarations but sometimes we need to disable this
warning for a good reason.
DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS is an existing macro but only
defined on Clang. Since "-Wdeprecated-declarations" is also available on
GCC (>= 3.4.0), this commit adds equivalent definition as Clang.
__GNUC__ and __GNUC_MINOR__ are not checked because this header file seems
to assume GCC >= 4.6 (with "GCC diagnostic push/pop").
include/ChangeLog:
* diagnostics.h (DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS):
Define also on GCC.
"-Wunused-but-set-variable" warning option can be helpful to track variables
that are written but not read thereafter. But it can be harmful if some of
the code is auto-generated and we have no ways to deal with it.
The particular example is Bison-generated code.
The new DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE macro can be helpful on
such cases. A typical use of this macro is to place this macro before the
end of user prologues on Bison (.y) files.
include/ChangeLog:
* diagnostics.h (DIAGNOSTIC_IGNORE_UNUSED_BUT_SET_VARIABLE): New.
User-defined warnings (on Clang, "-Wuser-defined-warnings") can be harmful
if we have specified "-Werror" and we have no control to disable the warning
ourself. The particular example is Gnulib.
Gnulib generates a warning if the system version of certain functions
are used (to redirect the developer to use Gnulib version). However,
it can be harmful if we cannot easily replace them (e.g. the target is in
the standard C++ library).
The new DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS macro can be helpful on such
cases. A typical use of this macro is to place this macro before including
certain system headers.
include/ChangeLog:
* diagnostics.h (DIAGNOSTIC_IGNORE_USER_DEFINED_WARNINGS): New.
When building trunk on openSUSE Leap 15.3 with system gcc 7.5.0, I run into:
...
In file included from ../bfd/bfd.h:46:0,
from gdb/defs.h:37,
from gdb/debuginfod-support.c:19:
gdb/debuginfod-support.c: In function ‘bool debuginfod_is_enabled()’:
gdb/../include/diagnostics.h:42:3: error: unknown option after \
‘#pragma GCC diagnostic’ kind [-Werror=pragmas]
_Pragma (DIAGNOSTIC_STRINGIFY (GCC diagnostic ignored option))
^
gdb/../include/diagnostics.h:80:3: note: in expansion of macro \
‘DIAGNOSTIC_IGNORE’
DIAGNOSTIC_IGNORE ("-Wstringop-overread")
^~~~~~~~~~~~~~~~~
gdb/debuginfod-support.c:201:4: note: in expansion of macro \
‘DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD’
DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD
^
...
The problem is that the warning -Wstringop-overread has been introduced for
gcc 11, and we can only tell gcc to ignore if it knows about it.
Fix this by guarding the DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD definition in
diagnostics.c with '#if __GNUC__ >= 11'.
Tested on x86_64-linux, by completing a build.
For some reason g++ 11.2.1 on s390x produces a spurious warning for
stringop-overread in debuginfod_is_enabled for url_view. Add a new
DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD macro to suppress this warning.
include/ChangeLog:
* diagnostics.h (DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD): New
macro.
gdb/ChangeLog:
* debuginfod-support.c (debuginfod_is_enabled): Use
DIAGNOSTIC_IGNORE_STRINGOP_OVERREAD on s390x.
The result of running etc/update-copyright.py --this-year, fixing all
the files whose mode is changed by the script, plus a build with
--enable-maintainer-mode --enable-cgen-maint=yes, then checking
out */po/*.pot which we don't update frequently.
The copy of cgen was with commit d1dd5fcc38ead reverted as that commit
breaks building of bfp opcodes files.
When introducing this code, I forgot that we had some macros for this.
Replace some "manual" pragma diagnostic with some DIAGNOSTIC_* macros,
provided by include/diagnostics.h.
In diagnostics.h:
- Add DIAGNOSTIC_ERROR, to enable a diagnostic at error level.
- Add DIAGNOSTIC_ERROR_SWITCH, to enable -Wswitch at error level, for
both gcc and clang.
Additionally, using DIAGNOSTIC_PUSH, DIAGNOSTIC_ERROR_SWITCH and
DIAGNOSTIC_POP seems to misbehave with g++ 4.8, where we see these
errors:
CXX ada-tasks.o
/home/smarchi/src/binutils-gdb/gdb/ada-tasks.c: In function void read_known_tasks():
/home/smarchi/src/binutils-gdb/gdb/ada-tasks.c:998:10: error: enumeration value ADA_TASKS_UNKNOWN not handled in switch [-Werror=switch]
switch (data->known_tasks_kind)
^
Because of the POP, the diagnostic should go back to being disabled,
since it was disabled in the beginning, but that's not what we see
here. Versions of GCC >= 5 compile correctly.
Work around this by making DIAGNOSTIC_ERROR_SWITCH a no-op for GCC < 5.
Note that this code (already as it exists in master today) enables
-Wswitch at the error level even if --disable-werror is passed. It
shouldn't be a problem, as it's not like a new enumerator will appear
out of nowhere and cause a build error if building with future
compilers. Still, for correctness, we would ideally want to ask the
compiler to enable -Wswitch at its default level (as if the user had
passed -Wswitch on the command-line). There doesn't seem to be a way to
do this.
Change-Id: Id33ebec3de39bd449409ea0bab59831289ffe82d
Building with a really old flex and a really new g++ is probably not
recommended, but it should not cause compile errors.
gdb/ChangeLog:
* ada-lex.l: Extend register warnings diagnostics comment for g++.
include/ChangeLog:
* diagnostics.h (DIAGNOSTIC_IGNORE_DEPRECATED_REGISTER): Also define
for GCC version 7.0 or higher.
Attempting to build GDB in Ubuntu 16.04.6 LTS on x86_64, I ran into warnings
that caused the build to fail:
binutils-gdb/gdb/gdbsupport/safe-strerror.c:44:1: error: ‘char* select_strerror_r(char*, char*)’ defined but not used [-Werror=unused-function] select_strerror_r (char *res, char *)
The diagnostic macro DIAGNOSTIC_IGNORE_UNUSED_FUNCTION seems to expand
correctly to its respective pragma, but this doesn't seem to have an effect on
the warning. I tried to use the pragma explicitly and got the same result.
ATTRIBUTE_UNUSED works fine in this case if you put it in both functions,
which should fix warnings for both gdb and gdbserver builds.
The compiler version is gcc (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609.
This is likely the result of PR64079 in GCC, which was fixed by commit
9e96f1e1b9731c4e1ef4fbbbf0997319973f0537.
To prevent other developers from attempting to use this macro, only to get
confused by it not working as expected, it seems better to not define this
particular macro.
gdb/ChangeLog:
2019-12-12 Luis Machado <luis.machado@linaro.org>
* gdbsupport/safe-strerror.c: Don't include diagnostics.h.
(select_strerror_r): Use ATTRIBUTE_UNUSED instead of the diagnostics
macros.
include/ChangeLog:
2019-12-12 Luis Machado <luis.machado@linaro.org>
* diagnostics.h (DIAGNOSTIC_IGNORE_UNUSED_FUNCTION). Remove
definitions.
Change-Id: Iad6123d61d76d111e3ef8d24aa8c60112304c749
commit 3322c5d9a1 ("Remove unneeded explicit .o targets") broke the
build with clang, because -Wno-format-nonliteral was in fact needed.
This patch fixes the problem by introducing
DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL and using it in printcmd.c. This
seems preferable to reverting the patch because now the warning
suppression is more targeted.
gdb/ChangeLog
2018-09-05 Simon Marchi <simon.marchi@ericsson.com>
* printcmd.c (printf_c_string): Use
DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL.
(printf_wide_c_string, printf_pointer, ui_printf): Likewise.
include/ChangeLog
2018-09-05 Simon Marchi <simon.marchi@ericsson.com>
* diagnostics.h (DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL): New macro.
Fixes a number of build errors like the following
.../elf32-arm.c: In function 'elf32_arm_nabi_write_core_note':
.../elf32-arm.c:2177: error: #pragma GCC diagnostic not allowed inside functions
.../elf32-arm.c:2186: error: #pragma GCC diagnostic not allowed inside functions
See the comment in diagnostics.h.
include/
* diagnostics.h: Comment on macro usage.
bfd/
* elf32-arm.c (elf32_arm_nabi_write_core_note): Don't use
DIAGNOTIC_PUSH and DIAGNOSTIC_POP unconditionally.
* elf32-ppc.c (ppc_elf_write_core_note): Likewise.
* elf32-s390.c (elf_s390_write_core_note): Likewise.
* elf64-ppc.c (ppc64_elf_write_core_note): Likewise.
* elf64-s390.c (elf_s390_write_core_note): Likewise.
* elfxx-aarch64.c (_bfd_aarch64_elf_write_core_note): Likewise.
This patch silences this warning:
/Users/simark/src/binutils-gdb/gdb/darwin-nat.c:839:10: error: 'syscall' is deprecated: first deprecated in macOS 10.12 - syscall(2) is unsupported; please switch to a supported interface. For SYS_kdebug_trace use kdebug_signpost(). [-Werror,-Wdeprecated-declarations]
res = syscall (SYS___pthread_kill, thread->gdb_port, nsignal);
^
/usr/include/unistd.h:745:6: note: 'syscall' has been explicitly marked deprecated here
int syscall(int, ...);
^
The comment of the new pthread_kill function explains why we use the
syscall function directly.
include/ChangeLog:
* diagnostics.h (DIAGNOSTIC_IGNORE_DEPRECATED_DECLARATIONS):
Define for clang.
gdb/ChangeLog:
* darwin-nat.c (darwin_pthread_kill): New function.
(darwin_resume_thread): Use darwin_pthread_kill.
Move gdb/common/diagnostics.h to include/diagnostics.h so that it can
be used in binutils.
gdb/
* ada-lex.l: Include "diagnostics.h" instead of
"common/diagnostics.h".
* unittests/environ-selftests.c: Likewise.
* common/diagnostics.h: Moved to ../include.
include/
* diagnostics.h: Moved from ../gdb/common/diagnostics.h.
