[2/2] gdb/c++: Improve error messages in overload resolution

  when resolving overloaded functions, GDB relies on knowing relationships
between types, i.e. if a type inherits from another. However, some
compilers may not add complete information for given types as a way to
reduce unnecessary debug information. In these cases, GDB would just say
that it couldn't resolve the method or function, with no extra
information.

The problem is that sometimes the user may not know that the type
information is incomplete, and may just assume that there is a bug in
GDB. To improve the user experience, we attempt to detect if the
overload match failed because of an incomplete type, and warn the user
of this.

This commit also adds a testcase confirming that the message is only
triggered in the correct scenario. This test was not developed as an
expansion of gdb.cp/overload.cc because it needed the dwarf assembler,
and porting all of overload.cc seemed unnecessary.
---
 .../gdb.cp/incomplete-type-overload.cc        |  45 +++++
 .../gdb.cp/incomplete-type-overload.exp       | 183 ++++++++++++++++++
 gdb/valops.c                                  |  49 ++++-
 3 files changed, 273 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.cp/incomplete-type-overload.cc
 create mode 100644 gdb/testsuite/gdb.cp/incomplete-type-overload.exp
  

Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:

> when resolving overloaded functions, GDB relies on knowing relationships
> between types, i.e. if a type inherits from another. However, some
> compilers may not add complete information for given types as a way to
> reduce unnecessary debug information. In these cases, GDB would just say
> that it couldn't resolve the method or function, with no extra
> information.
>
> The problem is that sometimes the user may not know that the type
> information is incomplete, and may just assume that there is a bug in
> GDB. To improve the user experience, we attempt to detect if the
> overload match failed because of an incomplete type, and warn the user
> of this.
>
> This commit also adds a testcase confirming that the message is only
> triggered in the correct scenario. This test was not developed as an
> expansion of gdb.cp/overload.cc because it needed the dwarf assembler,
> and porting all of overload.cc seemed unnecessary.
> ---
>  .../gdb.cp/incomplete-type-overload.cc        |  45 +++++
>  .../gdb.cp/incomplete-type-overload.exp       | 183 ++++++++++++++++++
>  gdb/valops.c                                  |  49 ++++-
>  3 files changed, 273 insertions(+), 4 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.cp/incomplete-type-overload.cc
>  create mode 100644 gdb/testsuite/gdb.cp/incomplete-type-overload.exp
>
> diff --git a/gdb/testsuite/gdb.cp/incomplete-type-overload.cc b/gdb/testsuite/gdb.cp/incomplete-type-overload.cc
> new file mode 100644
> index 00000000000..a7b85c92bbe
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/incomplete-type-overload.cc
> @@ -0,0 +1,45 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2022 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/>.  */
> +class
> +base {
> +public:
> +  int member;
> +};

I think this should be written as:

  struct base
  {
    int member;
  };

Having 'class' immediately followed by 'public:' is, if I understand
correctly, what 'struct' does, so we could just use that.  I might be
tempted to switch the 'class'-es below to struct too, but that's up to
you.

> +
> +class complete: public base { };
> +
> +class incomplete: public base { };
> +
> +complete comp;
> +complete *cp = &comp;
> +incomplete *inc;
> +int *ip;
> +
> +int
> +foo (base* b)
> +{
> +    asm ("foo_label: .globl foo_label");
> +    return 1;

Indent by 2 spaces.

> +}
> +
> +int
> +main (void)

No need for 'void' here in C++ code.

> +{
> +    asm("main_label: .globl main_label");

Missing space before '('.

> +    comp.member = 0;
> +    return 0;
> +}
> diff --git a/gdb/testsuite/gdb.cp/incomplete-type-overload.exp b/gdb/testsuite/gdb.cp/incomplete-type-overload.exp
> new file mode 100644
> index 00000000000..96ed25dd5d1
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/incomplete-type-overload.exp
> @@ -0,0 +1,183 @@
> +# Copyright 2022 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/>.
> +
> +# This file is part of GDB's testsuite.
> +
> +# This test intends to check the error message that GDB emits when unable
> +# to determine the correct overloaded function due to incomplete types.
> +
> +load_lib dwarf.exp
> +
> +if { [skip_cplus_tests] } { return }
> +
> +if { ![dwarf2_support] } { return }
> +
> +standard_testfile .cc .S
> +set asm_file [standard_output_file ${srcfile2}]
> +
> +if [prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}] {
> +    return
> +}
> +
> +if {[test_compiler_info clang-*-*]} {

Missing 'c++' language for test_compiler_info call.

> +    untested "gcc is required for dwarf assembler tests"
> +    return
> +}
> +
> +if ![runto_main] {
> +    return
> +}
> +
> +# Get important sizes to create fake dwarf for the test
> +set int_size [get_sizeof "int" -1]
> +set addr_size [get_sizeof "void *" -1]
> +set struct_base_size [get_sizeof "base" 4]
> +set struct_complete_size [get_sizeof "complete" 4]
> +get_func_info foo

