Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1285884 > unrolled thread

[GIT PULL 0/5] perf/urgent fixes

Started byArnaldo Carvalho de Melo <acme@kernel.org>
First post2015-12-07 21:10 +0100
Last post2015-12-08 06:20 +0100
Articles 6 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [GIT PULL 0/5] perf/urgent fixes Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-12-07 21:10 +0100
    [PATCH 5/5] perf hists browser: Fix segfault if use symbol filter in cmdline Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-12-07 21:10 +0100
    [PATCH 4/5] perf hists browser: Reset selection when refresh Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-12-07 21:10 +0100
    [PATCH 1/5] perf buildid-list: Show running kernel build id fix Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-12-07 21:10 +0100
    [PATCH 3/5] perf hists browser: Add NULL pointer check to prevent crash Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-12-07 21:10 +0100
    Re: [GIT PULL 0/5] perf/urgent fixes Ingo Molnar <mingo@kernel.org> - 2015-12-08 06:20 +0100

#1285884 — [GIT PULL 0/5] perf/urgent fixes

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2015-12-07 21:10 +0100
Subject[GIT PULL 0/5] perf/urgent fixes
Message-ID<qDagG-Lz-11@gated-at.bofh.it>
From: Arnaldo Carvalho de Melo <acme@redhat.com>

Hi Ingo,

	Please consider applying,

- Arnaldo

The following changes since commit 4e93ad601a4308d4a67673c81556580817d56940:

  perf: Do not send exit event twice (2015-12-06 12:54:49 +0100)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-urgent-for-mingo

for you to fetch changes up to 4938cf0c7a62025bbfbf3db7bcdcc2c33312bedb:

  perf hists browser: Fix segfault if use symbol filter in cmdline (2015-12-07 12:02:35 -0300)

----------------------------------------------------------------
perf/urgent fixes:

User visible:

- Fix showing the running kernel build id using: (Michael Petlan)

    $ perf buildid-list -k
    03c2a89c595616188f02f0282762a75b47069bc0

- hists browser (report, top) symbol filter segfault fixes (Wang Nan)

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

----------------------------------------------------------------
Michael Petlan (2):
      perf buildid-list: Show running kernel build id fix
      perf buildid-list: Fix return value of perf buildid-list -k

Wang Nan (3):
      perf hists browser: Add NULL pointer check to prevent crash
      perf hists browser: Reset selection when refresh
      perf hists browser: Fix segfault if use symbol filter in cmdline

 tools/perf/builtin-buildid-list.c | 2 +-
 tools/perf/ui/browsers/hists.c    | 8 ++++++++
 tools/perf/util/build-id.c        | 2 +-
 3 files changed, 10 insertions(+), 2 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1285885 — [PATCH 5/5] perf hists browser: Fix segfault if use symbol filter in cmdline

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2015-12-07 21:10 +0100
Subject[PATCH 5/5] perf hists browser: Fix segfault if use symbol filter in cmdline
Message-ID<qDagG-Lz-19@gated-at.bofh.it>
In reply to#1285884
From: Wang Nan <wangnan0@huawei.com>

If feed perf a symbol filter in cmdline and the result is empty,
pressing 'Enter' in the hist browser causes crash:

 # ./perf report perf.data   <-- Common mistake for beginners

Then press 'Enter':

 perf: Segmentation fault
 -------- backtrace --------
 /home/wangnan/perf[0x53e578]
 /lib64/libc.so.6(+0x3545f)[0x7f76bafe045f]
 /home/wangnan/perf[0x539dd4]
 /home/wangnan/perf(perf_evlist__tui_browse_hists+0x96)[0x53d216]
 /home/wangnan/perf(cmd_report+0x1b9f)[0x442c7f]
 /home/wangnan/perf[0x47efa2]
 /home/wangnan/perf(main+0x5f5)[0x432fa5]
 /lib64/libc.so.6(__libc_start_main+0xf4)[0x7f76bafccbd4]
 /home/wangnan/perf[0x4330d4]

This is because 'perf.data' is interpreted as a symbol filter, and the
result is empty, so selection is empty. However,
hist_browser__toggle_fold() forgets to check it.

This patch simply return false when selection is NULL.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1449455746-41952-2-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 84c8251f39a1..81def6c3f24b 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -298,6 +298,9 @@ static bool hist_browser__toggle_fold(struct hist_browser *browser)
 	struct callchain_list *cl = container_of(ms, struct callchain_list, ms);
 	bool has_children;
 
+	if (!he || !ms)
+		return false;
+
 	if (ms == &he->ms)
 		has_children = hist_entry__toggle_fold(he);
 	else
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1285888 — [PATCH 4/5] perf hists browser: Reset selection when refresh

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2015-12-07 21:10 +0100
Subject[PATCH 4/5] perf hists browser: Reset selection when refresh
Message-ID<qDagH-Lz-31@gated-at.bofh.it>
In reply to#1285884
From: Wang Nan <wangnan0@huawei.com>

With the following steps:

 Step 1: perf report

 Step 2: Use UP/DOWN to select an entry, don't press 'ENTER'

 Step 3: Use '/' to filter symbols, use a filter which returns
         empty result

 Step 4: Press 'ENTER'

We see that, even if we have filtered all the symbols (and the main
interface is empty), pressing 'ENTER' still selects one symbol. This
behavior surprises the user.

This patch resets browser->{he_,}selection in hist_browser__refresh()
and lets it choose default selection. In this case
browser->{he_,}selection keeps NULL so user won't see annotation item in
menu.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1449455746-41952-4-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 932e13d015b9..84c8251f39a1 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -928,6 +928,8 @@ static unsigned int hist_browser__refresh(struct ui_browser *browser)
 	}
 
 	ui_browser__hists_init_top(browser);
