[PATCH v3 00/14] Clean gdb.base when testing with clang

[PATCH v3 00/14] Clean gdb.base when testing with clang

[Bruno Larsen]

When testing GDB with clang, gdb.base had over 50 more failures than when
testing with gcc.  Examining the failed tests led to a few clang bugs, a
few GDB bugs, and many testsuite assumptions that could be changed.

After this patch series, nothing should be changed for testing with gcc,
and testing with clang should only show non-trivial failures for
maint.exp and macscp.exp, along with the same GCC failures.

Changes in v3:
    * Fixed some issues that only showed up on CXX_FOR_TARGET=clang
    * rebased on current master

Changes in v2:
    * Introduced gdb_step_until_regexp, based on Pedro's and Andrew's suggestions
    * reworked fixes for: skip.exp, skip-solib.exp and msym-bp-shl.exp
    * Used Pedro's suggestion for call-ar-st
    * reordered patches slightly

Bruno Larsen (14):
  gdb/testsuite: introduce gdb_step_until_regexp
  Change gdb.base/skip-solib.exp deal with lack of epilogue information
  change gdb.base/symbol-alias to xfail with clang
  change gdb.base/nodebug.c to not fail with clang
  update gdb.base/info-program.exp to not fail with clang
  fix gdb.base/access-mem-running.exp for clang testing
  Fix gdb.base/call-ar-st to work with Clang
  add xfails to gdb.base/complex-parts.exp when testing with clang
  gdb/testsuite: fix gdb.base/msym-bp-shl when running with Clang
  explicitly test for stderr in gdb.base/dprintf.exp
  gdb/testsuite: Update gdb.base/so-impl-ld.exp
  [gdb/testsuite]: fix gdb.base/jit-elf.exp when testing with clang
  gdb/testsuite: fix gdb.base/info-types-c++ with clang
  gdb.base/skip.exp: Use finish to exit functions

 gdb/testsuite/gdb.base/access-mem-running.c |   2 +-
 gdb/testsuite/gdb.base/call-ar-st.exp       |  13 ++-
 gdb/testsuite/gdb.base/complex-parts.exp    |   5 +
 gdb/testsuite/gdb.base/dprintf.exp          |  10 ++
 gdb/testsuite/gdb.base/info-program.exp     |   2 +-
 gdb/testsuite/gdb.base/info-types.exp.tcl   | 109 +++++++++++++-------
 gdb/testsuite/gdb.base/jit-elf.exp          |   2 +-
 gdb/testsuite/gdb.base/msym-bp-shl.exp      |   8 ++
 gdb/testsuite/gdb.base/nodebug.c            |   2 +-
 gdb/testsuite/gdb.base/nodebug.exp          |   2 +-
 gdb/testsuite/gdb.base/skip-inline.exp      |  18 ++--
 gdb/testsuite/gdb.base/skip-solib-lib.c     |   3 +-
 gdb/testsuite/gdb.base/skip-solib-main.c    |   3 +-
 gdb/testsuite/gdb.base/skip-solib.exp       |  12 ++-
 gdb/testsuite/gdb.base/skip.exp             |  32 +++---
 gdb/testsuite/gdb.base/so-impl-ld.exp       |  13 +--
 gdb/testsuite/gdb.base/solib1.c             |   5 +-
 gdb/testsuite/gdb.base/symbol-alias.exp     |   9 +-
 gdb/testsuite/lib/gdb.exp                   |  30 ++++++
 19 files changed, 195 insertions(+), 85 deletions(-)

-- 
2.31.1

[PATCH v3 01/14] gdb/testsuite: introduce gdb_step_until_regexp

[Bruno Larsen]

Currently, GDB's testsuite uses a set amount of step commands to exit
functions. This is a problem if a compiler emits different epilogue
information from gcc, or emits no epilogue information at all. It was
most noticeable if Clang was used to test GDB.

To fix this unreliability, this commit introduces a new proc that will
single step the inferior until it is stopped at a line that matches the
given regexp, or until it steps too many times - defined as an optional
argument. If the line is found, it shows up as a single PASS in the
test, and if the line is not found, a single FAIL is emitted.

This patch only introduces this proc, but does not add it to any
existing tests, these will be introduced in the following commit.
---

No change in v3

New patch in v2

---
 gdb/testsuite/lib/gdb.exp | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index b04fbb89e4e..c0ca1d04cc2 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -8648,5 +8648,35 @@ proc get_set_option_choices {set_cmd} {
     return $values
 }
 
+# This proc is used mainly to exit function in a compiler agnostic way
+# It makes gdb single step and evaluate the output at every step, to see
+# if the regexp is present.
+#
+# The proc takes 2 optional arguments, the first being the name of the
+# test and the second the maximum amount of iterations until we expect to
+# see the function. The default is 10 steps, since this is meant as the
+# last step by default, and we don't expect any compiler generated epilogue
+# longer than 10 steps.
+
+proc gdb_step_until_regexp { regexp {test_name "single stepping until regexp"} {max_steps 10} } {
+    global gdb_prompt
+
+    set count 0
+    gdb_test_multiple "step" "$test_name" {
+	-re "$regexp\r\n$gdb_prompt $" {
+	    pass $test_name
+	}
+	-re ".*$gdb_prompt $" {
+	    if {$count < $max_steps} {
+		incr count
+		send_gdb "step\n"
+		exp_continue
+	    } else {
+		fail $test_name
+	    }
+	}
+    }
+}
+
 # Always load compatibility stuff.
 load_lib future.exp
-- 
2.31.1

Re: [PATCH v3 01/14] gdb/testsuite: introduce gdb_step_until_regexp

[Andrew Burgess]

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

> Currently, GDB's testsuite uses a set amount of step commands to exit
> functions. This is a problem if a compiler emits different epilogue
> information from gcc, or emits no epilogue information at all. It was
> most noticeable if Clang was used to test GDB.
>
> To fix this unreliability, this commit introduces a new proc that will
> single step the inferior until it is stopped at a line that matches the
> given regexp, or until it steps too many times - defined as an optional
> argument. If the line is found, it shows up as a single PASS in the
> test, and if the line is not found, a single FAIL is emitted.
>
> This patch only introduces this proc, but does not add it to any
> existing tests, these will be introduced in the following commit.
> ---
>
> No change in v3
>
> New patch in v2
>
> ---
>  gdb/testsuite/lib/gdb.exp | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index b04fbb89e4e..c0ca1d04cc2 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -8648,5 +8648,35 @@ proc get_set_option_choices {set_cmd} {
>      return $values
>  }
>  
> +# This proc is used mainly to exit function in a compiler agnostic way
> +# It makes gdb single step and evaluate the output at every step, to see
> +# if the regexp is present.
> +#
> +# The proc takes 2 optional arguments, the first being the name of the
> +# test and the second the maximum amount of iterations until we expect to
> +# see the function. The default is 10 steps, since this is meant as the
> +# last step by default, and we don't expect any compiler generated epilogue
> +# longer than 10 steps.

I feel like you are being overly prescriptive in how this function
should be used.

I would rewrite this to just describe what the function does, and let
folk use it as they see fit.  Sure, initially it will only be used as
you imagine - that's why you're adding it.  But once it's there, who
knows what uses it might be put too.

I'd go with something like:

  # Single step until the pattern REGEXP is found.  Step at most
  # MAX_STEPS times, but stop stepping once REGEXP is found.
  #
  # If REGEXP is found then a single pass is emitted, otherwise, after
  # MAX_STEPS steps, a single fail is emitted.
  #
  # TEST_NAME is the name used in the pass/fail calls.

