Re: [PATCH v5 22/25] Extend "x" and "print" commands to support memory tagging

Simon Marchi via Gdb-patches
2021-02-05T05:02:26Z

> @@ -1208,6 +1254,26 @@ print_value (value *val, const value_print_options &opts)
> annotate_value_history_end ();
> }
>
> +/* Returns true if memory tags should be validated. False otherwise. */
> +
> +static bool
> +should_validate_memtags (struct value *value)
> +{
> + if (target_supports_memory_tagging ()
> + && gdbarch_tagged_address_p (target_gdbarch (), value))
> + {
> + gdb_assert (value && value_type (value));

!= nullptr twice

> @@ -3023,6 +3035,16 @@ will be replaced with either '{...}' or '(...)' depending on the language.\n\
> Use \"unlimited\" to print the complete structure.")
> },
>
> + boolean_option_def {
> + "memory-tag-violations",
> + [] (value_print_options *opt) { return &opt->memory_tag_violations; },
> + show_memory_tag_violations, /* show_cmd_cb */
> + N_("Set printing of memory tag violations for pointers."),
> + N_("Show printing of memory tag violations for pointers."),
> + N_("When logical tags in pointers do not match the allocation tags in\n\
> +memory, issue a warning."),

Related to what I asked you last time, whether all pointer dereferences
made in order to evaluate the value are checked or just the final
pointer value: the help message could wrongfully imply that all pointer
dereferences are checked and reported, when this isn't the case. Could
you spin it as: "Issue a warning when the printed value is a pointer
whose logical tag doesn't match the allocation tag of the memory
location it points to"? The important part being that it specifies that
the printed value is checked.

Otherwise, LGTM. Thanks for doing the options change.

Simon