All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] perf tools: Fix arm64 build error with gcc-11
@ 2021-02-17 11:58 Jianlin Lv
  2021-02-17 12:55 ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 2+ messages in thread
From: Jianlin Lv @ 2021-02-17 11:58 UTC (permalink / raw)
  To: john.garry, will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers,
	agerstmayr, kan.liang, adrian.hunter
  Cc: Jianlin.Lv, iecedge, linux-kernel

gcc version: 11.0.0 20210208 (experimental) (GCC)

Following build error on arm64:

.......
In function ‘printf’,
    inlined from ‘regs_dump__printf’ at util/session.c:1141:3,
    inlined from ‘regs__printf’ at util/session.c:1169:2:
/usr/include/aarch64-linux-gnu/bits/stdio2.h:107:10: \
  error: ‘%-5s’ directive argument is null [-Werror=format-overflow=]

107 |   return __printf_chk (__USE_FORTIFY_LEVEL - 1, __fmt, \
                __va_arg_pack ());

......
In function ‘fprintf’,
  inlined from ‘perf_sample__fprintf_regs.isra’ at \
    builtin-script.c:622:14:
/usr/include/aarch64-linux-gnu/bits/stdio2.h:100:10: \
    error: ‘%5s’ directive argument is null [-Werror=format-overflow=]
  100 |   return __fprintf_chk (__stream, __USE_FORTIFY_LEVEL - 1, __fmt,
  101 |                         __va_arg_pack ());

cc1: all warnings being treated as errors
.......

This patch fixes Wformat-overflow warnings. Add helper function to
convert NULL to "unknown".

Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
---
v2: Add ternary operator to avoid similar errors in other arch.
v3: Declared reg_name in inner block.
v4: Add helper function: perf_reg_name_str, update changelog.
---
 tools/perf/builtin-script.c                           |  2 +-
 tools/perf/util/perf_regs.h                           | 11 ++++++++++-
 .../perf/util/scripting-engines/trace-event-python.c  |  2 +-
 tools/perf/util/session.c                             |  2 +-
 4 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 42dad4a0f8cf..35cddca2c7a7 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -643,7 +643,7 @@ static int perf_sample__fprintf_regs(struct regs_dump *regs, uint64_t mask,
 
 	for_each_set_bit(r, (unsigned long *) &mask, sizeof(mask) * 8) {
 		u64 val = regs->regs[i++];
-		printed += fprintf(fp, "%5s:0x%"PRIx64" ", perf_reg_name(r), val);
+		printed += fprintf(fp, "%5s:0x%"PRIx64" ", perf_reg_name_str(r), val);
 	}
 
 	return printed;
diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
index a45499126184..e4a0a6f5408e 100644
--- a/tools/perf/util/perf_regs.h
+++ b/tools/perf/util/perf_regs.h
@@ -33,13 +33,22 @@ extern const struct sample_reg sample_reg_masks[];
 
 int perf_reg_value(u64 *valp, struct regs_dump *regs, int id);
 
+static inline const char *perf_reg_name_str(int id)
+{
+	const char *str = perf_reg_name(id);
+
+	if (!str)
+		return "unknown";
+	return str;
+}
+
 #else
 #define PERF_REGS_MASK	0
 #define PERF_REGS_MAX	0
 
 #define DWARF_MINIMAL_REGS PERF_REGS_MASK
 
-static inline const char *perf_reg_name(int id __maybe_unused)
+static inline const char *perf_reg_name_str(int id __maybe_unused)
 {
 	return "unknown";
 }
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index c83c2c6564e0..361307026485 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -702,7 +702,7 @@ static int regs_map(struct regs_dump *regs, uint64_t mask, char *bf, int size)
 
 		printed += scnprintf(bf + printed, size - printed,
 				     "%5s:0x%" PRIx64 " ",
-				     perf_reg_name(r), val);
+				     perf_reg_name_str(r), val);
 	}
 
 	return printed;
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 25adbcce0281..0737d3e7e698 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1140,7 +1140,7 @@ static void regs_dump__printf(u64 mask, u64 *regs)
 		u64 val = regs[i++];
 
 		printf(".... %-5s 0x%016" PRIx64 "\n",
-		       perf_reg_name(rid), val);
+		       perf_reg_name_str(rid), val);
 	}
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread
* Re: [PATCH v4] perf tools: Fix arm64 build error with gcc-11
  2021-02-17 11:58 [PATCH v4] perf tools: Fix arm64 build error with gcc-11 Jianlin Lv
@ 2021-02-17 12:55 ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 2+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-02-17 12:55 UTC (permalink / raw)
  To: Jianlin Lv
  Cc: john.garry, will, mathieu.poirier, leo.yan, peterz, mingo,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers,
	agerstmayr, kan.liang, adrian.hunter, iecedge, linux-kernel