Not that it makes much difference, but get_func_info has the ability to
take compiler options, so I think this should really be:

  get_func_info foo {debug c++}

> +
> +# Create fake DWARF for the .cc file.
> +# This is the best way to ensure we have an incomplete type.
> +Dwarf::assemble ${asm_file} {
> +    global srcdir subdir srcfile srcfile2 foo_start foo_end
> +    global int_size addr_size struct_base_size struct_complete_size
> +    declare_labels L
> +
> +    cu {} {
> +	DW_TAG_compile_unit {
> +	    {DW_AT_language @DW_LANG_C_plus_plus}
> +	    {name $srcfile}
> +	    {stmt_list $L DW_FORM_sec_offset}
> +	} {
> +	    declare_labels int_label base_label complete_label incomplete_label
> +	    declare_labels ptr_base_label ptr_inc_label ptr_comp_label ptr_int_label
> +
> +	    int_label: DW_TAG_base_type {
> +		{DW_AT_byte_size $int_size DW_FORM_sdata}
> +		{DW_AT_encoding @DW_ATE_signed}
> +		{DW_AT_name "int"}
> +	    }
> +
> +	    base_label: DW_TAG_class_type {
> +		{DW_AT_byte_size $struct_base_size DW_FORM_sdata}
> +		{DW_AT_name "base"}
> +	    } {
> +		DW_TAG_member {
> +		    {DW_AT_name "member"}
> +		    {DW_AT_type :$int_label}
> +		    {DW_AT_data_member_location 0 DW_FORM_sdata}
> +		}
> +	    }
> +
> +	    complete_label: DW_TAG_class_type {
> +		{DW_AT_byte_size $struct_complete_size DW_FORM_sdata}
> +		{DW_AT_name "complete"}
> +	    } {
> +		DW_TAG_inheritance {
> +		    {DW_AT_type :$base_label}
> +		    {DW_AT_data_member_location 0 DW_FORM_sdata}
> +		    {DW_AT_accessibility 1 DW_FORM_data1}
> +		}
> +	    }
> +
> +	    incomplete_label: DW_TAG_class_type {
> +		{DW_AT_name "incomplete"}
> +		{DW_AT_declaration 1 DW_FORM_flag_present}
> +	    }
> +
> +	    ptr_base_label: DW_TAG_pointer_type {
> +		{DW_AT_byte_size $addr_size DW_FORM_udata}
> +		{DW_AT_type :$base_label}
> +	    }
> +
> +	    ptr_inc_label: DW_TAG_pointer_type {
> +		{DW_AT_byte_size $addr_size DW_FORM_udata}
> +		{DW_AT_type :$incomplete_label}
> +	    }
> +
> +	    ptr_comp_label: DW_TAG_pointer_type {
> +		{DW_AT_byte_size $addr_size DW_FORM_udata}
> +		{DW_AT_type :$complete_label}
> +	    }
> +
> +	    ptr_int_label: DW_TAG_pointer_type {
> +		{DW_AT_byte_size $addr_size DW_FORM_udata}
> +		{DW_AT_type :$int_label}
> +	    }
> +
> +	DW_TAG_variable {
> +	    {DW_AT_name "comp"}
> +	    {DW_AT_type :$complete_label}
> +	    {DW_AT_location {DW_OP_addr [gdb_target_symbol "comp"]} SPECIAL_expr}
> +	    {DW_AT_external 1 DW_FORM_flag}
> +	}
> +
> +	DW_TAG_variable {
> +	    {DW_AT_name "cp"}
> +	    {DW_AT_type :$ptr_comp_label}
> +	    {DW_AT_location {DW_OP_addr [gdb_target_symbol "cp"]} SPECIAL_expr}
> +	    {DW_AT_external 1 DW_FORM_flag}
> +	}
> +
> +	DW_TAG_variable {
> +	    {DW_AT_name "inc"}
> +	    {DW_AT_type :$ptr_inc_label}
> +	    {DW_AT_location {DW_OP_addr [gdb_target_symbol "inc"]} SPECIAL_expr}
> +	    {DW_AT_external 1 DW_FORM_flag}
> +	}
> +
> +	DW_TAG_variable {
> +	    {DW_AT_name "ip"}
> +	    {DW_AT_type :$ptr_int_label}
> +	    {DW_AT_location {DW_OP_addr [gdb_target_symbol "ip"]} SPECIAL_expr}
> +	    {DW_AT_external 1 DW_FORM_flag}
> +	}
> +
> +	    DW_TAG_subprogram {
> +		{MACRO_AT_func {"main"}}
> +		{DW_AT_external 1 flag}
> +	    }
> +	    DW_TAG_subprogram {
> +		{MACRO_AT_func {"foo"}}

While looking at this patch I wondered about these two MACRO_AT_ calls.
These surely should be passing the language through too, just like the
get_func_info call earlier?

I've attached a patch at the end of this email that I wrote that allows
these to change to:

  {MACRO_AT_func {"main" {debug c++}}}
  {MACRO_AT_func {"foo" {debug c++}}}

I think this change would be better, but doesn't actually change the
results of the tests or not, so feel free to use this or not, up to
you.

> +		{DW_AT_type :$int_label}
> +		{DW_AT_external 1 flag}
> +	    } { formal_parameter {
> +		    {DW_AT_name "b"}
> +		    {DW_AT_type :$ptr_base_label}
> +		}
> +	    }
> +	}
> +    }
> +
> +    lines {version 2} L {
> +	include_dir "$srcdir/$subdir"
> +	file_name $srcfile 1
> +    }
> +}
> +
> +if [prepare_for_testing "failed to prepare" $testfile [list $asm_file $srcfile] {}] {
> +    return
> +}
> +
> +if ![runto_main] {
> +    return
> +}
> +
> +gdb_test "print foo(cp)" "= 1" "successful invocation"
> +gdb_test "print foo(inc)"\
> +	 "The type. 'incomplete .' isn't fully known to GDB.*"\
> +	 "unsuccessful because declaration"
> +gdb_test "print foo(ip)"\
> +	 "Cannot resolve function foo to any overloaded instance."\
> +	 "unsuccessful because incorrect"
> diff --git a/gdb/valops.c b/gdb/valops.c
> index 748f154252a..2054e9ec303 100644
> --- a/gdb/valops.c
> +++ b/gdb/valops.c
> @@ -41,6 +41,7 @@
>  #include "extension.h"
>  #include "gdbtypes.h"
>  #include "gdbsupport/byte-vector.h"
> +#include "typeprint.h"
>  
>  /* Local functions.  */
>  
> @@ -2933,14 +2934,54 @@ find_overload_match (gdb::array_view<value *> args,
>  
>    if (match_quality == INCOMPATIBLE)
>      {
> +      /* If we couldn't find a decent overload, it may be because we don't
> +	 have enough information about the requested types.  Warn user if
> +	 this is the case, so they can manually cast the types.  */
> +      int incomplete_types = 0;
> +      std::string incomplete_arg_names = "";

I think the ' = ""' can be removed here.

> +      for (struct value *arg: args)

I think GDB style is to have a space before and after ':'.  Also I think
you can make ARG const.

> +	{
> +	  struct type *t = value_type (arg);
> +	  while (t->code () == TYPE_CODE_PTR)
> +	    {
> +	      t = t->target_type();
> +	    }

Single statement while body should have the '{' and '}' removed.

> +	  if (t->is_stub ())
> +	    {
> +	      string_file buffer;
> +	      if(incomplete_types > 0)

Missing space before '(' here.

> +		incomplete_arg_names += ", ";
> +
> +	      current_language->print_type(value_type(arg), "", &buffer,
> +					   -1, 0, &type_print_raw_options);

... and here twice.

> +
> +	      incomplete_types++;
> +	      incomplete_arg_names += buffer.string();

... and here again.

> +	    }
> +	}
> +      std::string hint = "";

I don't think the ' = ""' is needed here.

> +      if (incomplete_types > 1)
> +	{
> +	  hint = string_printf (_("\nThe types: '%s' aren't fully known to GDB."
> +				  " Please cast them directly to the desired"
> +				  " typed in the function call."),
> +				  incomplete_arg_names.c_str ());
> +	}

This, and the following single statement if blocks should have the '{'
and '}' removed.

> +      else if (incomplete_types == 1)
> +	{
> +	  hint = string_printf (_("\nThe type: '%s' isn't fully known to GDB."
> +				  " Please cast it directly to the desired"
> +				  " typed in the function call."),
> +				  incomplete_arg_names.c_str ());
> +	}

I think I would move this entire block of code into a new static
function.  It only requires the ARGS gdb::array_view parameter, and
returns a std::string.

>        if (method == METHOD)
> -	error (_("Cannot resolve method %s%s%s to any overloaded instance"),
> +	error (_("Cannot resolve method %s%s%s to any overloaded instance.%s"),
>  	       obj_type_name,
>  	       (obj_type_name && *obj_type_name) ? "::" : "",
> -	       name);
> +	       name, hint.c_str());
>        else
> -	error (_("Cannot resolve function %s to any overloaded instance"),
> -	       func_name);
> +	error (_("Cannot resolve function %s to any overloaded instance.%s"),
> +	       func_name, hint.c_str());
>      }
>    else if (match_quality == NON_STANDARD)
>      {
> -- 
> 2.37.3

---

commit d98626400f0e0a5721e8ea9ebdc0ea0edc480af3
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Mon Oct 31 12:57:51 2022 +0000

    gdb/testsuite: allow options for MACRO_AT_range and MACRO_AT_func
    
    This commit allows the MACRO_AT_range and MACRO_AT_func, as used by
    the DWARF assembler, to accept an options list.  These options will
    then be forwarded to the function_range call that is contained within
    the implementation of MACRO_AT_range.
    
    This will allow tests to pass through the correct compilation flags if
    needed when these macros are used, though no such uses are added in
    this commit.

diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index b843b1acf75..593f4ea72f1 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -925,14 +925,20 @@ namespace eval Dwarf {
     proc _handle_macro_at_range { attr_value } {
 	variable _cu_is_fission
 
-	if {[llength $attr_value] != 1} {
-	    error "usage: MACRO_AT_range { func }"
+	if {[llength $attr_value] < 1 || [llength $attr_value] > 2} {
+	    error "usage: MACRO_AT_range { func {options {debug}} }"
+	}
+
+	if {[llength $attr_value] == 2} {
+	    set options [lindex $attr_value 1]
+	} else {
+	    set options {debug}
 	}
 
 	set func [lindex $attr_value 0]
 	global srcdir subdir srcfile
 	set src ${srcdir}/${subdir}/${srcfile}
-	set result [function_range $func $src]
+	set result [function_range $func $src $options]
 
 	set form DW_FORM_addr
 	if { $_cu_is_fission } {
@@ -947,9 +953,10 @@ namespace eval Dwarf {
     # Handle macro attribute MACRO_AT_func.
 
     proc _handle_macro_at_func { attr_value } {
-	if {[llength $attr_value] != 1} {
-	    error "usage: MACRO_AT_func { func file }"
+	if {[llength $attr_value] < 1 || [llength $attr_value] > 2} {
+	    error "usage: MACRO_AT_func { func {options {debug}} }"
 	}
+
 	_handle_attribute DW_AT_name [lindex $attr_value 0] DW_FORM_string
 	_handle_macro_at_range $attr_value
     }
  

Cheers,
Bruno

On 31/10/2022 16:04, Andrew Burgess wrote:
> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
>
>> when resolving overloaded functions, GDB relies on knowing relationships
>> between types, i.e. if a type inherits from another. However, some
>> compilers may not add complete information for given types as a way to
>> reduce unnecessary debug information. In these cases, GDB would just say
>> that it couldn't resolve the method or function, with no extra
>> information.
>>
>> The problem is that sometimes the user may not know that the type
>> information is incomplete, and may just assume that there is a bug in
>> GDB. To improve the user experience, we attempt to detect if the
>> overload match failed because of an incomplete type, and warn the user
>> of this.
>>
>> This commit also adds a testcase confirming that the message is only
>> triggered in the correct scenario. This test was not developed as an
>> expansion of gdb.cp/overload.cc because it needed the dwarf assembler,
>> and porting all of overload.cc seemed unnecessary.
>> ---
>>   .../gdb.cp/incomplete-type-overload.cc        |  45 +++++
>>   .../gdb.cp/incomplete-type-overload.exp       | 183 ++++++++++++++++++
>>   gdb/valops.c                                  |  49 ++++-
>>   3 files changed, 273 insertions(+), 4 deletions(-)
>>   create mode 100644 gdb/testsuite/gdb.cp/incomplete-type-overload.cc
>>   create mode 100644 gdb/testsuite/gdb.cp/incomplete-type-overload.exp
>>
>> diff --git a/gdb/testsuite/gdb.cp/incomplete-type-overload.cc b/gdb/testsuite/gdb.cp/incomplete-type-overload.cc
>> new file mode 100644
>> index 00000000000..a7b85c92bbe
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.cp/incomplete-type-overload.cc
>> @@ -0,0 +1,45 @@
>> +/* This testcase is part of GDB, the GNU debugger.
>> +
>> +   Copyright 2022 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/>.  */
>> +class
>> +base {
>> +public:
>> +  int member;
>> +};
> I think this should be written as:
>
>    struct base
>    {
>      int member;
>    };
>
> Having 'class' immediately followed by 'public:' is, if I understand
> correctly, what 'struct' does, so we could just use that.  I might be
> tempted to switch the 'class'-es below to struct too, but that's up to
> you.
Ok, I'll do that
>
>> +
>> +class complete: public base { };
>> +
>> +class incomplete: public base { };
>> +
>> +complete comp;
>> +complete *cp = &comp;
>> +incomplete *inc;
>> +int *ip;
>> +
>> +int
>> +foo (base* b)
>> +{
>> +    asm ("foo_label: .globl foo_label");
>> +    return 1;
> Indent by 2 spaces.
>
>> +}
>> +
>> +int
>> +main (void)
> No need for 'void' here in C++ code.
>
>> +{
>> +    asm("main_label: .globl main_label");
> Missing space before '('.
>
>> +    comp.member = 0;
>> +    return 0;
>> +}
>> diff --git a/gdb/testsuite/gdb.cp/incomplete-type-overload.exp b/gdb/testsuite/gdb.cp/incomplete-type-overload.exp
>> new file mode 100644
>> index 00000000000..96ed25dd5d1
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.cp/incomplete-type-overload.exp
>> @@ -0,0 +1,183 @@
>> +# Copyright 2022 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/>.
>> +
>> +# This file is part of GDB's testsuite.
>> +
>> +# This test intends to check the error message that GDB emits when unable
>> +# to determine the correct overloaded function due to incomplete types.
>> +
>> +load_lib dwarf.exp
>> +
>> +if { [skip_cplus_tests] } { return }
>> +
>> +if { ![dwarf2_support] } { return }
>> +
>> +standard_testfile .cc .S
>> +set asm_file [standard_output_file ${srcfile2}]
>> +
>> +if [prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}] {
>> +    return
>> +}
>> +
>> +if {[test_compiler_info clang-*-*]} {
> Missing 'c++' language for test_compiler_info call.
>
>> +    untested "gcc is required for dwarf assembler tests"
>> +    return
>> +}
>> +
>> +if ![runto_main] {
>> +    return
>> +}
>> +
>> +# Get important sizes to create fake dwarf for the test
>> +set int_size [get_sizeof "int" -1]
>> +set addr_size [get_sizeof "void *" -1]
>> +set struct_base_size [get_sizeof "base" 4]
>> +set struct_complete_size [get_sizeof "complete" 4]
>> +get_func_info foo
> Not that it makes much difference, but get_func_info has the ability to
> take compiler options, so I think this should really be:
>
>    get_func_info foo {debug c++}
>
>> +
>> +# Create fake DWARF for the .cc file.
>> +# This is the best way to ensure we have an incomplete type.
>> +Dwarf::assemble ${asm_file} {
>> +    global srcdir subdir srcfile srcfile2 foo_start foo_end
>> +    global int_size addr_size struct_base_size struct_complete_size
>> +    declare_labels L
>> +
>> +    cu {} {
>> +	DW_TAG_compile_unit {
>> +	    {DW_AT_language @DW_LANG_C_plus_plus}
>> +	    {name $srcfile}
>> +	    {stmt_list $L DW_FORM_sec_offset}
>> +	} {
>> +	    declare_labels int_label base_label complete_label incomplete_label
>> +	    declare_labels ptr_base_label ptr_inc_label ptr_comp_label ptr_int_label
>> +
>> +	    int_label: DW_TAG_base_type {
>> +		{DW_AT_byte_size $int_size DW_FORM_sdata}
>> +		{DW_AT_encoding @DW_ATE_signed}
>> +		{DW_AT_name "int"}
>> +	    }
>> +
>> +	    base_label: DW_TAG_class_type {
>> +		{DW_AT_byte_size $struct_base_size DW_FORM_sdata}
>> +		{DW_AT_name "base"}
>> +	    } {
>> +		DW_TAG_member {
>> +		    {DW_AT_name "member"}
>> +		    {DW_AT_type :$int_label}
>> +		    {DW_AT_data_member_location 0 DW_FORM_sdata}
>> +		}
>> +	    }
>> +
>> +	    complete_label: DW_TAG_class_type {
>> +		{DW_AT_byte_size $struct_complete_size DW_FORM_sdata}
>> +		{DW_AT_name "complete"}
>> +	    } {
>> +		DW_TAG_inheritance {
>> +		    {DW_AT_type :$base_label}
>> +		    {DW_AT_data_member_location 0 DW_FORM_sdata}
>> +		    {DW_AT_accessibility 1 DW_FORM_data1}
>> +		}
>> +	    }
>> +
>> +	    incomplete_label: DW_TAG_class_type {
>> +		{DW_AT_name "incomplete"}
>> +		{DW_AT_declaration 1 DW_FORM_flag_present}
>> +	    }
>> +
>> +	    ptr_base_label: DW_TAG_pointer_type {
>> +		{DW_AT_byte_size $addr_size DW_FORM_udata}
>> +		{DW_AT_type :$base_label}
>> +	    }
>> +
>> +	    ptr_inc_label: DW_TAG_pointer_type {
>> +		{DW_AT_byte_size $addr_size DW_FORM_udata}
>> +		{DW_AT_type :$incomplete_label}
>> +	    }
>> +
>> +	    ptr_comp_label: DW_TAG_pointer_type {
>> +		{DW_AT_byte_size $addr_size DW_FORM_udata}
>> +		{DW_AT_type :$complete_label}
>> +	    }
>> +
>> +	    ptr_int_label: DW_TAG_pointer_type {
>> +		{DW_AT_byte_size $addr_size DW_FORM_udata}
>> +		{DW_AT_type :$int_label}
>> +	    }
>> +
>> +	DW_TAG_variable {
>> +	    {DW_AT_name "comp"}
>> +	    {DW_AT_type :$complete_label}
>> +	    {DW_AT_location {DW_OP_addr [gdb_target_symbol "comp"]} SPECIAL_expr}
>> +	    {DW_AT_external 1 DW_FORM_flag}
>> +	}
>> +
>> +	DW_TAG_variable {
>> +	    {DW_AT_name "cp"}
>> +	    {DW_AT_type :$ptr_comp_label}
>> +	    {DW_AT_location {DW_OP_addr [gdb_target_symbol "cp"]} SPECIAL_expr}
>> +	    {DW_AT_external 1 DW_FORM_flag}
>> +	}
>> +
>> +	DW_TAG_variable {
>> +	    {DW_AT_name "inc"}
>> +	    {DW_AT_type :$ptr_inc_label}
>> +	    {DW_AT_location {DW_OP_addr [gdb_target_symbol "inc"]} SPECIAL_expr}
>> +	    {DW_AT_external 1 DW_FORM_flag}
>> +	}
>> +
>> +	DW_TAG_variable {
>> +	    {DW_AT_name "ip"}
>> +	    {DW_AT_type :$ptr_int_label}
>> +	    {DW_AT_location {DW_OP_addr [gdb_target_symbol "ip"]} SPECIAL_expr}
>> +	    {DW_AT_external 1 DW_FORM_flag}
>> +	}
>> +
>> +	    DW_TAG_subprogram {
>> +		{MACRO_AT_func {"main"}}
>> +		{DW_AT_external 1 flag}
>> +	    }
>> +	    DW_TAG_subprogram {
>> +		{MACRO_AT_func {"foo"}}
> While looking at this patch I wondered about these two MACRO_AT_ calls.
> These surely should be passing the language through too, just like the
> get_func_info call earlier?
>
> I've attached a patch at the end of this email that I wrote that allows
> these to change to:
>
>    {MACRO_AT_func {"main" {debug c++}}}
>    {MACRO_AT_func {"foo" {debug c++}}}
>
> I think this change would be better, but doesn't actually change the
> results of the tests or not, so feel free to use this or not, up to
> you.
This makes sense, but since it doesn't change anything I think you 
should post the patch separately and maybe have a second patch changing 
the relevant functions. Just so the patch series doesn't balloons.
>
>> +		{DW_AT_type :$int_label}
>> +		{DW_AT_external 1 flag}
>> +	    } { formal_parameter {
>> +		    {DW_AT_name "b"}
>> +		    {DW_AT_type :$ptr_base_label}
>> +		}
>> +	    }
>> +	}
>> +    }
>> +
>> +    lines {version 2} L {
>> +	include_dir "$srcdir/$subdir"
>> +	file_name $srcfile 1
>> +    }
>> +}
>> +
>> +if [prepare_for_testing "failed to prepare" $testfile [list $asm_file $srcfile] {}] {
>> +    return
>> +}
>> +
>> +if ![runto_main] {
>> +    return
>> +}
>> +
>> +gdb_test "print foo(cp)" "= 1" "successful invocation"
>> +gdb_test "print foo(inc)"\
>> +	 "The type. 'incomplete .' isn't fully known to GDB.*"\
>> +	 "unsuccessful because declaration"
>> +gdb_test "print foo(ip)"\
>> +	 "Cannot resolve function foo to any overloaded instance."\
>> +	 "unsuccessful because incorrect"
>> diff --git a/gdb/valops.c b/gdb/valops.c
>> index 748f154252a..2054e9ec303 100644
>> --- a/gdb/valops.c
>> +++ b/gdb/valops.c
>> @@ -41,6 +41,7 @@
>>   #include "extension.h"
>>   #include "gdbtypes.h"
>>   #include "gdbsupport/byte-vector.h"
>> +#include "typeprint.h"
>>   
>>   /* Local functions.  */
>>   
>> @@ -2933,14 +2934,54 @@ find_overload_match (gdb::array_view<value *> args,
>>   
>>     if (match_quality == INCOMPATIBLE)
>>       {
>> +      /* If we couldn't find a decent overload, it may be because we don't
>> +	 have enough information about the requested types.  Warn user if
>> +	 this is the case, so they can manually cast the types.  */
>> +      int incomplete_types = 0;
>> +      std::string incomplete_arg_names = "";
> I think the ' = ""' can be removed here.
>
>> +      for (struct value *arg: args)
> I think GDB style is to have a space before and after ':'.  Also I think
> you can make ARG const.
>
>> +	{
>> +	  struct type *t = value_type (arg);
>> +	  while (t->code () == TYPE_CODE_PTR)
>> +	    {
>> +	      t = t->target_type();
>> +	    }
> Single statement while body should have the '{' and '}' removed.
>
>> +	  if (t->is_stub ())
>> +	    {
>> +	      string_file buffer;
>> +	      if(incomplete_types > 0)
> Missing space before '(' here.
>
>> +		incomplete_arg_names += ", ";
>> +
>> +	      current_language->print_type(value_type(arg), "", &buffer,
>> +					   -1, 0, &type_print_raw_options);
> ... and here twice.
>
>> +
>> +	      incomplete_types++;
>> +	      incomplete_arg_names += buffer.string();
> ... and here again.
>
>> +	    }
>> +	}
>> +      std::string hint = "";
> I don't think the ' = ""' is needed here.
>
>> +      if (incomplete_types > 1)
>> +	{
>> +	  hint = string_printf (_("\nThe types: '%s' aren't fully known to GDB."
>> +				  " Please cast them directly to the desired"
>> +				  " typed in the function call."),
>> +				  incomplete_arg_names.c_str ());
>> +	}
> This, and the following single statement if blocks should have the '{'
> and '}' removed.
Ah, I thought the bit about multiple lines in the coding standard also 
applied to wrapping lines (at the end 
ofhttps://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Whitespaces)
>
>> +      else if (incomplete_types == 1)
>> +	{
>> +	  hint = string_printf (_("\nThe type: '%s' isn't fully known to GDB."
>> +				  " Please cast it directly to the desired"
>> +				  " typed in the function call."),
>> +				  incomplete_arg_names.c_str ());
>> +	}
> I think I would move this entire block of code into a new static
> function.  It only requires the ARGS gdb::array_view parameter, and
> returns a std::string.
>
>>         if (method == METHOD)
>> -	error (_("Cannot resolve method %s%s%s to any overloaded instance"),
>> +	error (_("Cannot resolve method %s%s%s to any overloaded instance.%s"),
>>   	       obj_type_name,
>>   	       (obj_type_name && *obj_type_name) ? "::" : "",
>> -	       name);
>> +	       name, hint.c_str());
>>         else
>> -	error (_("Cannot resolve function %s to any overloaded instance"),
>> -	       func_name);
>> +	error (_("Cannot resolve function %s to any overloaded instance.%s"),
>> +	       func_name, hint.c_str());
>>       }
>>     else if (match_quality == NON_STANDARD)
>>       {
>> -- 
>> 2.37.3
> ---
>
> commit d98626400f0e0a5721e8ea9ebdc0ea0edc480af3
> Author: Andrew Burgess <aburgess@redhat.com>
> Date:   Mon Oct 31 12:57:51 2022 +0000
>
>      gdb/testsuite: allow options for MACRO_AT_range and MACRO_AT_func
>      
>      This commit allows the MACRO_AT_range and MACRO_AT_func, as used by
>      the DWARF assembler, to accept an options list.  These options will
>      then be forwarded to the function_range call that is contained within
>      the implementation of MACRO_AT_range.
>      
>      This will allow tests to pass through the correct compilation flags if
>      needed when these macros are used, though no such uses are added in
>      this commit.
>
> diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
> index b843b1acf75..593f4ea72f1 100644
> --- a/gdb/testsuite/lib/dwarf.exp
> +++ b/gdb/testsuite/lib/dwarf.exp
> @@ -925,14 +925,20 @@ namespace eval Dwarf {
>       proc _handle_macro_at_range { attr_value } {
>   	variable _cu_is_fission
>   
> -	if {[llength $attr_value] != 1} {
> -	    error "usage: MACRO_AT_range { func }"
> +	if {[llength $attr_value] < 1 || [llength $attr_value] > 2} {
> +	    error "usage: MACRO_AT_range { func {options {debug}} }"
> +	}
> +
> +	if {[llength $attr_value] == 2} {
> +	    set options [lindex $attr_value 1]
> +	} else {
> +	    set options {debug}
>   	}
>   
>   	set func [lindex $attr_value 0]
>   	global srcdir subdir srcfile
>   	set src ${srcdir}/${subdir}/${srcfile}
> -	set result [function_range $func $src]
> +	set result [function_range $func $src $options]
>   
>   	set form DW_FORM_addr
>   	if { $_cu_is_fission } {
> @@ -947,9 +953,10 @@ namespace eval Dwarf {
>       # Handle macro attribute MACRO_AT_func.
>   
>       proc _handle_macro_at_func { attr_value } {
> -	if {[llength $attr_value] != 1} {
> -	    error "usage: MACRO_AT_func { func file }"
> +	if {[llength $attr_value] < 1 || [llength $attr_value] > 2} {
> +	    error "usage: MACRO_AT_func { func {options {debug}} }"
>   	}
> +
>   	_handle_attribute DW_AT_name [lindex $attr_value 0] DW_FORM_string
>   	_handle_macro_at_range $attr_value
>       }
>