diff mbox

[2/6] DW attribute macro MACRO_AT_func and MACRO_AT_range

Message ID 87fvdqijal.fsf@codesourcery.com
State New
Headers show

Commit Message

Yao Qi Nov. 11, 2014, 2:05 a.m. UTC
Doug Evans <dje@google.com> writes:

> OTOH, leaving such things around for debug purposes is important,
> or at least not making it difficult to keep such things (and editing source
> files to, e.g., comment out the "file delete", crosses a threshold of
> annoyance for me).

Yeah, I commented out the "file delete" from time to time, but I can
bear with that.

>
> I think the simplicity of implementation and assistance to debugability
> of just leaving the file there outweighs the extra code and any perceived
> increase in cleanliness.
>
> Plus the more remote file operations we add the more we slow
> things down.  There's a *ton* of files we leave behind,
> I wouldn't worry about leaving a few more behind.
> If there were another reason then that might be something
> to consider.
>
> [At least with gcc there is -save-temps.  We *could* have something
> like that here, but I'm ok with just leaving them.]

It has been discussed for several times that these artifacts should be
kept.  Let us have a try and start to do it now.  I'll think about this
problem systematically which I haven't done before, and post something
out when I make some progresses or get some conclusions.

The "remote_file host delete" is removed from the patch below.

Comments

Doug Evans Nov. 12, 2014, 7:01 a.m. UTC | #1
On Mon, Nov 10, 2014 at 6:05 PM, Yao Qi <yao@codesourcery.com> wrote:
> [...]
> 2014-11-10  Yao Qi  <yao@codesourcery.com>
>
>         * lib/dwarf.exp (function_range): New procedure.
>         (Dwarf::_handle_macro_at_func): New procedure.
>         (Dwarf::_handle_macro_at_range): New procedure.
>         (Dwarf): Handle MACRO_AT_func and MACRO_AT_range.

LGTM, with one nit that can be left for later.

> +
> +           if { [string equal "MACRO_AT_func" $attr_name] } {
> +               _handle_macro_at_func $attr_value
> +           } elseif { [string equal "MACRO_AT_range" $attr_name] } {
> +               _handle_macro_at_range $attr_value
>             } else {
> -               set attr_form [_guess_form $attr_value attr_value]
> -           }
> -           set attr_form [_map_name $attr_form _FORM]
> +               if {[llength $attr] > 2} {
> +                   set attr_form [lindex $attr 2]
> +               } else {
> +                   set attr_form [_guess_form $attr_value attr_value]
> +               }
> +               set attr_form [_map_name $attr_form _FORM]
>
> -           _handle_attribute $attr_name $attr_value $attr_form
> +               _handle_attribute $attr_name $attr_value $attr_form
> +           }
>         }
>
>         _defer_output $_abbrev_section {

The sequence of ifs to test for each macro name is akin to the switch
statement we removed.
It's less code of course, but it still involves continual additions
for each new macro.
I was thinking of still having a wrapper function that checks for macros,
but it could do "info proc _handle_$attr_name" or some such and
call(via eval?) the function if it exists or flag an error if it
doesn't.  We don't have to go down this road though until we need to.
Yao Qi Nov. 14, 2014, 12:59 a.m. UTC | #2
Doug Evans <dje@google.com> writes:

Hi Doug,

> The sequence of ifs to test for each macro name is akin to the switch
> statement we removed.
> It's less code of course, but it still involves continual additions
> for each new macro.
> I was thinking of still having a wrapper function that checks for macros,
> but it could do "info proc _handle_$attr_name" or some such and
> call(via eval?) the function if it exists or flag an error if it
> doesn't.  We don't have to go down this road though until we need to.

Yeah, that is a very clear trick.  I'll adapt the code for it when new
macro attribute is added later.

Thanks for your review, and this series is pushed in.
diff mbox

Patch

diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index 4986f83..cadda3e 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -86,6 +86,79 @@  proc build_executable_from_fission_assembler { testname executable sources optio
     return 0
 }
 
+# Return a list of expressions about function FUNC's address and length.
+# The first expression is the address of function FUNC, and the second
+# one is FUNC's length.  SRC is the source file having function FUNC.
+# An internal label ${func}_label must be defined inside FUNC:
+#
+#  int main (void)
+#  {
+#    asm ("main_label: .globl main_label");
+#    return 0;
+#  }
+#
+# This label is needed to compute the start address of function FUNC.
+# If the compiler is gcc, we can do the following to get function start
+# and end address too:
+#
+# asm ("func_start: .globl func_start");
+# static void func (void) {}
+# asm ("func_end: .globl func_end");
+#
+# however, this isn't portable, because other compilers, such as clang,
+# may not guarantee the order of global asms and function.  The code
+# becomes:
+#
+# asm ("func_start: .globl func_start");
+# asm ("func_end: .globl func_end");
+# static void func (void) {}
+#
+
+proc function_range { func src } {
+    global decimal gdb_prompt
+
+    set exe [standard_temp_file func_addr[pid].x]
+
+    gdb_compile $src $exe executable {debug}
+
+    gdb_exit
+    gdb_start
+    gdb_load "$exe"
+
+    # Compute the label offset, and we can get the function start address
+    # by "${func}_label - $func_label_offset".
+    set func_label_offset ""
+    set test "p ${func}_label - ${func}"
+    gdb_test_multiple $test $test {
+	-re ".* = ($decimal)\r\n$gdb_prompt $" {
+	    set func_label_offset $expect_out(1,string)
+	}
+    }
+
+    # Compute the function length.
+    global hex
+    set func_length ""
+    set test "disassemble $func"
+    gdb_test_multiple $test $test {
+	-re ".*$hex <\\+($decimal)>:\[^\r\n\]+\r\nEnd of assembler dump\.\r\n$gdb_prompt $" {
+	    set func_length $expect_out(1,string)
+	}
+    }
+
+    # Compute the size of the last instruction.
+    set test "x/2i $func+$func_length"
+    gdb_test_multiple $test $test {
+	-re ".*($hex) <$func\\+$func_length>:\[^\r\n\]+\r\n\[ \]+($hex).*\.\r\n$gdb_prompt $" {
+	    set start $expect_out(1,string)
+	    set end $expect_out(2,string)
+
+	    set func_length [expr $func_length + $end - $start]
+	}
+    }
+
+    return [list "${func}_label - $func_label_offset" $func_length]
+}
+
 # A DWARF assembler.
 #
 # All the variables in this namespace are private to the
@@ -121,6 +194,17 @@  proc build_executable_from_fission_assembler { testname executable sources optio
 # This can either be the full name, like 'DW_AT_name', or a shortened
 # name, like 'name'.  These are fully equivalent.
 #
+# Besides DWARF standard attributes, assembler supports 'macro' attribute
+# which will be substituted by one or more standard or macro attributes.
+# supported macro attributes are:
+#
+#  - MACRO_AT_range { FUNC FILE }
+#  It is substituted by DW_AT_low_pc and DW_AT_high_pc with the start and
+#  end address of function FUNC in file FILE.
+#
+#  - MACRO_AT_func { FUNC FILE }
+#  It is substituted by DW_AT_name with FUNC and MACRO_AT_range.
+#
 # If FORM is given, it should name a DW_FORM_ constant.
 # This can either be the short form, like 'DW_FORM_addr', or a
 # shortened version, like 'addr'.  If the form is given, VALUE
@@ -473,6 +557,33 @@  namespace eval Dwarf {
 	}
     }
 
+    # Handle macro attribute MACRO_AT_range.
+
+    proc _handle_macro_at_range { attr_value } {
+	if {[llength $attr_value] != 2} {
+	    error "usage: MACRO_AT_range { func file }"
+	}
+
+	set func [lindex $attr_value 0]
+	set src [lindex $attr_value 1]
+	set result [function_range $func $src]
+
+	_handle_attribute DW_AT_low_pc [lindex $result 0] \
+	    DW_FORM_addr
+	_handle_attribute DW_AT_high_pc \
+	    "[lindex $result 0] + [lindex $result 1]" DW_FORM_addr
+    }
+
+    # Handle macro attribute MACRO_AT_func.
+
+    proc _handle_macro_at_func { attr_value } {
+	if {[llength $attr_value] != 2} {
+	    error "usage: MACRO_AT_func { func file }"
+	}
+	_handle_attribute DW_AT_name [lindex $attr_value 0] DW_FORM_string
+	_handle_macro_at_range $attr_value
+    }
+
     proc _handle_DW_TAG {tag_name {attrs {}} {children {}}} {
 	variable _abbrev_section
 	variable _abbrev_num
@@ -494,14 +605,21 @@  namespace eval Dwarf {
 	foreach attr $attrs {
 	    set attr_name [_map_name [lindex $attr 0] _AT]
 	    set attr_value [uplevel 2 [list subst [lindex $attr 1]]]
-	    if {[llength $attr] > 2} {
-		set attr_form [lindex $attr 2]
+
+	    if { [string equal "MACRO_AT_func" $attr_name] } {
+		_handle_macro_at_func $attr_value
+	    } elseif { [string equal "MACRO_AT_range" $attr_name] } {
+		_handle_macro_at_range $attr_value
 	    } else {
-		set attr_form [_guess_form $attr_value attr_value]
-	    }
-	    set attr_form [_map_name $attr_form _FORM]
+		if {[llength $attr] > 2} {
+		    set attr_form [lindex $attr 2]
+		} else {
+		    set attr_form [_guess_form $attr_value attr_value]
+		}
+		set attr_form [_map_name $attr_form _FORM]
 
-	    _handle_attribute $attr_name $attr_value $attr_form
+		_handle_attribute $attr_name $attr_value $attr_form
+	    }
 	}
 
 	_defer_output $_abbrev_section {