LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Kim Phillips <kim.phillips@amd.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Ian Rogers <irogers@google.com>, Ingo Molnar <mingo@redhat.com>,
	Jiri Olsa <jolsa@redhat.com>,
	Joao Martins <joao.m.martins@oracle.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Michael Petlan <mpetlan@redhat.com>,
	Namhyung Kim <namhyung@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Robert Richter <robert.richter@amd.com>,
	Stephane Eranian <eranian@google.com>,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org
Subject: Re: [PATCH 0/3] perf report: Add support to print a textual representation of IBS raw sample data
Date: Fri, 10 Sep 2021 11:11:48 -0300	[thread overview]
Message-ID: <YTtnpAcaCNYozoJF@kernel.org> (raw)
In-Reply-To: <YTtmwrZ6iy0TzG3l@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 "

  reply	other threads:[~2021-09-10 14:11 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-17 22:15 Kim Phillips
2021-08-17 22:15 ` [PATCH 1/3] perf env: Add perf_env__cpuid, perf_env__{nr_}pmu_mappings Kim Phillips
2021-08-17 22:15 ` [PATCH 2/3] perf report: Add tools/arch/x86/include/asm/amd-ibs.h Kim Phillips
2021-08-17 22:15 ` [PATCH 3/3] perf report: Add support to print a textual representation of IBS raw sample data Kim Phillips
2021-09-09 21:58 ` [PATCH 0/3] " Kim Phillips
2021-09-10 13:47   ` Arnaldo Carvalho de Melo
2021-09-10 14:08     ` Arnaldo Carvalho de Melo
2021-09-10 14:11       ` Arnaldo Carvalho de Melo [this message]
2021-09-10 14:17         ` Arnaldo Carvalho de Melo
2021-09-10 19:48           ` Another bug: " Arnaldo Carvalho de Melo
2021-09-10 20:58             ` Arnaldo Carvalho de Melo
2021-09-10 21:40               ` Arnaldo Carvalho de Melo

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=YTtnpAcaCNYozoJF@kernel.org \
    --to=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=eranian@google.com \
    --cc=irogers@google.com \
    --cc=joao.m.martins@oracle.com \
    --cc=jolsa@redhat.com \
    --cc=kim.phillips@amd.com \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=mpetlan@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=robert.richter@amd.com \
    --subject='Re: [PATCH 0/3] perf report: Add support to print a textual representation of IBS raw sample data' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).