public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gdb/testsuite: make declare_labels use better default label names
@ 2020-12-03 23:19 Simon Marchi
  2020-12-04 19:46 ` Tom Tromey
  0 siblings, 1 reply; 3+ messages in thread
From: Simon Marchi @ 2020-12-03 23:19 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi

When using the single-element form of argument to declare_labels, the
generated label (in the assembly file) is of the format ".LlabelN",
where N is a number.

I propose making it use the name of the label by default.  Calling:

    declare_labels foo

will generate the ".LfooN" in the assembly file (again, where N is a
number).  When debugging the output of the DWARF assembler, it makes it
easier to map labels to the source.  Also, when defining the same label
twice by mistake in the Tcl code (like I d id), it's easier to track the
error from the message to the root cause:

    -/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/implptrpiece/implptrpiece-dw.S:62: Error: symbol `.Llabel5' is already defined
    +/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/implptrpiece/implptrpiece-dw.S:62: Error: symbol `.Lvar_label5' is already defined

This doesn't change anything for the test cases, it just makes the
assembly output a bit nicer.

gdb/testsuite/ChangeLog:

	* lib/dwarf.exp (declare_labels): Use name as text if text is
	not provided.

Change-Id: I63856c1fa6390498fd5b9d66f471f817ff0a465c
---
 gdb/testsuite/lib/dwarf.exp | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index c1596df58be..ecd438b205e 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -863,13 +863,13 @@ namespace eval Dwarf {
 	    set name [lindex $arg 0]
 	    set text [lindex $arg 1]
 
-	    upvar $name label_var
-	    if {$text == ""} {
-		set label_var [new_label]
-	    } else {
-		set label_var [new_label $text]
+	    if { $text == "" } {
+		set text $name
 	    }
 
+	    upvar $name label_var
+	    set label_var [new_label $text]
+
 	    proc ${name}: {args} [format {
 		define_label %s
 		uplevel $args
-- 
2.29.2


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] gdb/testsuite: make declare_labels use better default label names
  2020-12-03 23:19 [PATCH] gdb/testsuite: make declare_labels use better default label names Simon Marchi
@ 2020-12-04 19:46 ` Tom Tromey
  2020-12-04 20:08   ` Simon Marchi
  0 siblings, 1 reply; 3+ messages in thread
From: Tom Tromey @ 2020-12-04 19:46 UTC (permalink / raw)
  To: Simon Marchi via Gdb-patches; +Cc: Simon Marchi

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> When using the single-element form of argument to declare_labels, the
Simon> generated label (in the assembly file) is of the format ".LlabelN",
Simon> where N is a number.

Simon> I propose making it use the name of the label by default.  Calling:

Simon>     declare_labels foo

Simon> will generate the ".LfooN" in the assembly file (again, where N is a
Simon> number).

Seems like a good idea to me.

Simon> Also, when defining the same label
Simon> twice by mistake in the Tcl code (like I d id)

Can we make this a Tcl error at the point of redefinition?
For example declare_labels and define_label could use a new
namespace-local array to track when labels are declared and when they
are defined, to prevent non-declaration (if that is needed, I forget)
and redefinition.

Simon> gdb/testsuite/ChangeLog:

Simon> 	* lib/dwarf.exp (declare_labels): Use name as text if text is
Simon> 	not provided.

Looks good.

Tom

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] gdb/testsuite: make declare_labels use better default label names
  2020-12-04 19:46 ` Tom Tromey
@ 2020-12-04 20:08   ` Simon Marchi
  0 siblings, 0 replies; 3+ messages in thread
From: Simon Marchi @ 2020-12-04 20:08 UTC (permalink / raw)
  To: Tom Tromey, Simon Marchi via Gdb-patches

On 2020-12-04 2:46 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Simon> When using the single-element form of argument to declare_labels, the
> Simon> generated label (in the assembly file) is of the format ".LlabelN",
> Simon> where N is a number.
>
> Simon> I propose making it use the name of the label by default.  Calling:
>
> Simon>     declare_labels foo
>
> Simon> will generate the ".LfooN" in the assembly file (again, where N is a
> Simon> number).
>
> Seems like a good idea to me.
>
> Simon> Also, when defining the same label
> Simon> twice by mistake in the Tcl code (like I d id)
>
> Can we make this a Tcl error at the point of redefinition?
> For example declare_labels and define_label could use a new
> namespace-local array to track when labels are declared and when they
> are defined, to prevent non-declaration (if that is needed, I forget)
> and redefinition.

That would be nice indeed.

>
> Simon> gdb/testsuite/ChangeLog:
>
> Simon> 	* lib/dwarf.exp (declare_labels): Use name as text if text is
> Simon> 	not provided.
>
> Looks good.

Thanks, I'll merge this.  Even if the TCL code tracks declared labels to
check for duplication, it still sounds nice to have assembly label name
matching the TCL label names.

Simon

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-12-04 20:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-03 23:19 [PATCH] gdb/testsuite: make declare_labels use better default label names Simon Marchi
2020-12-04 19:46 ` Tom Tromey
2020-12-04 20:08   ` Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).