On 2015/05/09 18:55, He Kuang wrote:
> It is not easy for users to get the accurate byte offset or the line
> number where a local variable can be probed. With '--range' option,
> local variables in scope of the probe point are showed with byte offset
> range, and can be added according to this range information.
> 
> For example, there are some variables in function
> generic_perform_write():
> 
>   <generic_perform_write@mm/filemap.c:0>
>   0  ssize_t generic_perform_write(struct file *file,
>   1                                 struct iov_iter *i, loff_t pos)
>   2  {
>   3          struct address_space *mapping = file->f_mapping;
>   4          const struct address_space_operations *a_ops = mapping->a_ops;
>   ...
>   42                 status = a_ops->write_begin(file, mapping, pos, bytes, 
> flags,
>                                                &page, &fsdata);
>   44                 if (unlikely(status < 0))
> 
> But we got failed when we try to probe the variable 'a_ops' at line 42
> or 44.
> 
>   $ perf probe --add 'generic_perform_write:42 a_ops'
>   Failed to find the location of a_ops at this address.
>     Perhaps, it has been optimized out.
> 
> This is because source code do not match assembly, so a variable may not
> be available in the sourcecode line where it presents. After this patch,
> we can lookup the accurate byte offset range of a variable, 'INV'
> indicates that this variable is not valid at the given point, but
> available in scope:
> 
>   $ perf probe --vars 'generic_perform_write:42' --range
>   Available variables at generic_perform_write:42
>         @<generic_perform_write+141>
>                 [INV]   ssize_t written @<generic_perform_write+[324-331]>
>                 [INV]   struct address_space_operations*        a_ops   
> @<generic_perform_write+[55-61,170-176,223-246]>
>                 [VAL]   (unknown_type)  fsdata  
> @<generic_perform_write+[70-307,346-411]>
>                 [VAL]   loff_t  pos     
> @<generic_perform_write+[0-286,286-336,346-411]>
>                 [VAL]   long int        status  
> @<generic_perform_write+[83-342,346-411]>
>                 [VAL]   long unsigned int       bytes   
> @<generic_perform_write+[122-311,320-338,346-403,403-411]>
>                 [VAL]   struct address_space*   mapping 
> @<generic_perform_write+[35-344,346-411]>
>                 [VAL]   struct iov_iter*        i       
> @<generic_perform_write+[0-340,346-411]>
>                 [VAL]   struct page*    page    
> @<generic_perform_write+[70-307,346-411]>
> 

Thanks, this looks easier to understand :)

[...]
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index dcca551..30a1a1b 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -43,6 +43,9 @@
>  /* Kprobe tracer basic type is up to u64 */
>  #define MAX_BASIC_TYPE_BITS  64
>  
> +/* Variable location invalid at addr but valid in scope */
> +#define VARIABLE_LOCATION_INVALID_AT_ADDR    -10000

Hmm, could you use -ERANGE instead of this?
Other part is OK for me.

Thank you!


> +
>  /* Dwarf FL wrappers */
>  static char *debuginfo_path; /* Currently dummy */
>  
> @@ -177,7 +180,7 @@ static int convert_variable_location(Dwarf_Die *vr_die, 
> Dwarf_Addr addr,
>       Dwarf_Word offs = 0;
>       bool ref = false;
>       const char *regs;
> -     int ret;
> +     int ret, ret2 = 0;
>  
>       if (dwarf_attr(vr_die, DW_AT_external, &attr) != NULL)
>               goto static_var;
> @@ -187,9 +190,19 @@ static int convert_variable_location(Dwarf_Die *vr_die, 
> Dwarf_Addr addr,
>               return -EINVAL; /* Broken DIE ? */
>       if (dwarf_getlocation_addr(&attr, addr, &op, &nops, 1) <= 0) {
>               ret = dwarf_entrypc(sp_die, &tmp);
> -             if (ret || addr != tmp ||
> -                 dwarf_tag(vr_die) != DW_TAG_formal_parameter ||
> -                 dwarf_highpc(sp_die, &tmp))
> +             if (ret)
> +                     return -ENOENT;
> +
> +             if (probe_conf.show_location_range &&
> +                     (dwarf_tag(vr_die) == DW_TAG_variable)) {
> +                     ret2 = VARIABLE_LOCATION_INVALID_AT_ADDR;
> +             } else if (addr != tmp ||
> +                     dwarf_tag(vr_die) != DW_TAG_formal_parameter) {
> +                     return -ENOENT;
> +             }
> +
> +             ret = dwarf_highpc(sp_die, &tmp);
> +             if (ret)
>                       return -ENOENT;
>               /*
>                * This is fuzzed by fentry mcount. We try to find the
> @@ -210,7 +223,7 @@ found:
>       if (op->atom == DW_OP_addr) {
>  static_var:
>               if (!tvar)
> -                     return 0;
> +                     return ret2;
>               /* Static variables on memory (not stack), make @varname */
>               ret = strlen(dwarf_diename(vr_die));
>               tvar->value = zalloc(ret + 2);
> @@ -220,7 +233,7 @@ static_var:
>               tvar->ref = alloc_trace_arg_ref((long)offs);
>               if (tvar->ref == NULL)
>                       return -ENOMEM;
> -             return 0;
> +             return ret2;
>       }
>  
>       /* If this is based on frame buffer, set the offset */
> @@ -250,7 +263,7 @@ static_var:
>       }
>  
>       if (!tvar)
> -             return 0;
> +             return ret2;
>  
>       regs = get_arch_regstr(regn);
>       if (!regs) {
> @@ -269,7 +282,7 @@ static_var:
>               if (tvar->ref == NULL)
>                       return -ENOMEM;
>       }
> -     return 0;
> +     return ret2;
>  }
>  
>  #define BYTES_TO_BITS(nb)    ((nb) * BITS_PER_LONG / sizeof(long))
> @@ -1268,13 +1281,34 @@ static int collect_variables_cb(Dwarf_Die *die_mem, 
> void *data)
>               ret = convert_variable_location(die_mem, af->pf.addr,
>                                               af->pf.fb_ops, &af->pf.sp_die,
>                                               NULL);
> -             if (ret == 0) {
> +             if (ret == 0 || ret == VARIABLE_LOCATION_INVALID_AT_ADDR) {
> +                     int ret2;
> +                     bool externs = !af->child;
>                       struct strbuf buf;
>  
>                       strbuf_init(&buf, 64);
> -                     ret = die_get_varname(die_mem, &buf);
> -                     pr_debug2("Add new var: %s\n", buf.buf);
> -                     if (ret == 0) {
> +
> +                     if (probe_conf.show_location_range) {
> +                             if (!externs) {
> +                                     if (ret)
> +                                             strbuf_addf(&buf, "[INV]\t");
> +                                     else
> +                                             strbuf_addf(&buf, "[VAL]\t");
> +                             } else
> +                                     strbuf_addf(&buf, "[EXT]\t");
> +                     }
> +
> +                     ret2 = die_get_varname(die_mem, &buf);
> +
> +                     if (!ret2 && probe_conf.show_location_range &&
> +                             !externs) {
> +                             strbuf_addf(&buf, "\t");
> +                             ret2 = die_get_var_range(&af->pf.sp_die,
> +                                                     die_mem, &buf);
> +                     }
> +
> +                     pr_debug("Add new var: %s\n", buf.buf);
> +                     if (ret2 == 0) {
>                               strlist__add(vl->vars,
>                                       strbuf_detach(&buf, NULL));
>                       }
> 


-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu...@hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to