> +
> +proc gdb_step_until_regexp { regexp {test_name "single stepping until regexp"} {max_steps 10} } {

You should keep this line under 80 characters.  You can wrap the
arguments I believe, like:

  proc gdb_step_until_regexp { regexp
                               {test_name "single stepping until regexp"}
                               {max_steps 10} } {

However, I'd be tempted to take a different approach, like this:

  proc gdb_step_until_regexp { regexp {test_name ""} {max_steps 10} } {

    if { $test_name == "" } {
      set test_name "single stepping until regexp"
    }

The benefit I see in this approach is that if a user wants to adjust
max_steps, but doesn't care about the test name, they can do this:

  gdb_step_until_regexp "SOME_PATTERN" "" 50

And still get the default test name.

> +    global gdb_prompt

I think this is OK, there's certainly lots of precedent for this
approach, but I think more often these days, we just refer to the global
directly as:

  $::gdb_prompt

As this removes the need for the 'global gdb_prompt' line.

But I don't think this is a hard requirement if you prefer what you
have.

Thanks,
Andrew

> +
> +    set count 0
> +    gdb_test_multiple "step" "$test_name" {
> +	-re "$regexp\r\n$gdb_prompt $" {
> +	    pass $test_name
> +	}
> +	-re ".*$gdb_prompt $" {
> +	    if {$count < $max_steps} {
> +		incr count
> +		send_gdb "step\n"
> +		exp_continue
> +	    } else {
> +		fail $test_name
> +	    }
> +	}
> +    }
> +}
> +
>  # Always load compatibility stuff.
>  load_lib future.exp
> -- 
> 2.31.1

Re: [PATCH v3 01/14] gdb/testsuite: introduce gdb_step_until_regexp

[Bruno Larsen]

Hi Andrew,

thanks for the review!

On 5/27/22 13:19, Andrew Burgess wrote:
> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
>> Currently, GDB's testsuite uses a set amount of step commands to exit
>> functions. This is a problem if a compiler emits different epilogue
>> information from gcc, or emits no epilogue information at all. It was
>> most noticeable if Clang was used to test GDB.
>>
>> To fix this unreliability, this commit introduces a new proc that will
>> single step the inferior until it is stopped at a line that matches the
>> given regexp, or until it steps too many times - defined as an optional
>> argument. If the line is found, it shows up as a single PASS in the
>> test, and if the line is not found, a single FAIL is emitted.
>>
>> This patch only introduces this proc, but does not add it to any
>> existing tests, these will be introduced in the following commit.
>> ---
>>
>> No change in v3
>>
>> New patch in v2
>>
>> ---
>>   gdb/testsuite/lib/gdb.exp | 30 ++++++++++++++++++++++++++++++
>>   1 file changed, 30 insertions(+)
>>
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index b04fbb89e4e..c0ca1d04cc2 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -8648,5 +8648,35 @@ proc get_set_option_choices {set_cmd} {
>>       return $values
>>   }
>>   
>> +# This proc is used mainly to exit function in a compiler agnostic way
>> +# It makes gdb single step and evaluate the output at every step, to see
>> +# if the regexp is present.
>> +#
>> +# The proc takes 2 optional arguments, the first being the name of the
>> +# test and the second the maximum amount of iterations until we expect to
>> +# see the function. The default is 10 steps, since this is meant as the
>> +# last step by default, and we don't expect any compiler generated epilogue
>> +# longer than 10 steps.
> 
> I feel like you are being overly prescriptive in how this function
> should be used.
> 
> I would rewrite this to just describe what the function does, and let
> folk use it as they see fit.  Sure, initially it will only be used as
> you imagine - that's why you're adding it.  But once it's there, who
> knows what uses it might be put too.
> 
> I'd go with something like:
> 
>    # Single step until the pattern REGEXP is found.  Step at most
>    # MAX_STEPS times, but stop stepping once REGEXP is found.
>    #
>    # If REGEXP is found then a single pass is emitted, otherwise, after
>    # MAX_STEPS steps, a single fail is emitted.
>    #
>    # TEST_NAME is the name used in the pass/fail calls.
> 

Good point. I've used your version of the comment.

>> +
>> +proc gdb_step_until_regexp { regexp {test_name "single stepping until regexp"} {max_steps 10} } {
> 
> You should keep this line under 80 characters.  You can wrap the
> arguments I believe, like:
> 
>    proc gdb_step_until_regexp { regexp
>                                 {test_name "single stepping until regexp"}
>                                 {max_steps 10} } {
> 
> However, I'd be tempted to take a different approach, like this:
> 
>    proc gdb_step_until_regexp { regexp {test_name ""} {max_steps 10} } {
> 
>      if { $test_name == "" } {
>        set test_name "single stepping until regexp"
>      }
> 
> The benefit I see in this approach is that if a user wants to adjust
> max_steps, but doesn't care about the test name, they can do this:
> 
>    gdb_step_until_regexp "SOME_PATTERN" "" 50
> 
> And still get the default test name.

That's a good reason. I didn't have any for my choice other than it's what I was comfortable with. Using your version again

> 
>> +    global gdb_prompt
> 
> I think this is OK, there's certainly lots of precedent for this
> approach, but I think more often these days, we just refer to the global
> directly as:
> 
>    $::gdb_prompt
> 
> As this removes the need for the 'global gdb_prompt' line.
> 
> But I don't think this is a hard requirement if you prefer what you
> have.

I do like this version more too, more clear that it is a global variable for new contributors.

At this point, I wonder if I should add a co-authored tag to the patch.

> 
> Thanks,
> Andrew
> 
>> +
>> +    set count 0
>> +    gdb_test_multiple "step" "$test_name" {
>> +	-re "$regexp\r\n$gdb_prompt $" {
>> +	    pass $test_name
>> +	}
>> +	-re ".*$gdb_prompt $" {
>> +	    if {$count < $max_steps} {
>> +		incr count
>> +		send_gdb "step\n"
>> +		exp_continue
>> +	    } else {
>> +		fail $test_name
>> +	    }
>> +	}
>> +    }
>> +}
>> +
>>   # Always load compatibility stuff.
>>   load_lib future.exp
>> -- 
>> 2.31.1
> 
Cheers!
Bruno Larsen

Re: [PATCH v3 01/14] gdb/testsuite: introduce gdb_step_until_regexp

[Andrew Burgess]

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

> Hi Andrew,
>
> thanks for the review!
>
> On 5/27/22 13:19, Andrew Burgess wrote:
>> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
>> 
>>> Currently, GDB's testsuite uses a set amount of step commands to exit
>>> functions. This is a problem if a compiler emits different epilogue
>>> information from gcc, or emits no epilogue information at all. It was
>>> most noticeable if Clang was used to test GDB.
>>>
>>> To fix this unreliability, this commit introduces a new proc that will
>>> single step the inferior until it is stopped at a line that matches the
>>> given regexp, or until it steps too many times - defined as an optional
>>> argument. If the line is found, it shows up as a single PASS in the
>>> test, and if the line is not found, a single FAIL is emitted.
>>>
>>> This patch only introduces this proc, but does not add it to any
>>> existing tests, these will be introduced in the following commit.
>>> ---
>>>
>>> No change in v3
>>>
>>> New patch in v2
>>>
>>> ---
>>>   gdb/testsuite/lib/gdb.exp | 30 ++++++++++++++++++++++++++++++
>>>   1 file changed, 30 insertions(+)
>>>
>>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>>> index b04fbb89e4e..c0ca1d04cc2 100644
>>> --- a/gdb/testsuite/lib/gdb.exp
>>> +++ b/gdb/testsuite/lib/gdb.exp
>>> @@ -8648,5 +8648,35 @@ proc get_set_option_choices {set_cmd} {
>>>       return $values
>>>   }
>>>   
>>> +# This proc is used mainly to exit function in a compiler agnostic way
>>> +# It makes gdb single step and evaluate the output at every step, to see
>>> +# if the regexp is present.
>>> +#
>>> +# The proc takes 2 optional arguments, the first being the name of the
>>> +# test and the second the maximum amount of iterations until we expect to
>>> +# see the function. The default is 10 steps, since this is meant as the
>>> +# last step by default, and we don't expect any compiler generated epilogue
>>> +# longer than 10 steps.
>> 
>> I feel like you are being overly prescriptive in how this function
>> should be used.
>> 
>> I would rewrite this to just describe what the function does, and let
>> folk use it as they see fit.  Sure, initially it will only be used as
>> you imagine - that's why you're adding it.  But once it's there, who
>> knows what uses it might be put too.
>> 
>> I'd go with something like:
>> 
>>    # Single step until the pattern REGEXP is found.  Step at most
>>    # MAX_STEPS times, but stop stepping once REGEXP is found.
>>    #
>>    # If REGEXP is found then a single pass is emitted, otherwise, after
>>    # MAX_STEPS steps, a single fail is emitted.
>>    #
>>    # TEST_NAME is the name used in the pass/fail calls.
>> 
>
> Good point. I've used your version of the comment.
>
>>> +
>>> +proc gdb_step_until_regexp { regexp {test_name "single stepping until regexp"} {max_steps 10} } {
>> 
>> You should keep this line under 80 characters.  You can wrap the
>> arguments I believe, like:
>> 
>>    proc gdb_step_until_regexp { regexp
>>                                 {test_name "single stepping until regexp"}
>>                                 {max_steps 10} } {
>> 
>> However, I'd be tempted to take a different approach, like this:
>> 
>>    proc gdb_step_until_regexp { regexp {test_name ""} {max_steps 10} } {
>> 
>>      if { $test_name == "" } {
>>        set test_name "single stepping until regexp"
>>      }
>> 
>> The benefit I see in this approach is that if a user wants to adjust
>> max_steps, but doesn't care about the test name, they can do this:
>> 
>>    gdb_step_until_regexp "SOME_PATTERN" "" 50
>> 
>> And still get the default test name.
>
> That's a good reason. I didn't have any for my choice other than it's what I was comfortable with. Using your version again
>
>> 
>>> +    global gdb_prompt
>> 
>> I think this is OK, there's certainly lots of precedent for this
>> approach, but I think more often these days, we just refer to the global
>> directly as:
>> 
>>    $::gdb_prompt
>> 
>> As this removes the need for the 'global gdb_prompt' line.
>> 
>> But I don't think this is a hard requirement if you prefer what you
>> have.
>
> I do like this version more too, more clear that it is a global variable for new contributors.
>
> At this point, I wonder if I should add a co-authored tag to the
> patch.

That's up to you, but not required from my side.

Thanks,
Andrew

>
>> 
>> Thanks,
>> Andrew
>> 
>>> +
>>> +    set count 0
>>> +    gdb_test_multiple "step" "$test_name" {
>>> +	-re "$regexp\r\n$gdb_prompt $" {
>>> +	    pass $test_name
>>> +	}
>>> +	-re ".*$gdb_prompt $" {
>>> +	    if {$count < $max_steps} {
>>> +		incr count
>>> +		send_gdb "step\n"
>>> +		exp_continue
>>> +	    } else {
>>> +		fail $test_name
>>> +	    }
>>> +	}
>>> +    }
>>> +}
>>> +
>>>   # Always load compatibility stuff.
>>>   load_lib future.exp
>>> -- 
>>> 2.31.1
>> 
> Cheers!
> Bruno Larsen

Re: [PATCH v3 01/14] gdb/testsuite: introduce gdb_step_until_regexp

[Pedro Alves]

On 2022-05-27 17:19, Andrew Burgess via Gdb-patches wrote:
> 
> I'd go with something like:
> 
>   # Single step until the pattern REGEXP is found.  Step at most
>   # MAX_STEPS times, but stop stepping once REGEXP is found.

It all sounds fine, except that "single step" usually means "stepi", while
the proc uses "step".  So I'd suggest "Single step" -> "Step".

[PATCH v3 02/14] Change gdb.base/skip-solib.exp deal with lack of epilogue information

[Bruno Larsen]

When running gdb.base/skip-solib.exp, the backtrace tests could fail if
the compiler did not emit epilogue information for trivial epilogues,
despite the feature being fully functional.  As an example, when testing
skipping the function square, the testsuite would show

Breakpoint 1, main () at (...)/binutils-gdb/gdb/testsuite/gdb.base/skip-solib-main.c:5
5         return square(0);
(gdb) step
0x00007ffff7cef560 in __libc_start_call_main () from /lib64/libc.so.6
(gdb) PASS: gdb.base/skip-solib.exp: ignoring solib file: step
bt
 #0  0x00007ffff7cef560 in __libc_start_call_main () from /lib64/libc.so.6
 #1  0x00007ffff7cef60c in __libc_start_main_impl () from /lib64/libc.so.6
 #2  0x0000000000401065 in _start ()
(gdb) FAIL: gdb.base/skip-solib.exp: ignoring solib file: bt

Which means that the feature is working, the testsuite is just
mis-identifying it.  To avoid this problem, the skipped function calls
have been sent to a line before `return`, so epilogues won't factor in.

This commit has also changed a few hardcoded steps to leave functions to
the newly introduced gdb_step_until_regexp to leave those functions.
---

No change in v3

v2: chagned to use step_until_regexp instead of finish

---
 gdb/testsuite/gdb.base/skip-inline.exp   | 18 +++++++++++-------
 gdb/testsuite/gdb.base/skip-solib-lib.c  |  3 ++-
 gdb/testsuite/gdb.base/skip-solib-main.c |  3 ++-
 gdb/testsuite/gdb.base/skip-solib.exp    | 12 ++++++++++--
 4 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/gdb/testsuite/gdb.base/skip-inline.exp b/gdb/testsuite/gdb.base/skip-inline.exp
index f6e9926b66c..327ea676140 100644
--- a/gdb/testsuite/gdb.base/skip-inline.exp
+++ b/gdb/testsuite/gdb.base/skip-inline.exp
@@ -35,16 +35,20 @@ gdb_test "skip function foo" "Function foo will be skipped when stepping\."
 gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
 gdb_test "step" ".*" "step into baz, since foo will be skipped"
 gdb_test "bt" "\\s*\\#0\\s+baz.*" "in the baz, since foo was skipped"
-gdb_test "step" ".*" "step in the baz"
-gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz"
-gdb_test "step" ".*" "step back to main"
+gdb_step_until_regexp ".*x = 0; x = baz \\(foo \\(\\)\\).*"
 gdb_test "bt" "\\s*\\#0\\s+main.*" "again in the main"
 gdb_test "step" ".*" "step again into baz, since foo will be skipped"
 gdb_test "bt" "\\s*\\#0\\s+baz.*" "again in the baz"
-gdb_test "step" ".*" "step in the baz, again"
-gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz, again"
-gdb_test "step" ".*" "step back to main, again"
-gdb_test "bt" "\\s*\\#0\\s+main.*" "again back to main"
+gdb_step_until_regexp "main \\(\\) at .*" "step back to main, again"
+gdb_test "bt" "\\s*\\#0.*main.*" "again back to main"
+
+# because clang doesn't add epilogue information, having a set number of
+# steps puts clang more and more out of sync with gcc.  It is unlikely that
+# the effort of keeping both outputs will be useful.
+if {[test_compiler_info "clang-*"]} {
+    untested "Multiple steps are not supported with clang"
+    return
+}
 
 if ![runto_main] {
     return
diff --git a/gdb/testsuite/gdb.base/skip-solib-lib.c b/gdb/testsuite/gdb.base/skip-solib-lib.c
index b2c4d86d703..341f1440a3b 100644
--- a/gdb/testsuite/gdb.base/skip-solib-lib.c
+++ b/gdb/testsuite/gdb.base/skip-solib-lib.c
@@ -7,5 +7,6 @@ int multiply(int a, int b)
 
 int square(int num)
 {
-  return multiply(num, num);
+  int res = multiply(num, num);
+  return res;
 }
diff --git a/gdb/testsuite/gdb.base/skip-solib-main.c b/gdb/testsuite/gdb.base/skip-solib-main.c
index 746bb5f36bb..a3b6d417935 100644
--- a/gdb/testsuite/gdb.base/skip-solib-main.c
+++ b/gdb/testsuite/gdb.base/skip-solib-main.c
@@ -2,5 +2,6 @@ int square(int num);
 
 int main()
 {
-  return square(0);
+  int s = square(0);
+  return s;
 }
diff --git a/gdb/testsuite/gdb.base/skip-solib.exp b/gdb/testsuite/gdb.base/skip-solib.exp
index 0f2ce7e1ad8..8e61725ad1b 100644
--- a/gdb/testsuite/gdb.base/skip-solib.exp
+++ b/gdb/testsuite/gdb.base/skip-solib.exp
@@ -82,7 +82,7 @@ with_test_prefix "ignoring solib file" {
     # We shouldn't step into square(), since we skipped skip-solib-lib.c.
     #
     gdb_test "step" ""
-    gdb_test "bt" "#0\\s+main.*"
+    gdb_test "bt 1" "#0\\s+main.*"
 }
 
 #
@@ -114,5 +114,13 @@ with_test_prefix "ignoring solib function" {
     # the last line of square.
     #
     gdb_test "step" ""
-    gdb_test "bt" "#0\\s+square.*"
+    gdb_test "bt 1" "#0\\s+square.*" "skipped multiply"
+#    gdb_test_multiple "bt 1" "skipped multiply" {
+#	-re "#0\\s+square.*" {
+#	    pass "skipped multiply"
+#	}
+#	-re "#0.*main.*" {
+#	    pass "skipped multiply"
+#	}
+#    }
 }
-- 
2.31.1

Re: [PATCH v3 02/14] Change gdb.base/skip-solib.exp deal with lack of epilogue information

[Andrew Burgess]

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

> When running gdb.base/skip-solib.exp, the backtrace tests could fail if
> the compiler did not emit epilogue information for trivial epilogues,
> despite the feature being fully functional.  As an example, when testing
> skipping the function square, the testsuite would show
>
> Breakpoint 1, main () at (...)/binutils-gdb/gdb/testsuite/gdb.base/skip-solib-main.c:5
> 5         return square(0);
> (gdb) step
> 0x00007ffff7cef560 in __libc_start_call_main () from /lib64/libc.so.6
> (gdb) PASS: gdb.base/skip-solib.exp: ignoring solib file: step
> bt
>  #0  0x00007ffff7cef560 in __libc_start_call_main () from /lib64/libc.so.6
>  #1  0x00007ffff7cef60c in __libc_start_main_impl () from /lib64/libc.so.6
>  #2  0x0000000000401065 in _start ()
> (gdb) FAIL: gdb.base/skip-solib.exp: ignoring solib file: bt
>
> Which means that the feature is working, the testsuite is just
> mis-identifying it.  To avoid this problem, the skipped function calls
> have been sent to a line before `return`, so epilogues won't factor in.
>
> This commit has also changed a few hardcoded steps to leave functions to
> the newly introduced gdb_step_until_regexp to leave those functions.
> ---
>
> No change in v3
>
> v2: chagned to use step_until_regexp instead of finish
>
> ---
>  gdb/testsuite/gdb.base/skip-inline.exp   | 18 +++++++++++-------
>  gdb/testsuite/gdb.base/skip-solib-lib.c  |  3 ++-
>  gdb/testsuite/gdb.base/skip-solib-main.c |  3 ++-
>  gdb/testsuite/gdb.base/skip-solib.exp    | 12 ++++++++++--
>  4 files changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/skip-inline.exp b/gdb/testsuite/gdb.base/skip-inline.exp
> index f6e9926b66c..327ea676140 100644
> --- a/gdb/testsuite/gdb.base/skip-inline.exp
> +++ b/gdb/testsuite/gdb.base/skip-inline.exp
> @@ -35,16 +35,20 @@ gdb_test "skip function foo" "Function foo will be skipped when stepping\."
>  gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
>  gdb_test "step" ".*" "step into baz, since foo will be skipped"
>  gdb_test "bt" "\\s*\\#0\\s+baz.*" "in the baz, since foo was skipped"
> -gdb_test "step" ".*" "step in the baz"
> -gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz"
> -gdb_test "step" ".*" "step back to main"
> +gdb_step_until_regexp ".*x = 0; x = baz \\(foo \\(\\)\\).*"
>  gdb_test "bt" "\\s*\\#0\\s+main.*" "again in the main"
>  gdb_test "step" ".*" "step again into baz, since foo will be skipped"
>  gdb_test "bt" "\\s*\\#0\\s+baz.*" "again in the baz"
> -gdb_test "step" ".*" "step in the baz, again"
> -gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz, again"
> -gdb_test "step" ".*" "step back to main, again"
> -gdb_test "bt" "\\s*\\#0\\s+main.*" "again back to main"
> +gdb_step_until_regexp "main \\(\\) at .*" "step back to main, again"
> +gdb_test "bt" "\\s*\\#0.*main.*" "again back to main"
> +
> +# because clang doesn't add epilogue information, having a set number of

Capital letter - 'Because'.

> +# steps puts clang more and more out of sync with gcc.  It is unlikely that
> +# the effort of keeping both outputs will be useful.
> +if {[test_compiler_info "clang-*"]} {
> +    untested "Multiple steps are not supported with clang"

I'd reword this as "Multiple step tests are not supported with clang"
just to avoid confusion.  Multiple steps are fine, it's the tests
themselves we're not proposing to support with clang.

> +    return
> +}
>  
>  if ![runto_main] {
>      return
> diff --git a/gdb/testsuite/gdb.base/skip-solib-lib.c b/gdb/testsuite/gdb.base/skip-solib-lib.c
> index b2c4d86d703..341f1440a3b 100644
> --- a/gdb/testsuite/gdb.base/skip-solib-lib.c
> +++ b/gdb/testsuite/gdb.base/skip-solib-lib.c
> @@ -7,5 +7,6 @@ int multiply(int a, int b)
>  
>  int square(int num)
>  {
> -  return multiply(num, num);
> +  int res = multiply(num, num);
> +  return res;
>  }
> diff --git a/gdb/testsuite/gdb.base/skip-solib-main.c b/gdb/testsuite/gdb.base/skip-solib-main.c
> index 746bb5f36bb..a3b6d417935 100644
> --- a/gdb/testsuite/gdb.base/skip-solib-main.c
> +++ b/gdb/testsuite/gdb.base/skip-solib-main.c
> @@ -2,5 +2,6 @@ int square(int num);
>  
>  int main()
>  {
> -  return square(0);
> +  int s = square(0);
> +  return s;
>  }
> diff --git a/gdb/testsuite/gdb.base/skip-solib.exp b/gdb/testsuite/gdb.base/skip-solib.exp
> index 0f2ce7e1ad8..8e61725ad1b 100644
> --- a/gdb/testsuite/gdb.base/skip-solib.exp
> +++ b/gdb/testsuite/gdb.base/skip-solib.exp
> @@ -82,7 +82,7 @@ with_test_prefix "ignoring solib file" {
>      # We shouldn't step into square(), since we skipped skip-solib-lib.c.
>      #
>      gdb_test "step" ""
> -    gdb_test "bt" "#0\\s+main.*"
> +    gdb_test "bt 1" "#0\\s+main.*"

I don't object to this change, but it's not necessary.

>  }
>  
>  #
> @@ -114,5 +114,13 @@ with_test_prefix "ignoring solib function" {
>      # the last line of square.
>      #
>      gdb_test "step" ""
> -    gdb_test "bt" "#0\\s+square.*"
> +    gdb_test "bt 1" "#0\\s+square.*" "skipped multiply"
> +#    gdb_test_multiple "bt 1" "skipped multiply" {
> +#	-re "#0\\s+square.*" {
> +#	    pass "skipped multiply"
> +#	}
> +#	-re "#0.*main.*" {
> +#	    pass "skipped multiply"
> +#	}
> +#    }

So, I was worried while looking at this change that we might end up
removing all the places where we try to step through code like:

  int square(int num)
  {
    return multiply(num, num);
  }

Which I think would be a shame.  However, I notice you (accidentally)
left in some commented out code - and I think we might be able to use
that.

What if we reverted the change to the `square` function, and then, for
this last test did this:

    gdb_test_multiple "bt 1" "skipped multiply" {
	-re "#0\\s+square.*" {
	    pass $gdb_test_name
	}
	-re "#0.*main.*" {
	    # Clang doesn't add sufficient epilogue information to
	    # allow GDB to stop within square after the call to
	    # multiply.  We accept ending up back in main, but only
	    # for clang.
	    if [test_compiler_info clang*] {
		pass $gdb_test_name
	    }
	}
    }

For any compiler that, like gcc, emits some line info for the function
epilogue, the test passes.  But we let clang pass even though it appears
to unwind the extra frame.

Thoughts?

Thanks,
Andrew

Re: [PATCH v3 02/14] Change gdb.base/skip-solib.exp deal with lack of epilogue information

[Bruno Larsen]

Cheers!
Bruno Larsen

On 5/30/22 11:04, Andrew Burgess wrote:
> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
>> When running gdb.base/skip-solib.exp, the backtrace tests could fail if
>> the compiler did not emit epilogue information for trivial epilogues,
>> despite the feature being fully functional.  As an example, when testing
>> skipping the function square, the testsuite would show
>>
>> Breakpoint 1, main () at (...)/binutils-gdb/gdb/testsuite/gdb.base/skip-solib-main.c:5
>> 5         return square(0);
>> (gdb) step
>> 0x00007ffff7cef560 in __libc_start_call_main () from /lib64/libc.so.6
>> (gdb) PASS: gdb.base/skip-solib.exp: ignoring solib file: step
>> bt
>>   #0  0x00007ffff7cef560 in __libc_start_call_main () from /lib64/libc.so.6
>>   #1  0x00007ffff7cef60c in __libc_start_main_impl () from /lib64/libc.so.6
>>   #2  0x0000000000401065 in _start ()
>> (gdb) FAIL: gdb.base/skip-solib.exp: ignoring solib file: bt
>>
>> Which means that the feature is working, the testsuite is just
>> mis-identifying it.  To avoid this problem, the skipped function calls
>> have been sent to a line before `return`, so epilogues won't factor in.
>>
>> This commit has also changed a few hardcoded steps to leave functions to
>> the newly introduced gdb_step_until_regexp to leave those functions.
>> ---
>>
>> No change in v3
>>
>> v2: chagned to use step_until_regexp instead of finish
>>
>> ---
>>   gdb/testsuite/gdb.base/skip-inline.exp   | 18 +++++++++++-------
>>   gdb/testsuite/gdb.base/skip-solib-lib.c  |  3 ++-
>>   gdb/testsuite/gdb.base/skip-solib-main.c |  3 ++-
>>   gdb/testsuite/gdb.base/skip-solib.exp    | 12 ++++++++++--
>>   4 files changed, 25 insertions(+), 11 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.base/skip-inline.exp b/gdb/testsuite/gdb.base/skip-inline.exp
>> index f6e9926b66c..327ea676140 100644
>> --- a/gdb/testsuite/gdb.base/skip-inline.exp
>> +++ b/gdb/testsuite/gdb.base/skip-inline.exp
>> @@ -35,16 +35,20 @@ gdb_test "skip function foo" "Function foo will be skipped when stepping\."
>>   gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
>>   gdb_test "step" ".*" "step into baz, since foo will be skipped"
>>   gdb_test "bt" "\\s*\\#0\\s+baz.*" "in the baz, since foo was skipped"
>> -gdb_test "step" ".*" "step in the baz"
>> -gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz"
>> -gdb_test "step" ".*" "step back to main"
>> +gdb_step_until_regexp ".*x = 0; x = baz \\(foo \\(\\)\\).*"
>>   gdb_test "bt" "\\s*\\#0\\s+main.*" "again in the main"
>>   gdb_test "step" ".*" "step again into baz, since foo will be skipped"
>>   gdb_test "bt" "\\s*\\#0\\s+baz.*" "again in the baz"
>> -gdb_test "step" ".*" "step in the baz, again"
>> -gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz, again"
>> -gdb_test "step" ".*" "step back to main, again"
>> -gdb_test "bt" "\\s*\\#0\\s+main.*" "again back to main"
>> +gdb_step_until_regexp "main \\(\\) at .*" "step back to main, again"
>> +gdb_test "bt" "\\s*\\#0.*main.*" "again back to main"
>> +
>> +# because clang doesn't add epilogue information, having a set number of
> 
> Capital letter - 'Because'.

Fixed.

> 
>> +# steps puts clang more and more out of sync with gcc.  It is unlikely that
>> +# the effort of keeping both outputs will be useful.
>> +if {[test_compiler_info "clang-*"]} {
>> +    untested "Multiple steps are not supported with clang"
> 
> I'd reword this as "Multiple step tests are not supported with clang"
> just to avoid confusion.  Multiple steps are fine, it's the tests
> themselves we're not proposing to support with clang.

good catch! Fixed.

> 
>> +    return
>> +}
>>   
>>   if ![runto_main] {
>>       return
>> diff --git a/gdb/testsuite/gdb.base/skip-solib-lib.c b/gdb/testsuite/gdb.base/skip-solib-lib.c
>> index b2c4d86d703..341f1440a3b 100644
>> --- a/gdb/testsuite/gdb.base/skip-solib-lib.c
>> +++ b/gdb/testsuite/gdb.base/skip-solib-lib.c
>> @@ -7,5 +7,6 @@ int multiply(int a, int b)
>>   
>>   int square(int num)
>>   {
>> -  return multiply(num, num);
>> +  int res = multiply(num, num);
>> +  return res;
>>   }
>> diff --git a/gdb/testsuite/gdb.base/skip-solib-main.c b/gdb/testsuite/gdb.base/skip-solib-main.c
>> index 746bb5f36bb..a3b6d417935 100644
>> --- a/gdb/testsuite/gdb.base/skip-solib-main.c
>> +++ b/gdb/testsuite/gdb.base/skip-solib-main.c
>> @@ -2,5 +2,6 @@ int square(int num);
>>   
>>   int main()
>>   {
>> -  return square(0);
>> +  int s = square(0);
>> +  return s;
>>   }
>> diff --git a/gdb/testsuite/gdb.base/skip-solib.exp b/gdb/testsuite/gdb.base/skip-solib.exp
>> index 0f2ce7e1ad8..8e61725ad1b 100644
>> --- a/gdb/testsuite/gdb.base/skip-solib.exp
>> +++ b/gdb/testsuite/gdb.base/skip-solib.exp
>> @@ -82,7 +82,7 @@ with_test_prefix "ignoring solib file" {
>>       # We shouldn't step into square(), since we skipped skip-solib-lib.c.
>>       #
>>       gdb_test "step" ""
>> -    gdb_test "bt" "#0\\s+main.*"
>> +    gdb_test "bt 1" "#0\\s+main.*"
> 
> I don't object to this change, but it's not necessary.
> 
>>   }
>>   
>>   #
>> @@ -114,5 +114,13 @@ with_test_prefix "ignoring solib function" {
>>       # the last line of square.
>>       #
>>       gdb_test "step" ""
>> -    gdb_test "bt" "#0\\s+square.*"
>> +    gdb_test "bt 1" "#0\\s+square.*" "skipped multiply"
>> +#    gdb_test_multiple "bt 1" "skipped multiply" {
>> +#	-re "#0\\s+square.*" {
>> +#	    pass "skipped multiply"
>> +#	}
>> +#	-re "#0.*main.*" {
>> +#	    pass "skipped multiply"
>> +#	}
>> +#    }
> 
> So, I was worried while looking at this change that we might end up
> removing all the places where we try to step through code like:
> 
>    int square(int num)
>    {
>      return multiply(num, num);
>    }
> 
> Which I think would be a shame.  However, I notice you (accidentally)
> left in some commented out code - and I think we might be able to use
> that.
> 
> What if we reverted the change to the `square` function, and then, for
> this last test did this:
> 
>      gdb_test_multiple "bt 1" "skipped multiply" {
> 	-re "#0\\s+square.*" {
> 	    pass $gdb_test_name
> 	}
> 	-re "#0.*main.*" {
> 	    # Clang doesn't add sufficient epilogue information to
> 	    # allow GDB to stop within square after the call to
> 	    # multiply.  We accept ending up back in main, but only
> 	    # for clang.
> 	    if [test_compiler_info clang*] {
> 		pass $gdb_test_name
> 	    }
> 	}
>      }
> 
> For any compiler that, like gcc, emits some line info for the function
> epilogue, the test passes.  But we let clang pass even though it appears
> to unwind the extra frame.
> 
> Thoughts?

As we spoke off-list, I am of the opinion that tests should only be used to test what they are trying to test. So the ideal solution would be having a test that tries to step through epilogues in different configurations, and we can safely assume that it works for the rest of the tests. I am working in one such test, which I'll send soon.

In the meantime, I'm not opposed to the specific change you proposed, as it is readable and small, so I can also do this if you think it is worth doing.

> 
> Thanks,
> Andrew
> 

[PATCH] gdb/testsuite: Add test to step through function epilogue

[Bruno Larsen]

The testsuite implicitly tests GDB's ability to step through epilogues
in multiple tests, without doing it explicitly anywhere.  This is
unfortunate, as clang does not emit epilogue information, so using clang
on our testsuite makes many tests fail.  This patch adds a central,
explicit test for walking through the epilogue so we can safely remove
this from other tests and have them working with clang.

The test created attempts to step through a simple empilogue, an
epilogue that ends on another epilogue, and epilogues leading to other
function calls.
---
 .../gdb.base/step-through-epilogue.c          | 15 ++++
 .../gdb.base/step-through-epilogue.exp        | 86 +++++++++++++++++++
 2 files changed, 101 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/step-through-epilogue.c
 create mode 100644 gdb/testsuite/gdb.base/step-through-epilogue.exp

diff --git a/gdb/testsuite/gdb.base/step-through-epilogue.c b/gdb/testsuite/gdb.base/step-through-epilogue.c
new file mode 100644
index 00000000000..3e0ea5a2522
--- /dev/null
+++ b/gdb/testsuite/gdb.base/step-through-epilogue.c
@@ -0,0 +1,15 @@
+int multiply (int a, int b){
+    return a*b;
+}
+
+int square (int x){
+    return multiply (x, x);
+}
+
+int main(){
+    int x;
+    x = multiply (1,2);
+    x = square(2);
+    x = multiply (square (1), square (2));
+    return 0;
+}
diff --git a/gdb/testsuite/gdb.base/step-through-epilogue.exp b/gdb/testsuite/gdb.base/step-through-epilogue.exp
new file mode 100644
index 00000000000..762623cf72c
--- /dev/null
+++ b/gdb/testsuite/gdb.base/step-through-epilogue.exp
@@ -0,0 +1,86 @@
+#   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 test is used to confirm that GDB is able to step, stopping at an
+# epilogue line, then step out of the function.
+
+standard_testfile
+
+if { [get_compiler_info "clang-*"] } {
+    untested "This test can't be used with clang"
+    return
+}
+
+if { [prepare_for_testing "failed to prepare" "skip" \
+			  {step-through-epilogue.c}] } {
+    untested "failed to prepare"
+    return
+}
+
+if { ![runto_main] } {
+    untested "couldn't run to main"
+    return
+}
+
+# Some gcc versions, at least 6.5.0 and 9.2.0, can mess this test by
+# requiring extra steps in a few locations.  We test here and use
+# this knowledge later to decide if the extra steps need to be taken.
+set slow_gcc 0
+if { [test_compiler_info "gcc-6-5-0"] || [test_compiler_info "gcc-9-2-0"]} {
+    set slow_gcc 1
+}
+
+proc step_till_epilogue_multiply {} {
+    gdb_test "step" ".*return a.b;.*" "step into multiply"
+    gdb_test "step" ".*3\\s+\\\}.*" "stop at the epilogue of multiply"
+}
+
+proc step_till_epilogue_square {} {
+    gdb_test "step" ".*return multiply.*" "step into square"
+    step_till_epilogue_multiply
+    gdb_test "step" ".*7\\s+\\\}.*" "stop at epilogue of square"
+}
+
+with_test_prefix "multiply" {
+    step_till_epilogue_multiply
+    gdb_test "step" ".*x = square\\(2\\);" "leave function"
+}
+
+with_test_prefix "square" {
+    step_till_epilogue_square
+    gdb_test "step" ".*x = multiply \\(square \\(1\\), square \\(2\\)\\);.*"\
+	"leave function"
+}
+
+with_test_prefix "square, first argument" {
+    step_till_epilogue_square
+    if {$slow_gcc} {
+	gdb_test "step"\
+	    ".*x = multiply \\(square \\(1\\), square \\(2\\)\\);.*"\
+	    "step back to main"
+    }
+}
+with_test_prefix "square, second argument" {
+    step_till_epilogue_square
+    if {$slow_gcc} {
+	gdb_test "step"\
+	    ".*x = multiply \\(square \\(1\\), square \\(2\\)\\);.*"\
+	    "step back to main"
+    }
+}
+with_test_prefix "multiply with function args" {
+    step_till_epilogue_multiply
+    gdb_test "step" ".*return 0;.*" "leave last function"
+}
-- 
2.31.1

Re: [PATCH v3 02/14] Change gdb.base/skip-solib.exp deal with lack of epilogue information

[Pedro Alves]

On 2022-05-26 16:10, Bruno Larsen via Gdb-patches wrote:
> When running gdb.base/skip-solib.exp, the backtrace tests could fail if
> the compiler did not emit epilogue information for trivial epilogues,

I found "emit epilogue information" very confusing FWIW.  When I read that the first time,
I thought this was talking about unwind info for the prologue after the stack is destroyed,
and I wondered why not fix clang instead.

I think the following would be more accurate:

 "fail with compilers that associate prologue insns with the function's last
  statement line instead of the function's closing brace".

> despite the feature being fully functional.  As an example, when testing
> skipping the function square, the testsuite would show
> 


> ---
>  gdb/testsuite/gdb.base/skip-inline.exp   | 18 +++++++++++-------
>  gdb/testsuite/gdb.base/skip-solib-lib.c  |  3 ++-
>  gdb/testsuite/gdb.base/skip-solib-main.c |  3 ++-
>  gdb/testsuite/gdb.base/skip-solib.exp    | 12 ++++++++++--
>  4 files changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/skip-inline.exp b/gdb/testsuite/gdb.base/skip-inline.exp
> index f6e9926b66c..327ea676140 100644
> --- a/gdb/testsuite/gdb.base/skip-inline.exp
> +++ b/gdb/testsuite/gdb.base/skip-inline.exp
> @@ -35,16 +35,20 @@ gdb_test "skip function foo" "Function foo will be skipped when stepping\."
>  gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
>  gdb_test "step" ".*" "step into baz, since foo will be skipped"
>  gdb_test "bt" "\\s*\\#0\\s+baz.*" "in the baz, since foo was skipped"
> -gdb_test "step" ".*" "step in the baz"
> -gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz"
> -gdb_test "step" ".*" "step back to main"
> +gdb_step_until_regexp ".*x = 0; x = baz \\(foo \\(\\)\\).*"

FWIW, I'd remove the "regexp" from the function's name, just call it gdb_step_until.


>  gdb_test "bt" "\\s*\\#0\\s+main.*" "again in the main"
>  gdb_test "step" ".*" "step again into baz, since foo will be skipped"
>  gdb_test "bt" "\\s*\\#0\\s+baz.*" "again in the baz"
> -gdb_test "step" ".*" "step in the baz, again"
> -gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz, again"
> -gdb_test "step" ".*" "step back to main, again"
> -gdb_test "bt" "\\s*\\#0\\s+main.*" "again back to main"
> +gdb_step_until_regexp "main \\(\\) at .*" "step back to main, again"
> +gdb_test "bt" "\\s*\\#0.*main.*" "again back to main"
> +
> +# because clang doesn't add epilogue information, having a set number of

"add epilogue information" here falls in the "very confusing" camp for me.

> +# steps puts clang more and more out of sync with gcc.  It is unlikely that
> +# the effort of keeping both outputs will be useful.
> +if {[test_compiler_info "clang-*"]} {
> +    untested "Multiple steps are not supported with clang"
> +    return
> +}

That begs the question -- what if clang changes in the future?  Nobody
will remember to update this.

I think it would be better to add a procedure to lib/gdb.exp that detects this
automatically, if possible.  I.e., with the gdb.base/skip.exp program, note:

 (gdb) list foo
 36        return 0;
 37      }
 38
 39      int
 40      foo ()
 41      {
 42        return 0;
 43      }
 44
 45      static void
 (gdb) 
 46      test_skip (void)
 47      {
 48      }
 49
 50      static void
 51      end_test_skip_file_and_function (void)
 52      {
 53        abort ();
 54      }
 55
 (gdb) 

when the program is compiled with clang, we get this:

 (gdb) info line 42
 Line 42 of "../../../src/gdb/testsuite/gdb.base/skip.c" starts at address 0x201804 <foo+4> and ends at 0x201810 <test_skip_file_and_function>.
 (gdb) info line 43
 Line 43 of "../../../src/gdb/testsuite/gdb.base/skip.c" is at address 0x201830 <test_skip> but contains no code.

while if the program is compiled with gcc, we get this:

 (gdb) info line 42
 Line 42 of "/home/pedro/rocm/gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/skip.c" starts at address 0x117d <foo+8> and ends at 0x1182 <foo+13>.
 (gdb) info line 43
 Line 43 of "/home/pedro/rocm/gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/skip.c" starts at address 0x1182 <foo+13> and ends at 0x1184 <test_skip>.


Or even with a simple "just main" program, like so:

 (gdb) list
 1       int
 2       main ()
 3       {
 4         return 0;
 5       }

we get this with gcc:

 (gdb) info line 4
 Line 4 of "main.c" starts at address 0x1131 <main+8> and ends at 0x1136 <main+13>.
 (gdb) info line 5
 Line 5 of "main.c" starts at address 0x1136 <main+13> and ends at 0x1138.
 (gdb) 

and this with clang:

 (gdb) info line 4
 Line 4 of "main.c" starts at address 0x40111d <main+13> and ends at 0x40111f.
 (gdb) info line 5
 Line number 5 is out of range for "main.c".
 (gdb) 

I think that we can use that to write a caching proc like:

 # Return true if the compiler emits line information associating prologue insns with
 # the function's closing brace.  Return false if not, meaning the prologue
 # associates prologue instructions with function's last line with a statement.

 gdb_caching_proc have_prologue_line_info {
     use gdb_simple_compile the simple main program from above
     
     use "info line 5", and return false if we get "out of range", otherwise return true.
 }

Re: [PATCH v3 02/14] Change gdb.base/skip-solib.exp deal with lack of epilogue information

[Bruno Larsen]

On 6/8/22 12:37, Pedro Alves wrote:
> On 2022-05-26 16:10, Bruno Larsen via Gdb-patches wrote:
>> When running gdb.base/skip-solib.exp, the backtrace tests could fail if
>> the compiler did not emit epilogue information for trivial epilogues,
> 
> I found "emit epilogue information" very confusing FWIW.  When I read that the first time,
> I thought this was talking about unwind info for the prologue after the stack is destroyed,
> and I wondered why not fix clang instead.
> 
> I think the following would be more accurate:
> 
>   "fail with compilers that associate prologue insns with the function's last
>    statement line instead of the function's closing brace".

Yeah, I think this is a better explanation, but s/prologue/epilogue, right?

In the simplest terms, we need one more step to leave a function, because the pop and return instructions are not marked as belonging to the last brace. I tried to make this concept more specific by saying that clang doesn't emit epilogue information, Have I misunderstood or misnamed it?

> 
>> despite the feature being fully functional.  As an example, when testing
>> skipping the function square, the testsuite would show
>>
> 
> 
>> ---
>>   gdb/testsuite/gdb.base/skip-inline.exp   | 18 +++++++++++-------
>>   gdb/testsuite/gdb.base/skip-solib-lib.c  |  3 ++-
>>   gdb/testsuite/gdb.base/skip-solib-main.c |  3 ++-
>>   gdb/testsuite/gdb.base/skip-solib.exp    | 12 ++++++++++--
>>   4 files changed, 25 insertions(+), 11 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.base/skip-inline.exp b/gdb/testsuite/gdb.base/skip-inline.exp
>> index f6e9926b66c..327ea676140 100644
>> --- a/gdb/testsuite/gdb.base/skip-inline.exp
>> +++ b/gdb/testsuite/gdb.base/skip-inline.exp
>> @@ -35,16 +35,20 @@ gdb_test "skip function foo" "Function foo will be skipped when stepping\."
>>   gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
>>   gdb_test "step" ".*" "step into baz, since foo will be skipped"
>>   gdb_test "bt" "\\s*\\#0\\s+baz.*" "in the baz, since foo was skipped"
>> -gdb_test "step" ".*" "step in the baz"
>> -gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz"
>> -gdb_test "step" ".*" "step back to main"
>> +gdb_step_until_regexp ".*x = 0; x = baz \\(foo \\(\\)\\).*"
> 
> FWIW, I'd remove the "regexp" from the function's name, just call it gdb_step_until.

Alright, but this sounds more like a review of the first patch than this one.

> 
> 
>>   gdb_test "bt" "\\s*\\#0\\s+main.*" "again in the main"
>>   gdb_test "step" ".*" "step again into baz, since foo will be skipped"
>>   gdb_test "bt" "\\s*\\#0\\s+baz.*" "again in the baz"
>> -gdb_test "step" ".*" "step in the baz, again"
>> -gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz, again"
>> -gdb_test "step" ".*" "step back to main, again"
>> -gdb_test "bt" "\\s*\\#0\\s+main.*" "again back to main"
>> +gdb_step_until_regexp "main \\(\\) at .*" "step back to main, again"
>> +gdb_test "bt" "\\s*\\#0.*main.*" "again back to main"
>> +
>> +# because clang doesn't add epilogue information, having a set number of
> 
> "add epilogue information" here falls in the "very confusing" camp for me.
> 
>> +# steps puts clang more and more out of sync with gcc.  It is unlikely that
>> +# the effort of keeping both outputs will be useful.
>> +if {[test_compiler_info "clang-*"]} {
>> +    untested "Multiple steps are not supported with clang"
>> +    return
>> +}
> 
> That begs the question -- what if clang changes in the future?  Nobody
> will remember to update this.
> 
> I think it would be better to add a procedure to lib/gdb.exp that detects this
> automatically, if possible.  I.e., with the gdb.base/skip.exp program, note:
> 
>   (gdb) list foo
>   36        return 0;
>   37      }
>   38
>   39      int
>   40      foo ()
>   41      {
>   42        return 0;
>   43      }
>   44
>   45      static void
>   (gdb)
>   46      test_skip (void)
>   47      {
>   48      }
>   49
>   50      static void
>   51      end_test_skip_file_and_function (void)
>   52      {
>   53        abort ();
>   54      }
>   55
>   (gdb)
> 
> when the program is compiled with clang, we get this:
> 
>   (gdb) info line 42
>   Line 42 of "../../../src/gdb/testsuite/gdb.base/skip.c" starts at address 0x201804 <foo+4> and ends at 0x201810 <test_skip_file_and_function>.
>   (gdb) info line 43
>   Line 43 of "../../../src/gdb/testsuite/gdb.base/skip.c" is at address 0x201830 <test_skip> but contains no code.
> 
> while if the program is compiled with gcc, we get this:
> 
>   (gdb) info line 42
>   Line 42 of "/home/pedro/rocm/gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/skip.c" starts at address 0x117d <foo+8> and ends at 0x1182 <foo+13>.
>   (gdb) info line 43
>   Line 43 of "/home/pedro/rocm/gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/skip.c" starts at address 0x1182 <foo+13> and ends at 0x1184 <test_skip>.
> 
> 
> Or even with a simple "just main" program, like so:
> 
>   (gdb) list
>   1       int
>   2       main ()
>   3       {
>   4         return 0;
>   5       }
> 
> we get this with gcc:
> 
>   (gdb) info line 4
>   Line 4 of "main.c" starts at address 0x1131 <main+8> and ends at 0x1136 <main+13>.
>   (gdb) info line 5
>   Line 5 of "main.c" starts at address 0x1136 <main+13> and ends at 0x1138.
>   (gdb)
> 
> and this with clang:
> 
>   (gdb) info line 4
>   Line 4 of "main.c" starts at address 0x40111d <main+13> and ends at 0x40111f.
>   (gdb) info line 5
>   Line number 5 is out of range for "main.c".
>   (gdb)
> 
> I think that we can use that to write a caching proc like:
> 
>   # Return true if the compiler emits line information associating prologue insns with
>   # the function's closing brace.  Return false if not, meaning the prologue
>   # associates prologue instructions with function's last line with a statement.
> 
>   gdb_caching_proc have_prologue_line_info {
>       use gdb_simple_compile the simple main program from above
>       
>       use "info line 5", and return false if we get "out of range", otherwise return true.
>   }
> 

This sounds like a good idea, I would only change the default behavior to returning false, unless we saw specifically "start at address.*and ends at". This would make GDB changes more pronounced, as GCC would start having different results.

Cheers!
Bruno Larsen

Re: [PATCH v3 02/14] Change gdb.base/skip-solib.exp deal with lack of epilogue information

[Pedro Alves]

On 2022-06-09 17:27, Bruno Larsen wrote:
> 
> On 6/8/22 12:37, Pedro Alves wrote:
>> On 2022-05-26 16:10, Bruno Larsen via Gdb-patches wrote:
>>> When running gdb.base/skip-solib.exp, the backtrace tests could fail if
>>> the compiler did not emit epilogue information for trivial epilogues,
>>
>> I found "emit epilogue information" very confusing FWIW.  When I read that the first time,
>> I thought this was talking about unwind info for the prologue after the stack is destroyed,
>> and I wondered why not fix clang instead.
>>
>> I think the following would be more accurate:
>>
>>   "fail with compilers that associate prologue insns with the function's last
>>    statement line instead of the function's closing brace".
> 
> Yeah, I think this is a better explanation, but s/prologue/epilogue, right?

Right.  That was a typo.  I meant:

  "fail with compilers that associate epilogue insns with the function's last
   statement line instead of the function's closing brace".

> I tried to make this concept more specific by saying that clang doesn't emit epilogue information, Have I misunderstood or misnamed it?

To me "epilogue information" sounds like some special DWARF information specific to epilogues.
It reminds of me of special epilogue unwinders that GDB has, like amd64_epilogue_frame_unwind,
i386_epilogue_frame_unwind, rs6000_epilogue_frame_unwind, etc.  By not saying "line" anywhere,
I have nothing to connect "epilogue information" to "line info".  Once you make the connection,
you understand that there's nothing special about "epilogue information", it's just that clang
and gcc choose different lines numbers for the epilogue instructions.


>>> diff --git a/gdb/testsuite/gdb.base/skip-inline.exp b/gdb/testsuite/gdb.base/skip-inline.exp
>>> index f6e9926b66c..327ea676140 100644
>>> --- a/gdb/testsuite/gdb.base/skip-inline.exp
>>> +++ b/gdb/testsuite/gdb.base/skip-inline.exp
>>> @@ -35,16 +35,20 @@ gdb_test "skip function foo" "Function foo will be skipped when stepping\."
>>>   gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
>>>   gdb_test "step" ".*" "step into baz, since foo will be skipped"
>>>   gdb_test "bt" "\\s*\\#0\\s+baz.*" "in the baz, since foo was skipped"
>>> -gdb_test "step" ".*" "step in the baz"
>>> -gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz"
>>> -gdb_test "step" ".*" "step back to main"
>>> +gdb_step_until_regexp ".*x = 0; x = baz \\(foo \\(\\)\\).*"
>>
>> FWIW, I'd remove the "regexp" from the function's name, just call it gdb_step_until.
> 
> Alright, but this sounds more like a review of the first patch than this one.

It only stood out to me when I saw it in use, as in the function name is long
and regexp stuck out like a sore thumb here.  :-)

>> Or even with a simple "just main" program, like so:
>>
>>   (gdb) list
>>   1       int
>>   2       main ()
>>   3       {
>>   4         return 0;
>>   5       }
>>
>> we get this with gcc:
>>
>>   (gdb) info line 4
>>   Line 4 of "main.c" starts at address 0x1131 <main+8> and ends at 0x1136 <main+13>.
>>   (gdb) info line 5
>>   Line 5 of "main.c" starts at address 0x1136 <main+13> and ends at 0x1138.
>>   (gdb)
>>
>> and this with clang:
>>
>>   (gdb) info line 4
>>   Line 4 of "main.c" starts at address 0x40111d <main+13> and ends at 0x40111f.
>>   (gdb) info line 5
>>   Line number 5 is out of range for "main.c".
>>   (gdb)
>>
>> I think that we can use that to write a caching proc like:
>>
>>   # Return true if the compiler emits line information associating prologue insns with
>>   # the function's closing brace.  Return false if not, meaning the prologue
>>   # associates prologue instructions with function's last line with a statement.
>>
>>   gdb_caching_proc have_prologue_line_info {
>>       use gdb_simple_compile the simple main program from above
>>             use "info line 5", and return false if we get "out of range", otherwise return true.
>>   }
>>
> 
> This sounds like a good idea, I would only change the default behavior to returning false, unless we saw specifically "start at address.*and ends at". 

...

> This would make GDB changes more pronounced, as GCC would start having different results.

Note sure what you mean here.

Re: [PATCH v3 02/14] Change gdb.base/skip-solib.exp deal with lack of epilogue information

[Bruno Larsen]

On 6/9/22 15:25, Pedro Alves wrote:
> On 2022-06-09 17:27, Bruno Larsen wrote:
>>
>> On 6/8/22 12:37, Pedro Alves wrote:
>>> On 2022-05-26 16:10, Bruno Larsen via Gdb-patches wrote:
>>>> When running gdb.base/skip-solib.exp, the backtrace tests could fail if
>>>> the compiler did not emit epilogue information for trivial epilogues,
>>>
>>> I found "emit epilogue information" very confusing FWIW.  When I read that the first time,
>>> I thought this was talking about unwind info for the prologue after the stack is destroyed,
>>> and I wondered why not fix clang instead.
>>>
>>> I think the following would be more accurate:
>>>
>>>    "fail with compilers that associate prologue insns with the function's last
>>>     statement line instead of the function's closing brace".
>>
>> Yeah, I think this is a better explanation, but s/prologue/epilogue, right?
> 
> Right.  That was a typo.  I meant:
> 
>    "fail with compilers that associate epilogue insns with the function's last
>     statement line instead of the function's closing brace".
> 
>> I tried to make this concept more specific by saying that clang doesn't emit epilogue information, Have I misunderstood or misnamed it?
> 
> To me "epilogue information" sounds like some special DWARF information specific to epilogues.
> It reminds of me of special epilogue unwinders that GDB has, like amd64_epilogue_frame_unwind,
> i386_epilogue_frame_unwind, rs6000_epilogue_frame_unwind, etc.  By not saying "line" anywhere,
> I have nothing to connect "epilogue information" to "line info".  Once you make the connection,
> you understand that there's nothing special about "epilogue information", it's just that clang
> and gcc choose different lines numbers for the epilogue instructions.

Ah, I see! Then  yes, I can change it to what you suggested. Just wanted to confirm my understanding :)

> 
> 
>>>> diff --git a/gdb/testsuite/gdb.base/skip-inline.exp b/gdb/testsuite/gdb.base/skip-inline.exp
>>>> index f6e9926b66c..327ea676140 100644
>>>> --- a/gdb/testsuite/gdb.base/skip-inline.exp
>>>> +++ b/gdb/testsuite/gdb.base/skip-inline.exp
>>>> @@ -35,16 +35,20 @@ gdb_test "skip function foo" "Function foo will be skipped when stepping\."
>>>>    gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
>>>>    gdb_test "step" ".*" "step into baz, since foo will be skipped"
>>>>    gdb_test "bt" "\\s*\\#0\\s+baz.*" "in the baz, since foo was skipped"
>>>> -gdb_test "step" ".*" "step in the baz"
>>>> -gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz"
>>>> -gdb_test "step" ".*" "step back to main"
>>>> +gdb_step_until_regexp ".*x = 0; x = baz \\(foo \\(\\)\\).*"
>>>
>>> FWIW, I'd remove the "regexp" from the function's name, just call it gdb_step_until.
>>
>> Alright, but this sounds more like a review of the first patch than this one.
> 
> It only stood out to me when I saw it in use, as in the function name is long
> and regexp stuck out like a sore thumb here.  :-)
> 
>>> Or even with a simple "just main" program, like so:
>>>
>>>    (gdb) list
>>>    1       int
>>>    2       main ()
>>>    3       {
>>>    4         return 0;
>>>    5       }
>>>
>>> we get this with gcc:
>>>
>>>    (gdb) info line 4
>>>    Line 4 of "main.c" starts at address 0x1131 <main+8> and ends at 0x1136 <main+13>.
>>>    (gdb) info line 5
>>>    Line 5 of "main.c" starts at address 0x1136 <main+13> and ends at 0x1138.
>>>    (gdb)
>>>
>>> and this with clang:
>>>
>>>    (gdb) info line 4
>>>    Line 4 of "main.c" starts at address 0x40111d <main+13> and ends at 0x40111f.
>>>    (gdb) info line 5
>>>    Line number 5 is out of range for "main.c".
>>>    (gdb)
>>>
>>> I think that we can use that to write a caching proc like:
>>>
>>>    # Return true if the compiler emits line information associating prologue insns with
>>>    # the function's closing brace.  Return false if not, meaning the prologue
>>>    # associates prologue instructions with function's last line with a statement.
>>>
>>>    gdb_caching_proc have_prologue_line_info {
>>>        use gdb_simple_compile the simple main program from above
>>>              use "info line 5", and return false if we get "out of range", otherwise return true.
>>>    }
>>>
>>
>> This sounds like a good idea, I would only change the default behavior to returning false, unless we saw specifically "start at address.*and ends at".
> 
> ...
> 
>> This would make GDB changes more pronounced, as GCC would start having different results.
> 
> Note sure what you mean here.
> 

I meant that if something changes in GDB and the epilogue detection fails, making gcc's test result different would be easier to detect than making clang's results different.

Cheers!
Bruno Larsen

Re: [PATCH v3 02/14] Change gdb.base/skip-solib.exp deal with lack of epilogue information

[Pedro Alves]

On 2022-06-09 19:55, Bruno Larsen wrote:
> 
> On 6/9/22 15:25, Pedro Alves wrote:

>>>> I think that we can use that to write a caching proc like:
>>>>
>>>>    # Return true if the compiler emits line information associating prologue insns with
>>>>    # the function's closing brace.  Return false if not, meaning the prologue
>>>>    # associates prologue instructions with function's last line with a statement.
>>>>
>>>>    gdb_caching_proc have_prologue_line_info {
>>>>        use gdb_simple_compile the simple main program from above
>>>>              use "info line 5", and return false if we get "out of range", otherwise return true.
>>>>    }
>>>>
>>>
>>> This sounds like a good idea, I would only change the default behavior to returning false, unless we saw specifically "start at address.*and ends at".
>>
>> ...
>>
>>> This would make GDB changes more pronounced, as GCC would start having different results.
>>
>> Note sure what you mean here.
>>
> 
> I meant that if something changes in GDB and the epilogue detection fails, making gcc's test result different would be easier to detect than making clang's results different.

I see.  Sounds fine.

[PATCH v3 03/14] change gdb.base/symbol-alias to xfail with clang

[Bruno Larsen]

Clang does not issue alias information, even when using -gfull.  This
commit adds an xfail to that test in case we are using clang to reduce
noise when testing.
---

No change in v3

no change in v2

---
 gdb/testsuite/gdb.base/symbol-alias.exp | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/symbol-alias.exp b/gdb/testsuite/gdb.base/symbol-alias.exp
index 289f49bbc3f..f2b675e09ff 100644
--- a/gdb/testsuite/gdb.base/symbol-alias.exp
+++ b/gdb/testsuite/gdb.base/symbol-alias.exp
@@ -31,6 +31,11 @@ foreach f {"func" "func_alias"} {
 }
 
 # Variables.
-foreach v {"g_var_s" "g_var_s_alias"} {
-    gdb_test "p $v" "= {field1 = 1, field2 = 2}"
+gdb_test "p g_var_s" "= {field1 = 1, field2 = 2}"
+
+# clang doesn't include variable alias information in the dwarf:
+# https://github.com/llvm/llvm-project/issues/52664
+if [test_compiler_info {clang*}] {
+    setup_xfail "clang/52664" *-*-*
 }
+gdb_test "p g_var_s_alias" "= {field1 = 1, field2 = 2}"
-- 
2.31.1

RE: [PATCH v3 03/14] change gdb.base/symbol-alias to xfail with clang

[George, Jini Susan]

[Public]

Hi Bruno,

Now that with Kavitha's changes, we have clang emitting debug information for alias variables (https://reviews.llvm.org/D120989)  and since we have her gdb patch (awaiting final approval) to enable support for clang emitted alias debug information, (https://sourceware.org/pipermail/gdb-patches/2022-April/188354.html), maybe we can wait on this ? If her patch goes through, we wouldn't have to make this test case an xfail.

Let me know if you think otherwise.

Thanks,
Jini.


>>-----Original Message-----
>>From: Gdb-patches <gdb-patches-
>>bounces+jigeorge=amd.com@sourceware.org> On Behalf Of Bruno Larsen via
>>Gdb-patches
>>Sent: Thursday, May 26, 2022 8:41 PM
>>To: gdb-patches@sourceware.org
>>Subject: [PATCH v3 03/14] change gdb.base/symbol-alias to xfail with clang
>>
>>[CAUTION: External Email]
>>
>>Clang does not issue alias information, even when using -gfull.  This commit
>>adds an xfail to that test in case we are using clang to reduce noise when
>>testing.
>>---
>>
>>No change in v3
>>
>>no change in v2
>>
>>---
>> gdb/testsuite/gdb.base/symbol-alias.exp | 9 +++++++--
>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>
>>diff --git a/gdb/testsuite/gdb.base/symbol-alias.exp
>>b/gdb/testsuite/gdb.base/symbol-alias.exp
>>index 289f49bbc3f..f2b675e09ff 100644
>>--- a/gdb/testsuite/gdb.base/symbol-alias.exp
>>+++ b/gdb/testsuite/gdb.base/symbol-alias.exp
>>@@ -31,6 +31,11 @@ foreach f {"func" "func_alias"} {  }
>>
>> # Variables.
>>-foreach v {"g_var_s" "g_var_s_alias"} {
>>-    gdb_test "p $v" "= {field1 = 1, field2 = 2}"
>>+gdb_test "p g_var_s" "= {field1 = 1, field2 = 2}"
>>+
>>+# clang doesn't include variable alias information in the dwarf:
>>+#
>>+https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithu
>>+b.com%2Fllvm%2Fllvm-
>>project%2Fissues%2F52664&amp;data=05%7C01%7CJiniSus
>>+an.George%40amd.com%7C8b9c790bd30245184ac808da3f2a368f%7C3dd896
>>1fe4884e
>>+608e11a82d994e183d%7C0%7C0%7C637891747783140459%7CUnknown%7C
>>TWFpbGZsb3d
>>+8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
>>D%7C3
>>+000%7C%7C%7C&amp;sdata=8P8eeL5Y%2FlthVzjmehnJ8i1PTBMI1zy8t8TgEHF
>>%2FnHc%
>>+3D&amp;reserved=0
>>+if [test_compiler_info {clang*}] {
>>+    setup_xfail "clang/52664" *-*-*
>> }
>>+gdb_test "p g_var_s_alias" "= {field1 = 1, field2 = 2}"
>>--
>>2.31.1

RE: [PATCH v3 03/14] change gdb.base/symbol-alias to xfail with clang

[Bruno Larsen]

Hi Jini,

Great to hear that Kavitha's changes have landed on clang! I do think
that it is still important to have xfails, however, since only new
clangs would add the information, and GDB is tested in all manner of
systems.

I have changed the patch to assume that clang 15 has Kavitha's patches,
and changed the clang compiler test.  Does this look acceptable?

[PATCH v4 03/14] gdb/testsuite: Change gdb.base/symbol-alias to xfail with old clang

Clang didn't use to add alias information, even when using -gfull.  This
commit checks if the clang version being used is already providing alias
information (15 or newer), otherwise it adds an XFAIL.
---
 gdb/testsuite/gdb.base/symbol-alias.exp | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/symbol-alias.exp b/gdb/testsuite/gdb.base/symbol-alias.exp
index 289f49bbc3f..078158dc101 100644
--- a/gdb/testsuite/gdb.base/symbol-alias.exp
+++ b/gdb/testsuite/gdb.base/symbol-alias.exp
@@ -31,6 +31,11 @@ foreach f {"func" "func_alias"} {
 }
 
 # Variables.
-foreach v {"g_var_s" "g_var_s_alias"} {
-    gdb_test "p $v" "= {field1 = 1, field2 = 2}"
+gdb_test "p g_var_s" "= {field1 = 1, field2 = 2}"
+
+# Clang didn't include alias information until version 15.
+if {[test_compiler_info {clang-[1-9]*}]
+     || [test_compiler_info {clang-1[0-4]*}]} {
+    setup_xfail "clang/52664" *-*-*
 }
+gdb_test "p g_var_s_alias" "= {field1 = 1, field2 = 2}"
-- 
2.31.1

RE: [PATCH v3 03/14] change gdb.base/symbol-alias to xfail with clang

[George, Jini Susan]

[Public]

Thanks, Bruno. This seems reasonable.

Thanks,
Jini.

>>-----Original Message-----
>>From: Gdb-patches <gdb-patches-
>>bounces+jigeorge=amd.com@sourceware.org> On Behalf Of Bruno Larsen via
>>Gdb-patches
>>Sent: Tuesday, June 7, 2022 6:24 PM
>>To: gdb-patches@sourceware.org
>>Subject: RE: [PATCH v3 03/14] change gdb.base/symbol-alias to xfail with clang
>>
>>[CAUTION: External Email]
>>
>>Hi Jini,
>>
>>Great to hear that Kavitha's changes have landed on clang! I do think that it is
>>still important to have xfails, however, since only new clangs would add the
>>information, and GDB is tested in all manner of systems.
>>
>>I have changed the patch to assume that clang 15 has Kavitha's patches, and
>>changed the clang compiler test.  Does this look acceptable?
>>
>>[PATCH v4 03/14] gdb/testsuite: Change gdb.base/symbol-alias to xfail with old
>>clang
>>
>>Clang didn't use to add alias information, even when using -gfull.  This commit
>>checks if the clang version being used is already providing alias information (15
>>or newer), otherwise it adds an XFAIL.
>>---
>> gdb/testsuite/gdb.base/symbol-alias.exp | 9 +++++++--
>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>
>>diff --git a/gdb/testsuite/gdb.base/symbol-alias.exp
>>b/gdb/testsuite/gdb.base/symbol-alias.exp
>>index 289f49bbc3f..078158dc101 100644
>>--- a/gdb/testsuite/gdb.base/symbol-alias.exp
>>+++ b/gdb/testsuite/gdb.base/symbol-alias.exp
>>@@ -31,6 +31,11 @@ foreach f {"func" "func_alias"} {  }
>>
>> # Variables.
>>-foreach v {"g_var_s" "g_var_s_alias"} {
>>-    gdb_test "p $v" "= {field1 = 1, field2 = 2}"
>>+gdb_test "p g_var_s" "= {field1 = 1, field2 = 2}"
>>+
>>+# Clang didn't include alias information until version 15.
>>+if {[test_compiler_info {clang-[1-9]*}]
>>+     || [test_compiler_info {clang-1[0-4]*}]} {
>>+    setup_xfail "clang/52664" *-*-*
>> }
>>+gdb_test "p g_var_s_alias" "= {field1 = 1, field2 = 2}"
>>--
>>2.31.1

RE: [PATCH v3 03/14] change gdb.base/symbol-alias to xfail with clang

[Andrew Burgess]

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

> Hi Jini,
>
> Great to hear that Kavitha's changes have landed on clang! I do think
> that it is still important to have xfails, however, since only new
> clangs would add the information, and GDB is tested in all manner of
> systems.
>
> I have changed the patch to assume that clang 15 has Kavitha's patches,
> and changed the clang compiler test.  Does this look acceptable?
>
> [PATCH v4 03/14] gdb/testsuite: Change gdb.base/symbol-alias to xfail with old clang
>
> Clang didn't use to add alias information, even when using -gfull.  This
> commit checks if the clang version being used is already providing alias
> information (15 or newer), otherwise it adds an XFAIL.

My understanding from Jini's email was that for this test to pass we
would also need this gdb patch:

  https://sourceware.org/pipermail/gdb-patches/2022-April/188354.html

So, if I had clang 15 right now, this test would still fail, right?

I guess you either need to hold this patch back until the above is
merged, or put this in with a generic "all clang" pattern (like you
originally had) and then assume someone will spot the KPASS and fix up
the test later.

Also...


> ---
>  gdb/testsuite/gdb.base/symbol-alias.exp | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/symbol-alias.exp b/gdb/testsuite/gdb.base/symbol-alias.exp
> index 289f49bbc3f..078158dc101 100644
> --- a/gdb/testsuite/gdb.base/symbol-alias.exp
> +++ b/gdb/testsuite/gdb.base/symbol-alias.exp
> @@ -31,6 +31,11 @@ foreach f {"func" "func_alias"} {
>  }
>  
>  # Variables.
> -foreach v {"g_var_s" "g_var_s_alias"} {
> -    gdb_test "p $v" "= {field1 = 1, field2 = 2}"
> +gdb_test "p g_var_s" "= {field1 = 1, field2 = 2}"
> +
> +# Clang didn't include alias information until version 15.
> +if {[test_compiler_info {clang-[1-9]*}]
> +     || [test_compiler_info {clang-1[0-4]*}]} {

Wouldn't clang-15 match the first of these patterns?

Thannks,
Andrew


> +    setup_xfail "clang/52664" *-*-*
>  }
> +gdb_test "p g_var_s_alias" "= {field1 = 1, field2 = 2}"
> -- 
> 2.31.1

Re: [PATCH v3 03/14] change gdb.base/symbol-alias to xfail with clang

[Bruno Larsen]

On 6/10/22 08:01, Andrew Burgess wrote:
> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
>> Hi Jini,
>>
>> Great to hear that Kavitha's changes have landed on clang! I do think
>> that it is still important to have xfails, however, since only new
>> clangs would add the information, and GDB is tested in all manner of
>> systems.
>>
>> I have changed the patch to assume that clang 15 has Kavitha's patches,
>> and changed the clang compiler test.  Does this look acceptable?
>>
>> [PATCH v4 03/14] gdb/testsuite: Change gdb.base/symbol-alias to xfail with old clang
>>
>> Clang didn't use to add alias information, even when using -gfull.  This
>> commit checks if the clang version being used is already providing alias
>> information (15 or newer), otherwise it adds an XFAIL.
> 
> My understanding from Jini's email was that for this test to pass we
> would also need this gdb patch:
> 
>    https://sourceware.org/pipermail/gdb-patches/2022-April/188354.html
> 
> So, if I had clang 15 right now, this test would still fail, right?

Yes. My thought was that for clang < 15, it is an XFAIL, because there is nothing we can do about it, and I'd leave all other outputs the same.

> 
> I guess you either need to hold this patch back until the above is
> merged, or put this in with a generic "all clang" pattern (like you
> originally had) and then assume someone will spot the KPASS and fix up
> the test later.

If I'm honest, I thought that patch would land before clang-15 was released. I can hold off on this until then.

> 
> Also...
> 
> 
>> ---
>>   gdb/testsuite/gdb.base/symbol-alias.exp | 9 +++++++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.base/symbol-alias.exp b/gdb/testsuite/gdb.base/symbol-alias.exp
>> index 289f49bbc3f..078158dc101 100644
>> --- a/gdb/testsuite/gdb.base/symbol-alias.exp
>> +++ b/gdb/testsuite/gdb.base/symbol-alias.exp
>> @@ -31,6 +31,11 @@ foreach f {"func" "func_alias"} {
>>   }
>>   
>>   # Variables.
>> -foreach v {"g_var_s" "g_var_s_alias"} {
>> -    gdb_test "p $v" "= {field1 = 1, field2 = 2}"
>> +gdb_test "p g_var_s" "= {field1 = 1, field2 = 2}"
>> +
>> +# Clang didn't include alias information until version 15.
>> +if {[test_compiler_info {clang-[1-9]*}]
>> +     || [test_compiler_info {clang-1[0-4]*}]} {
> 
> Wouldn't clang-15 match the first of these patterns?

Now that you mention it, it should, but doesn't. My guess is that TCL implicitly adds [^1-9] for the first character of '*', assuming that if you meant to have it, you'd have written [1-9]+ instead. However, I will change it to "clang-[1-9]-*" to not rely on that unexplained behavior.

Cheers!
Bruno Larsen

> 
> Thannks,
> Andrew
> 
> 
>> +    setup_xfail "clang/52664" *-*-*
>>   }
>> +gdb_test "p g_var_s_alias" "= {field1 = 1, field2 = 2}"
>> -- 
>> 2.31.1
> 

RE: [PATCH v3 03/14] change gdb.base/symbol-alias to xfail with clang

[George, Jini Susan]

[Public]

Now that Kavitha's patch just got committed (details below), guess we might not require this.

commit 6df97c56ea0f3086c96743ec47148ee69fd8cf71 (HEAD -> master, origin/master, origin/HEAD)
Author: Kavitha Natarajan <kavitha.natarajan@amd.com>
Date:   Tue Jun 14 10:37:46 2022 +0530

    Debug support for global alias variable

    Starting with (future) Clang 15 (since
    https://reviews.llvm.org/D120989), Clang emits the DWARF information
    of global alias variables as DW_TAG_imported_declaration.  However,
    GDB does not handle it.  It incorrectly always reads this tag as
    C++/Fortran imported declaration (type alias, namespace alias and
    Fortran module).  This commit adds support to handle this tag as an
    alias variable.

    This change fixes the failures in the gdb.base/symbol-alias.exp
    testcase with current git Clang.  This testcase is also updated to
    test nested (recursive) aliases.

Thanks,
Jini.

>>-----Original Message-----
>>From: Gdb-patches <gdb-patches-
>>bounces+jigeorge=amd.com@sourceware.org> On Behalf Of Andrew Burgess
>>via Gdb-patches
>>Sent: Friday, June 10, 2022 4:31 PM
>>To: Bruno Larsen <blarsen@redhat.com>; gdb-patches@sourceware.org
>>Subject: RE: [PATCH v3 03/14] change gdb.base/symbol-alias to xfail with clang
>>
>>[CAUTION: External Email]
>>
>>Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
>>
>>> Hi Jini,
>>>
>>> Great to hear that Kavitha's changes have landed on clang! I do think
>>> that it is still important to have xfails, however, since only new
>>> clangs would add the information, and GDB is tested in all manner of
>>> systems.
>>>
>>> I have changed the patch to assume that clang 15 has Kavitha's
>>> patches, and changed the clang compiler test.  Does this look acceptable?
>>>
>>> [PATCH v4 03/14] gdb/testsuite: Change gdb.base/symbol-alias to xfail
>>> with old clang
>>>
>>> Clang didn't use to add alias information, even when using -gfull.
>>> This commit checks if the clang version being used is already
>>> providing alias information (15 or newer), otherwise it adds an XFAIL.
>>
>>My understanding from Jini's email was that for this test to pass we would also
>>need this gdb patch:
>>
>>
>>https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsource
>>ware.org%2Fpipermail%2Fgdb-patches%2F2022-
>>April%2F188354.html&amp;data=05%7C01%7CJiniSusan.George%40amd.com%
>>7Ca2aaecea2df0476d781808da4ad0988b%7C3dd8961fe4884e608e11a82d994e
>>183d%7C0%7C0%7C637904557029019597%7CUnknown%7CTWFpbGZsb3d8eyJ
>>WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C
>>3000%7C%7C%7C&amp;sdata=8%2FUKHMbpyAKXN4HOPHRJ2vP62yKK5i5Qeau
>>v0GQtWHo%3D&amp;reserved=0
>>
>>So, if I had clang 15 right now, this test would still fail, right?
>>
>>I guess you either need to hold this patch back until the above is merged, or put
>>this in with a generic "all clang" pattern (like you originally had) and then assume
>>someone will spot the KPASS and fix up the test later.
>>
>>Also...
>>
>>
>>> ---
>>>  gdb/testsuite/gdb.base/symbol-alias.exp | 9 +++++++--
>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/gdb/testsuite/gdb.base/symbol-alias.exp
>>> b/gdb/testsuite/gdb.base/symbol-alias.exp
>>> index 289f49bbc3f..078158dc101 100644
>>> --- a/gdb/testsuite/gdb.base/symbol-alias.exp
>>> +++ b/gdb/testsuite/gdb.base/symbol-alias.exp
>>> @@ -31,6 +31,11 @@ foreach f {"func" "func_alias"} {  }
>>>
>>>  # Variables.
>>> -foreach v {"g_var_s" "g_var_s_alias"} {
>>> -    gdb_test "p $v" "= {field1 = 1, field2 = 2}"
>>> +gdb_test "p g_var_s" "= {field1 = 1, field2 = 2}"
>>> +
>>> +# Clang didn't include alias information until version 15.
>>> +if {[test_compiler_info {clang-[1-9]*}]
>>> +     || [test_compiler_info {clang-1[0-4]*}]} {
>>
>>Wouldn't clang-15 match the first of these patterns?
>>
>>Thannks,
>>Andrew
>>
>>
>>> +    setup_xfail "clang/52664" *-*-*
>>>  }
>>> +gdb_test "p g_var_s_alias" "= {field1 = 1, field2 = 2}"
>>> --
>>> 2.31.1

RE: [PATCH v3 03/14] change gdb.base/symbol-alias to xfail with clang

[George, Jini Susan]

[Public]

I missed mentioning that Kavitha's patch takes care of making symbol-alias.exp xfail for clang versions less than 15.

Thanks,
Jini.

>>-----Original Message-----
>>From: Gdb-patches <gdb-patches-
>>bounces+jigeorge=amd.com@sourceware.org> On Behalf Of George, Jini Susan
>>via Gdb-patches
>>Sent: Tuesday, June 14, 2022 12:44 PM
>>To: Andrew Burgess <aburgess@redhat.com>; Bruno Larsen
>><blarsen@redhat.com>; gdb-patches@sourceware.org
>>Cc: Natarajan, Kavitha <Kavitha.Natarajan@amd.com>
>>Subject: RE: [PATCH v3 03/14] change gdb.base/symbol-alias to xfail with clang
>>
>>[Public]
>>
>>[CAUTION: External Email]
>>
>>[Public]
>>
>>Now that Kavitha's patch just got committed (details below), guess we might not
>>require this.
>>
>>commit 6df97c56ea0f3086c96743ec47148ee69fd8cf71 (HEAD -> master,
>>origin/master, origin/HEAD)
>>Author: Kavitha Natarajan <kavitha.natarajan@amd.com>
>>Date:   Tue Jun 14 10:37:46 2022 +0530
>>
>>    Debug support for global alias variable
>>
>>    Starting with (future) Clang 15 (since
>>
>>https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.
>>llvm.org%2FD120989&amp;data=05%7C01%7CJiniSusan.George%40amd.com%
>>7Ce7ea873468214efcf9d908da4dd648a2%7C3dd8961fe4884e608e11a82d994e
>>183d%7C0%7C0%7C637907879987330246%7CUnknown%7CTWFpbGZsb3d8eyJ
>>WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C
>>3000%7C%7C%7C&amp;sdata=bNOSusnr0cUBWttQ1PM8JrnijdjPJl2ee7lBpPsStY
>>A%3D&amp;reserved=0), Clang emits the DWARF information
>>    of global alias variables as DW_TAG_imported_declaration.  However,
>>    GDB does not handle it.  It incorrectly always reads this tag as
>>    C++/Fortran imported declaration (type alias, namespace alias and
>>    Fortran module).  This commit adds support to handle this tag as an
>>    alias variable.
>>
>>    This change fixes the failures in the gdb.base/symbol-alias.exp
>>    testcase with current git Clang.  This testcase is also updated to
>>    test nested (recursive) aliases.
>>
>>Thanks,
>>Jini.
>>
>>>>-----Original Message-----
>>>>From: Gdb-patches <gdb-patches-
>>>>bounces+jigeorge=amd.com@sourceware.org> On Behalf Of Andrew Burgess
>>>>via Gdb-patches
>>>>Sent: Friday, June 10, 2022 4:31 PM
>>>>To: Bruno Larsen <blarsen@redhat.com>; gdb-patches@sourceware.org
>>>>Subject: RE: [PATCH v3 03/14] change gdb.base/symbol-alias to xfail
>>>>with clang
>>>>
>>>>[CAUTION: External Email]
>>>>
>>>>Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
>>>>
>>>>> Hi Jini,
>>>>>
>>>>> Great to hear that Kavitha's changes have landed on clang! I do
>>>>> think that it is still important to have xfails, however, since only
>>>>> new clangs would add the information, and GDB is tested in all
>>>>> manner of systems.
>>>>>
>>>>> I have changed the patch to assume that clang 15 has Kavitha's
>>>>> patches, and changed the clang compiler test.  Does this look acceptable?
>>>>>
>>>>> [PATCH v4 03/14] gdb/testsuite: Change gdb.base/symbol-alias to
>>>>> xfail with old clang
>>>>>
>>>>> Clang didn't use to add alias information, even when using -gfull.
>>>>> This commit checks if the clang version being used is already
>>>>> providing alias information (15 or newer), otherwise it adds an XFAIL.
>>>>
>>>>My understanding from Jini's email was that for this test to pass we
>>>>would also need this gdb patch:
>>>>
>>>>
>>>>https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsour
>>>>ce
>>>>ware.org%2Fpipermail%2Fgdb-patches%2F2022-
>>>>April%2F188354.html&amp;data=05%7C01%7CJiniSusan.George%40amd.com
>>%
>>>>7Ca2aaecea2df0476d781808da4ad0988b%7C3dd8961fe4884e608e11a82d99
>>4e
>>>>183d%7C0%7C0%7C637904557029019597%7CUnknown%7CTWFpbGZsb3d8e
>>yJ
>>>>WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
>>7C
>>>>3000%7C%7C%7C&amp;sdata=8%2FUKHMbpyAKXN4HOPHRJ2vP62yKK5i5Qe
>>au
>>>>v0GQtWHo%3D&amp;reserved=0
>>>>
>>>>So, if I had clang 15 right now, this test would still fail, right?
>>>>
>>>>I guess you either need to hold this patch back until the above is
>>>>merged, or put this in with a generic "all clang" pattern (like you
>>>>originally had) and then assume someone will spot the KPASS and fix up the
>>test later.
>>>>
>>>>Also...
>>>>
>>>>
>>>>> ---
>>>>>  gdb/testsuite/gdb.base/symbol-alias.exp | 9 +++++++--
>>>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/gdb/testsuite/gdb.base/symbol-alias.exp
>>>>> b/gdb/testsuite/gdb.base/symbol-alias.exp
>>>>> index 289f49bbc3f..078158dc101 100644
>>>>> --- a/gdb/testsuite/gdb.base/symbol-alias.exp
>>>>> +++ b/gdb/testsuite/gdb.base/symbol-alias.exp
>>>>> @@ -31,6 +31,11 @@ foreach f {"func" "func_alias"} {  }
>>>>>
>>>>>  # Variables.
>>>>> -foreach v {"g_var_s" "g_var_s_alias"} {
>>>>> -    gdb_test "p $v" "= {field1 = 1, field2 = 2}"
>>>>> +gdb_test "p g_var_s" "= {field1 = 1, field2 = 2}"
>>>>> +
>>>>> +# Clang didn't include alias information until version 15.
>>>>> +if {[test_compiler_info {clang-[1-9]*}]
>>>>> +     || [test_compiler_info {clang-1[0-4]*}]} {
>>>>
>>>>Wouldn't clang-15 match the first of these patterns?
>>>>
>>>>Thannks,
>>>>Andrew
>>>>
>>>>
>>>>> +    setup_xfail "clang/52664" *-*-*
>>>>>  }
>>>>> +gdb_test "p g_var_s_alias" "= {field1 = 1, field2 = 2}"
>>>>> --
>>>>> 2.31.1

Re: [PATCH v3 03/14] change gdb.base/symbol-alias to xfail with clang

[Bruno Larsen]

Good catch. I'll drop this patch.

Cheers!
Bruno Larsen

On 6/14/22 04:23, George, Jini Susan wrote:
> [Public]
> 
> I missed mentioning that Kavitha's patch takes care of making symbol-alias.exp xfail for clang versions less than 15.
> 
> Thanks,
> Jini.
> 
>>> -----Original Message-----
>>> From: Gdb-patches <gdb-patches-
>>> bounces+jigeorge=amd.com@sourceware.org> On Behalf Of George, Jini Susan
>>> via Gdb-patches
>>> Sent: Tuesday, June 14, 2022 12:44 PM
>>> To: Andrew Burgess <aburgess@redhat.com>; Bruno Larsen
>>> <blarsen@redhat.com>; gdb-patches@sourceware.org
>>> Cc: Natarajan, Kavitha <Kavitha.Natarajan@amd.com>
>>> Subject: RE: [PATCH v3 03/14] change gdb.base/symbol-alias to xfail with clang
>>>
>>> [Public]
>>>
>>> [CAUTION: External Email]
>>>
>>> [Public]
>>>
>>> Now that Kavitha's patch just got committed (details below), guess we might not
>>> require this.
>>>
>>> commit 6df97c56ea0f3086c96743ec47148ee69fd8cf71 (HEAD -> master,
>>> origin/master, origin/HEAD)
>>> Author: Kavitha Natarajan <kavitha.natarajan@amd.com>
>>> Date:   Tue Jun 14 10:37:46 2022 +0530
>>>
>>>     Debug support for global alias variable
>>>
>>>     Starting with (future) Clang 15 (since
>>>
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.
>>> llvm.org%2FD120989&amp;data=05%7C01%7CJiniSusan.George%40amd.com%
>>> 7Ce7ea873468214efcf9d908da4dd648a2%7C3dd8961fe4884e608e11a82d994e
>>> 183d%7C0%7C0%7C637907879987330246%7CUnknown%7CTWFpbGZsb3d8eyJ
>>> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C
>>> 3000%7C%7C%7C&amp;sdata=bNOSusnr0cUBWttQ1PM8JrnijdjPJl2ee7lBpPsStY
>>> A%3D&amp;reserved=0), Clang emits the DWARF information
>>>     of global alias variables as DW_TAG_imported_declaration.  However,
>>>     GDB does not handle it.  It incorrectly always reads this tag as
>>>     C++/Fortran imported declaration (type alias, namespace alias and
>>>     Fortran module).  This commit adds support to handle this tag as an
>>>     alias variable.
>>>
>>>     This change fixes the failures in the gdb.base/symbol-alias.exp
>>>     testcase with current git Clang.  This testcase is also updated to
>>>     test nested (recursive) aliases.
>>>
>>> Thanks,
>>> Jini.
>>>
>>>>> -----Original Message-----
>>>>> From: Gdb-patches <gdb-patches-
>>>>> bounces+jigeorge=amd.com@sourceware.org> On Behalf Of Andrew Burgess
>>>>> via Gdb-patches
>>>>> Sent: Friday, June 10, 2022 4:31 PM
>>>>> To: Bruno Larsen <blarsen@redhat.com>; gdb-patches@sourceware.org
>>>>> Subject: RE: [PATCH v3 03/14] change gdb.base/symbol-alias to xfail
>>>>> with clang
>>>>>
>>>>> [CAUTION: External Email]
>>>>>
>>>>> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
>>>>>
>>>>>> Hi Jini,
>>>>>>
>>>>>> Great to hear that Kavitha's changes have landed on clang! I do
>>>>>> think that it is still important to have xfails, however, since only
>>>>>> new clangs would add the information, and GDB is tested in all
>>>>>> manner of systems.
>>>>>>
>>>>>> I have changed the patch to assume that clang 15 has Kavitha's
>>>>>> patches, and changed the clang compiler test.  Does this look acceptable?
>>>>>>
>>>>>> [PATCH v4 03/14] gdb/testsuite: Change gdb.base/symbol-alias to
>>>>>> xfail with old clang
>>>>>>
>>>>>> Clang didn't use to add alias information, even when using -gfull.
>>>>>> This commit checks if the clang version being used is already
>>>>>> providing alias information (15 or newer), otherwise it adds an XFAIL.
>>>>>
>>>>> My understanding from Jini's email was that for this test to pass we
>>>>> would also need this gdb patch:
>>>>>
>>>>>
>>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsour
>>>>> ce
>>>>> ware.org%2Fpipermail%2Fgdb-patches%2F2022-
>>>>> April%2F188354.html&amp;data=05%7C01%7CJiniSusan.George%40amd.com
>>> %
>>>>> 7Ca2aaecea2df0476d781808da4ad0988b%7C3dd8961fe4884e608e11a82d99
>>> 4e
>>>>> 183d%7C0%7C0%7C637904557029019597%7CUnknown%7CTWFpbGZsb3d8e
>>> yJ
>>>>> WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
>>> 7C
>>>>> 3000%7C%7C%7C&amp;sdata=8%2FUKHMbpyAKXN4HOPHRJ2vP62yKK5i5Qe
>>> au
>>>>> v0GQtWHo%3D&amp;reserved=0
>>>>>
>>>>> So, if I had clang 15 right now, this test would still fail, right?
>>>>>
>>>>> I guess you either need to hold this patch back until the above is
>>>>> merged, or put this in with a generic "all clang" pattern (like you
>>>>> originally had) and then assume someone will spot the KPASS and fix up the
>>> test later.
>>>>>
>>>>> Also...
>>>>>
>>>>>
>>>>>> ---
>>>>>>   gdb/testsuite/gdb.base/symbol-alias.exp | 9 +++++++--
>>>>>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/gdb/testsuite/gdb.base/symbol-alias.exp
>>>>>> b/gdb/testsuite/gdb.base/symbol-alias.exp
>>>>>> index 289f49bbc3f..078158dc101 100644
>>>>>> --- a/gdb/testsuite/gdb.base/symbol-alias.exp
>>>>>> +++ b/gdb/testsuite/gdb.base/symbol-alias.exp
>>>>>> @@ -31,6 +31,11 @@ foreach f {"func" "func_alias"} {  }
>>>>>>
>>>>>>   # Variables.
>>>>>> -foreach v {"g_var_s" "g_var_s_alias"} {
>>>>>> -    gdb_test "p $v" "= {field1 = 1, field2 = 2}"
>>>>>> +gdb_test "p g_var_s" "= {field1 = 1, field2 = 2}"
>>>>>> +
>>>>>> +# Clang didn't include alias information until version 15.
>>>>>> +if {[test_compiler_info {clang-[1-9]*}]
>>>>>> +     || [test_compiler_info {clang-1[0-4]*}]} {
>>>>>
>>>>> Wouldn't clang-15 match the first of these patterns?
>>>>>
>>>>> Thannks,
>>>>> Andrew
>>>>>
>>>>>
>>>>>> +    setup_xfail "clang/52664" *-*-*
>>>>>>   }
>>>>>> +gdb_test "p g_var_s_alias" "= {field1 = 1, field2 = 2}"
>>>>>> --
>>>>>> 2.31.1
> 

[PATCH v3 04/14] change gdb.base/nodebug.c to not fail with clang

[Bruno Larsen]

Clang organizes the variables differently to gcc in the original version
of this code, leading to the following differences when testing
p (int*) &dataglobal + 1

gcc:
$16 = (int *) 0x404034 <datalocal>

clang:
$16 = (int *) 0x404034 <dataglobal8>

The code change to nodebug.c makes it so gcc and clang will place the
same variable under dataglobal8, generating the same output.
---

No change in v3.

No change in v2.

---
 gdb/testsuite/gdb.base/nodebug.c   | 2 +-
 gdb/testsuite/gdb.base/nodebug.exp | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/nodebug.c b/gdb/testsuite/gdb.base/nodebug.c
index c7bc93991b8..572255697bb 100644
--- a/gdb/testsuite/gdb.base/nodebug.c
+++ b/gdb/testsuite/gdb.base/nodebug.c
@@ -3,8 +3,8 @@
 
 /* Test that things still (sort of) work when compiled without -g.  */
 
-int dataglobal = 3;			/* Should go in global data */
 static int datalocal = 4;		/* Should go in local data */
+int dataglobal = 3;			/* Should go in global data */
 int bssglobal;				/* Should go in global bss */
 static int bsslocal;			/* Should go in local bss */
 
diff --git a/gdb/testsuite/gdb.base/nodebug.exp b/gdb/testsuite/gdb.base/nodebug.exp
index 0dbd0ad153e..fbff6ecb395 100644
--- a/gdb/testsuite/gdb.base/nodebug.exp
+++ b/gdb/testsuite/gdb.base/nodebug.exp
@@ -187,7 +187,7 @@ if [nodebug_runto inner] then {
 	{"dataglobal + 1"		""   $dataglobal_unk_re					$dataglobal_unk_re}
 	{"&dataglobal"			""   "\\($data_var_type \\*\\) $hex <dataglobal>"	" = $data_var_type \\*"}
 	{"&dataglobal + 1"		""   $ptr_math_re					$ptr_math_re}
-	{"(int *) &dataglobal + 1"	""   " = \\(int \\*\\) $hex <datalocal>"		"int \\*"}
+	{"(int *) &dataglobal + 1"	""   " = \\(int \\*\\) $hex <dataglobal8>"		"int \\*"}
 	{"&(int) dataglobal + 1"	""   $not_mem_re					$not_mem_re}
 	{"&dataglobal, &dataglobal"	""   "\\($data_var_type \\*\\) $hex <dataglobal>"	" = $data_var_type \\*"}
 	{"*dataglobal"			""   $dataglobal_unk_re					$dataglobal_unk_re}
-- 
2.31.1

Re: [PATCH v3 04/14] change gdb.base/nodebug.c to not fail with clang

[Andrew Burgess]

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

> Clang organizes the variables differently to gcc in the original version
> of this code, leading to the following differences when testing
> p (int*) &dataglobal + 1
>
> gcc:
> $16 = (int *) 0x404034 <datalocal>
>
> clang:
> $16 = (int *) 0x404034 <dataglobal8>
>
> The code change to nodebug.c makes it so gcc and clang will place the
> same variable under dataglobal8, generating the same output.
> ---
>
> No change in v3.
>
> No change in v2.
>
> ---
>  gdb/testsuite/gdb.base/nodebug.c   | 2 +-
>  gdb/testsuite/gdb.base/nodebug.exp | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/nodebug.c b/gdb/testsuite/gdb.base/nodebug.c
> index c7bc93991b8..572255697bb 100644
> --- a/gdb/testsuite/gdb.base/nodebug.c
> +++ b/gdb/testsuite/gdb.base/nodebug.c
> @@ -3,8 +3,8 @@
>  
>  /* Test that things still (sort of) work when compiled without -g.  */
>  
> -int dataglobal = 3;			/* Should go in global data */
>  static int datalocal = 4;		/* Should go in local data */
> +int dataglobal = 3;			/* Should go in global data */
>  int bssglobal;				/* Should go in global bss */
>  static int bsslocal;			/* Should go in local bss */

Something about just reordering the symbols like this makes me a little
uncomfortable.

I spent some time thinking about this test, wondering what the point of
the check was.  The test was added in commit:

  commit 46a4882b3c7d9ec981568b8b13a3c9c39c8f8e61
  Date:   Mon Sep 4 20:21:15 2017 +0100
  
      Stop assuming no-debug-info variables have type int

But there's no hint what this specific test was for.

My best guess is that we're checking that the '+ 1' correctly increases
the address by the size of an int.  With that in mind, I wonder if the
label after the address is really that important.  I mean, right not it
kind-of acts to confirm that we increased the address by the correct
amount, but, we could achieve that same goal by just comparing
addresses.

And so, I wonder about the patch below instead?  Instead of looking for
a specific label I now accept any label.  I then add an extra test
afterwards that checks that '+ 1' increases the address by the size of
an int.

What do you think of this?

Thanks,
Andrew

---

diff --git a/gdb/testsuite/gdb.base/nodebug.exp b/gdb/testsuite/gdb.base/nodebug.exp
index 0dbd0ad153e..a4db483d6af 100644
--- a/gdb/testsuite/gdb.base/nodebug.exp
+++ b/gdb/testsuite/gdb.base/nodebug.exp
@@ -174,6 +174,7 @@ if [nodebug_runto inner] then {
     set unk_type_re "has unknown type.*to its declared type"
     set ptr_math_re "Cannot perform pointer math on incomplete type \"$data_var_type\", try casting to a known type, or void \\*\\."
     set not_mem_re "Attempt to take address of value not located in memory\\."
+    set any_label_regexp "<\[^>\]+>"
 
     set dataglobal_unk_re "dataglobal.*$unk_type_re"
 
@@ -187,7 +188,7 @@ if [nodebug_runto inner] then {
 	{"dataglobal + 1"		""   $dataglobal_unk_re					$dataglobal_unk_re}
 	{"&dataglobal"			""   "\\($data_var_type \\*\\) $hex <dataglobal>"	" = $data_var_type \\*"}
 	{"&dataglobal + 1"		""   $ptr_math_re					$ptr_math_re}
-	{"(int *) &dataglobal + 1"	""   " = \\(int \\*\\) $hex <datalocal>"		"int \\*"}
+	{"(int *) &dataglobal + 1"	""   " = \\(int \\*\\) $hex $any_label_regexp"		"int \\*"}
 	{"&(int) dataglobal + 1"	""   $not_mem_re					$not_mem_re}
 	{"&dataglobal, &dataglobal"	""   "\\($data_var_type \\*\\) $hex <dataglobal>"	" = $data_var_type \\*"}
 	{"*dataglobal"			""   $dataglobal_unk_re					$dataglobal_unk_re}
@@ -218,7 +219,14 @@ if [nodebug_runto inner] then {
 	gdb_test "whatis $exp" $whatis
 	gdb_test "ptype $exp" $whatis
     }
-    
+
+    # Check that pointer arithmetic works as expected.
+    set addr1 [get_hexadecimal_valueof "&dataglobal" "*UNKNOWN*"]
+    set addr2 [get_hexadecimal_valueof "(int *) &dataglobal + 1" "*UNKNOWN*"]
+    set offset [expr $addr2 - $addr1]
+    set int_size [get_integer_valueof "sizeof (int)" "*UNKNOWN*"]
+    gdb_assert { $offset == $int_size }
+
     # The only symbol xcoff puts out for statics is for the TOC entry.
     # Possible, but hairy, for gdb to deal.  Right now it doesn't, it
     # doesn't know the variables exist at all.

[PATCH v3 05/14] update gdb.base/info-program.exp to not fail with clang

[Bruno Larsen]

The updated test specifically mentions that it doesn't care where the
program stops, however it was still testing for something.  With this
correction, the test works even if the compiler doesn't add epilogue
information to functions.
---

No change in v3.

No change in v2.

---
 gdb/testsuite/gdb.base/info-program.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/info-program.exp b/gdb/testsuite/gdb.base/info-program.exp
index facc13efa2f..f652cfbf426 100644
--- a/gdb/testsuite/gdb.base/info-program.exp
+++ b/gdb/testsuite/gdb.base/info-program.exp
@@ -28,7 +28,7 @@ gdb_test "info program" "Program stopped at $hex\.\r\nIt stopped at breakpoint $
 
 # We don't really care where this step lands, so long as it gets
 # the inferior pushed off the breakpoint it's currently on...
-gdb_test "next" "$decimal\t.*" "step before info program"
+gdb_test "next" ".*" "step before info program"
 
 gdb_test "info program" "Program stopped at $hex\.\r\nIt stopped after being stepped\.\r\nType \"info stack\" or \"info registers\" for more information\." \
     "info program after next"
-- 
2.31.1

Re: [PATCH v3 05/14] update gdb.base/info-program.exp to not fail with clang

[Andrew Burgess]

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

> The updated test specifically mentions that it doesn't care where the
> program stops, however it was still testing for something.  With this
> correction, the test works even if the compiler doesn't add epilogue
> information to functions.

Considering Pedro's feedback on an earlier patch about "epilogue
information" being potentially confusing, maybe this should be replaced
with something like:

  "... if the compiler doesn't emit line table entries for the function
  epilogue."

Or something similar.

This patch is OK with that change.

Thanks,
Andrew


> ---
>
> No change in v3.
>
> No change in v2.
>
> ---
>  gdb/testsuite/gdb.base/info-program.exp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gdb/testsuite/gdb.base/info-program.exp b/gdb/testsuite/gdb.base/info-program.exp
> index facc13efa2f..f652cfbf426 100644
> --- a/gdb/testsuite/gdb.base/info-program.exp
> +++ b/gdb/testsuite/gdb.base/info-program.exp
> @@ -28,7 +28,7 @@ gdb_test "info program" "Program stopped at $hex\.\r\nIt stopped at breakpoint $
>  
>  # We don't really care where this step lands, so long as it gets
>  # the inferior pushed off the breakpoint it's currently on...
> -gdb_test "next" "$decimal\t.*" "step before info program"
> +gdb_test "next" ".*" "step before info program"
>  
>  gdb_test "info program" "Program stopped at $hex\.\r\nIt stopped after being stepped\.\r\nType \"info stack\" or \"info registers\" for more information\." \
>      "info program after next"
> -- 
> 2.31.1

[PATCH v3 06/14] fix gdb.base/access-mem-running.exp for clang testing

[Bruno Larsen]

Clang was optimizing global_var away because it was not being used
anywhere. this commit fixes that by adding the attribute used it.
---

No change in v3.

No change in v2.

---
 gdb/testsuite/gdb.base/access-mem-running.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/access-mem-running.c b/gdb/testsuite/gdb.base/access-mem-running.c
index 6335f1bf199..cff6f0da820 100644
--- a/gdb/testsuite/gdb.base/access-mem-running.c
+++ b/gdb/testsuite/gdb.base/access-mem-running.c
@@ -19,7 +19,7 @@
 
 static unsigned int global_counter = 1;
 
-static volatile unsigned int global_var = 123;
+static volatile unsigned int __attribute__((used)) global_var = 123;
 
 static void
 maybe_stop_here ()
-- 
2.31.1

Re: [PATCH v3 06/14] fix gdb.base/access-mem-running.exp for clang testing

[Andrew Burgess]

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

> Clang was optimizing global_var away because it was not being used
> anywhere. this commit fixes that by adding the attribute used it.

LGTM.

Thanks,
Andrew

> ---
>
> No change in v3.
>
> No change in v2.
>
> ---
>  gdb/testsuite/gdb.base/access-mem-running.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gdb/testsuite/gdb.base/access-mem-running.c b/gdb/testsuite/gdb.base/access-mem-running.c
> index 6335f1bf199..cff6f0da820 100644
> --- a/gdb/testsuite/gdb.base/access-mem-running.c
> +++ b/gdb/testsuite/gdb.base/access-mem-running.c
> @@ -19,7 +19,7 @@
>  
>  static unsigned int global_counter = 1;
>  
> -static volatile unsigned int global_var = 123;
> +static volatile unsigned int __attribute__((used)) global_var = 123;
>  
>  static void
>  maybe_stop_here ()
> -- 
> 2.31.1

[PATCH v3 07/14] Fix gdb.base/call-ar-st to work with Clang

[Bruno Larsen]

When running gdb.base/call-ar-st.exp against Clang, we see one FAIL,
like so:

 print_all_arrays (array_i=<main.integer_array>, array_c=<main.char_array> "ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa
 ZaZaZaZaZaZaZaZaZaZaZaZa", array_f=<main.float_array>, array_d=<main.double_array>) at ../../../src/gdb/testsuite/gdb.base/call-ar-st.c:274
 274       print_int_array(array_i);     /* -step1- */
 (gdb) FAIL: gdb.base/call-ar-st.exp: step inside print_all_arrays

With GCC we instead see:

 print_all_arrays (array_i=<integer_array>, array_c=<char_array> "ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa", array_f=<float_array>, array_d=<double_array>) at /home/pedro/gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/call-ar-st.c:274
 274       print_int_array(array_i);     /* -step1- */
 (gdb) PASS: gdb.base/call-ar-st.exp: step inside print_all_arrays

The difference is that with Clang we get:

 array_i=<main.integer_array>, ...

instead of

 array_i = <integer_array>, ...

These symbols are local static variables, and "main" is the name of
the function they are defined in.  GCC instead appends a sequence
number to the linkage name:

 $ nm -A call-ar-st.gcc | grep integer_
 call-ar-st/call-ar-st:00000000000061a0 b integer_array.3968

 $ nm -A call-ar-st.clang | grep integer_
 call-ar-st:00000000004061a0 b main.integer_array

This commit changes the testcase to accept both outputs, as they are
functionally identical.

Co-Authored-By: Pedro Alves <pedro@palves.net>
Change-Id: Iaf2ccdb9d5996e0268ed12f595a6e04b368bfcb4
---

No change in v3.

v2: Rewored the patch according to Pedro's suggestions.

---
 gdb/testsuite/gdb.base/call-ar-st.exp | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/call-ar-st.exp b/gdb/testsuite/gdb.base/call-ar-st.exp
index 239cfa4c518..0c7dcc7c36e 100644
--- a/gdb/testsuite/gdb.base/call-ar-st.exp
+++ b/gdb/testsuite/gdb.base/call-ar-st.exp
@@ -151,10 +151,21 @@ if {!$skip_float_test && \
     gdb_test "continue" ".*" ""
 }
 
+# Return a regexp that matches the linkage name of SYM, assuming SYM
+# is a local static variable inside the main function.
+proc main_static_local_re {sym} {
+    # Clang prepends the function name + '.'.
+    return "(main\\.)?${sym}"
+}
+
 #step
 set stop_line [gdb_get_line_number "-step1-"]
 gdb_test "step" \
-    "print_all_arrays \\(array_i=<integer_array.*>, array_c=<char_array.*> .ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa., array_f=<float_array.*>, array_d=<double_array.*>\\) at .*$srcfile:$stop_line\[ \t\r\n\]+$stop_line.*print_int_array\\(array_i\\);.*" \
+    "print_all_arrays \\(array_i=<[main_static_local_re integer_array]>,\
+			 array_c=<[main_static_local_re char_array]> .ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa.,\
+			 array_f=<[main_static_local_re float_array]>,\
+			 array_d=<[main_static_local_re double_array]>\\)\
+			 at .*$srcfile:$stop_line\[ \t\r\n\]+$stop_line.*print_int_array\\(array_i\\);.*" \
     "step inside print_all_arrays"
 
 #step -over
-- 
2.31.1

[PATCH v3 08/14] add xfails to gdb.base/complex-parts.exp when testing with clang

[Bruno Larsen]

clang doesn't add encoding to the name of complex variables, only says
that the type name is complex, making the relevant tests fail.
This patch adds the xfails to the tests that expect the variable name to
include it.
---

No change in v3.

No change in v2.

---
 gdb/testsuite/gdb.base/complex-parts.exp | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gdb/testsuite/gdb.base/complex-parts.exp b/gdb/testsuite/gdb.base/complex-parts.exp
index e67fd482268..7fa94c72cd4 100644
--- a/gdb/testsuite/gdb.base/complex-parts.exp
+++ b/gdb/testsuite/gdb.base/complex-parts.exp
@@ -30,8 +30,13 @@ gdb_test "p z1" " = 1.5 \\+ 4.5i"
 gdb_test "p z2" " = 2.5 \\+ -5.5i"
 gdb_test "p z3" " = 3.5 \\+ 6.5i"
 
+# The following 3 tests are broken for Clang.
+# More info at https://github.com/llvm/llvm-project/issues/52996.
+if {[test_compiler_info clang-*-*]} { setup_xfail *-*-* }
 gdb_test "ptype z1" " = complex double"
+if {[test_compiler_info clang-*-*]} { setup_xfail *-*-* }
 gdb_test "ptype z2" " = complex float"
+if {[test_compiler_info clang-*-*]} { setup_xfail *-*-* }
 gdb_test "ptype z3" " = complex long double"
 
 with_test_prefix "double imaginary" {
-- 
2.31.1

[PATCH v3 09/14] gdb/testsuite: fix gdb.base/msym-bp-shl when running with Clang

[Bruno Larsen]

Because Clang's -O0 is not as unoptimized as gcc's, one of the functions
of gdb.base/msym-bp-shl was being optimized away, making the entire test
fail. A lot of the test fail like so:

(gdb) break foo
Breakpoint 1 at 0x401030
(gdb) FAIL: gdb.base/msym-bp-shl.exp: debug=0: before run: break foo
info breakpoint
Num     Type           Disp Enb Address            What
1       breakpoint     keep y   0x0000000000401030 <foo@plt>
(gdb) FAIL: gdb.base/msym-bp-shl.exp: debug=0: before run: info breakpoint

As the test expects 2 breakpoints to be placed. This can't be easily fixed
by adding __attribute__ ((used)) to the function, since Clang will still
optimize away the function. Because of this, the test is skipped when
the it detects that Clang is being used
---

No change in v3.

v2: Reworded commit message

---
 gdb/testsuite/gdb.base/msym-bp-shl.exp | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gdb/testsuite/gdb.base/msym-bp-shl.exp b/gdb/testsuite/gdb.base/msym-bp-shl.exp
index 42adcb191dd..dd7d05bab52 100644
--- a/gdb/testsuite/gdb.base/msym-bp-shl.exp
+++ b/gdb/testsuite/gdb.base/msym-bp-shl.exp
@@ -22,6 +22,14 @@ if {[skip_shlib_tests]} {
     return 0
 }
 
+# Clang will optimize away the static foo, regardless of using
+# __attribute__((used)), so we'll always get a single breakpoint
+# making this test useless
+if {[test_compiler_info {clang-*-*}]} {
+    untested "Clang optimizes away one of the functions"
+    return
+}
+
 standard_testfile msym-bp-shl-main.c msym-bp-shl-main-2.c msym-bp-shl-lib.c
 set srcfile ${srcdir}/${subdir}/${srcfile}
 set srcfile2 ${srcdir}/${subdir}/${srcfile2}
-- 
2.31.1

[PATCH v3 10/14] explicitly test for stderr in gdb.base/dprintf.exp

[Bruno Larsen]

Not all compilers add stderr debug information when compiling a
program. Clang, for instance, prefers to add nothing from standard
libraries and let an external debug package have this information.
Because of this, gdb.base/dprintf.exp was failing when GDB attempted to
use dprintf as a call to fprintf(stderrr, ...), like this:

 (gdb) PASS: gdb.base/dprintf.exp: call: fprintf: set dprintf style to call
 continue
 Continuing.
 kickoff 1234
 also to stderr 1234
 'stderr' has unknown type; cast it to its declared type
 (gdb) FAIL: gdb.base/dprintf.exp: call: fprintf: 1st dprintf (timeout)

To avoid this false positive, we explicitly test to see if
the compiler has added information about stderr at all, and abort
testing dprintf as an fprintf call if it is unavailable.
---

No change in v3.

v2: reworded commit message

---
 gdb/testsuite/gdb.base/dprintf.exp | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/gdb/testsuite/gdb.base/dprintf.exp b/gdb/testsuite/gdb.base/dprintf.exp
index 0b209c02a62..e214531f6dc 100644
--- a/gdb/testsuite/gdb.base/dprintf.exp
+++ b/gdb/testsuite/gdb.base/dprintf.exp
@@ -111,6 +111,16 @@ proc test_call {} {
 	test_dprintf "At foo entry.*arg=1235, g=2222\r\n" "2nd dprintf"
     }
 
+    gdb_test_multiple "print stderr" "stderr symbol check" {
+	-re "\\'stderr\\' has unknown type.*" {
+	    untested "No information available for stderr, exiting early"
+	    return
+	}
+	-re "\\\$1.*" {
+	    pass "stderr is available"
+	}
+    }
+
     with_test_prefix "fprintf" {
 	restart
 
-- 
2.31.1

[PATCH v3 11/14] gdb/testsuite: Update gdb.base/so-impl-ld.exp

[Bruno Larsen]

gdb.base/so-impl-ld.exp was setup assuming that the compiler would add
epilogue information and that GDB would stop in the } line.  This would
make clang tests fail like so:

 step^M
 solib_main (arg=10000) at ../../../common/git-repos/binutils-gdb/gdb/testsuite/gdb.base/solib1.c:7^M
 7|__  return arg*arg;|__|___/* HERE */^M
 (gdb) PASS: gdb.base/so-impl-ld.exp: step into solib call
 next^M
 main () at ../../../common/git-repos/binutils-gdb/gdb/testsuite/gdb.base/so-impl-ld.c:22^M
 22|_  return 0;^M
 (gdb) FAIL: gdb.base/so-impl-ld.exp: step in solib call
 next^M
 0x00007ffff7cef560 in __libc_start_call_main () from /lib64/libc.so.6^M
 (gdb) FAIL: gdb.base/so-impl-ld.exp: step out of solib call

This patch changes it so solib_main has 2 lines where GDB can stop
regardless of compiler weirdness, and updates the exp file to
generically deal with unknown number of steps until leaving that
function.
---

New in v3.

---
 gdb/testsuite/gdb.base/so-impl-ld.exp | 13 ++-----------
 gdb/testsuite/gdb.base/solib1.c       |  5 +++--
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/gdb/testsuite/gdb.base/so-impl-ld.exp b/gdb/testsuite/gdb.base/so-impl-ld.exp
index 69c35d4c7ac..6602aaadae0 100644
--- a/gdb/testsuite/gdb.base/so-impl-ld.exp
+++ b/gdb/testsuite/gdb.base/so-impl-ld.exp
@@ -60,21 +60,12 @@ gdb_test "step" "solib_main .arg=10000. at.*${libfile}.c:${decimal}.* HERE .*" \
 
 # Verify that we can step within the shlib call.
 #
-gdb_test "next" "${decimal}\[ \t\]*\}.* STEP .*" "step in solib call"
+gdb_test "next" "${decimal}\[ \t\]*return ans;.* STEP .*" "step in solib call"
 
 # Verify that we can step out of the shlib call, and back out into
 # the caller.
 #
-gdb_test_multiple "next" "step out of solib call" {
-    -re "0x\[0-9a-f\]*\[ \t\]*9\[ \t\]*.*$gdb_prompt $" {
-	gdb_test "next" \
-	    "main .. at.*so-impl-ld.c:22.*" \
-	    "step out of solib call"
-    }
-    -re "main .. at.*so-impl-ld.c:22.*$gdb_prompt $" {
-	pass "step out of solib call"
-    }
-}
+gdb_step_until_regexp ".*main .. at.*return 0;.*" "step out of solib call"
 
 gdb_exit
 return 0
diff --git a/gdb/testsuite/gdb.base/solib1.c b/gdb/testsuite/gdb.base/solib1.c
index 16b72338f26..bf52beec5de 100644
--- a/gdb/testsuite/gdb.base/solib1.c
+++ b/gdb/testsuite/gdb.base/solib1.c
@@ -4,5 +4,6 @@ extern "C"
 int
 solib_main (int arg)
 {
-  return arg*arg;		/* HERE */
-}				/* STEP */
+  int ans = arg*arg;		/* HERE */
+  return ans;			/* STEP */
+}
-- 
2.31.1

[PATCH v3 12/14] [gdb/testsuite]: fix gdb.base/jit-elf.exp when testing with clang

[Bruno Larsen]

When using clang as the compiler for the target, gdb.base/jit-elf.exp
was failing with the following output:

       (gdb) attach 3674146
       Attaching to program: /home/blarsen/Documents/gdb-build/gdb/testsuite/outputs/gdb.base/jit-elf/jit-elf-main, process 3674146
       Reading symbols from /lib64/libm.so.6...
       Reading symbols from .gnu_debugdata for /lib64/libm.so.6...
       (No debugging symbols found in .gnu_debugdata for /lib64/libm.so.6)
       Reading symbols from /lib64/libc.so.6...
       (No debugging symbols found in /lib64/libc.so.6)
       Reading symbols from /lib64/ld-linux-x86-64.so.2...
       [Thread debugging using libthread_db enabled]
       Using host libthread_db library "/lib64/libthread_db.so.1".
       0x00000000004013ff in main (argc=3, argv=0x7fffffffd820) at ../../../common/git-repos/binutils-gdb/gdb/testsuite/gdb.base/jit-elf-main.c:118
       118|  WAIT_FOR_GDB; i = 0;  /* gdb break here 1 */
       (gdb) FAIL: gdb.base/jit-elf.exp: attach: one_jit_test-2: break here 1: attach

While gcc's output is as follows:

       (gdb) attach 3592961
       Attaching to program: /home/blarsen/Documents/gdb-build/gdb/testsuite/outputs/gdb.base/jit-elf/jit-elf-main, process 3592961
       Reading symbols from /lib64/libm.so.6...
       Reading symbols from .gnu_debugdata for /lib64/libm.so.6...
       (No debugging symbols found in .gnu_debugdata for /lib64/libm.so.6)
       Reading symbols from /lib64/libc.so.6...
       (No debugging symbols found in /lib64/libc.so.6)
       Reading symbols from /lib64/ld-linux-x86-64.so.2...
       [Thread debugging using libthread_db enabled]
       Using host libthread_db library "/lib64/libthread_db.so.1".
       main (argc=3, argv=0x7fffffffd860) at /home/blarsen/Documents/gdb-build/gdb/testsuite/../../../common/git-repos/binutils-gdb/gdb/testsuite/gdb.base/jit-elf-main.c:118
       118|  WAIT_FOR_GDB; i = 0;  /* gdb break here 1 */
       (gdb) PASS: gdb.base/jit-elf.exp: attach: one_jit_test-2: break here 1: attach

Clang-compiled code is clearly working, as gdb is attaching and running
to the established breakpoint. To fix the false positive, the regexp for
checking where gdb has stopped was relaxed a little, to allow for the
address at the start of the line, and to allow the relative path.
---

New in v3.

---
 gdb/testsuite/gdb.base/jit-elf.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/jit-elf.exp b/gdb/testsuite/gdb.base/jit-elf.exp
index 4b75188a00d..c9929670b18 100644
--- a/gdb/testsuite/gdb.base/jit-elf.exp
+++ b/gdb/testsuite/gdb.base/jit-elf.exp
@@ -59,7 +59,7 @@ proc clean_reattach {} {
     clean_restart ${main_binfile}
 
     if { ![gdb_attach $testpid \
-	      -pattern "main.*at .*$::main_srcfile:.*"] } {
+	      -pattern ".*main.*at .*$::main_basename.c:.*"] } {
 	return 0
     }
 
-- 
2.31.1

[PATCH v3 13/14] gdb/testsuite: fix gdb.base/info-types-c++ with clang

[Bruno Larsen]

When g++ compiles nameles structs defined in a typedef, it adds a
DW_AT_linkage_name with the name defined in the typedef.  So when
running gdb.base/info-types-c++.exp by default, we get the following
output

    All defined types:

    File ../../../common/git-repos/binutils-gdb/gdb/testsuite/gdb.base/info-types.c:
    98:     CL;
    42:     anon_struct_t;
    65:     anon_union_t;
    21:     baz_t;
    33:     enum_t;
    56:     union_t;
    52:     typedef enum {...} anon_enum_t;
    45:     typedef anon_struct_t anon_struct_t;
    68:     typedef anon_union_t anon_union_t;

Clang[++] does not add DW_AT_linkage_name, and so it's output looks like
this:

    All defined types:

    File ../../../common/git-repos/binutils-gdb/gdb/testsuite/gdb.base/info-types.c:
    98:     CL;
    21:     baz_t;
    33:     enum_t;
    56:     union_t;
    52:     typedef enum {...} anon_enum_t;
    45:     typedef struct {...} anon_struct_t;
    68:     typedef union {...} anon_union_t;

Which is still correct output for GDB, but shows up as a failure when
running the test. This commit changes the test to allow for this output
when the compiler is clang.
---

New in v3.

---
 gdb/testsuite/gdb.base/info-types.exp.tcl | 109 +++++++++++++++-------
 1 file changed, 73 insertions(+), 36 deletions(-)

diff --git a/gdb/testsuite/gdb.base/info-types.exp.tcl b/gdb/testsuite/gdb.base/info-types.exp.tcl
index 2dd9b9e5489..b6a03276f69 100644
--- a/gdb/testsuite/gdb.base/info-types.exp.tcl
+++ b/gdb/testsuite/gdb.base/info-types.exp.tcl
@@ -41,42 +41,79 @@ proc run_test { lang } {
     set file_re "File .*[string_to_regexp $srcfile]:"
 
     if { $lang == "c++" } {
-	set output_lines \
-	    [list \
-		 "^All defined types:" \
-		 ".*" \
-		 $file_re \
-		 "98:\[\t \]+CL;" \
-		 "42:\[\t \]+anon_struct_t;" \
-		 "65:\[\t \]+anon_union_t;" \
-		 "21:\[\t \]+baz_t;" \
-		 "33:\[\t \]+enum_t;" \
-		 "56:\[\t \]+union_t;" \
-		 "52:\[\t \]+typedef enum {\\.\\.\\.} anon_enum_t;" \
-		 "45:\[\t \]+typedef anon_struct_t anon_struct_t;" \
-		 "68:\[\t \]+typedef anon_union_t anon_union_t;" \
-		 "28:\[\t \]+typedef baz_t baz;" \
-		 "31:\[\t \]+typedef baz_t \\* baz_ptr;" \
-		 "27:\[\t \]+typedef baz_t baz_t;" \
-		 "\[\t \]+double" \
-		 "\[\t \]+float" \
-		 "\[\t \]+int" \
-		 "103:\[\t \]+typedef CL my_cl;" \
-		 "38:\[\t \]+typedef enum_t my_enum_t;" \
-		 "17:\[\t \]+typedef float my_float_t;" \
-		 "16:\[\t \]+typedef int my_int_t;" \
-		 "104:\[\t \]+typedef CL \\* my_ptr;" \
-		 "54:\[\t \]+typedef enum {\\.\\.\\.} nested_anon_enum_t;" \
-		 "47:\[\t \]+typedef anon_struct_t nested_anon_struct_t;" \
-		 "70:\[\t \]+typedef anon_union_t nested_anon_union_t;" \
-		 "30:\[\t \]+typedef baz_t nested_baz;" \
-		 "29:\[\t \]+typedef baz_t nested_baz_t;" \
-		 "39:\[\t \]+typedef enum_t nested_enum_t;" \
-		 "19:\[\t \]+typedef float nested_float_t;" \
-		 "18:\[\t \]+typedef int nested_int_t;" \
-		 "62:\[\t \]+typedef union_t nested_union_t;(" \
-		 "\[\t \]+unsigned int)?" \
-		 "($|\r\n.*)"]
+	if { [test_compiler_info "clang-*"] } {
+	    set output_lines \
+		[list \
+		     "^All defined types:" \
+		     ".*" \
+		     $file_re \
+		     "98:\[\t \]+CL;" \
+		     "21:\[\t \]+baz_t;" \
+		     "33:\[\t \]+enum_t;" \
+		     "56:\[\t \]+union_t;" \
+		     "52:\[\t \]+typedef enum {\\.\\.\\.} anon_enum_t;" \
+		     "45:\[\t \]+typedef struct {\\.\\.\\.} anon_struct_t;" \
+		     "68:\[\t \]+typedef union {\\.\\.\\.} anon_union_t;" \
+		     "28:\[\t \]+typedef baz_t baz;" \
+		     "31:\[\t \]+typedef baz_t \\* baz_ptr;" \
+		     "27:\[\t \]+typedef baz_t baz_t;" \
+		     "\[\t \]+double" \
+		     "\[\t \]+float" \
+		     "\[\t \]+int" \
+		     "103:\[\t \]+typedef CL my_cl;" \
+		     "38:\[\t \]+typedef enum_t my_enum_t;" \
+		     "17:\[\t \]+typedef float my_float_t;" \
+		     "16:\[\t \]+typedef int my_int_t;" \
+		     "104:\[\t \]+typedef CL \\* my_ptr;" \
+		     "54:\[\t \]+typedef enum {\\.\\.\\.} nested_anon_enum_t;" \
+		     "47:\[\t \]+typedef struct {\\.\\.\\.} nested_anon_struct_t;" \
+		     "70:\[\t \]+typedef union {\\.\\.\\.} nested_anon_union_t;" \
+		     "30:\[\t \]+typedef baz_t nested_baz;" \
+		     "29:\[\t \]+typedef baz_t nested_baz_t;" \
+		     "39:\[\t \]+typedef enum_t nested_enum_t;" \
+		     "19:\[\t \]+typedef float nested_float_t;" \
+		     "18:\[\t \]+typedef int nested_int_t;" \
+		     "62:\[\t \]+typedef union_t nested_union_t;(" \
+		     "\[\t \]+unsigned int)?" \
+		     "($|\r\n.*)"]
+	 } else {
+	    set output_lines \
+		[list \
+		     "^All defined types:" \
+		     ".*" \
+		     $file_re \
+		     "98:\[\t \]+CL;" \
+		     "42:\[\t \]+anon_struct_t;" \
+		     "65:\[\t \]+anon_union_t;" \
+		     "21:\[\t \]+baz_t;" \
+		     "33:\[\t \]+enum_t;" \
+		     "56:\[\t \]+union_t;" \
+		     "52:\[\t \]+typedef enum {\\.\\.\\.} anon_enum_t;" \
+		     "45:\[\t \]+typedef anon_struct_t anon_struct_t;" \
+		     "68:\[\t \]+typedef anon_union_t anon_union_t;" \
+		     "28:\[\t \]+typedef baz_t baz;" \
+		     "31:\[\t \]+typedef baz_t \\* baz_ptr;" \
+		     "27:\[\t \]+typedef baz_t baz_t;" \
+		     "\[\t \]+double" \
+		     "\[\t \]+float" \
+		     "\[\t \]+int" \
+		     "103:\[\t \]+typedef CL my_cl;" \
+		     "38:\[\t \]+typedef enum_t my_enum_t;" \
+		     "17:\[\t \]+typedef float my_float_t;" \
+		     "16:\[\t \]+typedef int my_int_t;" \
+		     "104:\[\t \]+typedef CL \\* my_ptr;" \
+		     "54:\[\t \]+typedef enum {\\.\\.\\.} nested_anon_enum_t;" \
+		     "47:\[\t \]+typedef anon_struct_t nested_anon_struct_t;" \
+		     "70:\[\t \]+typedef anon_union_t nested_anon_union_t;" \
+		     "30:\[\t \]+typedef baz_t nested_baz;" \
+		     "29:\[\t \]+typedef baz_t nested_baz_t;" \
+		     "39:\[\t \]+typedef enum_t nested_enum_t;" \
+		     "19:\[\t \]+typedef float nested_float_t;" \
+		     "18:\[\t \]+typedef int nested_int_t;" \
+		     "62:\[\t \]+typedef union_t nested_union_t;(" \
+		     "\[\t \]+unsigned int)?" \
+		     "($|\r\n.*)"]
+	 }
     } else {
 	set output_lines \
 	    [list \
-- 
2.31.1

[PATCH v3 14/14] gdb.base/skip.exp: Use finish to exit functions

[Bruno Larsen]

gdb.base/skip.exp was making use of a fixed amount of step commands to
exit some functions.  This caused some problems when using clang to test
GDB, as GDB would need fewer steps to reach the desired spots.  For
instance, when testing in the section "step after disabling 3", the log
looks like this:

    Breakpoint 4, main () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:32
    32        x = baz ((bar (), foo ()));
    (gdb) step
    bar () at binutils-gdb/gdb/testsuite/gdb.base/skip1.c:21
    21        return 1;
    (gdb) PASS: gdb.base/skip.exp: step after disabling 3: step 1
    step
    foo () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:42
    42        return 0;
    (gdb) PASS: gdb.base/skip.exp: step after disabling 3: step 2
    step
    main () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:34
    34        test_skip_file_and_function ();
    (gdb) step
    test_skip_file_and_function () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:59
    59        test_skip ();
    (gdb) FAIL: gdb.base/skip.exp: step after disabling 3: step 3
    step
    test_skip () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:48
    48      }
    (gdb) PASS: gdb.base/skip.exp: step after disabling 3: step 4
    step
    test_skip_file_and_function () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:60
    60        skip1_test_skip_file_and_function ();
    (gdb) FAIL: gdb.base/skip.exp: step after disabling 3: step 5

This shows that the feature is working, but it is not easy to use steps
to test this feature without analyzing all possible outputs, such as
using gdb_step_until_regexp.  Instead, skip.exp now uses finish to leave
functions, synchronizing through compilers and compiler versions.  Some
test names were also changed to be a bit more descriptive.  The new log
looks like this, independently of compiler used:

    Breakpoint 4, main () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:32
    32        x = baz ((bar (), foo ()));
    (gdb) step
    bar () at binutils-gdb/gdb/testsuite/gdb.base/skip1.c:21
    21        return 1;
    (gdb) PASS: gdb.base/skip.exp: step after disabling 3: step into bar
    finish
    Run till exit from #0  bar () at binutils-gdb/gdb/testsuite/gdb.base/skip1.c:21
    main () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:32
    32        x = baz ((bar (), foo ()));
    Value returned is $2 = 1
    (gdb) PASS: gdb.base/skip.exp: step after disabling 3: return from bar
    step
    foo () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:42
    42        return 0;
    (gdb) PASS: gdb.base/skip.exp: step after disabling 3: step into foo
    finish
    Run till exit from #0  foo () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:42
    main () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:32
    32        x = baz ((bar (), foo ()));
    Value returned is $3 = 0
    (gdb) PASS: gdb.base/skip.exp: step after disabling 3: Return from foo
    step
    34        test_skip_file_and_function ();
    (gdb) PASS: gdb.base/skip.exp: step after disabling 3: step and skip baz
---

v3: Reworked again, as another patch rewrote much of this test and
showed a better way to handle the case.

v2: Reworked the whole test, as it was mis using features.

---
 gdb/testsuite/gdb.base/skip.exp | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/gdb/testsuite/gdb.base/skip.exp b/gdb/testsuite/gdb.base/skip.exp
index e6b660004d9..5b08cde93b7 100644
--- a/gdb/testsuite/gdb.base/skip.exp
+++ b/gdb/testsuite/gdb.base/skip.exp
@@ -100,6 +100,12 @@ if ![runto_main] {
 gdb_test "step" ".*" "step in the main"
 gdb_test "bt" "\\s*\\#0\\s+main.*" "step after all ignored"
 
+proc step_foo_skip_bar_baz {} {
+    gdb_test "step" "foo \\(\\) at.*" "step and skip bar"
+    gdb_test "finish" ".*" "return from bar"
+    gdb_test "step" ".*test_skip_file_and_function.*" "step and skip baz"
+}
+
 # Now remove skip.c from the skiplist.  Our first step should take us
 # into foo(), and our second step should take us to the next line in main().
 
@@ -117,21 +123,19 @@ with_test_prefix "step after deleting 1" {
 	return
     }
 
-    gdb_test "step" "foo \\(\\) at.*" "step 1"
-    gdb_test "step" ".*" "step 2" ; # Return from foo()
-    gdb_test "step" "main \\(\\) at.*" "step 3"
+    step_foo_skip_bar_baz
 }
 
 # Test that we step into foo(), then into bar(), but not into baz().
 proc step_bar_foo_skip_baz {} {
-    gdb_test "step" "bar \\(\\) at.*" "step 1"
-    gdb_test "step" ".*" "step 2"; # Return from bar()
+    gdb_test "step" "bar \\(\\) at.*" "step into bar"
+    gdb_test "finish" ".*" "return from bar"
 
     # With at least gcc 6.5.0 and 9.2.0, we jump once back to main
     # before entering foo here.  If that happens try to step a second
     # time.
     set stepped_again 0
-    gdb_test_multiple "step" "step 3" {
+    gdb_test_multiple "step" "step into foo" {
 	-re -wrap "foo \\(\\) at.*" {
 	    pass $gdb_test_name
 	}
@@ -144,8 +148,8 @@ proc step_bar_foo_skip_baz {} {
 	}
     }
 
-    gdb_test "step" ".*" "step 4"; # Return from foo()
-    gdb_test "step" "main \\(\\) at.*" "step 5"
+    gdb_test "finish" ".*" "Return from foo"
+    gdb_test "step" ".*test_skip_file_and_function.*" "step and skip baz"
 }
 
 # Now disable the skiplist entry for  skip1.c.  We should now
@@ -178,9 +182,7 @@ with_test_prefix "step after enable 3" {
 	return
     }
 
-    gdb_test "step" "foo \\(\\) at.*" "step 1"
-    gdb_test "step" ".*" "step 2"; # Return from foo()
-    gdb_test "step" "main \\(\\) at.*" "step 3"
+    step_foo_skip_bar_baz
 }
 
 # Admin tests (disable,enable,delete).
@@ -249,9 +251,7 @@ with_test_prefix "step using -fi" {
 
     gdb_test_no_output "skip disable"
     gdb_test_no_output "skip enable 5"
-    gdb_test "step" "foo \\(\\) at.*" "step 1"
-    gdb_test "step" ".*" "step 2"; # Return from foo()
-    gdb_test "step" "main \\(\\) at.*" "step 3"
+    step_foo_skip_bar_baz
 }
 
 with_test_prefix "step using -gfi" {
@@ -261,9 +261,7 @@ with_test_prefix "step using -gfi" {
 
     gdb_test_no_output "skip disable"
     gdb_test_no_output "skip enable 6"
-    gdb_test "step" "foo \\(\\) at.*" "step 1"
-    gdb_test "step" ".*" "step 2"; # Return from foo()
-    gdb_test "step" "main \\(\\) at.*" "step 3"
+    step_foo_skip_bar_baz
 }
 
 with_test_prefix "step using -fu for baz" {
-- 
2.31.1