+	hb->he_selection = NULL;
+	hb->selection = NULL;
 
 	for (nd = browser->top; nd; nd = rb_next(nd)) {
 		struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1285889 — [PATCH 1/5] perf buildid-list: Show running kernel build id fix

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2015-12-07 21:10 +0100
Subject[PATCH 1/5] perf buildid-list: Show running kernel build id fix
Message-ID<qDagH-Lz-25@gated-at.bofh.it>
In reply to#1285884
From: Michael Petlan <mpetlan@redhat.com>

The --kernel option of perf buildid-list tool should show the running
kernel buildid.  The functionality has been lost during other changes of
the related code.

The build_id__sprintf() function should return length of the build-id
string,  but it was the length of the build-id raw data instead. Due to
that, some return value checking caused that the final string was not
printed out.

With this patch the build_id__sprintf() returns the correct value, so
the --kernel option works again.

Before:

	# perf buildid-list --kernel
	#

After:

	# perf buildid-list --kernel
	972c1edab5bdc06cc224af45d510af662a3c6972
	#

Signed-off-by: Michael Petlan <mpetlan@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
LPU-Reference: 1448632089.24573.114.camel@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/build-id.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 217b5a60e2ab..6a7e273a514a 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -91,7 +91,7 @@ int build_id__sprintf(const u8 *build_id, int len, char *bf)
 		bid += 2;
 	}
 
-	return raw - build_id;
+	return (bid - bf) + 1;
 }
 
 int sysfs__sprintf_build_id(const char *root_dir, char *sbuild_id)
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1285890 — [PATCH 3/5] perf hists browser: Add NULL pointer check to prevent crash

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2015-12-07 21:10 +0100
Subject[PATCH 3/5] perf hists browser: Add NULL pointer check to prevent crash
Message-ID<qDagH-Lz-35@gated-at.bofh.it>
In reply to#1285884
From: Wang Nan <wangnan0@huawei.com>

Before this patch we can trigger a segfault by following steps:

 Step 0: Use 'perf record' to generate a perf.data without callchain

 Step 1: perf report

 Step 2: Use UP/DOWN to select an entry, don't press 'ENTER'

 Step 3: Use '/' to filter symbols, use a filter which returns
         empty result

 Step 4: Press 'ENTER' (notice here that the old selection is still
		        there. This is another problem)

 Step 5: Press 'ENTER' to annotate that symbol

 Step 6: Press 'LEFT' to go out.

 Result: segfault:

 perf: Segmentation fault
 -------- backtrace --------
 /home/wangnan/perf[0x53e568]
 /lib64/libc.so.6(+0x3545f)[0x7fba75d3245f]
 /home/wangnan/perf[0x537516]
 /home/wangnan/perf[0x533fef]
 /home/wangnan/perf[0x53b347]
 /home/wangnan/perf(perf_evlist__tui_browse_hists+0x96)[0x53d206]
 /home/wangnan/perf(cmd_report+0x1b9f)[0x442c7f]
 /home/wangnan/perf[0x47efa2]
 /home/wangnan/perf(main+0x5f5)[0x432fa5]
 /lib64/libc.so.6(__libc_start_main+0xf4)[0x7fba75d1ebd4]
 /home/wangnan/perf[0x4330d4]

This is because in this case 'nd' could be NULL in
ui_browser__hists_seek(), but that function never checks it.

This patch adds checker for potential NULL pointer in that function.
After this patch the above steps won't segfault.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1449455746-41952-3-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index fa9eb92c9e24..932e13d015b9 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1033,6 +1033,9 @@ static void ui_browser__hists_seek(struct ui_browser *browser,
 	 * and stop when we printed enough lines to fill the screen.
 	 */
 do_offset:
+	if (!nd)
+		return;
+
 	if (offset > 0) {
 		do {
 			h = rb_entry(nd, struct hist_entry, rb_node);
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1286239

FromIngo Molnar <mingo@kernel.org>
Date2015-12-08 06:20 +0100
Message-ID<qDiQW-6q2-5@gated-at.bofh.it>
In reply to#1285884
* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> Hi Ingo,
> 
> 	Please consider applying,
> 
> - Arnaldo
> 
> The following changes since commit 4e93ad601a4308d4a67673c81556580817d56940:
> 
>   perf: Do not send exit event twice (2015-12-06 12:54:49 +0100)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-urgent-for-mingo
> 
> for you to fetch changes up to 4938cf0c7a62025bbfbf3db7bcdcc2c33312bedb:
> 
>   perf hists browser: Fix segfault if use symbol filter in cmdline (2015-12-07 12:02:35 -0300)
> 
> ----------------------------------------------------------------
> perf/urgent fixes:
> 
> User visible:
> 
> - Fix showing the running kernel build id using: (Michael Petlan)
> 
>     $ perf buildid-list -k
>     03c2a89c595616188f02f0282762a75b47069bc0
> 
> - hists browser (report, top) symbol filter segfault fixes (Wang Nan)
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> ----------------------------------------------------------------
> Michael Petlan (2):
>       perf buildid-list: Show running kernel build id fix
>       perf buildid-list: Fix return value of perf buildid-list -k
> 
> Wang Nan (3):
>       perf hists browser: Add NULL pointer check to prevent crash
>       perf hists browser: Reset selection when refresh
>       perf hists browser: Fix segfault if use symbol filter in cmdline
> 
>  tools/perf/builtin-buildid-list.c | 2 +-
>  tools/perf/ui/browsers/hists.c    | 8 ++++++++
>  tools/perf/util/build-id.c        | 2 +-
>  3 files changed, 10 insertions(+), 2 deletions(-)

Pulled, thanks a lot Arnaldo!

I also merged the fixes into perf/core and pushed it all out, so you can base 
subsequent changes on top of the merged result.

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web