[PATCH 0/6] The DWARF assembler and Clang

[PATCH 0/6] The DWARF assembler and Clang

[Andrew Burgess]

In a reply to a recent mailing list message of mine, it was mentioned
that the DWARF assembler doesn't work with Clang.

I know that once upon a time this was the case, but I thought those
problems had long been fixed, but I did some investigation anyway.

I ran all the tests that mention 'Dwarf::assemble' and compared the
results between gcc and Clang.  This series addresses all the
regressions that I found.

The general summary is that the DWARF assembler works fine with Clang.
All the issues I found were minor, and specific to individual tests.

The final patch converts an existing test to use the DWARF assembler,
when I proposed this earlier the feedback was the DWARF assembler
doesn't work with Clang.  My claim is that it's fine, and switching to
the DWARF assembler will increase test coverage.

Thanks,
Andrew

---

Andrew Burgess (6):
  gdb/testsuite: don't avoid DWARF assembler tests with Clang
  gdb/testsuite: fix gdb.trace/unavailable-dwarf-piece.exp with Clang
  gdb/testsuite: fix gdb.compile/compile-ops.exp with clang
  gdb/testsuite: add (and use) a new build-id compile option
  gdb/testsuite: fix gdb.debuginfod/fetch_src_and_symbols.exp with Clang
  gdb/testsuite: rewrite gdb.cp/call-method-register.exp with dwarf
    assembler

 .../gdb.base/until-trailing-insns.exp         |   6 -
 gdb/testsuite/gdb.compile/compile-ops.c       |   8 +-
 gdb/testsuite/gdb.compile/compile-ops.exp     |  18 ++-
 gdb/testsuite/gdb.cp/call-method-register.cc  |  49 +-------
 gdb/testsuite/gdb.cp/call-method-register.exp | 108 +++++++++++++-----
 .../gdb.cp/incomplete-type-overload.exp       |   5 -
 .../gdb.debuginfod/fetch_src_and_symbols.exp  |  25 +++-
 .../gdb.trace/unavailable-dwarf-piece.exp     |   4 +-
 gdb/testsuite/lib/gdb.exp                     |   9 ++
 9 files changed, 130 insertions(+), 102 deletions(-)


base-commit: db2e277d1a840091f56185d94f9d39c6736d2556
-- 
2.25.4

[PATCH 1/6] gdb/testsuite: don't avoid DWARF assembler tests with Clang

[Andrew Burgess]

Two tests make the claim that the DWARF assembler requires gcc,
however, this isn't true.  I think at one point, when the DWARF
assembler was first added, we did use some techniques that were not
portable (see the comments in lib/dwarf.exp on function_range for
details), however, I think most DWARF assembler tests will now work
fine with Clang.

The two tests that I modify in this commit both work fine with Clang,
at least, I've tested with Clang 9.0.1 and 15.0.2, and don't see any
problems, so I'm removing the early return logic that stops these
tests from running with Clang.
---
 gdb/testsuite/gdb.base/until-trailing-insns.exp   | 6 ------
 gdb/testsuite/gdb.cp/incomplete-type-overload.exp | 5 -----
 2 files changed, 11 deletions(-)

diff --git a/gdb/testsuite/gdb.base/until-trailing-insns.exp b/gdb/testsuite/gdb.base/until-trailing-insns.exp
index 6396b6650c1..56f6cf038e2 100644
--- a/gdb/testsuite/gdb.base/until-trailing-insns.exp
+++ b/gdb/testsuite/gdb.base/until-trailing-insns.exp
@@ -84,12 +84,6 @@ if {![dwarf2_support]} {
     return 0
 }
 
-# The DWARF assembler requires the gcc compiler.
-if {![is_c_compiler_gcc]} {
-    unsupported "gcc is required for this test"
-    return 0
-}
-
 standard_testfile .c .S
 
 if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
diff --git a/gdb/testsuite/gdb.cp/incomplete-type-overload.exp b/gdb/testsuite/gdb.cp/incomplete-type-overload.exp
index 96ed25dd5d1..024014d68ba 100644
--- a/gdb/testsuite/gdb.cp/incomplete-type-overload.exp
+++ b/gdb/testsuite/gdb.cp/incomplete-type-overload.exp
@@ -31,11 +31,6 @@ if [prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}] {
     return
 }
 
-if {[test_compiler_info clang-*-*]} {
-    untested "gcc is required for dwarf assembler tests"
-    return
-}
-
 if ![runto_main] {
     return
 }
-- 
2.25.4

[PATCH 2/6] gdb/testsuite: fix gdb.trace/unavailable-dwarf-piece.exp with Clang

[Andrew Burgess]

I noticed that the test gdb.trace/unavailable-dwarf-piece.exp was
failing when run with Clang.  Or rather, the test was not running as
the test executable failed to compile.

The problem is that Clang was emitting this warning:

  warning: argument unused during compilation: '-fdiagnostics-color=never' [-Wunused-command-line-argument]

This warning is emitted when compiling the assembler file generated
by the DWARF assembler.

Most DWARF assembler tests generate the assembler file into a file
with the '.S' extension.  However, this particular test uses a '.s'
extension.

Now a .S file will be passed through the preprocessor, while a .s will
be sent straight to the assembler.  My guess is that Clang doesn't
support the -fdiagnostics-color=never option for the assembler, but
does for the preprocessor.

That's a little annoying, but easily worked around.  We don't care if
our assembler file is passed through the preprocessor, so, in this
commit, I just change the file extension from .s to .S, and the
problem is fixed.

Currently, the unavailable-dwarf-piece.exp script names the assembler
file using standard_output_file, in this commit I've switched to make
use of standard_testfile, as that seems to be the more common way of
doing this sort of thing.

With these changes the test now passes with Clang 9.0.1 and 15.0.2,
and also still passes with gcc.
---
 gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp b/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
index f80f8005fcf..13c6f38737c 100644
--- a/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
+++ b/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
@@ -20,9 +20,9 @@ if {![dwarf2_support]} {
     return 0
 }
 
-standard_testfile .c
+standard_testfile .c -dbg.S
 
