[v2,03/10] Use unsigned as base type for some enums
Commit Message
-fsanitize=undefined complains about using operator~ on various enum
types that are used with DEF_ENUM_FLAGS_TYPE. This patch fixes these
problems by explicitly setting the base type for these enums to
unsigned. It also adds a static assert to enum_flags to ensure that
future enums used this way have an unsigned underlying type.
gdb/ChangeLog
2018-10-01 Tom Tromey <tom@tromey.com>
* common/enum-flags.h (enum_flags::operator~): Add static assert.
* symfile-add-flags.h (enum symfile_add_flag): Use unsigned as
base type.
* objfile-flags.h (enum objfile_flag): Use unsigned as base type.
* gdbtypes.h (enum type_instance_flag_value): Use unsigned as base
type.
* c-lang.h (enum c_string_type_values): Use unsigned as base
type.
* btrace.h (enum btrace_thread_flag): Use unsigned as base type.
---
gdb/ChangeLog | 12 ++++++++++++
gdb/btrace.h | 2 +-
gdb/c-lang.h | 2 +-
gdb/common/enum-flags.h | 1 +
gdb/gdbtypes.h | 2 +-
gdb/objfile-flags.h | 2 +-
gdb/symfile-add-flags.h | 2 +-
7 files changed, 18 insertions(+), 5 deletions(-)
Comments
On 10/02/2018 05:44 AM, Tom Tromey wrote:
> -fsanitize=undefined complains about using operator~ on various enum
> types that are used with DEF_ENUM_FLAGS_TYPE. This patch fixes these
> problems by explicitly setting the base type for these enums to
> unsigned. It also adds a static assert to enum_flags to ensure that
> future enums used this way have an unsigned underlying type.
>
> gdb/ChangeLog
This LGTM, except a nit. During the discussion around v1, the conclusion
was that we can't add the assertion to the class, but adding it to
operator~ would work. That information is lost on whoever ends up reading
this code later on. Could you add a comment? Or if you prefer, update the
commit log to mention it?
> enum_flags operator~ () const
> {
> + gdb_static_assert (std::is_unsigned<underlying_type>::value);
> return (enum_type) ~underlying_value ();
> }
Thanks,
Pedro Alves
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> This LGTM, except a nit. During the discussion around v1, the conclusion
Pedro> was that we can't add the assertion to the class, but adding it to
Pedro> operator~ would work. That information is lost on whoever ends up reading
Pedro> this code later on. Could you add a comment? Or if you prefer, update the
Pedro> commit log to mention it?
I've added a comment.
Tom
@@ -228,7 +228,7 @@ struct btrace_call_history
};
/* Branch trace thread flags. */
-enum btrace_thread_flag
+enum btrace_thread_flag : unsigned
{
/* The thread is to be stepped forwards. */
BTHR_STEP = (1 << 0),
@@ -35,7 +35,7 @@ struct parser_state;
/* The various kinds of C string and character. Note that these
values are chosen so that they may be or'd together in certain
ways. */
-enum c_string_type_values
+enum c_string_type_values : unsigned
{
/* An ordinary string: "value". */
C_STRING = 0,
@@ -164,6 +164,7 @@ public:
}
enum_flags operator~ () const
{
+ gdb_static_assert (std::is_unsigned<underlying_type>::value);
return (enum_type) ~underlying_value ();
}
@@ -193,7 +193,7 @@ enum type_code
/* * Some bits for the type's instance_flags word. See the macros
below for documentation on each bit. */
-enum type_instance_flag_value
+enum type_instance_flag_value : unsigned
{
TYPE_INSTANCE_FLAG_CONST = (1 << 0),
TYPE_INSTANCE_FLAG_VOLATILE = (1 << 1),
@@ -25,7 +25,7 @@
/* Defines for the objfile flags field. Defined in a separate file to
break circular header dependencies. */
-enum objfile_flag
+enum objfile_flag : unsigned
{
/* When an object file has its functions reordered (currently
Irix-5.2 shared libraries exhibit this behaviour), we will need
@@ -26,7 +26,7 @@
symbol_file_add, etc. Defined in a separate file to break circular
header dependencies. */
-enum symfile_add_flag
+enum symfile_add_flag : unsigned
{
/* Be chatty about what you are doing. */
SYMFILE_VERBOSE = 1 << 1,