Re: [PATCH 0/3] perf report: Add support to print a textual representation of IBS raw sample data

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Fri, Sep 10, 2021 at 11:08:02AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Sep 10, 2021 at 10:47:16AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Sep 09, 2021 at 04:58:12PM -0500, Kim Phillips escreveu:
> > > Hi Arnaldo,
> > > 
> > > Can you please take a look at applying this series?  Its kernel-side
> > > dependent series has already been applied and is in Linus' master.
> > 
> > Sure, I'm now trying to fix this:
> > 
> >   CC      /tmp/build/perf/util/amd-sample-raw.o
> > util/amd-sample-raw.c: In function ‘evlist__amd_sample_raw’:
> > util/amd-sample-raw.c:125:42: error: ‘ bytes’ directive output may be truncated writing 6 bytes into a region of size between 4 and 7 [-Werror=format-truncation=]
> >   125 |                          " OpMemWidth %2d bytes", 1 << (reg.op_mem_width - 1));
> >       |                                          ^~~~~~
> > In file included from /usr/include/stdio.h:866,
> >                  from util/amd-sample-raw.c:7:
> > /usr/include/bits/stdio2.h:71:10: note: ‘__builtin___snprintf_chk’ output between 21 and 24 bytes into a destination of size 21
> >    71 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
> >       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    72 |                                    __glibc_objsize (__s), __fmt,
> >       |                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    73 |                                    __va_arg_pack ());
> >       |                                    ~~~~~~~~~~~~~~~~~
> > cc1: all warnings being treated as errors
> > make[4]: *** [/var/home/acme/git/perf/tools/build/Makefile.build:96: /tmp/build/perf/util/amd-sample-raw.o] Error 1
> 
> So, that trick with using sizeof and that string 3 times is cumbersome
> and prone to truncation, at least the compiler can't say that the number
> you're passing to %2d will have just 2 digits:
> 
> [acme@quaco c]$ cat printf.c
> #include <stdio.h>
> #include <stdlib.h>
> 
> int main(int argc, char *argv[])
> {
> 	char bf[64];
> 	int len = snprintf(bf, sizeof(bf), "%2d", atoi(argv[1]));
> 
> 	printf("strlen(%s): %u\n", bf, len);
> 
> 	return 0;
> }
> [acme@quaco c]$ ./printf 1234567
> strlen(1234567): 7
> [acme@quaco c]$
> 
> I'm trying to rework this.

So below is the minimal fix, the other cases the compiler somehow thinks
its ok, so I'll keep as is, will just remove the sizeof(string) to
sizeof(var), as I did below for the offending case.

- Arnaldo

diff --git a/tools/perf/util/amd-sample-raw.c b/tools/perf/util/amd-sample-raw.c
index fc1f670e88562b2e..e24025022735acdc 100644
--- a/tools/perf/util/amd-sample-raw.c
+++ b/tools/perf/util/amd-sample-raw.c
@@ -106,7 +106,7 @@ static void pr_ibs_op_data2(union ibs_op_data2 reg)
 static void pr_ibs_op_data3(union ibs_op_data3 reg)
 {
 	char l2_miss_str[sizeof(" L2Miss _")] = "";
-	char op_mem_width_str[sizeof(" OpMemWidth __ bytes")] = "";
+	char op_mem_width_str[sizeof(" OpMemWidth _____ bytes")] = "";
 	char op_dc_miss_open_mem_reqs_str[sizeof(" OpDcMissOpenMemReqs __")] = "";
 
 	/*
@@ -121,7 +121,7 @@ static void pr_ibs_op_data3(union ibs_op_data3 reg)
 	}
 
 	if (reg.op_mem_width)
-		snprintf(op_mem_width_str, sizeof(" OpMemWidth __ bytes"),
+		snprintf(op_mem_width_str, sizeof(op_mem_width_str),
 			 " OpMemWidth %2d bytes", 1 << (reg.op_mem_width - 1));
 
 	printf("ibs_op_data3:\t%016llx LdOp %d StOp %d DcL1TlbMiss %d DcL2TlbMiss %d "