All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Thomas Richter <tmricht@linux.ibm.com>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
	Sumanth Korikkar <sumanthk@linux.ibm.com>
Subject: Re: [PATCH 2/2] perf-probe: Change function definition check due to broken dwarf
Date: Fri, 27 Nov 2020 14:36:29 -0300	[thread overview]
Message-ID: <20201127173629.GP70905@kernel.org> (raw)
In-Reply-To: <160645613571.2824037.7441351537890235895.stgit@devnote2>

Em Fri, Nov 27, 2020 at 02:48:55PM +0900, Masami Hiramatsu escreveu:
> Since some gcc generates a broken DWARF which lacks DW_AT_declaration
> attribute from the subprogram DIE of function prototype.
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97060)
> 
> So, in addition to the DW_AT_declaration check, we also check the
> subprogram DIE has DW_AT_inline or actual entry pc.

Thanks, applied and tested both patches on a fedora 33 system where
previously those tests were failing:

    Committer testing:

      # cat /etc/fedora-release
      Fedora release 33 (Thirty Three)
      #

    Before:

      # perf test vfs_getname
      78: Use vfs_getname probe to get syscall args filenames             : FAILED!
      79: Check open filename arg using perf trace + vfs_getname          : FAILED!
      81: Add vfs_getname probe to get syscall args filenames             : FAILED!
      #

    After:

      # perf test vfs_getname
      78: Use vfs_getname probe to get syscall args filenames             : Ok
      79: Check open filename arg using perf trace + vfs_getname          : Ok
      81: Add vfs_getname probe to get syscall args filenames             : Ok
      #


- Arnaldo
 
> Reported-by: Thomas Richter <tmricht@linux.ibm.com>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  tools/perf/util/dwarf-aux.c    |   20 ++++++++++++++++++--
>  tools/perf/util/probe-finder.c |    3 +--
>  2 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index 03c1a39c312a..7b2d471a6419 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -356,9 +356,25 @@ bool die_is_signed_type(Dwarf_Die *tp_die)
>  bool die_is_func_def(Dwarf_Die *dw_die)
>  {
>  	Dwarf_Attribute attr;
> +	Dwarf_Addr addr = 0;
> +
> +	if (dwarf_tag(dw_die) != DW_TAG_subprogram)
> +		return false;
> +
> +	if (dwarf_attr(dw_die, DW_AT_declaration, &attr))
> +		return false;
>  
> -	return (dwarf_tag(dw_die) == DW_TAG_subprogram &&
> -		dwarf_attr(dw_die, DW_AT_declaration, &attr) == NULL);
> +	/*
> +	 * DW_AT_declaration can be lost from function declaration
> +	 * by gcc's bug #97060.
> +	 * So we need to check this subprogram DIE has DW_AT_inline
> +	 * or an entry address.
> +	 */
> +	if (!dwarf_attr(dw_die, DW_AT_inline, &attr) &&
> +	    die_entrypc(dw_die, &addr) < 0)
> +		return false;
> +
> +	return true;
>  }
>  
>  /**
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index 2c4061035f77..76dd349aa48d 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -1885,8 +1885,7 @@ static int line_range_search_cb(Dwarf_Die *sp_die, void *data)
>  	if (lr->file && strtailcmp(lr->file, dwarf_decl_file(sp_die)))
>  		return DWARF_CB_OK;
>  
> -	if (die_is_func_def(sp_die) &&
> -	    die_match_name(sp_die, lr->function)) {
> +	if (die_match_name(sp_die, lr->function) && die_is_func_def(sp_die)) {
>  		lf->fname = dwarf_decl_file(sp_die);
>  		dwarf_decl_line(sp_die, &lr->offset);
>  		pr_debug("fname: %s, lineno:%d\n", lf->fname, lr->offset);
> 

-- 

- Arnaldo

      reply	other threads:[~2020-11-27 17:36 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-18 16:37 Fedora 33 and perf probe failures Thomas Richter
2020-11-19  1:07 ` Masami Hiramatsu
2020-11-19  8:16   ` Thomas Richter
2020-11-19 13:04     ` Masami Hiramatsu
2020-11-26 13:08     ` Masami Hiramatsu
2020-11-26 17:28       ` Arnaldo Carvalho de Melo
2020-11-27  0:10         ` Masami Hiramatsu
2020-11-27 11:36           ` Arnaldo Carvalho de Melo
2020-11-27  5:48         ` [PATCH 1/2] perf-probe: Fix to die_entrypc() returns error correctly Masami Hiramatsu
2020-11-27  5:48         ` [PATCH 2/2] perf-probe: Change function definition check due to broken dwarf Masami Hiramatsu
2020-11-27 17:36           ` Arnaldo Carvalho de Melo [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201127173629.GP70905@kernel.org \
    --to=acme@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=sumanthk@linux.ibm.com \
    --cc=tmricht@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.