On Sat, Sep 20 2014, Mark Wielaard wrote:
> When adding DW_TAG_restrict_type I made a mistake when updating the
> code that handled types with multiple modifiers. This patch fixes it
> by putting the logic for finding the "sub-qualified" type in a separate
> function and fall back to adding the modifiers separately if there is
> no such existing type. The old tests didn't catch this case because
> there always was an existing sub-qualified type already. The new testcase
> fails before and succeeds after this patch.
>
> gcc/ChangeLog
>
> * dwarf2out.c (existing_sub_qualified_type): New function.
> (modified_type_die): Use existing_sub_qualified_type. Fall
> back to adding modifiers one by one of there is no existing
> sub-qualified type.
>
> gcc/testsuite/ChangeLog
>
> * gcc.dg/guality/pr63300-const-volatile.c: New testcase.
> ---
> gcc/dwarf2out.c | 85
> ++++++++++++++++++----
> .../gcc.dg/guality/pr63300-const-volatile.c | 12 +++
> 2 files changed, 84 insertions(+), 13 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/guality/pr63300-const-volatile.c
>
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index e87ade2..0cbc316 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -10461,6 +10461,51 @@ decl_quals (const_tree decl)
> ? TYPE_QUAL_VOLATILE : TYPE_UNQUALIFIED));
> }
>
> +/* Returns true if CV_QUALS contains QUAL and we have a qualified
> + variant of TYPE that has at least one other qualifier found in
> + CV_QUALS. Returns false if CV_QUALS doesn't contain QUAL, if
> + CV_QUALS is empty after subtracting QUAL, or if we don't have a
> + TYPE that has at least one qualifier from CV_QUALS minus QUAL. */
> +static bool
> +existing_sub_qualified_type (tree type, int cv_quals, int qual)
> +{
> + int sub_qual, sub_quals = cv_quals & ~qual;
> + if ((cv_quals & qual) == TYPE_UNQUALIFIED || sub_quals == TYPE_UNQUALIFIED)
> + return false;
> +
> + sub_qual = TYPE_QUAL_CONST;
> + if ((sub_quals & ~sub_qual) != TYPE_UNQUALIFIED
> + && get_qualified_type (type, sub_quals & ~sub_qual) != NULL_TREE)
> + return true;
> +
> + sub_qual = TYPE_QUAL_VOLATILE;
> + if ((sub_quals & ~sub_qual) != TYPE_UNQUALIFIED
> + && get_qualified_type (type, sub_quals & ~sub_qual) != NULL_TREE)
> + return true;
> +
> + sub_qual = TYPE_QUAL_RESTRICT;
> + if ((sub_quals & ~sub_qual) != TYPE_UNQUALIFIED
> + && get_qualified_type (type, sub_quals & ~sub_qual) != NULL_TREE)
> + return true;
> +
> + sub_qual = TYPE_QUAL_CONST & TYPE_QUAL_VOLATILE;
You probably mean '|' instead of '&' here.
> + if ((sub_quals & ~sub_qual) != TYPE_UNQUALIFIED
> + && get_qualified_type (type, sub_quals & ~sub_qual) != NULL_TREE)
> + return true;
> +
> + sub_qual = TYPE_QUAL_CONST & TYPE_QUAL_RESTRICT;
See above.
> + if ((sub_quals & ~sub_qual) != TYPE_UNQUALIFIED
> + && get_qualified_type (type, sub_quals & ~sub_qual) != NULL_TREE)
> + return true;
> +
> + sub_qual = TYPE_QUAL_VOLATILE & TYPE_QUAL_RESTRICT;
See above.
> + if ((sub_quals & ~sub_qual) != TYPE_UNQUALIFIED
> + && get_qualified_type (type, sub_quals & ~sub_qual) != NULL_TREE)
> + return true;
> +
> + return false;
> +}
IIUC, 'sub_qual' above represents the qualifiers to *omit* from the ones
we're interested in, right? Maybe it would be more straightforward to
reverse the logic, i.e., start with
sub_qual = TYPE_QUAL_VOLATILE | TYPE_QUAL_RESTRICT;
and then always use sub_qual instead of ~sub_qual.
Also note that the logic wouldn't scale too well for yet more
qualifiers...
> +
> /* Given a pointer to an arbitrary ..._TYPE tree node, return a debugging
> entry that chains various modifiers in front of the given type. */
>
> @@ -10543,34 +10588,48 @@ modified_type_die (tree type, int cv_quals,
> dw_die_ref context_die)
>
> mod_scope = scope_die_for (type, context_die);
>
> - if ((cv_quals & TYPE_QUAL_CONST)
> - /* If there are multiple type modifiers, prefer a path which
> - leads to a qualified type. */
> - && (((cv_quals & ~TYPE_QUAL_CONST) == TYPE_UNQUALIFIED)
> - || get_qualified_type (type, cv_quals) == NULL_TREE
> - || (get_qualified_type (type, cv_quals & ~TYPE_QUAL_CONST)
> - != NULL_TREE)))
> + /* If there are multiple type modifiers, prefer a path which
> + leads to a qualified type. */
> + if (existing_sub_qualified_type (type, cv_quals, TYPE_QUAL_CONST))
> {
> mod_type_die = new_die (DW_TAG_const_type, mod_scope, type);
> sub_die = modified_type_die (type, cv_quals & ~TYPE_QUAL_CONST,
> context_die);
> }
> - else if ((cv_quals & TYPE_QUAL_VOLATILE)
> - && (((cv_quals & ~TYPE_QUAL_VOLATILE) == TYPE_UNQUALIFIED)
> - || get_qualified_type (type, cv_quals) == NULL_TREE
> - || (get_qualified_type (type, cv_quals & ~TYPE_QUAL_VOLATILE)
> - != NULL_TREE)))
> + else if (existing_sub_qualified_type (type, cv_quals, TYPE_QUAL_VOLATILE))
> {
> mod_type_die = new_die (DW_TAG_volatile_type, mod_scope, type);
> sub_die = modified_type_die (type, cv_quals & ~TYPE_QUAL_VOLATILE,
> context_die);
> }
> - else if (cv_quals & TYPE_QUAL_RESTRICT)
> + else if (existing_sub_qualified_type (type, cv_quals, TYPE_QUAL_RESTRICT))
> {
> mod_type_die = new_die (DW_TAG_restrict_type, mod_scope, type);
> sub_die = modified_type_die (type, cv_quals & ~TYPE_QUAL_RESTRICT,
> context_die);
> }
> + else if (cv_quals)
> + {
> + /* No existing path, just add qualifiers one by one. */
> + if (cv_quals & TYPE_QUAL_CONST)
> + {
> + mod_type_die = new_die (DW_TAG_const_type, mod_scope, type);
> + sub_die = modified_type_die (type, cv_quals & ~TYPE_QUAL_CONST,
> + context_die);
> + }
> + else if (cv_quals & TYPE_QUAL_VOLATILE)
> + {
> + mod_type_die = new_die (DW_TAG_volatile_type, mod_scope, type);
> + sub_die = modified_type_die (type, cv_quals & ~TYPE_QUAL_VOLATILE,
> + context_die);
> + }
> + else
> + {
> + mod_type_die = new_die (DW_TAG_restrict_type, mod_scope, type);
> + sub_die = modified_type_die (type, cv_quals & ~TYPE_QUAL_RESTRICT,
> + context_die);
> + }
> + }
> else if (code == POINTER_TYPE)
> {
> mod_type_die = new_die (DW_TAG_pointer_type, mod_scope, type);
> diff --git a/gcc/testsuite/gcc.dg/guality/pr63300-const-volatile.c
> b/gcc/testsuite/gcc.dg/guality/pr63300-const-volatile.c
> new file mode 100644
> index 0000000..b8d75ed
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/guality/pr63300-const-volatile.c
> @@ -0,0 +1,12 @@
> +/* PR63300 'const volatile' sometimes stripped in debug info */
> +/* { dg-do run } */
> +/* { dg-options "-g" } */
> +
> +int
> +main (int argc, char **argv)
> +{
> + const volatile int v = argc;
> + return v - argc;
> +}
> +
> +/* { dg-final { gdb-test 9 "type:v" "const volatile int" } } */
