[PATCH] perf: increase size of buf in perf_evsel__hists_browse()

[PATCH] perf: increase size of buf in perf_evsel__hists_browse()

[Song Liu]

Making perf with gcc-9.1.1 generates the following warning:

  CC       ui/browsers/hists.o
ui/browsers/hists.c: In function 'perf_evsel__hists_browse':
ui/browsers/hists.c:3078:61: error: '%d' directive output may be \
truncated writing between 1 and 11 bytes into a region of size \
between 2 and 12 [-Werror=format-truncation=]

 3078 |       "Max event group index to sort is %d (index from 0 to %d)",
      |                                                             ^~
ui/browsers/hists.c:3078:7: note: directive argument in the range [-2147483648, 8]
 3078 |       "Max event group index to sort is %d (index from 0 to %d)",
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/stdio.h:937,
                 from ui/browsers/hists.c:5:

IOW, the string in line 3078 might be too long for buf[] of 64 bytes.

Fix this by increasing the size of buf[] to 128.

Fixes: dbddf1747441  ("perf report/top TUI: Support hotkeys to let user select any event for sorting")
Cc: stable <stable@vger.kernel.org> # v5.7+
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Signed-off-by: Song Liu <songliubraving@fb.com>
---
 tools/perf/ui/browsers/hists.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index a07626f072087..b0e1880cf992b 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -2963,7 +2963,7 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
 	struct popup_action actions[MAX_OPTIONS];
 	int nr_options = 0;
 	int key = -1;
-	char buf[64];
+	char buf[128];
 	int delay_secs = hbt ? hbt->refresh : 0;
 
 #define HIST_BROWSER_HELP_COMMON					\
-- 
2.24.1

RE: [PATCH] perf: increase size of buf in perf_evsel__hists_browse()

[David Laight]

From: Song Liu
> Sent: 30 October 2020 23:55
> 
> Making perf with gcc-9.1.1 generates the following warning:
> 
>   CC       ui/browsers/hists.o
> ui/browsers/hists.c: In function 'perf_evsel__hists_browse':
> ui/browsers/hists.c:3078:61: error: '%d' directive output may be \
> truncated writing between 1 and 11 bytes into a region of size \
> between 2 and 12 [-Werror=format-truncation=]
> 
>  3078 |       "Max event group index to sort is %d (index from 0 to %d)",
>       |                                                             ^~
> ui/browsers/hists.c:3078:7: note: directive argument in the range [-2147483648, 8]
>  3078 |       "Max event group index to sort is %d (index from 0 to %d)",
>       |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from /usr/include/stdio.h:937,
>                  from ui/browsers/hists.c:5:
> 
> IOW, the string in line 3078 might be too long for buf[] of 64 bytes.
> 
> Fix this by increasing the size of buf[] to 128.

ISTM that something should be unsigned so that the bound check
that puts an upper bound of 8 implies a lower bound.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] perf: increase size of buf in perf_evsel__hists_browse()

[Jiri Olsa]

On Fri, Oct 30, 2020 at 04:54:31PM -0700, Song Liu wrote:
> Making perf with gcc-9.1.1 generates the following warning:
> 
>   CC       ui/browsers/hists.o
> ui/browsers/hists.c: In function 'perf_evsel__hists_browse':
> ui/browsers/hists.c:3078:61: error: '%d' directive output may be \
> truncated writing between 1 and 11 bytes into a region of size \
> between 2 and 12 [-Werror=format-truncation=]
> 
>  3078 |       "Max event group index to sort is %d (index from 0 to %d)",
>       |                                                             ^~
> ui/browsers/hists.c:3078:7: note: directive argument in the range [-2147483648, 8]
>  3078 |       "Max event group index to sort is %d (index from 0 to %d)",
>       |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from /usr/include/stdio.h:937,
>                  from ui/browsers/hists.c:5:
> 
> IOW, the string in line 3078 might be too long for buf[] of 64 bytes.
> 
> Fix this by increasing the size of buf[] to 128.
> 
> Fixes: dbddf1747441  ("perf report/top TUI: Support hotkeys to let user select any event for sorting")
> Cc: stable <stable@vger.kernel.org> # v5.7+
> Cc: Jin Yao <yao.jin@linux.intel.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> Signed-off-by: Song Liu <songliubraving@fb.com>

