[07/11] gdb/testsuite: skip gdb.cp/anon-struct.exp when using clang

Message ID 20221004170747.154307-9-blarsen@redhat.com
State Committed
Headers
Series Cleanup gdb.cp tests when running with clang |

Commit Message

Guinevere Larsen Oct. 4, 2022, 5:07 p.m. UTC
  When clang compiles anonymous structures, it does not add linkage names in
their dwarf representations. This is compounded by clang not adding linkage
names to subprograms of those anonymous structs (for instance, the
constructor).

Since this isn't a bug on clang or GDB, but there is no way to make
anon-struct.exp work when using clang, just mark that test as untested.

Since I was already touching the file, I also added a comment at the top
of the file explaining what it is testing for.
---
 gdb/testsuite/gdb.cp/anon-struct.exp | 8 ++++++++
 1 file changed, 8 insertions(+)
  

Comments

Andrew Burgess Oct. 26, 2022, 2:49 p.m. UTC | #1
Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:

> When clang compiles anonymous structures, it does not add linkage names in
> their dwarf representations. This is compounded by clang not adding linkage
> names to subprograms of those anonymous structs (for instance, the
> constructor).
>
> Since this isn't a bug on clang or GDB, but there is no way to make
> anon-struct.exp work when using clang, just mark that test as untested.
>
> Since I was already touching the file, I also added a comment at the top
> of the file explaining what it is testing for.
> ---
>  gdb/testsuite/gdb.cp/anon-struct.exp | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/gdb/testsuite/gdb.cp/anon-struct.exp b/gdb/testsuite/gdb.cp/anon-struct.exp
> index 2c709ab9ecc..774ec882a07 100644
> --- a/gdb/testsuite/gdb.cp/anon-struct.exp
> +++ b/gdb/testsuite/gdb.cp/anon-struct.exp
> @@ -14,12 +14,20 @@
>  # 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 test is used to verify GDB's abiility to refer to linkage names
> +# for types and functions.

Type: 'ability'.

Also, I wonder if this comment should explicitly say something like:

      "...for types and functions within anonymous structs."

maybe?

> +
>  standard_testfile .cc
>  
>  if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} {debug c++}] } {
>       return -1
>  }
>  
> +if {[test_compiler_info clang-*-*]} {

Missing the language argument to test_compiler_info.

> +    untested "clang does not use linkage name in this case"
> +    return

You specifically call out the missing linkage name here, but both the
DW_AT_name and DW_AT_linkage name are filled in by GCC, and not with
Clang.  Is it the linkage name in particular that is critical here?

Either way, I wonder if a more extended comment within this if block
might be useful, just touching on the differences in behaviour between
GCC and Clang, and why than means this test will not work with Clang.

Thanks,
Andrew


> +}
> +
>  if { [is_aarch32_target] } {
>      gdb_test "ptype t::t" "type = struct t {\r\n    C m;\r\n} \\*\\(t \\* const\\)" \
>  	"print type of t::t"
> -- 
> 2.37.3
  
Guinevere Larsen Oct. 27, 2022, 1:46 p.m. UTC | #2
On 26/10/2022 16:49, Andrew Burgess wrote:
> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
>
>> When clang compiles anonymous structures, it does not add linkage names in
>> their dwarf representations. This is compounded by clang not adding linkage
>> names to subprograms of those anonymous structs (for instance, the
>> constructor).
>>
>> Since this isn't a bug on clang or GDB, but there is no way to make
>> anon-struct.exp work when using clang, just mark that test as untested.
>>
>> Since I was already touching the file, I also added a comment at the top
>> of the file explaining what it is testing for.
>> ---
>>   gdb/testsuite/gdb.cp/anon-struct.exp | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/gdb/testsuite/gdb.cp/anon-struct.exp b/gdb/testsuite/gdb.cp/anon-struct.exp
>> index 2c709ab9ecc..774ec882a07 100644
>> --- a/gdb/testsuite/gdb.cp/anon-struct.exp
>> +++ b/gdb/testsuite/gdb.cp/anon-struct.exp
>> @@ -14,12 +14,20 @@
>>   # 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 test is used to verify GDB's abiility to refer to linkage names
>> +# for types and functions.
> Type: 'ability'.
>
> Also, I wonder if this comment should explicitly say something like:
>
>        "...for types and functions within anonymous structs."
>
> maybe?
Yeah, good call.
>
>> +
>>   standard_testfile .cc
>>   
>>   if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} {debug c++}] } {
>>        return -1
>>   }
>>   
>> +if {[test_compiler_info clang-*-*]} {
> Missing the language argument to test_compiler_info.
>
>> +    untested "clang does not use linkage name in this case"
>> +    return
> You specifically call out the missing linkage name here, but both the
> DW_AT_name and DW_AT_linkage name are filled in by GCC, and not with
> Clang.  Is it the linkage name in particular that is critical here?

Well, I guess I need to workshop my explanation.

The problem is that GCC generates a linkage name (t in this case), and 
uses this name for the constructor, so we can use t::t() to refer to the 
anonymous structure's constructor. Clang doesn't use the linkage name to 
generate the name of the constructor (or give it any name at all) so it 
ends with the constructor t::(), which isn't a valid expression for GDB.

I decided not to fix this because I'm not convinced calling the 
constructor of an anonymous structure makes much sense, but I'm open to 
being convinced.

Cheers,
Bruno

>
> Either way, I wonder if a more extended comment within this if block
> might be useful, just touching on the differences in behaviour between
> GCC and Clang, and why than means this test will not work with Clang.
>
> Thanks,
> Andrew
>
>
>> +}
>> +
>>   if { [is_aarch32_target] } {
>>       gdb_test "ptype t::t" "type = struct t {\r\n    C m;\r\n} \\*\\(t \\* const\\)" \
>>   	"print type of t::t"
>> -- 
>> 2.37.3
  

Patch

diff --git a/gdb/testsuite/gdb.cp/anon-struct.exp b/gdb/testsuite/gdb.cp/anon-struct.exp
index 2c709ab9ecc..774ec882a07 100644
--- a/gdb/testsuite/gdb.cp/anon-struct.exp
+++ b/gdb/testsuite/gdb.cp/anon-struct.exp
@@ -14,12 +14,20 @@ 
 # 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 test is used to verify GDB's abiility to refer to linkage names
+# for types and functions.
+
 standard_testfile .cc
 
 if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} {debug c++}] } {
      return -1
 }
 
+if {[test_compiler_info clang-*-*]} {
+    untested "clang does not use linkage name in this case"
+    return
+}
+
 if { [is_aarch32_target] } {
     gdb_test "ptype t::t" "type = struct t {\r\n    C m;\r\n} \\*\\(t \\* const\\)" \
 	"print type of t::t"