Re: [PATCH 16/35] perf annotate: Add samples into struct annotation_line

[Jiri Olsa]

On Mon, Nov 13, 2017 at 09:14:38PM +0100, Jiri Olsa wrote:
> On Mon, Nov 13, 2017 at 09:16:20PM +0530, Ravi Bangoria wrote:
> > Hi Jiri,
> > 
> > This patch seems to be causing segfault with "perf top --stdio".
> > 
> > Steps to reproduce:
> > 1. start "perf top --stdio" in one terminal
> > 2. run some simple workload in another terminal, let it get finished.
> > 3. annotate function from previous workload in perf top (press 'a' followed
> > by 's')
> > 
> > Perf will crash with:
> > 
> >   perf: Segmentation fault
> >   Obtained 8 stack frames.
> >   ./perf(sighandler_dump_stack+0x3e) [0x4f1b6e]
> >   /lib64/libc.so.6(+0x36a7f) [0x7ff3aa7e4a7f]
> >   ./perf() [0x4a27fd]
> >   ./perf(symbol__annotate+0x199) [0x4a4439]
> >   ./perf() [0x44e32d]
> >   ./perf() [0x44f098]
> >   /lib64/libpthread.so.0(+0x736c) [0x7ff3acee836c]
> >   /lib64/libc.so.6(clone+0x3e) [0x7ff3aa8bee1e]
> > 
> > Can you please check.
> 
> hum, I'm getting following crash after resizing the terminal window:
> 
> perf: Floating point exception
> Obtained 8 stack frames.
> ./perf(dump_stack+0x2e) [0x510c89]
> ./perf(sighandler_dump_stack+0x2e) [0x510d69]
> /lib64/libc.so.6(+0x36a80) [0x7f9419588a80]
> ./perf(perf_top__header_snprintf+0x208) [0x4f42c1]
> ./perf() [0x453c09]
> ./perf() [0x454ddb]
> /lib64/libpthread.so.0(+0x736d) [0x7f941bc8c36d]
> /lib64/libc.so.6(clone+0x3f) [0x7f9419662e1f]
> Floating point exception (core dumped)
> 
> working on fix

so my crash is caused by bogus resize code, I have it working with fix for
memory corruption happening in SIGWINCH signal handler (attached)
could you please check if that fixes the code for you?

I'll check if that does not break the TUI

jirka


---
 tools/perf/builtin-top.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index adfeeb488f1a..7a2a7d7931c6 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -77,6 +77,7 @@
 #include "sane_ctype.h"
 
 static volatile int done;
+static volatile int resize;
 
 #define HEADER_LINE_NR  5
 
@@ -86,10 +87,13 @@ static void perf_top__update_print_entries(struct perf_top *top)
 }
 
 static void perf_top__sig_winch(int sig __maybe_unused,
-				siginfo_t *info __maybe_unused, void *arg)
+				siginfo_t *info __maybe_unused, void *arg __maybe_unused)
 {
-	struct perf_top *top = arg;
+	resize = 1;
+}
 
+static void perf_top__resize(struct perf_top *top)
+{
 	get_term_dimensions(&top->winsize);
 	perf_top__update_print_entries(top);
 }
@@ -475,9 +479,8 @@ static bool perf_top__handle_keypress(struct perf_top *top, int c)
 			if (top->print_entries == 0) {
 				struct sigaction act = {
 					.sa_sigaction = perf_top__sig_winch,
-					.sa_flags     = SA_SIGINFO,
 				};
-				perf_top__sig_winch(SIGWINCH, NULL, top);
+				perf_top__resize(top);
 				sigaction(SIGWINCH, &act, NULL);
 			} else {
 				signal(SIGWINCH, SIG_DFL);
@@ -1030,6 +1033,11 @@ static int __cmd_top(struct perf_top *top)
 
 		if (hits == top->samples)
 			ret = perf_evlist__poll(top->evlist, 100);
+
+		if (resize) {
+			perf_top__resize(top);
+			resize = 0;
+		}
 	}
 
 	ret = 0;
@@ -1354,9 +1362,8 @@ int cmd_top(int argc, const char **argv)
 	if (top.print_entries == 0) {
 		struct sigaction act = {
 			.sa_sigaction = perf_top__sig_winch,
-			.sa_flags     = SA_SIGINFO,
 		};
-		perf_top__update_print_entries(&top);
+		perf_top__resize(&top);
 		sigaction(SIGWINCH, &act, NULL);
 	}
 
-- 
2.9.5