LinuxLists.cc - [PATCH v1 0/6] Simplify C/C++ compiler flags

[permalink] [raw]

Subject: [PATCH v1 4/6] perf build: Disable fewer flex warnings

If flex is version 2.6.4, reduce the number of flex C warnings
disabled. Earlier flex versions have all C warnings disabled.

Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/Build | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 96f4ea1d45c5..32239c4b0393 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -279,13 +279,9 @@ $(OUTPUT)util/bpf-filter-bison.c $(OUTPUT)util/bpf-filter-bison.h: util/bpf-filt
$(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) $(BISON_FILE_PREFIX_MAP) \
-o $(OUTPUT)util/bpf-filter-bison.c -p perf_bpf_filter_

-FLEX_GE_26 := $(shell expr $(shell $(FLEX) --version | sed -e 's/flex $[0-9]\+$.$[0-9]\+$/\1\2/g') \>\= 26)
-ifeq ($(FLEX_GE_26),1)
- flex_flags := -Wno-switch-enum -Wno-switch-default -Wno-unused-function -Wno-redundant-decls -Wno-sign-compare -Wno-unused-parameter -Wno-missing-prototypes -Wno-missing-declarations
- CC_HASNT_MISLEADING_INDENTATION := $(shell echo "int main(void) { return 0 }" | $(CC) -Werror -Wno-misleading-indentation -o /dev/null -xc - 2>&1 | grep -q -- -Wno-misleading-indentation ; echo $$?)
- ifeq ($(CC_HASNT_MISLEADING_INDENTATION), 1)
- flex_flags += -Wno-misleading-indentation
- endif
+FLEX_GE_264 := $(shell expr $(shell $(FLEX) --version | sed -e 's/flex $[0-9]\+$.$[0-9]\+$.$[0-9]\+$/\1\2\3/g') \>\= 264)
+ifeq ($(FLEX_GE_264),1)
+ flex_flags := -Wno-redundant-decls -Wno-switch-default -Wno-unused-function
else
flex_flags := -w
endif
--
2.41.0.487.g6d72f3e995-goog

2023-07-28 07:45:39

[permalink] [raw]

Subject: [PATCH v1 5/6] perf build: Disable fewer bison warnings

If bison is version 3.8.2, reduce the number of bison C warnings
disabled. Earlier bison versions have all C warnings disabled. Avoid
implicit declarations of yylex by adding the declaration in the C
file. A header can't be included as a circular dependency would occur
due to the lexer using the bison defined tokens.

Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/Build | 6 +++---
tools/perf/util/bpf-filter.y | 2 ++
tools/perf/util/expr.y | 4 +++-
tools/perf/util/parse-events.y | 1 +
tools/perf/util/pmu.y | 3 +++
5 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 32239c4b0393..20aa8545b127 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -291,9 +291,9 @@ CFLAGS_expr-flex.o += $(flex_flags)
CFLAGS_bpf-filter-flex.o += $(flex_flags)

bison_flags := -DYYENABLE_NLS=0
-BISON_GE_35 := $(shell expr $(shell $(BISON) --version | grep bison | sed -e 's/.\+ $[0-9]\+$.$[0-9]\+$/\1\2/g') \>\= 35)
-ifeq ($(BISON_GE_35),1)
- bison_flags += -Wno-unused-parameter -Wno-nested-externs -Wno-implicit-function-declaration -Wno-switch-enum -Wno-unused-but-set-variable -Wno-unknown-warning-option
+BISON_GE_382 := $(shell expr $(shell $(BISON) --version | grep bison | sed -e 's/.\+ $[0-9]\+$.$[0-9]\+$.$[0-9]\+$/\1\2\3/g') \>\= 382)
+ifeq ($(BISON_GE_382),1)
+ bison_flags += -Wno-switch-enum
else
bison_flags += -w
endif
diff --git a/tools/perf/util/bpf-filter.y b/tools/perf/util/bpf-filter.y
index 07d6c7926c13..5dfa948fc986 100644
--- a/tools/perf/util/bpf-filter.y
+++ b/tools/perf/util/bpf-filter.y
@@ -9,6 +9,8 @@
#include <linux/list.h>
#include "bpf-filter.h"

+int perf_bpf_filter_lex(void);
+
static void perf_bpf_filter_error(struct list_head *expr __maybe_unused,
char const *msg)
{
diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
index dd504afd8f36..65d54a6f29ad 100644
--- a/tools/perf/util/expr.y
+++ b/tools/perf/util/expr.y
@@ -7,6 +7,8 @@
#include "util/debug.h"
#define IN_EXPR_Y 1
#include "expr.h"
+#include "expr-bison.h"
+int expr_lex(YYSTYPE * yylval_param , void *yyscanner);
%}