Acked-by: Jiri Olsa <jolsa@kernel.org>

jirka

> ---
>  tools/perf/ui/browsers/hists.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index a07626f072087..b0e1880cf992b 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -2963,7 +2963,7 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
>  	struct popup_action actions[MAX_OPTIONS];
>  	int nr_options = 0;
>  	int key = -1;
> -	char buf[64];
> +	char buf[128];
>  	int delay_secs = hbt ? hbt->refresh : 0;
>  
>  #define HIST_BROWSER_HELP_COMMON					\
> -- 
> 2.24.1
> 

Re: [PATCH] perf: increase size of buf in perf_evsel__hists_browse()

[Song Liu]

> On Oct 31, 2020, at 4:27 AM, David Laight <David.Laight@ACULAB.COM> wrote:
> 
> From: Song Liu
>> Sent: 30 October 2020 23:55
>> 
>> Making perf with gcc-9.1.1 generates the following warning:
>> 
>>  CC       ui/browsers/hists.o
>> ui/browsers/hists.c: In function 'perf_evsel__hists_browse':
>> ui/browsers/hists.c:3078:61: error: '%d' directive output may be \
>> truncated writing between 1 and 11 bytes into a region of size \
>> between 2 and 12 [-Werror=format-truncation=]
>> 
>> 3078 |       "Max event group index to sort is %d (index from 0 to %d)",
>>      |                                                             ^~
>> ui/browsers/hists.c:3078:7: note: directive argument in the range [-2147483648, 8]
>> 3078 |       "Max event group index to sort is %d (index from 0 to %d)",
>>      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> In file included from /usr/include/stdio.h:937,
>>                 from ui/browsers/hists.c:5:
>> 
>> IOW, the string in line 3078 might be too long for buf[] of 64 bytes.
>> 
>> Fix this by increasing the size of buf[] to 128.
> 
> ISTM that something should be unsigned so that the bound check
> that puts an upper bound of 8 implies a lower bound.
> 
> 	David

Changing both "%d" in this line to "%u" does fix the warning. But we 
are printing "evsel->core.nr_members - 1" here, and nr_members is 
signed int. So I feel more comfortable keep the "%d"s and increase 
the buffer size. 

Thanks,
Song

Re: [PATCH] perf: increase size of buf in perf_evsel__hists_browse()

[Arnaldo Carvalho de Melo]

Em Sat, Oct 31, 2020 at 09:29:20PM +0100, Jiri Olsa escreveu:
> On Fri, Oct 30, 2020 at 04:54:31PM -0700, Song Liu wrote:
> > Making perf with gcc-9.1.1 generates the following warning:
> > 
> >   CC       ui/browsers/hists.o
> > ui/browsers/hists.c: In function 'perf_evsel__hists_browse':
> > ui/browsers/hists.c:3078:61: error: '%d' directive output may be \
> > truncated writing between 1 and 11 bytes into a region of size \
> > between 2 and 12 [-Werror=format-truncation=]
> > 
> >  3078 |       "Max event group index to sort is %d (index from 0 to %d)",
> >       |                                                             ^~
> > ui/browsers/hists.c:3078:7: note: directive argument in the range [-2147483648, 8]
> >  3078 |       "Max event group index to sort is %d (index from 0 to %d)",
> >       |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > In file included from /usr/include/stdio.h:937,
> >                  from ui/browsers/hists.c:5:
> > 
> > IOW, the string in line 3078 might be too long for buf[] of 64 bytes.
> > 
> > Fix this by increasing the size of buf[] to 128.
> > 
> > Fixes: dbddf1747441  ("perf report/top TUI: Support hotkeys to let user select any event for sorting")
> > Cc: stable <stable@vger.kernel.org> # v5.7+
> > Cc: Jin Yao <yao.jin@linux.intel.com>
> > Cc: Jiri Olsa <jolsa@kernel.org>
> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
> > Signed-off-by: Song Liu <songliubraving@fb.com>
> 
> Acked-by: Jiri Olsa <jolsa@kernel.org>



Thanks, applied.

- Arnaldo