-set asm_file [standard_output_file "${testfile}-dbg.s"]
+set asm_file $srcfile2
 set opts {}
 
 if  { [gdb_compile ${srcdir}/${subdir}/${srcfile} ${binfile}1.o \
-- 
2.25.4

[PATCH 3/6] gdb/testsuite: fix gdb.compile/compile-ops.exp with clang

[Andrew Burgess]

I noticed that the gdb.compile/compile-ops.exp test was failing when
run with Clang as the compiler.

This test makes use of the DWARF assembler, and, it turns out, uses
a technique which is not portable to Clang.   This problem is
described in the comment on the function_range proc in lib/dwarf.exp,
the explanation is:

  # If the compiler is gcc, we can do the following to get function start
  # and end address too:
  #
  # asm ("func_start: .globl func_start");
  # static void func (void) {}
  # asm ("func_end: .globl func_end");
  #
  # however, this isn't portable, because other compilers, such as clang,
  # may not guarantee the order of global asms and function.  The code
  # becomes:
  #
  # asm ("func_start: .globl func_start");
  # asm ("func_end: .globl func_end");
  # static void func (void) {}

These start/end labels are used for computing the function start, end,
and length.  The portable solution is to place a label within the
function, like this:

  #  int main (void)
  #  {
  #    asm ("main_label: .globl main_label");
  #    return 0;
  #  }

And make use of 'proc function_range' (from lib/dwarf.exp).

So, that's what I do in this commit.

One consequence of this change is that we need to compile the source
file, and have it loaded into a GDB session, before calling
function_range, so I've added an early call to prepare_for_testing.

Additionally, this test script was generating the DWARF assembler into
a file called gdbjit-ops.S, I suspect a copy and paste issue there, so
I've switched this to use compile-ops-dbg.S instead, which is more
inline with what other DWARF assembler tests do.

The only other change, which might be a problem, is that I also
deleted these two lines from the source file:

  asm (".section \".text\"");
  asm (".balign 8");

These lines were setting the alignment of the .text section.  What I
don't know is whether this was significant or not.  If it is
significant, then I can't see why.

On x86-64, the test still passes fine without these lines, but that
doesn't mean the test wont start failing on some other architecture.

Still, I figure, lets remove them, then, if/when we find a test that
starts failing, we can add the lines back, along with an explanation
for why the extra alignment is required.

But, if people would prefer to be more conservative, then I'm happy to
just add the lines back.
---
 gdb/testsuite/gdb.compile/compile-ops.c   |  8 ++------
 gdb/testsuite/gdb.compile/compile-ops.exp | 18 +++++++++++++-----
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/gdb/testsuite/gdb.compile/compile-ops.c b/gdb/testsuite/gdb.compile/compile-ops.c
index 28afe628e35..7a35909e21e 100644
--- a/gdb/testsuite/gdb.compile/compile-ops.c
+++ b/gdb/testsuite/gdb.compile/compile-ops.c
@@ -18,20 +18,16 @@
 int value = 0xdeadf00d;
 int *ptr = &value;
 
-asm (".section	\".text\"");
-asm (".balign 8");
-asm ("func_start: .globl func_start");
-
 static void
 func (void)
 {
+  asm ("func_label: .globl func_label");
 }
 
-asm ("func_end: .globl func_end");
-
 int
 main (void)
 {
+  asm ("main_label: .globl main_label");
   func ();
   return 0;
 }
diff --git a/gdb/testsuite/gdb.compile/compile-ops.exp b/gdb/testsuite/gdb.compile/compile-ops.exp
index b3b14d949d8..76f284f52f3 100644
--- a/gdb/testsuite/gdb.compile/compile-ops.exp
+++ b/gdb/testsuite/gdb.compile/compile-ops.exp
@@ -23,7 +23,7 @@ if {![dwarf2_support]} {
     return 0
 }
 
-standard_testfile .c gdbjit-ops.S
+standard_testfile .c -dbg.S
 
 #
 # A port of the pr10770.c test code to the DWARF assembler format.
@@ -354,9 +354,17 @@ set program [subst {
     addr ptr
 }]
 
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
+    return -1
+}
+
 # Make some DWARF for the test.
 set asm_file [standard_output_file $srcfile2]
 Dwarf::assemble $asm_file {
+
+    # Find start, end, and length of "func".
+    get_func_info func
+
     # Creating a CU with 4-byte addresses lets this test link on both
     # 32- and 64-bit machines.
     cu { addr_size 4 } {
@@ -366,8 +374,8 @@ Dwarf::assemble $asm_file {
 	compile_unit {
 	    {name file1.txt}
 	    {language @DW_LANG_C}
-	    {low_pc func_start addr}
-	    {high_pc func_end addr}
+	    {low_pc $func_start addr}
+	    {high_pc $func_end addr}
 	} {
 	    global program
 
@@ -380,8 +388,8 @@ Dwarf::assemble $asm_file {
 	    subprogram {
 		{external 1 flag}
 		{name func}
-		{low_pc func_start addr}
-		{high_pc func_end addr}
+		{low_pc $func_start addr}
+		{high_pc $func_end addr}
 	    } {
 		formal_parameter {
 		    {name param}
-- 
2.25.4

[PATCH 4/6] gdb/testsuite: add (and use) a new build-id compile option

[Andrew Burgess]

I noticed that the gdb.debuginfod/fetch_src_and_symbols.exp test was
failing when run with Clang as the compiler.

This test relies on the compiled binaries having a build-id within
them.  For GCC, really GNU ld, the default is to always include a
build-id.

When compiling with Clang though, the default is for no build-id.

I did consider *always* turning on the build-id feature when the
compiler is Clang, but that felt a little weird.

Instead, I propose that we add a new 'build-id' compiler option to
gdb_compile, this flag indicates that the test _requires_ a build-id.
In gcc_compile we can then add the required flags if the compiler is
Clang so that we do get a build-id.

With this change the gdb.debuginfod/fetch_src_and_symbols.exp test
now (mostly) passes with Clang 9.0.1 and 15.0.2, and still passes with
gcc.  The 'mostly' part is an unrelated issue, and will be addressed
in a later commit in this series.
---
 gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp | 4 ++--
 gdb/testsuite/lib/gdb.exp                              | 9 +++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
index 9bffb3397ec..b57b3201cf7 100644
--- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
+++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
@@ -32,12 +32,12 @@ if { [catch {file copy -force ${srcdir}/${subdir}/${srcfile} \
     return -1
 }
 
-if { [gdb_compile "$sourcetmp" "$binfile" executable {debug}] != "" } {
+if { [gdb_compile "$sourcetmp" "$binfile" executable {debug build-id}] != "" } {
     untested "failed to compile"
     return -1
 }
 
-if { [gdb_compile "$sourcetmp" "${binfile}2" executable {debug}] != "" } {
+if { [gdb_compile "$sourcetmp" "${binfile}2" executable {debug build-id}] != "" } {
     fail "compile"
     return -1
 }
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 3e0a46445ca..b006d0fe855 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -4684,6 +4684,7 @@ set gdb_saved_set_unbuffered_mode_obj ""
 #   - macros: Add the required compiler flag to include macro information in
 #     debug information
 #   - text_segment=addr: Tell the linker to place the text segment at ADDR.
+#   - build-id: Ensure the final binary includes a build-id.
 #
 # And here are some of the not too obscure options understood by DejaGnu that
 # influence the compilation:
@@ -4746,6 +4747,14 @@ proc gdb_compile {source dest type options} {
 	}
     }
 
+    # If the 'build-id' option is used, then ensure that we generate a
+    # build-id.  GCC does this by default, but Clang does not, so
+    # enable it now.
+    if {[lsearch -exact $options build-id] > 0
+	&& [test_compiler_info "clang-*"]} {
+	lappend new_options "additional_flags=-Wl,--build-id"
+    }
+
     # Treating .c input files as C++ is deprecated in Clang, so
     # explicitly force C++ language.
     if { !$getting_compiler_info
-- 
2.25.4

[PATCH 5/6] gdb/testsuite: fix gdb.debuginfod/fetch_src_and_symbols.exp with Clang

[Andrew Burgess]

The gdb.debuginfod/fetch_src_and_symbols.exp test is showing a single
failure when run with some older versions of Clang, e.g. 9.0.1.

The problem appears to be with Clang's generated line table.  The test
source looks like this:

  int
  main()
  {
    asm ("main_label: .globl main_label");
    return 0;
  }

In GDB, when we 'start', we expect to stop at the 'return 0;' line.
This is the behaviour when the compiler is gcc, or later versions of
Clang.

However, with Clang 9.0.2, I see GDB stop on the 'asm' line.

I propose adding some code to the test script to step GDB onto the
next line if we stop on the 'asm' line.  With this change in place, I
now see the test fully passing with Clang 9.0.1 and 15.0.2, as well as
with gcc.
---
 .../gdb.debuginfod/fetch_src_and_symbols.exp  | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
index b57b3201cf7..cebaa435102 100644
--- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
+++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
@@ -161,6 +161,27 @@ proc_with_prefix no_url { } {
     if ![runto_main] {
 	return -1
     }
+
+    # Some older versions of Clang (e.g. 9.0.1) were observed stopped
+    # at the 'asm' line rather than the 'return' line in main.c.  This
+    # messes up some of the expected patterns later in the test.
+    set rtn_lineno [gdb_get_line_number "return 0;"]
+    set prev_lineno [expr $rtn_lineno - 1]
+    gdb_test_multiple "info line" "" {
+	-wrap -re "\r\nLine ${prev_lineno} of .*" {
+	    gdb_test "step"
+	    pass $gdb_test_name
+	}
+	-wrap -re "\r\nLine ${rtn_lineno} of .*" {
+	    pass $gdb_test_name
+	}
+	-wrap -re "\r\nLine ${::decimal} of .*" {
+	    # Not sure what line this stopped on, but later tests are
+	    # probably going to fail.
+	    fail $gdb_test_name
+	}
+    }
+
     gdb_test "generate-core-file $::corefile" "Saved corefile $::corefile" \
 	"file [file tail $::corefile] gen"
     file rename -force ${binfile}2 $debugdir
-- 
2.25.4

[PATCH 6/6] gdb/testsuite: rewrite gdb.cp/call-method-register.exp with dwarf assembler

[Andrew Burgess]

Convert the gdb.cp/call-method-register.exp test to make use of the
DWARF assembler.

The existing gdb.cp/call-method-register.exp test relies on a GCC
extension - forcing a local variable into a particular named register.

This means that the test will only work with Clang, and, as we have to
name the register into which the variable will be placed, will only
work for those targets where we've selected a suitable register,
currently this is x86-64, i386, and ppc64.

By switching to the DWARF assembler, the test will work with gcc and
clang, and should work on most, if not all, architectures.

The test creates a small structure, something that can fit within a
register, and then tries to call a method on the structure from within
GDB.  This should fail because GDB can't take the address of the in
register structure (for the `this` pointer).

As the test is for a failure case, then we don't really care _which_
register the structure is in, and I take advantage of this for the
DWARF assembler test, I just declare that the variable is in
DW_OP_reg0, whatever that might be.  I've tested the new test on
x86-64, ppc, aarch64, and risc-v, and the test runs, and passes on all
these architectures, which is already more than we used to cover.

Additionally, on x86-64, I've tested with Clang and gcc, and the test
runs and passed with both compilers.
---
 gdb/testsuite/gdb.cp/call-method-register.cc  |  49 +-------
 gdb/testsuite/gdb.cp/call-method-register.exp | 108 +++++++++++++-----
 2 files changed, 81 insertions(+), 76 deletions(-)

diff --git a/gdb/testsuite/gdb.cp/call-method-register.cc b/gdb/testsuite/gdb.cp/call-method-register.cc
index d60fee03701..91cbf13ebca 100644
--- a/gdb/testsuite/gdb.cp/call-method-register.cc
+++ b/gdb/testsuite/gdb.cp/call-method-register.cc
@@ -1,6 +1,6 @@
 /* This testcase is part of GDB, the GNU debugger.
 
-   Copyright 1993-2022 Free Software Foundation, Inc.
+   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
@@ -15,54 +15,9 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#if defined __x86_64__
-# define ASM_REG "rax"
-#elif defined __i386__
-# define ASM_REG "eax"
-#elif defined __powerpc64__
-# define ASM_REG "r9"
-#else
-# error "port me"
-#endif
-
-/* A class small enough that it fits in a register.  */
-struct small
-{
-  int x;
-  int method ();
-};
-
-int
-small::method ()
-{
-  return x + 5;
-}
-
-int
-register_class ()
-{
-  /* Given the use of the GNU register-asm local variables extension,
-     the compiler puts this variable in a register.  This means that
-     GDB can't call any methods for this variable, which is what we
-     want to test.  */
-  register small v asm (ASM_REG);
-
-  int i;
-
-  /* Perform a computation sufficiently complicated that optimizing
-     compilers won't optimize out the variable.  If some compiler
-     constant-folds this whole loop, maybe using a parameter to this
-     function here would help.  */
-  v.x = 0;
-  for (i = 0; i < 13; ++i)
-    v.x += i;
-  --v.x; /* v.x is now 77 */
-  return v.x + 5; /* set breakpoint here */
-}
-
 int
 main ()
 {
-  register_class ();
+  asm ("main_label: .globl main_label");
   return 0;
 }
diff --git a/gdb/testsuite/gdb.cp/call-method-register.exp b/gdb/testsuite/gdb.cp/call-method-register.exp
index a98b11e4c11..a86ea1e44f9 100644
--- a/gdb/testsuite/gdb.cp/call-method-register.exp
+++ b/gdb/testsuite/gdb.cp/call-method-register.exp
@@ -19,43 +19,93 @@
 if { [skip_cplus_tests] } { continue }
 
 load_lib "cp-support.exp"
+load_lib dwarf.exp
 
-standard_testfile .cc
+standard_testfile .cc -dw.S
 
-if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
+	 {debug c++}]} {
     return -1
 }
 
-if {![test_compiler_info gcc-*-* c++]} {
-    untested "test relies on a gcc extension"
-    return
-}
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+
+    set main_result \
+	[function_range main ${::srcdir}/${::subdir}/${::srcfile}]
+    set main_start [lindex $main_result 0]
+    set main_length [lindex $main_result 1]
+
+    cu {} {
+	compile_unit {
+	    {DW_AT_language @DW_LANG_C_plus_plus}
+	    {DW_AT_name	    $::srcfile}
+	    {DW_AT_comp_dir /tmp}
+	} {
+	    declare_labels int_type_label struct_type_label \
+		struct_ptr_type_label
+	    set ptr_size [get_sizeof "void *" 96]
+
+	    DW_TAG_subprogram {
+		{name main}
+		{low_pc $main_start addr}
+		{high_pc $main_length data8}
+		{DW_AT_type :$int_type_label}
+	    }
+
+	    int_type_label: DW_TAG_base_type {
+		{DW_AT_byte_size 4 DW_FORM_sdata}
+		{DW_AT_encoding @DW_ATE_signed}
+		{DW_AT_name int}
+	    }
+
+	    struct_type_label: DW_TAG_structure_type {
+		{DW_AT_byte_size 4 DW_FORM_sdata}
+		{DW_AT_name small}
+	    } {
+		member {
+		    {name xxx}
+		    {type :$int_type_label}
+		    {data_member_location 0 data1}
+		}
+		subprogram {
+		    {name yyy}
+		    {type :$int_type_label}
+		    {data_member_location 0 data1}
+		} {
+		    formal_parameter {
+			{type :$struct_ptr_type_label}
+			{artificial 1 flag_present}
+		    }
+		}
+	    }
 
-proc test_call_register_class {} {
-    global gdb_prompt
+	    struct_ptr_type_label: DW_TAG_pointer_type {
+		{DW_AT_byte_size $ptr_size DW_FORM_data1}
+		{type :$struct_type_label}
+	    }
 
-    if ![runto_main] {
-	return
+	    DW_TAG_variable {
+		{DW_AT_name global_var}
+		{DW_AT_type :$struct_type_label}
+		{DW_AT_location {
+		    DW_OP_reg0
+		} SPECIAL_expr}
+		{external 1 flag}
+	    }
+	}
     }
+}
 
-    set bp_location [gdb_get_line_number "set breakpoint here"]
-    gdb_breakpoint $bp_location
-    gdb_continue_to_breakpoint "break here"
-
-    # This class is so small that an instance of it can fit in a register.
-    # When gdb tries to call a method, it gets embarrassed about taking
-    # the address of a register.
-    #
-    # That message is a PASS, not an XFAIL, because gdb prints an
-    # informative message and declines to do something impossible.
-    #
-    # The method call actually succeeds if the compiler allocates very
-    # small classes in memory instead of registers.  If that happens,
-    # it's a FAIL, because the testcase is written in a form such that
-    # it should not happen.
-    gdb_test "print v.method ()" \
-	"Address requested for identifier \"v\" which is in register .*" \
-	"call method on register local"
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
 }
 
-test_call_register_class
+gdb_test "print global_var.yyy ()" \
+    "Address requested for identifier \"global_var\" which is in register .*" \
+    "call method on register local"
-- 
2.25.4

Re: [PATCH 6/6] gdb/testsuite: rewrite gdb.cp/call-method-register.exp with dwarf assembler

[Lancelot SIX]

Hi Andrew


On Fri, Nov 11, 2022 at 04:36:25PM +0000, Andrew Burgess via Gdb-patches wrote:
> Convert the gdb.cp/call-method-register.exp test to make use of the
> DWARF assembler.
> 
> The existing gdb.cp/call-method-register.exp test relies on a GCC
> extension - forcing a local variable into a particular named register.
> 
> This means that the test will only work with Clang, and, as we have to
> name the register into which the variable will be placed, will only
> work for those targets where we've selected a suitable register,
> currently this is x86-64, i386, and ppc64.
> 
> By switching to the DWARF assembler, the test will work with gcc and
> clang, and should work on most, if not all, architectures.
> 
> The test creates a small structure, something that can fit within a
> register, and then tries to call a method on the structure from within
> GDB.  This should fail because GDB can't take the address of the in
> register structure (for the `this` pointer).
> 
> As the test is for a failure case, then we don't really care _which_
> register the structure is in, and I take advantage of this for the
> DWARF assembler test, I just declare that the variable is in
> DW_OP_reg0, whatever that might be.  I've tested the new test on
> x86-64, ppc, aarch64, and risc-v, and the test runs, and passes on all
> these architectures, which is already more than we used to cover.
> 
> Additionally, on x86-64, I've tested with Clang and gcc, and the test
> runs and passed with both compilers.
> ---
>  gdb/testsuite/gdb.cp/call-method-register.cc  |  49 +-------
>  gdb/testsuite/gdb.cp/call-method-register.exp | 108 +++++++++++++-----
>  2 files changed, 81 insertions(+), 76 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.cp/call-method-register.cc b/gdb/testsuite/gdb.cp/call-method-register.cc
> index d60fee03701..91cbf13ebca 100644
> --- a/gdb/testsuite/gdb.cp/call-method-register.cc
> +++ b/gdb/testsuite/gdb.cp/call-method-register.cc
> @@ -1,6 +1,6 @@
>  /* This testcase is part of GDB, the GNU debugger.
>  
> -   Copyright 1993-2022 Free Software Foundation, Inc.
> +   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
> @@ -15,54 +15,9 @@
>     You should have received a copy of the GNU General Public License
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
> -#if defined __x86_64__
> -# define ASM_REG "rax"
> -#elif defined __i386__
> -# define ASM_REG "eax"
> -#elif defined __powerpc64__
> -# define ASM_REG "r9"
> -#else
> -# error "port me"
> -#endif
> -
> -/* A class small enough that it fits in a register.  */
> -struct small
> -{
> -  int x;
> -  int method ();
> -};
> -
> -int
> -small::method ()
> -{
> -  return x + 5;
> -}
> -
> -int
> -register_class ()
> -{
> -  /* Given the use of the GNU register-asm local variables extension,
> -     the compiler puts this variable in a register.  This means that
> -     GDB can't call any methods for this variable, which is what we
> -     want to test.  */
> -  register small v asm (ASM_REG);
> -
> -  int i;
> -
> -  /* Perform a computation sufficiently complicated that optimizing
> -     compilers won't optimize out the variable.  If some compiler
> -     constant-folds this whole loop, maybe using a parameter to this
> -     function here would help.  */
> -  v.x = 0;
> -  for (i = 0; i < 13; ++i)
> -    v.x += i;
> -  --v.x; /* v.x is now 77 */
> -  return v.x + 5; /* set breakpoint here */
> -}
> -
>  int
>  main ()
>  {
> -  register_class ();
> +  asm ("main_label: .globl main_label");
>    return 0;
>  }
> diff --git a/gdb/testsuite/gdb.cp/call-method-register.exp b/gdb/testsuite/gdb.cp/call-method-register.exp
> index a98b11e4c11..a86ea1e44f9 100644
> --- a/gdb/testsuite/gdb.cp/call-method-register.exp
> +++ b/gdb/testsuite/gdb.cp/call-method-register.exp
> @@ -19,43 +19,93 @@
>  if { [skip_cplus_tests] } { continue }
>  
>  load_lib "cp-support.exp"
> +load_lib dwarf.exp
>  
> -standard_testfile .cc
> +standard_testfile .cc -dw.S
>  
> -if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
> +	 {debug c++}]} {
>      return -1
>  }
>  
> -if {![test_compiler_info gcc-*-* c++]} {
> -    untested "test relies on a gcc extension"
> -    return
> -}
> +set asm_file [standard_output_file $srcfile2]
> +Dwarf::assemble $asm_file {
> +
> +    set main_result \
> +	[function_range main ${::srcdir}/${::subdir}/${::srcfile}]
> +    set main_start [lindex $main_result 0]
> +    set main_length [lindex $main_result 1]
> +
> +    cu {} {
> +	compile_unit {
> +	    {DW_AT_language @DW_LANG_C_plus_plus}
> +	    {DW_AT_name	    $::srcfile}
> +	    {DW_AT_comp_dir /tmp}
> +	} {
> +	    declare_labels int_type_label struct_type_label \
> +		struct_ptr_type_label
> +	    set ptr_size [get_sizeof "void *" 96]
> +
> +	    DW_TAG_subprogram {
> +		{name main}
> +		{low_pc $main_start addr}
> +		{high_pc $main_length data8}
> +		{DW_AT_type :$int_type_label}
> +	    }
> +
> +	    int_type_label: DW_TAG_base_type {
> +		{DW_AT_byte_size 4 DW_FORM_sdata}
> +		{DW_AT_encoding @DW_ATE_signed}
> +		{DW_AT_name int}
> +	    }
> +
> +	    struct_type_label: DW_TAG_structure_type {
> +		{DW_AT_byte_size 4 DW_FORM_sdata}
> +		{DW_AT_name small}
> +	    } {
> +		member {
> +		    {name xxx}
> +		    {type :$int_type_label}
> +		    {data_member_location 0 data1}
> +		}
> +		subprogram {
> +		    {name yyy}
> +		    {type :$int_type_label}
> +		    {data_member_location 0 data1}

I do not think a DW_AT_data_member_location makes sense for a
DW_TAG_subprogram.  I do not see it in the Dwarf5's Appendix A
(Attributes by Tag Value) in the list of attributed valid for
subprogram.

On x86_64 the test is still passing wich gcc-11 and Clang-15 if I remove
this line.

> +		} {
> +		    formal_parameter {
> +			{type :$struct_ptr_type_label}
> +			{artificial 1 flag_present}
> +		    }
> +		}
> +	    }
>  
> -proc test_call_register_class {} {
> -    global gdb_prompt
> +	    struct_ptr_type_label: DW_TAG_pointer_type {
> +		{DW_AT_byte_size $ptr_size DW_FORM_data1}
> +		{type :$struct_type_label}
> +	    }
>  
> -    if ![runto_main] {
> -	return
> +	    DW_TAG_variable {
> +		{DW_AT_name global_var}
> +		{DW_AT_type :$struct_type_label}
> +		{DW_AT_location {
> +		    DW_OP_reg0
> +		} SPECIAL_expr}
> +		{external 1 flag}
> +	    }
> +	}
>      }
> +}
>  
> -    set bp_location [gdb_get_line_number "set breakpoint here"]
> -    gdb_breakpoint $bp_location
> -    gdb_continue_to_breakpoint "break here"
> -
> -    # This class is so small that an instance of it can fit in a register.
> -    # When gdb tries to call a method, it gets embarrassed about taking
> -    # the address of a register.
> -    #
> -    # That message is a PASS, not an XFAIL, because gdb prints an
> -    # informative message and declines to do something impossible.
> -    #
> -    # The method call actually succeeds if the compiler allocates very
> -    # small classes in memory instead of registers.  If that happens,
> -    # it's a FAIL, because the testcase is written in a form such that
> -    # it should not happen.
> -    gdb_test "print v.method ()" \
> -	"Address requested for identifier \"v\" which is in register .*" \
> -	"call method on register local"
> +if { [prepare_for_testing "failed to prepare" ${testfile} \
> +	  [list $srcfile $asm_file] {nodebug}] } {
> +    return -1
> +}
> +
> +if ![runto_main] {
> +    return -1
>  }
>  
> -test_call_register_class
> +gdb_test "print global_var.yyy ()" \
> +    "Address requested for identifier \"global_var\" which is in register .*" \
> +    "call method on register local"
> -- 
> 2.25.4
> 

Other than this, and for what it is worth the series seems reasonable to
me.

Patch 5 (fix gdb.debuginfod/fetch_src_and_symbols.exp with Clang) got me
curious on what the line table looks like.  I do not have a Clang-9
handy to reproduce the problem.  Anyway, your patch is fine to work
around the issue.

So with one change in this patch, feel free to apply my RB tag on the
series:

Reviewed-By: Lancelot SIX <lancelot.six@amd.com>

Best,
Lancelot.

Re: [PATCH 2/6] gdb/testsuite: fix gdb.trace/unavailable-dwarf-piece.exp with Clang

[Bruno Larsen]

On 11/11/2022 17:36, Andrew Burgess via Gdb-patches wrote:
> I noticed that the test gdb.trace/unavailable-dwarf-piece.exp was
> failing when run with Clang.  Or rather, the test was not running as
> the test executable failed to compile.
>
> The problem is that Clang was emitting this warning:
>
>    warning: argument unused during compilation: '-fdiagnostics-color=never' [-Wunused-command-line-argument]
>
> This warning is emitted when compiling the assembler file generated
> by the DWARF assembler.
>
> Most DWARF assembler tests generate the assembler file into a file
> with the '.S' extension.  However, this particular test uses a '.s'
> extension.
>
> Now a .S file will be passed through the preprocessor, while a .s will
> be sent straight to the assembler.  My guess is that Clang doesn't
> support the -fdiagnostics-color=never option for the assembler, but
> does for the preprocessor.
>
> That's a little annoying, but easily worked around.  We don't care if
> our assembler file is passed through the preprocessor, so, in this
> commit, I just change the file extension from .s to .S, and the
> problem is fixed.
>
> Currently, the unavailable-dwarf-piece.exp script names the assembler
> file using standard_output_file, in this commit I've switched to make
> use of standard_testfile, as that seems to be the more common way of
> doing this sort of thing.
>
> With these changes the test now passes with Clang 9.0.1 and 15.0.2,
> and also still passes with gcc.
> ---
>   gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp b/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
> index f80f8005fcf..13c6f38737c 100644
> --- a/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
> +++ b/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
> @@ -20,9 +20,9 @@ if {![dwarf2_support]} {
>       return 0
>   }
>   
> -standard_testfile .c
> +standard_testfile .c -dbg.S
>   
> -set asm_file [standard_output_file "${testfile}-dbg.s"]
> +set asm_file $srcfile2
>   set opts {}
>   
>   if  { [gdb_compile ${srcdir}/${subdir}/${srcfile} ${binfile}1.o \

I tried running this test with these changes using clang 14.0.5 and 
clang 16.0.0, and both times I got the following output:

builtin_spawn -ignore SIGHUP clang-14 -fdiagnostics-color=never 
-Wno-unknown-warning-option -w -c -g -o 
/home/blarsen/Documents/fsf_build/gdb/testsuite/temp/1163842/is_ilp32_target-1163842.o 
/home/blarsen/Documents/fsf_build/gdb/testsuite/temp/1163842/is_ilp32_target-1163842.c
/home/blarsen/Documents/fsf_build/gdb/testsuite/temp/1163842/is_ilp32_target-1163842.c:2:12: 
error: 'dummy' declared as an array with a negative size
         int dummy[sizeof (int) == 4
                   ^~~~~~~~~~~~~~~~~
1 error generated.


And I get an "unsupported" test. The error message says that the target 
doesn't support trace, but I'm not sure if it isn't related to the 
compilation failure. Do you see anything similar?

-- 
Cheers,
Bruno

Re: [PATCH 3/6] gdb/testsuite: fix gdb.compile/compile-ops.exp with clang

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> The only other change, which might be a problem, is that I also
Andrew> deleted these two lines from the source file:

Andrew>   asm (".section \".text\"");
Andrew>   asm (".balign 8");

Andrew> These lines were setting the alignment of the .text section.  What I
Andrew> don't know is whether this was significant or not.  If it is
Andrew> significant, then I can't see why.

According to 'git annotate', I wrote these lines.  I don't remember this
at all, I assume I copied it from some other place.  Removing seems
fine.

Tom

Re: [PATCH 5/6] gdb/testsuite: fix gdb.debuginfod/fetch_src_and_symbols.exp with Clang

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew>   int
Andrew>   main()
Andrew>   {
Andrew>     asm ("main_label: .globl main_label");
Andrew>     return 0;
Andrew>   }

Andrew> In GDB, when we 'start', we expect to stop at the 'return 0;' line.
Andrew> This is the behaviour when the compiler is gcc, or later versions of
Andrew> Clang.

Andrew> However, with Clang 9.0.2, I see GDB stop on the 'asm' line.

Andrew> I propose adding some code to the test script to step GDB onto the
Andrew> next line if we stop on the 'asm' line.  With this change in place, I
Andrew> now see the test fully passing with Clang 9.0.1 and 15.0.2, as well as
Andrew> with gcc.

It seems to me that the usual approach of adding a "/* STOP */" comment
and then running to that point would be less reliant on compiler
decisions.

Tom

Re: [PATCH 0/6] The DWARF assembler and Clang

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> The general summary is that the DWARF assembler works fine with Clang.
Andrew> All the issues I found were minor, and specific to individual tests.

I sent a couple of notes; but aside from those and the other responses,
everything looks good to me.  Thank you for doing this.

Tom

Re: [PATCH 2/6] gdb/testsuite: fix gdb.trace/unavailable-dwarf-piece.exp with Clang

[Andrew Burgess]

Bruno Larsen <blarsen@redhat.com> writes:

> On 11/11/2022 17:36, Andrew Burgess via Gdb-patches wrote:
>> I noticed that the test gdb.trace/unavailable-dwarf-piece.exp was
>> failing when run with Clang.  Or rather, the test was not running as
>> the test executable failed to compile.
>>
>> The problem is that Clang was emitting this warning:
>>
>>    warning: argument unused during compilation: '-fdiagnostics-color=never' [-Wunused-command-line-argument]
>>
>> This warning is emitted when compiling the assembler file generated
>> by the DWARF assembler.
>>
>> Most DWARF assembler tests generate the assembler file into a file
>> with the '.S' extension.  However, this particular test uses a '.s'
>> extension.
>>
>> Now a .S file will be passed through the preprocessor, while a .s will
>> be sent straight to the assembler.  My guess is that Clang doesn't
>> support the -fdiagnostics-color=never option for the assembler, but
>> does for the preprocessor.
>>
>> That's a little annoying, but easily worked around.  We don't care if
>> our assembler file is passed through the preprocessor, so, in this
>> commit, I just change the file extension from .s to .S, and the
>> problem is fixed.
>>
>> Currently, the unavailable-dwarf-piece.exp script names the assembler
>> file using standard_output_file, in this commit I've switched to make
>> use of standard_testfile, as that seems to be the more common way of
>> doing this sort of thing.
>>
>> With these changes the test now passes with Clang 9.0.1 and 15.0.2,
>> and also still passes with gcc.
>> ---
>>   gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp b/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
>> index f80f8005fcf..13c6f38737c 100644
>> --- a/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
>> +++ b/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
>> @@ -20,9 +20,9 @@ if {![dwarf2_support]} {
>>       return 0
>>   }
>>   
>> -standard_testfile .c
>> +standard_testfile .c -dbg.S
>>   
>> -set asm_file [standard_output_file "${testfile}-dbg.s"]
>> +set asm_file $srcfile2
>>   set opts {}
>>   
>>   if  { [gdb_compile ${srcdir}/${subdir}/${srcfile} ${binfile}1.o \
>
> I tried running this test with these changes using clang 14.0.5 and 
> clang 16.0.0, and both times I got the following output:
>
> builtin_spawn -ignore SIGHUP clang-14 -fdiagnostics-color=never 
> -Wno-unknown-warning-option -w -c -g -o 
> /home/blarsen/Documents/fsf_build/gdb/testsuite/temp/1163842/is_ilp32_target-1163842.o 
> /home/blarsen/Documents/fsf_build/gdb/testsuite/temp/1163842/is_ilp32_target-1163842.c
> /home/blarsen/Documents/fsf_build/gdb/testsuite/temp/1163842/is_ilp32_target-1163842.c:2:12: 
> error: 'dummy' declared as an array with a negative size
>          int dummy[sizeof (int) == 4
>                    ^~~~~~~~~~~~~~~~~
> 1 error generated.

That error is part of the is_ilp32_target proc, and is a result of this
C source code:

	int dummy[sizeof (int) == 4
		  && sizeof (void *) == 4
		  && sizeof (long) == 4 ? 1 : -1];

So on a non ilp32 target (e.g. x86-64) the array length will be -1, and
the code, by design, will fail to compile.  We use the failure to
compile as an indication that the target is non-ilp32.

> And I get an "unsupported" test. The error message says that the target 
> doesn't support trace, but I'm not sure if it isn't related to the 
> compilation failure. Do you see anything similar?

The unsupported is not related to the error described above.

I also see the "unsupported".

But the critical thing is that previously, when compiling with Clang I
didn't get to the unsupported, I instead saw a failure to compile the
test program (error in the original commit message).  With this fix the
Clang program now compiles.

There is a small chance that for [reasons] the test might fail on a
target that does support tracing when compiled with Clang.  But I'm
going to leave that for others to figure out.  Any fix for that problem
would be a completely unrelated fix to the one I'm proposing here
anyway, so it would always be a separate patch.

Thanks,
Andrew

[PATCHv2 0/2] Fix gdb.debuginfod/fetch_src_and_symbols.exp with Clang

[Andrew Burgess]

Tom,

Thanks for the feedback.  I've updated the test inline with your suggestion.

While making this change I wondered if there was any reason why the
source file for this test is called 'main.c' rather than the expected
fetch_src_and_symbols.c.  I couldn't see any reason, so in the first
patch below I rename the source file, then the second patch below is
an improved version of my v1 patch.

Thanks,
Andrewx


---

Andrew Burgess (2):
  gdb/testsuite: rename source file gdb.debuginfod/main.c
  gdb/testsuite: fix gdb.debuginfod/fetch_src_and_symbols.exp with Clang

 .../gdb.debuginfod/{main.c => fetch_src_and_symbols.c}   | 5 ++++-
 gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp   | 9 +++++++--
 2 files changed, 11 insertions(+), 3 deletions(-)
 rename gdb/testsuite/gdb.debuginfod/{main.c => fetch_src_and_symbols.c} (91%)

-- 
2.25.4

[PATCHv2 1/2] gdb/testsuite: rename source file gdb.debuginfod/main.c

[Andrew Burgess]

The test gdb.debuginfod/fetch_src_and_symbols.exp uses a source file
named main.c.  I can't see any particular reason why the file is named
as such.

Usually test source files are named after the test script.

This commit just renames the source file inline with the test script,
and updates the call to standard_testfile (removing the reference to
main.c).

There's no particular reason for this change other than seeing the
file named main.c made me thing that the source file must be shared
with some other test (it isn't).

There should be no change in what is tested after this commit.
---
 .../gdb.debuginfod/{main.c => fetch_src_and_symbols.c}          | 0
 gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp          | 2 +-
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename gdb/testsuite/gdb.debuginfod/{main.c => fetch_src_and_symbols.c} (100%)

diff --git a/gdb/testsuite/gdb.debuginfod/main.c b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.c
similarity index 100%
rename from gdb/testsuite/gdb.debuginfod/main.c
rename to gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.c
diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
index b57b3201cf7..8b3c2cf709e 100644
--- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
+++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
@@ -15,7 +15,7 @@
 
 # Test debuginfod functionality
 
-standard_testfile main.c
+standard_testfile
 
 load_lib dwarf.exp
 load_lib debuginfod-support.exp
-- 
2.25.4

[PATCHv2 2/2] gdb/testsuite: fix gdb.debuginfod/fetch_src_and_symbols.exp with Clang

[Andrew Burgess]

The gdb.debuginfod/fetch_src_and_symbols.exp test is showing a single
failure when run with some older versions of Clang, e.g. 9.0.1.

The problem appears to be with Clang's generated line table.  The test
source looks like this:

  int
  main()
  {
    asm ("main_label: .globl main_label");
    return 0;
  }

In GDB, when we 'start', we expect to stop at the 'return 0;' line.
This is the behaviour when the compiler is gcc, or later versions of
Clang.

However, with Clang 9.0.2, I see GDB stop on the 'asm' line.

In this commit I'll fix this issue by placing a breakpoint on the
return line, and then using gdb_continue_to_breakpoint to ensure we
have stopped in the correct place.

Of course, using gdb_continue_to_breakpoint will only work if we are
not already stopped at the breakpoint location, so I've added some
filler work before the 'return 0;' line.  With this done we can use
gdb_continue_to_breakpoint in all cases.

As a result of adding the new filler work, one of the later tests,
that used the 'list' command, no longer see the correct expected
output (the top line of the source file is no longer included in the
output).  I've fixed this by listing a known specific line, the test
is checking that GDB managed to find the source file, it doesn't
matter which source line we list, as long as we can list something.
---
 gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.c   | 5 ++++-
 gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp | 7 ++++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.c b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.c
index 412bd53edda..7215e3c6484 100644
--- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.c
+++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.c
@@ -15,11 +15,14 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+volatile int global_var = 0;
+
 /* Dummy main function.  */
 
 int
 main()
 {
   asm ("main_label: .globl main_label");
-  return 0;
+  ++global_var;
+  return 0;	/* Breakpoint here.  */
 }
diff --git a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
index 8b3c2cf709e..e95526a069f 100644
--- a/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
+++ b/gdb/testsuite/gdb.debuginfod/fetch_src_and_symbols.exp
@@ -161,6 +161,10 @@ proc_with_prefix no_url { } {
     if ![runto_main] {
 	return -1
     }
+
+    gdb_breakpoint [gdb_get_line_number "Breakpoint here"]
+    gdb_continue_to_breakpoint "stop at last line of main"
+
     gdb_test "generate-core-file $::corefile" "Saved corefile $::corefile" \
 	"file [file tail $::corefile] gen"
     file rename -force ${binfile}2 $debugdir
@@ -217,7 +221,8 @@ proc_with_prefix local_url { } {
     gdb_test_no_output "set substitute-path $outputdir /dev/null" \
 	"set substitute-path"
     gdb_test "br main" "Breakpoint 1 at.*file.*"
-    gdb_test "l" ".*This program is distributed in the hope.*"
+    set lineno [gdb_get_line_number "Breakpoint here"]
+    gdb_test "list $lineno" "return 0;\[^\r\n\]+Breakpoint here\\. .*"
 
     # GDB should now find the executable file.
     clean_restart
-- 
2.25.4

Re: [PATCHv2 0/2] Fix gdb.debuginfod/fetch_src_and_symbols.exp with Clang

[Tom Tromey]

Andrew> Thanks for the feedback.  I've updated the test inline with your suggestion.

Andrew> While making this change I wondered if there was any reason why the
Andrew> source file for this test is called 'main.c' rather than the expected
Andrew> fetch_src_and_symbols.c.  I couldn't see any reason, so in the first
Andrew> patch below I rename the source file, then the second patch below is
Andrew> an improved version of my v1 patch.

These look good to me.  Thank you.

Tom

Re: [PATCH 0/6] The DWARF assembler and Clang

[Andrew Burgess]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Andrew> The general summary is that the DWARF assembler works fine with Clang.
> Andrew> All the issues I found were minor, and specific to individual tests.
>
> I sent a couple of notes; but aside from those and the other responses,
> everything looks good to me.  Thank you for doing this.

Thanks for the reviews.  I made the changes Lancelot suggested, and
pushed this series.

Andrew

Re: [PATCH 2/6] gdb/testsuite: fix gdb.trace/unavailable-dwarf-piece.exp with Clang

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> Currently, the unavailable-dwarf-piece.exp script names the assembler
Andrew> file using standard_output_file, in this commit I've switched to make
Andrew> use of standard_testfile, as that seems to be the more common way of
Andrew> doing this sort of thing.

I think this caused the .S file to be created outside of the test's
directory.  Using standard_output_file would fix this.  I think it's
more normal to do this as well; searching for "set asm_file" yields a
lot of hits of the form:

    set asm_file [standard_output_file $srcfile2]

So if this change isn't needed to make the test work with clang, I'd
suggest reverting this part.

Tom

Re: [PATCH 2/6] gdb/testsuite: fix gdb.trace/unavailable-dwarf-piece.exp with Clang

[Andrew Burgess]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Andrew> Currently, the unavailable-dwarf-piece.exp script names the assembler
> Andrew> file using standard_output_file, in this commit I've switched to make
> Andrew> use of standard_testfile, as that seems to be the more common way of
> Andrew> doing this sort of thing.
>
> I think this caused the .S file to be created outside of the test's
> directory.  Using standard_output_file would fix this.  I think it's
> more normal to do this as well; searching for "set asm_file" yields a
> lot of hits of the form:
>
>     set asm_file [standard_output_file $srcfile2]
>
> So if this change isn't needed to make the test work with clang, I'd
> suggest reverting this part.

Sorry for this.  I pushed the patch below to fix this issue.

Thanks,
Andrew

---

commit dc3fb44540b586c02ec2f841e106a8d2cc2a422d
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Tue Dec 13 15:37:17 2022 +0000

    gdb/testsuite: avoid creating temp file in gdb/testsuite/ directory
    
    After this commit:
    
      commit 33c1395cf5e9deec7733691ba32c450e5c27f757
      Date:   Fri Nov 11 15:26:46 2022 +0000
    
          gdb/testsuite: fix gdb.trace/unavailable-dwarf-piece.exp with Clang
    
    The gdb.trace/unavailable-dwarf-piece.exp test script was creating a
    temporary file in the build/gdb/testsuite/ directory, instead of in
    the expected place in the outputs directory.
    
    Fix this by adding a call to standard_output_file.

diff --git a/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp b/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
index 13c6f38737c..d73b9f1e33f 100644
--- a/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
+++ b/gdb/testsuite/gdb.trace/unavailable-dwarf-piece.exp
@@ -22,7 +22,7 @@ if {![dwarf2_support]} {
 
 standard_testfile .c -dbg.S
 
-set asm_file $srcfile2
+set asm_file [standard_output_file $srcfile2]
 set opts {}
 
 if  { [gdb_compile ${srcdir}/${subdir}/${srcfile} ${binfile}1.o \