%define api.pure full
@@ -56,7 +58,7 @@
static void expr_error(double *final_val __maybe_unused,
struct expr_parse_ctx *ctx __maybe_unused,
bool compute_ids __maybe_unused,
- void *scanner,
+ void *scanner __maybe_unused,
const char *s)
{
pr_debug("%s\n", s);
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 454577f7aff6..251b7d2fde32 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -20,6 +20,7 @@
#include "parse-events.h"
#include "parse-events-bison.h"

+int parse_events_lex(YYSTYPE * yylval_param, YYLTYPE * yylloc_param , void *yyscanner);
void parse_events_error(YYLTYPE *loc, void *parse_state, void *scanner, char const *msg);

#define PE_ABORT(val) \
diff --git a/tools/perf/util/pmu.y b/tools/perf/util/pmu.y
index dff4e892ac4d..3d46cca3bb94 100644
--- a/tools/perf/util/pmu.y
+++ b/tools/perf/util/pmu.y
@@ -11,6 +11,9 @@
#include <linux/bitmap.h>
#include <string.h>
#include "pmu.h"
+#include "pmu-bison.h"
+
+int perf_pmu_lex(YYSTYPE * yylval_param , void *yyscanner);

#define ABORT_ON(val) \
do { \
--
2.41.0.487.g6d72f3e995-goog

2023-07-28 07:49:44

[permalink] [raw]

Subject: [PATCH v1 6/6] perf build: Remove -Wno-redundant-decls in 2 cases

Properly fix a warning and remove the -Wno-redundant-decls C flag.

Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/Build | 2 --
tools/perf/util/parse-events.c | 1 -
2 files changed, 3 deletions(-)

diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 20aa8545b127..b8f1e9ba8c6f 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -312,8 +312,6 @@ CFLAGS_find_bit.o += -Wno-unused-parameter -DETC_PERFCONFIG="BUILD_STR($(ET
CFLAGS_rbtree.o += -Wno-unused-parameter -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
CFLAGS_libstring.o += -Wno-unused-parameter -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
CFLAGS_hweight.o += -Wno-unused-parameter -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
-CFLAGS_parse-events.o += -Wno-redundant-decls
-CFLAGS_expr.o += -Wno-redundant-decls
CFLAGS_header.o += -include $(OUTPUT)PERF-VERSION-FILE
CFLAGS_arm-spe.o += -I$(srctree)/tools/arch/arm64/include/

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 926d3ac97324..ac315e1be2bc 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -35,7 +35,6 @@
#ifdef PARSER_DEBUG
extern int parse_events_debug;
#endif
-int parse_events_parse(void *parse_state, void *scanner);
static int get_config_terms(struct list_head *head_config,
struct list_head *head_terms __maybe_unused);

--
2.41.0.487.g6d72f3e995-goog

2023-07-28 07:55:53

[permalink] [raw]

Subject: [PATCH v1 2/6] perf build: Don't always set -funwind-tables and -ggdb3

Commit 6a40cd90f5de ("perf tools: Add libunwind dependency for DWARF
CFI unwinding") added libunwind support but also -funwind-tables and
-ggdb3 to the standard build. These build flags aren't necessary so
remove, set -g when DEBUG is enabled for the build.

Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/Makefile.config | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index a9cfe83638a9..14709a6bd622 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -246,6 +246,9 @@ ifeq ($(CC_NO_CLANG), 0)
else
CORE_CFLAGS += -O6
endif
+else
+ CORE_CFLAGS += -g
+ CXXFLAGS += -g
endif

ifdef PARSER_DEBUG
@@ -324,8 +327,6 @@ FEATURE_CHECK_LDFLAGS-disassembler-four-args = -lbfd -lopcodes -ldl
FEATURE_CHECK_LDFLAGS-disassembler-init-styled = -lbfd -lopcodes -ldl

CORE_CFLAGS += -fno-omit-frame-pointer
-CORE_CFLAGS += -ggdb3
-CORE_CFLAGS += -funwind-tables
CORE_CFLAGS += -Wall
CORE_CFLAGS += -Wextra
CORE_CFLAGS += -std=gnu11
@@ -333,8 +334,6 @@ CORE_CFLAGS += -std=gnu11
CXXFLAGS += -std=gnu++14 -fno-exceptions -fno-rtti
CXXFLAGS += -Wall
CXXFLAGS += -fno-omit-frame-pointer
-CXXFLAGS += -ggdb3
-CXXFLAGS += -funwind-tables
CXXFLAGS += -Wno-strict-aliasing

HOSTCFLAGS += -Wall
--
2.41.0.487.g6d72f3e995-goog

2023-07-28 07:59:50

[permalink] [raw]

Subject: [PATCH v1 1/6] perf bpf-loader: Remove unneeded diagnostic pragma

Added during the progress to libbpf 1.0 the deprecated functions are
no longer used and so the pragma can be removed.

Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/bpf-loader.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index 44cde27d6389..8f4c76f2265a 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -32,9 +32,6 @@

#include <internal/xyarray.h>

-/* temporarily disable libbpf deprecation warnings */
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-
static int libbpf_perf_print(enum libbpf_print_level level __attribute__((unused)),
const char *fmt, va_list args)
{
--
2.41.0.487.g6d72f3e995-goog

2023-07-28 08:04:43

[permalink] [raw]

Subject: [PATCH v1 3/6] perf build: Add Wextra for C++ compilation

Commit d58ac0bf8d1e ("perf build: Add clang and llvm compile and
linking support") added -Wall and -Wno-strict-aliasing for CXXFLAGS,
but not -Wextra. -Wno-strict-aliasing is no longer necessary, adding
-Wextra for CXXFLAGS requires adding -Wno-unused-parameter clang.cpp
and clang-test.cpp for LIBCLANGLLVM=1 to build.

Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/Makefile.config | 2 +-
tools/perf/util/c++/Build | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 14709a6bd622..fe7afe6d8529 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -333,8 +333,8 @@ CORE_CFLAGS += -std=gnu11

CXXFLAGS += -std=gnu++14 -fno-exceptions -fno-rtti
CXXFLAGS += -Wall
+CXXFLAGS += -Wextra
CXXFLAGS += -fno-omit-frame-pointer
-CXXFLAGS += -Wno-strict-aliasing

HOSTCFLAGS += -Wall
HOSTCFLAGS += -Wextra
diff --git a/tools/perf/util/c++/Build b/tools/perf/util/c++/Build
index 613ecfd76527..8610d032ac19 100644
--- a/tools/perf/util/c++/Build
+++ b/tools/perf/util/c++/Build
@@ -1,2 +1,5 @@
perf-$(CONFIG_CLANGLLVM) += clang.o
perf-$(CONFIG_CLANGLLVM) += clang-test.o
+
+CXXFLAGS_clang.o += -Wno-unused-parameter
+CXXFLAGS_clang-test.o += -Wno-unused-parameter
--
2.41.0.487.g6d72f3e995-goog

2023-07-28 09:15:09

[permalink] [raw]

Subject: Re: [PATCH v1 2/6] perf build: Don't always set -funwind-tables and -ggdb3

On 28/07/2023 07:49, Ian Rogers wrote:
> Commit 6a40cd90f5de ("perf tools: Add libunwind dependency for DWARF
> CFI unwinding") added libunwind support but also -funwind-tables and
> -ggdb3 to the standard build. These build flags aren't necessary so
> remove, set -g when DEBUG is enabled for the build.
>
> Signed-off-by: Ian Rogers <[email protected]>
> ---
> tools/perf/Makefile.config | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index a9cfe83638a9..14709a6bd622 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -246,6 +246,9 @@ ifeq ($(CC_NO_CLANG), 0)
> else
> CORE_CFLAGS += -O6
> endif
> +else
> + CORE_CFLAGS += -g
> + CXXFLAGS += -g
> endif
>
> ifdef PARSER_DEBUG
> @@ -324,8 +327,6 @@ FEATURE_CHECK_LDFLAGS-disassembler-four-args = -lbfd -lopcodes -ldl
> FEATURE_CHECK_LDFLAGS-disassembler-init-styled = -lbfd -lopcodes -ldl
>
> CORE_CFLAGS += -fno-omit-frame-pointer
> -CORE_CFLAGS += -ggdb3
> -CORE_CFLAGS += -funwind-tables
> CORE_CFLAGS += -Wall
> CORE_CFLAGS += -Wextra
> CORE_CFLAGS += -std=gnu11
> @@ -333,8 +334,6 @@ CORE_CFLAGS += -std=gnu11
> CXXFLAGS += -std=gnu++14 -fno-exceptions -fno-rtti
> CXXFLAGS += -Wall
> CXXFLAGS += -fno-omit-frame-pointer
> -CXXFLAGS += -ggdb3
> -CXXFLAGS += -funwind-tables
> CXXFLAGS += -Wno-strict-aliasing
>
> HOSTCFLAGS += -Wall

Acked-by: James Clark <[email protected]>

2023-07-28 09:15:10

[permalink] [raw]

Subject: Re: [PATCH v1 3/6] perf build: Add Wextra for C++ compilation

On 28/07/2023 07:49, Ian Rogers wrote:
> Commit d58ac0bf8d1e ("perf build: Add clang and llvm compile and
> linking support") added -Wall and -Wno-strict-aliasing for CXXFLAGS,
> but not -Wextra. -Wno-strict-aliasing is no longer necessary, adding
> -Wextra for CXXFLAGS requires adding -Wno-unused-parameter clang.cpp
> and clang-test.cpp for LIBCLANGLLVM=1 to build.
>
> Signed-off-by: Ian Rogers <[email protected]>
> ---
> tools/perf/Makefile.config | 2 +-
> tools/perf/util/c++/Build | 3 +++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index 14709a6bd622..fe7afe6d8529 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -333,8 +333,8 @@ CORE_CFLAGS += -std=gnu11
>
> CXXFLAGS += -std=gnu++14 -fno-exceptions -fno-rtti
> CXXFLAGS += -Wall
> +CXXFLAGS += -Wextra
> CXXFLAGS += -fno-omit-frame-pointer
> -CXXFLAGS += -Wno-strict-aliasing
>
> HOSTCFLAGS += -Wall
> HOSTCFLAGS += -Wextra
> diff --git a/tools/perf/util/c++/Build b/tools/perf/util/c++/Build
> index 613ecfd76527..8610d032ac19 100644
> --- a/tools/perf/util/c++/Build
> +++ b/tools/perf/util/c++/Build
> @@ -1,2 +1,5 @@
> perf-$(CONFIG_CLANGLLVM) += clang.o
> perf-$(CONFIG_CLANGLLVM) += clang-test.o
> +
> +CXXFLAGS_clang.o += -Wno-unused-parameter
> +CXXFLAGS_clang-test.o += -Wno-unused-parameter

Acked-by: James Clark <[email protected]>

2023-07-28 09:16:46

[permalink] [raw]

Subject: Re: [PATCH v1 4/6] perf build: Disable fewer flex warnings

On 28/07/2023 07:49, Ian Rogers wrote:
> If flex is version 2.6.4, reduce the number of flex C warnings
> disabled. Earlier flex versions have all C warnings disabled.

Hi Ian,

I get a build error with either this one or the bison warning change:

$ make LLVM=1 -C tools/perf NO_BPF_SKEL=1 DEBUG=1

util/pmu-bison.c:855:9: error: variable 'perf_pmu_nerrs' set but not
used [-Werror,-Wunused-but-set-variable]
int yynerrs = 0;

I tried a clean build which normally fixes these kind of bison errors.
Let me know if you need any version info.

James

>
> Signed-off-by: Ian Rogers <[email protected]>
> ---
> tools/perf/util/Build | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 96f4ea1d45c5..32239c4b0393 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -279,13 +279,9 @@ $(OUTPUT)util/bpf-filter-bison.c $(OUTPUT)util/bpf-filter-bison.h: util/bpf-filt
> $(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) $(BISON_FILE_PREFIX_MAP) \
> -o $(OUTPUT)util/bpf-filter-bison.c -p perf_bpf_filter_
>
> -FLEX_GE_26 := $(shell expr $(shell $(FLEX) --version | sed -e 's/flex $[0-9]\+$.$[0-9]\+$/\1\2/g') \>\= 26)
> -ifeq ($(FLEX_GE_26),1)
> - flex_flags := -Wno-switch-enum -Wno-switch-default -Wno-unused-function -Wno-redundant-decls -Wno-sign-compare -Wno-unused-parameter -Wno-missing-prototypes -Wno-missing-declarations
> - CC_HASNT_MISLEADING_INDENTATION := $(shell echo "int main(void) { return 0 }" | $(CC) -Werror -Wno-misleading-indentation -o /dev/null -xc - 2>&1 | grep -q -- -Wno-misleading-indentation ; echo $$?)
> - ifeq ($(CC_HASNT_MISLEADING_INDENTATION), 1)
> - flex_flags += -Wno-misleading-indentation
> - endif
> +FLEX_GE_264 := $(shell expr $(shell $(FLEX) --version | sed -e 's/flex $[0-9]\+$.$[0-9]\+$.$[0-9]\+$/\1\2\3/g') \>\= 264)
> +ifeq ($(FLEX_GE_264),1)
> + flex_flags := -Wno-redundant-decls -Wno-switch-default -Wno-unused-function
> else
> flex_flags := -w
> endif

2023-07-28 09:54:51

[permalink] [raw]

Subject: Re: [PATCH v1 1/6] perf bpf-loader: Remove unneeded diagnostic pragma

On 28/07/2023 07:49, Ian Rogers wrote:
> Added during the progress to libbpf 1.0 the deprecated functions are
> no longer used and so the pragma can be removed.
>
> Signed-off-by: Ian Rogers <[email protected]>
> ---
> tools/perf/util/bpf-loader.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> index 44cde27d6389..8f4c76f2265a 100644
> --- a/tools/perf/util/bpf-loader.c
> +++ b/tools/perf/util/bpf-loader.c
> @@ -32,9 +32,6 @@
>
> #include <internal/xyarray.h>
>
> -/* temporarily disable libbpf deprecation warnings */
> -#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> -
> static int libbpf_perf_print(enum libbpf_print_level level __attribute__((unused)),
> const char *fmt, va_list args)
> {

Acked-by: James Clark <[email protected]>

2023-07-28 14:30:13

[permalink] [raw]

Subject: Re: [PATCH v1 4/6] perf build: Disable fewer flex warnings

Em Fri, Jul 28, 2023 at 09:50:59AM +0100, James Clark escreveu:
>
>
> On 28/07/2023 07:49, Ian Rogers wrote:
> > If flex is version 2.6.4, reduce the number of flex C warnings
> > disabled. Earlier flex versions have all C warnings disabled.
>
> Hi Ian,
>
> I get a build error with either this one or the bison warning change:
>
> $ make LLVM=1 -C tools/perf NO_BPF_SKEL=1 DEBUG=1
>
> util/pmu-bison.c:855:9: error: variable 'perf_pmu_nerrs' set but not
> used [-Werror,-Wunused-but-set-variable]
> int yynerrs = 0;
>
> I tried a clean build which normally fixes these kind of bison errors.
> Let me know if you need any version info.

Trying to build it with the command line above I get:

CC util/expr.o
CC util/parse-events.o
CC util/parse-events-flex.o
util/parse-events-flex.c:7503:13: error: misleading indentation; statement is not part of the previous 'if' [-Werror,-Wmisleading-indentation]
if ( ! yyg->yy_state_buf )
^
util/parse-events-flex.c:7501:9: note: previous statement is here
if ( ! yyg->yy_state_buf )
^
1 error generated.
make[4]: *** [/var/home/acme/git/perf-tools-next/tools/build/Makefile.build:97: util/parse-events-flex.o] Error 1
make[4]: *** Waiting for unfinished jobs....
LD util/scripting-engines/perf-in.o
make[3]: *** [/var/home/acme/git/perf-tools-next/tools/build/Makefile.build:140: util] Error 2
make[2]: *** [Makefile.perf:682: perf-in.o] Error 2
make[2]: *** Waiting for unfinished jobs....
CC pmu-events/pmu-events.o
LD pmu-events/pmu-events-in.o
make[1]: *** [Makefile.perf:242: sub-make] Error 2
make: *** [Makefile:70: all] Error 2

⬢[acme@toolbox perf-tools-next]$ clang --version
clang version 14.0.5 (Fedora 14.0.5-2.fc36)
Target: x86_64-redhat-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
⬢[acme@toolbox perf-tools-next]$

2023-07-28 15:30:27

[permalink] [raw]

Subject: Re: [PATCH v1 4/6] perf build: Disable fewer flex warnings

Em Fri, Jul 28, 2023 at 10:59:38AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Jul 28, 2023 at 09:50:59AM +0100, James Clark escreveu:
> >
> >
> > On 28/07/2023 07:49, Ian Rogers wrote:
> > > If flex is version 2.6.4, reduce the number of flex C warnings
> > > disabled. Earlier flex versions have all C warnings disabled.
> >
> > Hi Ian,
> >
> > I get a build error with either this one or the bison warning change:
> >
> > $ make LLVM=1 -C tools/perf NO_BPF_SKEL=1 DEBUG=1
> >
> > util/pmu-bison.c:855:9: error: variable 'perf_pmu_nerrs' set but not
> > used [-Werror,-Wunused-but-set-variable]
> > int yynerrs = 0;
> >
> > I tried a clean build which normally fixes these kind of bison errors.
> > Let me know if you need any version info.
>
> Trying to build it with the command line above I get:
>
> CC util/expr.o
> CC util/parse-events.o
> CC util/parse-events-flex.o
> util/parse-events-flex.c:7503:13: error: misleading indentation; statement is not part of the previous 'if' [-Werror,-Wmisleading-indentation]
> if ( ! yyg->yy_state_buf )
> ^
> util/parse-events-flex.c:7501:9: note: previous statement is here
> if ( ! yyg->yy_state_buf )
> ^

I added this to the patch to get it moving:

make: Leaving directory '/var/home/acme/git/perf-tools-next/tools/perf'
⬢[acme@toolbox perf-tools-next]$ git diff
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 32239c4b0393c319..afa93eff495811cf 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -281,7 +281,7 @@ $(OUTPUT)util/bpf-filter-bison.c $(OUTPUT)util/bpf-filter-bison.h: util/bpf-filt

FLEX_GE_264 := $(shell expr $(shell $(FLEX) --version | sed -e 's/flex $[0-9]\+$.$[0-9]\+$.$[0-9]\+$/\1\2\3/g') \>\= 264)
ifeq ($(FLEX_GE_264),1)
- flex_flags := -Wno-redundant-decls -Wno-switch-default -Wno-unused-function
+ flex_flags := -Wno-redundant-decls -Wno-switch-default -Wno-unused-function -Wno-misleading-indentation
else
flex_flags := -w
endif
⬢[acme@toolbox perf-tools-next]$

> 1 error generated.
> make[4]: *** [/var/home/acme/git/perf-tools-next/tools/build/Makefile.build:97: util/parse-events-flex.o] Error 1
> make[4]: *** Waiting for unfinished jobs....
> LD util/scripting-engines/perf-in.o
> make[3]: *** [/var/home/acme/git/perf-tools-next/tools/build/Makefile.build:140: util] Error 2
> make[2]: *** [Makefile.perf:682: perf-in.o] Error 2
> make[2]: *** Waiting for unfinished jobs....
> CC pmu-events/pmu-events.o
> LD pmu-events/pmu-events-in.o
> make[1]: *** [Makefile.perf:242: sub-make] Error 2
> make: *** [Makefile:70: all] Error 2
>
> ⬢[acme@toolbox perf-tools-next]$ clang --version
> clang version 14.0.5 (Fedora 14.0.5-2.fc36)
> Target: x86_64-redhat-linux-gnu
> Thread model: posix
> InstalledDir: /usr/bin
> ⬢[acme@toolbox perf-tools-next]$

--

- Arnaldo

2023-07-28 15:53:32

[permalink] [raw]

Subject: Re: [PATCH v1 4/6] perf build: Disable fewer flex warnings

On Fri, Jul 28, 2023 at 7:02 AM Arnaldo Carvalho de Melo
<[email protected]> wrote:
>
> Em Fri, Jul 28, 2023 at 10:59:38AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Fri, Jul 28, 2023 at 09:50:59AM +0100, James Clark escreveu:
> > >
> > >
> > > On 28/07/2023 07:49, Ian Rogers wrote:
> > > > If flex is version 2.6.4, reduce the number of flex C warnings
> > > > disabled. Earlier flex versions have all C warnings disabled.
> > >
> > > Hi Ian,
> > >
> > > I get a build error with either this one or the bison warning change:
> > >
> > > $ make LLVM=1 -C tools/perf NO_BPF_SKEL=1 DEBUG=1
> > >
> > > util/pmu-bison.c:855:9: error: variable 'perf_pmu_nerrs' set but not
> > > used [-Werror,-Wunused-but-set-variable]
> > > int yynerrs = 0;
> > >
> > > I tried a clean build which normally fixes these kind of bison errors.
> > > Let me know if you need any version info.
> >
> > Trying to build it with the command line above I get:
> >
> > CC util/expr.o
> > CC util/parse-events.o
> > CC util/parse-events-flex.o
> > util/parse-events-flex.c:7503:13: error: misleading indentation; statement is not part of the previous 'if' [-Werror,-Wmisleading-indentation]
> > if ( ! yyg->yy_state_buf )
> > ^
> > util/parse-events-flex.c:7501:9: note: previous statement is here
> > if ( ! yyg->yy_state_buf )
> > ^
>
> I added this to the patch to get it moving:
>
> make: Leaving directory '/var/home/acme/git/perf-tools-next/tools/perf'
> ⬢[acme@toolbox perf-tools-next]$ git diff
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 32239c4b0393c319..afa93eff495811cf 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -281,7 +281,7 @@ $(OUTPUT)util/bpf-filter-bison.c $(OUTPUT)util/bpf-filter-bison.h: util/bpf-filt
>
> FLEX_GE_264 := $(shell expr $(shell $(FLEX) --version | sed -e 's/flex $[0-9]\+$.$[0-9]\+$.$[0-9]\+$/\1\2\3/g') \>\= 264)
> ifeq ($(FLEX_GE_264),1)
> - flex_flags := -Wno-redundant-decls -Wno-switch-default -Wno-unused-function
> + flex_flags := -Wno-redundant-decls -Wno-switch-default -Wno-unused-function -Wno-misleading-indentation
> else
> flex_flags := -w
> endif
> ⬢[acme@toolbox perf-tools-next]$
>
>
> > 1 error generated.
> > make[4]: *** [/var/home/acme/git/perf-tools-next/tools/build/Makefile.build:97: util/parse-events-flex.o] Error 1
> > make[4]: *** Waiting for unfinished jobs....
> > LD util/scripting-engines/perf-in.o
> > make[3]: *** [/var/home/acme/git/perf-tools-next/tools/build/Makefile.build:140: util] Error 2
> > make[2]: *** [Makefile.perf:682: perf-in.o] Error 2
> > make[2]: *** Waiting for unfinished jobs....
> > CC pmu-events/pmu-events.o
> > LD pmu-events/pmu-events-in.o
> > make[1]: *** [Makefile.perf:242: sub-make] Error 2
> > make: *** [Makefile:70: all] Error 2
> >
> > ⬢[acme@toolbox perf-tools-next]$ clang --version
> > clang version 14.0.5 (Fedora 14.0.5-2.fc36)
> > Target: x86_64-redhat-linux-gnu
> > Thread model: posix
> > InstalledDir: /usr/bin
> > ⬢[acme@toolbox perf-tools-next]$

Thanks James/Arnaldo, I was trying to be aggressive in having more
flags, but it seems too aggressive. We should probably bring back the
logic to make this flag only added if it is supported:
CC_HASNT_MISLEADING_INDENTATION := $(shell echo "int main(void) {
return 0 }" | $(CC) -Werror -Wno-misleading-indentation -o /dev/null
-xc - 2>&1 | grep -q -- -Wno-misleading-indentation ; echo $$?)
ifeq ($(CC_HASNT_MISLEADING_INDENTATION), 1)
flex_flags += -Wno-misleading-indentation
endif
Arnaldo, is the misleading indentation in the bison generated code or
something copy-pasted from the parse-events.l ? If the latter we may
be able to fix the .l file to keep the warning.

Thanks,
Ian

> --
>
> - Arnaldo

2023-07-28 18:40:55

[permalink] [raw]

Subject: Re: [PATCH v1 4/6] perf build: Disable fewer flex warnings

Em Fri, Jul 28, 2023 at 08:26:54AM -0700, Ian Rogers escreveu:
> On Fri, Jul 28, 2023 at 7:02 AM Arnaldo Carvalho de Melo
> <[email protected]> wrote:
> >
> > Em Fri, Jul 28, 2023 at 10:59:38AM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Fri, Jul 28, 2023 at 09:50:59AM +0100, James Clark escreveu:
> > > >
> > > >
> > > > On 28/07/2023 07:49, Ian Rogers wrote:
> > > > > If flex is version 2.6.4, reduce the number of flex C warnings
> > > > > disabled. Earlier flex versions have all C warnings disabled.
> > > >
> > > > Hi Ian,
> > > >
> > > > I get a build error with either this one or the bison warning change:
> > > >
> > > > $ make LLVM=1 -C tools/perf NO_BPF_SKEL=1 DEBUG=1
> > > >
> > > > util/pmu-bison.c:855:9: error: variable 'perf_pmu_nerrs' set but not
> > > > used [-Werror,-Wunused-but-set-variable]
> > > > int yynerrs = 0;
> > > >
> > > > I tried a clean build which normally fixes these kind of bison errors.
> > > > Let me know if you need any version info.
> > >
> > > Trying to build it with the command line above I get:
> > >
> > > CC util/expr.o
> > > CC util/parse-events.o
> > > CC util/parse-events-flex.o
> > > util/parse-events-flex.c:7503:13: error: misleading indentation; statement is not part of the previous 'if' [-Werror,-Wmisleading-indentation]
> > > if ( ! yyg->yy_state_buf )
> > > ^
> > > util/parse-events-flex.c:7501:9: note: previous statement is here
> > > if ( ! yyg->yy_state_buf )
> > > ^
> >
> > I added this to the patch to get it moving:
> >
> > make: Leaving directory '/var/home/acme/git/perf-tools-next/tools/perf'
> > ⬢[acme@toolbox perf-tools-next]$ git diff
> > diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> > index 32239c4b0393c319..afa93eff495811cf 100644
> > --- a/tools/perf/util/Build
> > +++ b/tools/perf/util/Build
> > @@ -281,7 +281,7 @@ $(OUTPUT)util/bpf-filter-bison.c $(OUTPUT)util/bpf-filter-bison.h: util/bpf-filt
> >
> > FLEX_GE_264 := $(shell expr $(shell $(FLEX) --version | sed -e 's/flex $[0-9]\+$.$[0-9]\+$.$[0-9]\+$/\1\2\3/g') \>\= 264)
> > ifeq ($(FLEX_GE_264),1)
> > - flex_flags := -Wno-redundant-decls -Wno-switch-default -Wno-unused-function
> > + flex_flags := -Wno-redundant-decls -Wno-switch-default -Wno-unused-function -Wno-misleading-indentation
> > else
> > flex_flags := -w
> > endif
> > ⬢[acme@toolbox perf-tools-next]$
> >
> >
> > > 1 error generated.
> > > make[4]: *** [/var/home/acme/git/perf-tools-next/tools/build/Makefile.build:97: util/parse-events-flex.o] Error 1
> > > make[4]: *** Waiting for unfinished jobs....
> > > LD util/scripting-engines/perf-in.o
> > > make[3]: *** [/var/home/acme/git/perf-tools-next/tools/build/Makefile.build:140: util] Error 2
> > > make[2]: *** [Makefile.perf:682: perf-in.o] Error 2
> > > make[2]: *** Waiting for unfinished jobs....
> > > CC pmu-events/pmu-events.o
> > > LD pmu-events/pmu-events-in.o
> > > make[1]: *** [Makefile.perf:242: sub-make] Error 2
> > > make: *** [Makefile:70: all] Error 2
> > >
> > > ⬢[acme@toolbox perf-tools-next]$ clang --version
> > > clang version 14.0.5 (Fedora 14.0.5-2.fc36)
> > > Target: x86_64-redhat-linux-gnu
> > > Thread model: posix
> > > InstalledDir: /usr/bin
> > > ⬢[acme@toolbox perf-tools-next]$
>
> Thanks James/Arnaldo, I was trying to be aggressive in having more
> flags, but it seems too aggressive. We should probably bring back the
> logic to make this flag only added if it is supported:
> CC_HASNT_MISLEADING_INDENTATION := $(shell echo "int main(void) {
> return 0 }" | $(CC) -Werror -Wno-misleading-indentation -o /dev/null
> -xc - 2>&1 | grep -q -- -Wno-misleading-indentation ; echo $$?)
> ifeq ($(CC_HASNT_MISLEADING_INDENTATION), 1)
> flex_flags += -Wno-misleading-indentation
> endif
> Arnaldo, is the misleading indentation in the bison generated code or
> something copy-pasted from the parse-events.l ? If the latter we may
> be able to fix the .l file to keep the warning.

I haven't checked, lemme do it now.

- Arnaldo

2023-07-28 19:16:43

[permalink] [raw]

Subject: Re: [PATCH v1 4/6] perf build: Disable fewer flex warnings

Em Fri, Jul 28, 2023 at 03:10:05PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Jul 28, 2023 at 08:26:54AM -0700, Ian Rogers escreveu:
> > On Fri, Jul 28, 2023 at 7:02 AM Arnaldo Carvalho de Melo
> > > I added this to the patch to get it moving:
> > >
> > > make: Leaving directory '/var/home/acme/git/perf-tools-next/tools/perf'
> > > ⬢[acme@toolbox perf-tools-next]$ git diff
> > > diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> > > index 32239c4b0393c319..afa93eff495811cf 100644
> > > --- a/tools/perf/util/Build
> > > +++ b/tools/perf/util/Build
> > > @@ -281,7 +281,7 @@ $(OUTPUT)util/bpf-filter-bison.c $(OUTPUT)util/bpf-filter-bison.h: util/bpf-filt
> > >
> > > FLEX_GE_264 := $(shell expr $(shell $(FLEX) --version | sed -e 's/flex $[0-9]\+$.$[0-9]\+$.$[0-9]\+$/\1\2\3/g') \>\= 264)
> > > ifeq ($(FLEX_GE_264),1)
> > > - flex_flags := -Wno-redundant-decls -Wno-switch-default -Wno-unused-function
> > > + flex_flags := -Wno-redundant-decls -Wno-switch-default -Wno-unused-function -Wno-misleading-indentation
> > > else
> > > flex_flags := -w
> > > endif
> > > ⬢[acme@toolbox perf-tools-next]$
> > >
> > >
> > > > 1 error generated.
> > > > make[4]: *** [/var/home/acme/git/perf-tools-next/tools/build/Makefile.build:97: util/parse-events-flex.o] Error 1
> > > > make[4]: *** Waiting for unfinished jobs....
> > > > LD util/scripting-engines/perf-in.o
> > > > make[3]: *** [/var/home/acme/git/perf-tools-next/tools/build/Makefile.build:140: util] Error 2
> > > > make[2]: *** [Makefile.perf:682: perf-in.o] Error 2
> > > > make[2]: *** Waiting for unfinished jobs....
> > > > CC pmu-events/pmu-events.o
> > > > LD pmu-events/pmu-events-in.o
> > > > make[1]: *** [Makefile.perf:242: sub-make] Error 2
> > > > make: *** [Makefile:70: all] Error 2
> > > >
> > > > ⬢[acme@toolbox perf-tools-next]$ clang --version
> > > > clang version 14.0.5 (Fedora 14.0.5-2.fc36)
> > > > Target: x86_64-redhat-linux-gnu
> > > > Thread model: posix
> > > > InstalledDir: /usr/bin
> > > > ⬢[acme@toolbox perf-tools-next]$
> >
> > Thanks James/Arnaldo, I was trying to be aggressive in having more
> > flags, but it seems too aggressive. We should probably bring back the
> > logic to make this flag only added if it is supported:
> > CC_HASNT_MISLEADING_INDENTATION := $(shell echo "int main(void) {
> > return 0 }" | $(CC) -Werror -Wno-misleading-indentation -o /dev/null
> > -xc - 2>&1 | grep -q -- -Wno-misleading-indentation ; echo $$?)
> > ifeq ($(CC_HASNT_MISLEADING_INDENTATION), 1)
> > flex_flags += -Wno-misleading-indentation
> > endif
> > Arnaldo, is the misleading indentation in the bison generated code or
> > something copy-pasted from the parse-events.l ? If the latter we may
> > be able to fix the .l file to keep the warning.
>
> I haven't checked, lemme do it now.

It comes directly from flex's m4 files:

https://github.com/westes/flex/blob/master/src/c99-flex.skl#L2044

So I'll keep the -Wno-misleading-indentation, ok?

- Arnaldo

2023-07-31 22:50:37

[permalink] [raw]

Subject: Re: [PATCH v1 4/6] perf build: Disable fewer flex warnings

Em Fri, Jul 28, 2023 at 12:05:56PM -0700, Ian Rogers escreveu:
> On Fri, Jul 28, 2023, 11:43 AM Arnaldo Carvalho de Melo <[email protected]>
> > > I haven't checked, lemme do it now.

> > It comes directly from flex's m4 files:

> > https://github.com/westes/flex/blob/master/src/c99-flex.skl#L2044

> > So I'll keep the -Wno-misleading-indentation, ok?

> Makes sense, yes.

continuing, changed the version check to:

commit f4da4419574536691c6b7843b6c48a3f97240404
Author: Ian Rogers <[email protected]>
Date: Thu Jul 27 23:49:15 2023 -0700

perf build: Disable fewer flex warnings

If flex is version 2.6.4, reduce the number of flex C warnings
disabled. Earlier flex versions have all C warnings disabled.

Committer notes:

Added this to the list of ignored warnings to get it building on
a Fedora 36 machine with flex 2.6.4:

-Wno-misleading-indentation

Noticed when building with:

$ make LLVM=1 -C tools/perf NO_BPF_SKEL=1 DEBUG=1

Take two:

We can't just try to canonicalize flex versions by just removing the
dots, as we end up with:

2.6.4 >= 2.5.37

becoming:

264 >= 2537

Failing the build on flex 2.5.37, so instead use the back to the past
added $(call version_ge3,2.6.4,$(FLEX_VERSION)) variant to check for
that.

Making sure $(FLEX_VERSION) keeps the dots as we may want to use 'sort
-V' or something nicer when available everywhere.

Signed-off-by: Ian Rogers <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Andrii Nakryiko <[email protected]>
Cc: Eduard Zingerman <[email protected]>
Cc: Gaosheng Cui <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Kan Liang <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Nathan Chancellor <[email protected]>
Cc: Nick Desaulniers <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Tom Rix <[email protected]>
Cc: [email protected]
Cc: [email protected]
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>

diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index bb08149179e405ac..ae91e2786f1a4f55 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -1,3 +1,5 @@
+include $(srctree)/tools/scripts/utilities.mak
+
perf-y += arm64-frame-pointer-unwind-support.o
perf-y += addr_location.o
perf-y += annotate.o
@@ -279,13 +281,11 @@ $(OUTPUT)util/bpf-filter-bison.c $(OUTPUT)util/bpf-filter-bison.h: util/bpf-filt
$(Q)$(call echo-cmd,bison)$(BISON) -v $< -d $(PARSER_DEBUG_BISON) $(BISON_FILE_PREFIX_MAP) \
-o $(OUTPUT)util/bpf-filter-bison.c -p perf_bpf_filter_

-FLEX_GE_26 := $(shell expr $(shell $(FLEX) --version | sed -e 's/flex $[0-9]\+$.$[0-9]\+$/\1\2/g') \>\= 26)
-ifeq ($(FLEX_GE_26),1)
- flex_flags := -Wno-switch-enum -Wno-switch-default -Wno-unused-function -Wno-redundant-decls -Wno-sign-compare -Wno-unused-parameter -Wno-missing-prototypes -Wno-missing-declarations
- CC_HASNT_MISLEADING_INDENTATION := $(shell echo "int main(void) { return 0 }" | $(CC) -Werror -Wno-misleading-indentation -o /dev/null -xc - 2>&1 | grep -q -- -Wno-misleading-indentation ; echo $$?)
- ifeq ($(CC_HASNT_MISLEADING_INDENTATION), 1)
- flex_flags += -Wno-misleading-indentation
- endif
+FLEX_VERSION := $(shell $(FLEX) --version | cut -d' ' -f2)
+
+FLEX_GE_264 := $(call version_ge3,2.6.4,$(FLEX_VERSION))
+ifeq ($(FLEX_GE_264),1)
+ flex_flags := -Wno-redundant-decls -Wno-switch-default -Wno-unused-function -Wno-misleading-indentation
else
flex_flags := -w
endif

--------------------------------------------------------------------------

with version_ge3 being:

commit aa9e655a4d755c3c75eb3d200d87a3b695f602cf
Author: Arnaldo Carvalho de Melo <[email protected]>
Date: Mon Jul 31 16:19:21 2023 -0300

tools build: Add a 3-component greater or equal version comparator

The next cset needs to compare if a flex version is greater or equal
than another, but since there is no canonical, generally available way
to compare versions in the command line (sort -V, yeah, but...), just
use awk to canonicalize the versions like is also done in
scripts/rust_is_available.sh.

Cc: Adrian Hunter <[email protected]>
Cc: Ian Rogers <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>

diff --git a/tools/scripts/utilities.mak b/tools/scripts/utilities.mak
index 172e47273b5d995a..568a6541ecf98075 100644
--- a/tools/scripts/utilities.mak
+++ b/tools/scripts/utilities.mak
@@ -177,3 +177,13 @@ $(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2)))
endef
_ge_attempt = $(or $(get-executable),$(call _gea_err,$(2)))
_gea_err = $(if $(1),$(error Please set '$(1)' appropriately))
+
+# version-ge3
+#
+# Usage $(call version_ge3,2.6.4,$(FLEX_VERSION))
+#
+# To compare if a 3 component version is greater or equal to anoter, first use
+# was to check the flex version to see if we can use compiler warnings as
+# errors for one of the cases flex generates code C compilers complains about.
+#
+version_ge3 = $(shell awk -F'.' '{ printf("%d\n", (10000000 * $$1 + 10000 * $$2 + $$3) >= (10000000 * $$4 + 10000 * $$5 + $$6)) }' <<< "$(1).$(2)")

2023-08-01 02:55:53