Em Wed, Feb 17, 2021 at 07:58:30PM +0800, Jianlin Lv escreveu:
> gcc version: 11.0.0 20210208 (experimental) (GCC)
> 
> Following build error on arm64:
> 
> .......
> In function ‘printf’,
>     inlined from ‘regs_dump__printf’ at util/session.c:1141:3,
>     inlined from ‘regs__printf’ at util/session.c:1169:2:
> /usr/include/aarch64-linux-gnu/bits/stdio2.h:107:10: \
>   error: ‘%-5s’ directive argument is null [-Werror=format-overflow=]
> 
> 107 |   return __printf_chk (__USE_FORTIFY_LEVEL - 1, __fmt, \
>                 __va_arg_pack ());
> 
> ......
> In function ‘fprintf’,
>   inlined from ‘perf_sample__fprintf_regs.isra’ at \
>     builtin-script.c:622:14:
> /usr/include/aarch64-linux-gnu/bits/stdio2.h:100:10: \
>     error: ‘%5s’ directive argument is null [-Werror=format-overflow=]
>   100 |   return __fprintf_chk (__stream, __USE_FORTIFY_LEVEL - 1, __fmt,
>   101 |                         __va_arg_pack ());
> 
> cc1: all warnings being treated as errors
> .......
> 
> This patch fixes Wformat-overflow warnings. Add helper function to
> convert NULL to "unknown".

I think this is the right approach, but since both return a string, it
is strange that only one of them have its _str() at the end, what we
usually do in such cases is to have:

  const char *__perf_reg_name(int id)
  {
  	return NULL if id unknown;
  }

  And:

  static inline const char *perf_reg_name(int id)
  {
  	const char *name = __perf_reg_name(id);
	return name ?: "unknown";
  }

Ok?

- Arnaldo
 
> Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
> ---
> v2: Add ternary operator to avoid similar errors in other arch.
> v3: Declared reg_name in inner block.
> v4: Add helper function: perf_reg_name_str, update changelog.
> ---
>  tools/perf/builtin-script.c                           |  2 +-
>  tools/perf/util/perf_regs.h                           | 11 ++++++++++-
>  .../perf/util/scripting-engines/trace-event-python.c  |  2 +-
>  tools/perf/util/session.c                             |  2 +-
>  4 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 42dad4a0f8cf..35cddca2c7a7 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -643,7 +643,7 @@ static int perf_sample__fprintf_regs(struct regs_dump *regs, uint64_t mask,
>  
>  	for_each_set_bit(r, (unsigned long *) &mask, sizeof(mask) * 8) {
>  		u64 val = regs->regs[i++];
> -		printed += fprintf(fp, "%5s:0x%"PRIx64" ", perf_reg_name(r), val);
> +		printed += fprintf(fp, "%5s:0x%"PRIx64" ", perf_reg_name_str(r), val);
>  	}
>  
>  	return printed;
> diff --git a/tools/perf/util/perf_regs.h b/tools/perf/util/perf_regs.h
> index a45499126184..e4a0a6f5408e 100644
> --- a/tools/perf/util/perf_regs.h
> +++ b/tools/perf/util/perf_regs.h
> @@ -33,13 +33,22 @@ extern const struct sample_reg sample_reg_masks[];
>  
>  int perf_reg_value(u64 *valp, struct regs_dump *regs, int id);
>  
> +static inline const char *perf_reg_name_str(int id)
> +{
> +	const char *str = perf_reg_name(id);
> +
> +	if (!str)
> +		return "unknown";
> +	return str;
> +}
> +
>  #else
>  #define PERF_REGS_MASK	0
>  #define PERF_REGS_MAX	0
>  
>  #define DWARF_MINIMAL_REGS PERF_REGS_MASK
>  
> -static inline const char *perf_reg_name(int id __maybe_unused)
> +static inline const char *perf_reg_name_str(int id __maybe_unused)
>  {
>  	return "unknown";
>  }
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index c83c2c6564e0..361307026485 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -702,7 +702,7 @@ static int regs_map(struct regs_dump *regs, uint64_t mask, char *bf, int size)
>  
>  		printed += scnprintf(bf + printed, size - printed,
>  				     "%5s:0x%" PRIx64 " ",
> -				     perf_reg_name(r), val);
> +				     perf_reg_name_str(r), val);
>  	}
>  
>  	return printed;
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 25adbcce0281..0737d3e7e698 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -1140,7 +1140,7 @@ static void regs_dump__printf(u64 mask, u64 *regs)
>  		u64 val = regs[i++];
>  
>  		printf(".... %-5s 0x%016" PRIx64 "\n",
> -		       perf_reg_name(rid), val);
> +		       perf_reg_name_str(rid), val);
>  	}
>  }
>  
> -- 
> 2.25.1
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-02-17 12:56 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-17 11:58 [PATCH v4] perf tools: Fix arm64 build error with gcc-11 Jianlin Lv
2021-02-17 12:55 ` Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.