Currently jevents builds without any complier warning flags enabled. So
use newly-defined HOSTCFLAGS, which comes from EXTRA_WARNINGS.
Changes for v2:
- Add Werror, Wall, and Wextra (James Clark suggestion)
Baseline is acme perf/core + mainline commit b94729919db2 ("perf jevents:
Free the sys_event_tables list after processing entries"):
680453e63302 perf jevents: Free the sys_event_tables list after processing entries
be8ecc57f180 (origin/perf/core) perf srcline: Use long-running addr2line per DSO
2b775152bbe8 perf tests vmlinux-kallsyms: Ignore hidden symbols
John Garry (2):
perf jevents: Fix some would-be warnings
perf jevents: Enable warnings through HOSTCFLAGS
tools/perf/Makefile.config | 5 +++++
tools/perf/Makefile.perf | 2 +-
tools/perf/pmu-events/Build | 2 +-
tools/perf/pmu-events/jevents.c | 10 ++++------
4 files changed, 11 insertions(+), 8 deletions(-)
--
2.17.1
Currently no compiler warnings at all are enabled for building jevents,
so help catch bugs at compile time by enabling through HOSTCFLAGS.
Signed-off-by: John Garry <[email protected]>
---
tools/perf/Makefile.config | 5 +++++
tools/perf/Makefile.perf | 2 +-
tools/perf/pmu-events/Build | 2 +-
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 0ae2e3d8b832..374f65b52157 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -17,6 +17,7 @@ detected = $(shell echo "$(1)=y" >> $(OUTPUT).config-detected)
detected_var = $(shell echo "$(1)=$($(1))" >> $(OUTPUT).config-detected)
CFLAGS := $(EXTRA_CFLAGS) $(filter-out -Wnested-externs,$(EXTRA_WARNINGS))
+HOSTCFLAGS := $(filter-out -Wnested-externs,$(EXTRA_WARNINGS))
include $(srctree)/tools/scripts/Makefile.arch
@@ -211,6 +212,7 @@ endif
ifneq ($(WERROR),0)
CORE_CFLAGS += -Werror
CXXFLAGS += -Werror
+ HOSTCFLAGS += -Werror
endif
ifndef DEBUG
@@ -292,6 +294,9 @@ CXXFLAGS += -ggdb3
CXXFLAGS += -funwind-tables
CXXFLAGS += -Wno-strict-aliasing
+HOSTCFLAGS += -Wall
+HOSTCFLAGS += -Wextra
+
# Enforce a non-executable stack, as we may regress (again) in the future by
# adding assembler files missing the .GNU-stack linker note.
LDFLAGS += -Wl,-z,noexecstack
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 7df13e74450c..118bcdc70bb4 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -226,7 +226,7 @@ else
endif
export srctree OUTPUT RM CC CXX LD AR CFLAGS CXXFLAGS V BISON FLEX AWK
-export HOSTCC HOSTLD HOSTAR
+export HOSTCC HOSTLD HOSTAR HOSTCFLAGS
include $(srctree)/tools/build/Makefile.include
diff --git a/tools/perf/pmu-events/Build b/tools/perf/pmu-events/Build
index a055dee6a46a..d5c287f069a2 100644
--- a/tools/perf/pmu-events/Build
+++ b/tools/perf/pmu-events/Build
@@ -1,7 +1,7 @@
hostprogs := jevents
jevents-y += json.o jsmn.o jevents.o
-HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include
+HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include $(HOSTCFLAGS)
pmu-events-y += pmu-events.o
JDIR = pmu-events/arch/$(SRCARCH)
JSON = $(shell [ -d $(JDIR) ] && \
--
2.17.1
Before enabling warnings through HOSTCFLAGS, fix the would-be warnings:
HOSTCC pmu-events/jevents.o
pmu-events/jevents.c:74:22: warning: no previous prototype for ‘convert’ [-Wmissing-prototypes]
74 | enum aggr_mode_class convert(const char *aggr_mode)
| ^~~~~~~
pmu-events/jevents.c: In function ‘print_events_table_entry’:
pmu-events/jevents.c:373:8: warning: declaration of ‘topic’ shadows a global declaration [-Wshadow]
373 | char *topic = pd->topic;
| ^~~~~
pmu-events/jevents.c:316:14: note: shadowed declaration is here
316 | static char *topic;
| ^~~~~
pmu-events/jevents.c: In function ‘json_events’:
pmu-events/jevents.c:554:9: warning: declaration of ‘func’ shadows a global declaration [-Wshadow]
554 | int (*func)(void *data, struct json_event *je),
| ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pmu-events/jevents.c:85:15: note: shadowed declaration is here
85 | typedef int (*func)(void *data, struct json_event *je);
| ^~~~
pmu-events/jevents.c: In function ‘main’:
pmu-events/jevents.c:1211:25: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
1211 | char *err_string_ext = "";
| ^~
pmu-events/jevents.c:1304:17: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
1304 | err_string_ext = " for std arch event";
| ^
Signed-off-by: John Garry <[email protected]>
---
tools/perf/pmu-events/jevents.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index b7c8aae6bcf1..70eacc22dfef 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -71,7 +71,7 @@ struct json_event {
char *metric_constraint;
};
-enum aggr_mode_class convert(const char *aggr_mode)
+static enum aggr_mode_class convert(const char *aggr_mode)
{
if (!strcmp(aggr_mode, "PerCore"))
return PerCore;
@@ -82,8 +82,6 @@ enum aggr_mode_class convert(const char *aggr_mode)
return -1;
}
-typedef int (*func)(void *data, struct json_event *je);
-
static LIST_HEAD(sys_event_tables);
struct sys_event_table {
@@ -370,7 +368,7 @@ static int print_events_table_entry(void *data, struct json_event *je)
{
struct perf_entry_data *pd = data;
FILE *outfp = pd->outfp;
- char *topic = pd->topic;
+ char *topic_local = pd->topic;
/*
* TODO: Remove formatting chars after debugging to reduce
@@ -385,7 +383,7 @@ static int print_events_table_entry(void *data, struct json_event *je)
fprintf(outfp, "\t.desc = \"%s\",\n", je->desc);
if (je->compat)
fprintf(outfp, "\t.compat = \"%s\",\n", je->compat);
- fprintf(outfp, "\t.topic = \"%s\",\n", topic);
+ fprintf(outfp, "\t.topic = \"%s\",\n", topic_local);
if (je->long_desc && je->long_desc[0])
fprintf(outfp, "\t.long_desc = \"%s\",\n", je->long_desc);
if (je->pmu)
@@ -1208,7 +1206,7 @@ int main(int argc, char *argv[])
const char *arch;
const char *output_file;
const char *start_dirname;
- char *err_string_ext = "";
+ const char *err_string_ext = "";
struct stat stbuf;
prog = basename(argv[0]);
--
2.17.1
On Thu, Oct 21, 2021 at 05:16:45PM +0800, John Garry wrote:
> Currently no compiler warnings at all are enabled for building jevents,
> so help catch bugs at compile time by enabling through HOSTCFLAGS.
>
> Signed-off-by: John Garry <[email protected]>
> ---
> tools/perf/Makefile.config | 5 +++++
> tools/perf/Makefile.perf | 2 +-
> tools/perf/pmu-events/Build | 2 +-
> 3 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index 0ae2e3d8b832..374f65b52157 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -17,6 +17,7 @@ detected = $(shell echo "$(1)=y" >> $(OUTPUT).config-detected)
> detected_var = $(shell echo "$(1)=$($(1))" >> $(OUTPUT).config-detected)
>
> CFLAGS := $(EXTRA_CFLAGS) $(filter-out -Wnested-externs,$(EXTRA_WARNINGS))
> +HOSTCFLAGS := $(filter-out -Wnested-externs,$(EXTRA_WARNINGS))
>
> include $(srctree)/tools/scripts/Makefile.arch
>
> @@ -211,6 +212,7 @@ endif
> ifneq ($(WERROR),0)
> CORE_CFLAGS += -Werror
> CXXFLAGS += -Werror
> + HOSTCFLAGS += -Werror
> endif
>
> ifndef DEBUG
> @@ -292,6 +294,9 @@ CXXFLAGS += -ggdb3
> CXXFLAGS += -funwind-tables
> CXXFLAGS += -Wno-strict-aliasing
>
> +HOSTCFLAGS += -Wall
> +HOSTCFLAGS += -Wextra
> +
> # Enforce a non-executable stack, as we may regress (again) in the future by
> # adding assembler files missing the .GNU-stack linker note.
> LDFLAGS += -Wl,-z,noexecstack
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 7df13e74450c..118bcdc70bb4 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -226,7 +226,7 @@ else
> endif
>
> export srctree OUTPUT RM CC CXX LD AR CFLAGS CXXFLAGS V BISON FLEX AWK
> -export HOSTCC HOSTLD HOSTAR
> +export HOSTCC HOSTLD HOSTAR HOSTCFLAGS
>
> include $(srctree)/tools/build/Makefile.include
>
> diff --git a/tools/perf/pmu-events/Build b/tools/perf/pmu-events/Build
> index a055dee6a46a..d5c287f069a2 100644
> --- a/tools/perf/pmu-events/Build
> +++ b/tools/perf/pmu-events/Build
> @@ -1,7 +1,7 @@
> hostprogs := jevents
>
> jevents-y += json.o jsmn.o jevents.o
> -HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include
> +HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include $(HOSTCFLAGS)
so the the host cflags are made of:
host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
can't you use KBUILD_HOSTCFLAGS?
also perhaps we could rename KBUILD_HOSTCFLAGS to HOSTCFLAGS?
the name seems like leftover from kbuild move
jirka
> pmu-events-y += pmu-events.o
> JDIR = pmu-events/arch/$(SRCARCH)
> JSON = $(shell [ -d $(JDIR) ] && \
> --
> 2.17.1
>
Em Thu, Oct 21, 2021 at 05:16:43PM +0800, John Garry escreveu:
> Currently jevents builds without any complier warning flags enabled. So
> use newly-defined HOSTCFLAGS, which comes from EXTRA_WARNINGS.
>
> Changes for v2:
> - Add Werror, Wall, and Wextra (James Clark suggestion)
Thanks, applied.
- Arnaldo
> Baseline is acme perf/core + mainline commit b94729919db2 ("perf jevents:
> Free the sys_event_tables list after processing entries"):
>
> 680453e63302 perf jevents: Free the sys_event_tables list after processing entries
> be8ecc57f180 (origin/perf/core) perf srcline: Use long-running addr2line per DSO
> 2b775152bbe8 perf tests vmlinux-kallsyms: Ignore hidden symbols
>
> John Garry (2):
> perf jevents: Fix some would-be warnings
> perf jevents: Enable warnings through HOSTCFLAGS
>
> tools/perf/Makefile.config | 5 +++++
> tools/perf/Makefile.perf | 2 +-
> tools/perf/pmu-events/Build | 2 +-
> tools/perf/pmu-events/jevents.c | 10 ++++------
> 4 files changed, 11 insertions(+), 8 deletions(-)
>
> --
> 2.17.1
--
- Arnaldo
On 21/10/2021 13:55, Arnaldo Carvalho de Melo wrote:
> Em Thu, Oct 21, 2021 at 05:16:43PM +0800, John Garry escreveu:
>> Currently jevents builds without any complier warning flags enabled. So
>> use newly-defined HOSTCFLAGS, which comes from EXTRA_WARNINGS.
>>
>> Changes for v2:
>> - Add Werror, Wall, and Wextra (James Clark suggestion)
>
> Thanks, applied.
>
Hi Arnaldo,
Can you please wait until I check the review response from jirka?
Thanks
> - Arnaldo
>
>
>> Baseline is acme perf/core + mainline commit b94729919db2 ("perf jevents:
>> Free the sys_event_tables list after processing entries"):
>>
>> 680453e63302 perf jevents: Free the sys_event_tables list after processing entries
>> be8ecc57f180 (origin/perf/core) perf srcline: Use long-running addr2line per DSO
>> 2b775152bbe8 perf tests vmlinux-kallsyms: Ignore hidden symbols
>>
>> John Garry (2):
>> perf jevents: Fix some would-be warnings
>> perf jevents: Enable warnings through HOSTCFLAGS
>>
>> tools/perf/Makefile.config | 5 +++++
>> tools/perf/Makefile.perf | 2 +-
>> tools/perf/pmu-events/Build | 2 +-
>> tools/perf/pmu-events/jevents.c | 10 ++++------
>> 4 files changed, 11 insertions(+), 8 deletions(-)
>>
>> --
>> 2.17.1
>
On October 21, 2021 10:02:41 AM GMT-03:00, John Garry <[email protected]> wrote:
>On 21/10/2021 13:55, Arnaldo Carvalho de Melo wrote:
>> Em Thu, Oct 21, 2021 at 05:16:43PM +0800, John Garry escreveu:
>>> Currently jevents builds without any complier warning flags enabled. So
>>> use newly-defined HOSTCFLAGS, which comes from EXTRA_WARNINGS.
>>>
>>> Changes for v2:
>>> - Add Werror, Wall, and Wextra (James Clark suggestion)
>>
>> Thanks, applied.
>>
>
>Hi Arnaldo,
>
>Can you please wait until I check the review response from jirka?
Sure, it's not published, and will not be until some fixes are made. Having it in my local tree gets it tested together with the whole shebang in lots of containers.
I collect reviewed-by, etc as they appear.
- Arnaldo
>>
>>
>>> Baseline is acme perf/core + mainline commit b94729919db2 ("perf jevents:
>>> Free the sys_event_tables list after processing entries"):
>>>
>>> 680453e63302 perf jevents: Free the sys_event_tables list after processing entries
>>> be8ecc57f180 (origin/perf/core) perf srcline: Use long-running addr2line per DSO
>>> 2b775152bbe8 perf tests vmlinux-kallsyms: Ignore hidden symbols
>>>
>>> John Garry (2):
>>> perf jevents: Fix some would-be warnings
>>> perf jevents: Enable warnings through HOSTCFLAGS
>>>
>>> tools/perf/Makefile.config | 5 +++++
>>> tools/perf/Makefile.perf | 2 +-
>>> tools/perf/pmu-events/Build | 2 +-
>>> tools/perf/pmu-events/jevents.c | 10 ++++------
>>> 4 files changed, 11 insertions(+), 8 deletions(-)
>>>
>>> --
>>> 2.17.1
>>
>
On 21/10/2021 13:48, Jiri Olsa wrote:
>> +HOSTCFLAGS += -Wall
>> +HOSTCFLAGS += -Wextra
>> +
>> # Enforce a non-executable stack, as we may regress (again) in the future by
>> # adding assembler files missing the .GNU-stack linker note.
>> LDFLAGS += -Wl,-z,noexecstack
>> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
>> index 7df13e74450c..118bcdc70bb4 100644
>> --- a/tools/perf/Makefile.perf
>> +++ b/tools/perf/Makefile.perf
>> @@ -226,7 +226,7 @@ else
>> endif
>>
>> export srctree OUTPUT RM CC CXX LD AR CFLAGS CXXFLAGS V BISON FLEX AWK
>> -export HOSTCC HOSTLD HOSTAR
>> +export HOSTCC HOSTLD HOSTAR HOSTCFLAGS
>>
>> include $(srctree)/tools/build/Makefile.include
>>
>> diff --git a/tools/perf/pmu-events/Build b/tools/perf/pmu-events/Build
>> index a055dee6a46a..d5c287f069a2 100644
>> --- a/tools/perf/pmu-events/Build
>> +++ b/tools/perf/pmu-events/Build
>> @@ -1,7 +1,7 @@
>> hostprogs := jevents
>>
>> jevents-y += json.o jsmn.o jevents.o
>> -HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include
>> +HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include $(HOSTCFLAGS)
> so the the host cflags are made of:
>
> host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
>
> can't you use KBUILD_HOSTCFLAGS?
Maybe. However, it seems to be empty when building perf/pmu-events. Even
in building tools/objtool - which currently references it - it is empty
AFAICS. I'm not sure if it's being imported properly.
Thanks,
John
>
> also perhaps we could rename KBUILD_HOSTCFLAGS to HOSTCFLAGS?
> the name seems like leftover from kbuild move
On Thu, Oct 21, 2021 at 04:50:09PM +0100, John Garry wrote:
> On 21/10/2021 13:48, Jiri Olsa wrote:
> > > +HOSTCFLAGS += -Wall
> > > +HOSTCFLAGS += -Wextra
> > > +
> > > # Enforce a non-executable stack, as we may regress (again) in the future by
> > > # adding assembler files missing the .GNU-stack linker note.
> > > LDFLAGS += -Wl,-z,noexecstack
> > > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > > index 7df13e74450c..118bcdc70bb4 100644
> > > --- a/tools/perf/Makefile.perf
> > > +++ b/tools/perf/Makefile.perf
> > > @@ -226,7 +226,7 @@ else
> > > endif
> > > export srctree OUTPUT RM CC CXX LD AR CFLAGS CXXFLAGS V BISON FLEX AWK
> > > -export HOSTCC HOSTLD HOSTAR
> > > +export HOSTCC HOSTLD HOSTAR HOSTCFLAGS
> > > include $(srctree)/tools/build/Makefile.include
> > > diff --git a/tools/perf/pmu-events/Build b/tools/perf/pmu-events/Build
> > > index a055dee6a46a..d5c287f069a2 100644
> > > --- a/tools/perf/pmu-events/Build
> > > +++ b/tools/perf/pmu-events/Build
> > > @@ -1,7 +1,7 @@
> > > hostprogs := jevents
> > > jevents-y += json.o jsmn.o jevents.o
> > > -HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include
> > > +HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include $(HOSTCFLAGS)
> > so the the host cflags are made of:
> >
> > host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
> >
> > can't you use KBUILD_HOSTCFLAGS?
>
> Maybe. However, it seems to be empty when building perf/pmu-events. Even in
> building tools/objtool - which currently references it - it is empty AFAICS.
> I'm not sure if it's being imported properly.
>
I meant change like this (on top of yours)
jirka
---
diff --git a/tools/build/Build.include b/tools/build/Build.include
index 2cf3b1bde86e..c2a95ab47379 100644
--- a/tools/build/Build.include
+++ b/tools/build/Build.include
@@ -99,7 +99,7 @@ cxx_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(CXXFLAGS) -D"BUILD_STR(s)=\#s" $(CXX
###
## HOSTCC C flags
-host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
+host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
# output directory for tests below
TMPOUT = .tmp_$$$$
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 0ae2e3d8b832..374f65b52157 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -17,6 +17,7 @@ detected = $(shell echo "$(1)=y" >> $(OUTPUT).config-detected)
detected_var = $(shell echo "$(1)=$($(1))" >> $(OUTPUT).config-detected)
CFLAGS := $(EXTRA_CFLAGS) $(filter-out -Wnested-externs,$(EXTRA_WARNINGS))
+HOSTCFLAGS := $(filter-out -Wnested-externs,$(EXTRA_WARNINGS))
include $(srctree)/tools/scripts/Makefile.arch
@@ -211,6 +212,7 @@ endif
ifneq ($(WERROR),0)
CORE_CFLAGS += -Werror
CXXFLAGS += -Werror
+ HOSTCFLAGS += -Werror
endif
ifndef DEBUG
@@ -292,6 +294,9 @@ CXXFLAGS += -ggdb3
CXXFLAGS += -funwind-tables
CXXFLAGS += -Wno-strict-aliasing
+HOSTCFLAGS += -Wall
+HOSTCFLAGS += -Wextra
+
# Enforce a non-executable stack, as we may regress (again) in the future by
# adding assembler files missing the .GNU-stack linker note.
LDFLAGS += -Wl,-z,noexecstack
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 7df13e74450c..118bcdc70bb4 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -226,7 +226,7 @@ else
endif
export srctree OUTPUT RM CC CXX LD AR CFLAGS CXXFLAGS V BISON FLEX AWK
-export HOSTCC HOSTLD HOSTAR
+export HOSTCC HOSTLD HOSTAR HOSTCFLAGS
include $(srctree)/tools/build/Makefile.include
On 21/10/2021 13:48, Jiri Olsa wrote:
>> +HOSTCFLAGS += -Wall
>> +HOSTCFLAGS += -Wextra
>> +
>> # Enforce a non-executable stack, as we may regress (again) in the future by
>> # adding assembler files missing the .GNU-stack linker note.
>> LDFLAGS += -Wl,-z,noexecstack
>> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
>> index 7df13e74450c..118bcdc70bb4 100644
>> --- a/tools/perf/Makefile.perf
>> +++ b/tools/perf/Makefile.perf
>> @@ -226,7 +226,7 @@ else
>> endif
>>
>> export srctree OUTPUT RM CC CXX LD AR CFLAGS CXXFLAGS V BISON FLEX AWK
>> -export HOSTCC HOSTLD HOSTAR
>> +export HOSTCC HOSTLD HOSTAR HOSTCFLAGS
>>
>> include $(srctree)/tools/build/Makefile.include
>>
>> diff --git a/tools/perf/pmu-events/Build b/tools/perf/pmu-events/Build
>> index a055dee6a46a..d5c287f069a2 100644
>> --- a/tools/perf/pmu-events/Build
>> +++ b/tools/perf/pmu-events/Build
>> @@ -1,7 +1,7 @@
>> hostprogs := jevents
>>
>> jevents-y += json.o jsmn.o jevents.o
>> -HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include
>> +HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include $(HOSTCFLAGS)
> so the the host cflags are made of:
>
> host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
>
ok, so IIRC, then the rule for building .o from .c in
tools/build/Makefile.build will pick up HOSTCFLAGS through this
variable, so we then don't need to explicitly mention it in the
per-target rule, so can have this as before in pmu-events/Build
HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include
right?
(Indeed I guess that we can get rid of -I$(srctree)/tools/include as well)
Thanks,
John
> can't you use KBUILD_HOSTCFLAGS?
>
> also perhaps we could rename KBUILD_HOSTCFLAGS to HOSTCFLAGS?
> the name seems like leftover from kbuild move
>
> jirka
>
>> pmu-events-y += pmu-events.o
>> JDIR = pmu-events/arch/$(SRCARCH)
>> JSON = $(shell [ -d $(JDIR) ] &&
On Fri, Oct 22, 2021 at 10:42:11AM +0100, John Garry wrote:
> On 21/10/2021 13:48, Jiri Olsa wrote:
> > > +HOSTCFLAGS += -Wall
> > > +HOSTCFLAGS += -Wextra
> > > +
> > > # Enforce a non-executable stack, as we may regress (again) in the future by
> > > # adding assembler files missing the .GNU-stack linker note.
> > > LDFLAGS += -Wl,-z,noexecstack
> > > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > > index 7df13e74450c..118bcdc70bb4 100644
> > > --- a/tools/perf/Makefile.perf
> > > +++ b/tools/perf/Makefile.perf
> > > @@ -226,7 +226,7 @@ else
> > > endif
> > > export srctree OUTPUT RM CC CXX LD AR CFLAGS CXXFLAGS V BISON FLEX AWK
> > > -export HOSTCC HOSTLD HOSTAR
> > > +export HOSTCC HOSTLD HOSTAR HOSTCFLAGS
> > > include $(srctree)/tools/build/Makefile.include
> > > diff --git a/tools/perf/pmu-events/Build b/tools/perf/pmu-events/Build
> > > index a055dee6a46a..d5c287f069a2 100644
> > > --- a/tools/perf/pmu-events/Build
> > > +++ b/tools/perf/pmu-events/Build
> > > @@ -1,7 +1,7 @@
> > > hostprogs := jevents
> > > jevents-y += json.o jsmn.o jevents.o
> > > -HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include
> > > +HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include $(HOSTCFLAGS)
> > so the the host cflags are made of:
> >
> > host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
> >
>
> ok, so IIRC, then the rule for building .o from .c in
> tools/build/Makefile.build will pick up HOSTCFLAGS through this variable, so
> we then don't need to explicitly mention it in the per-target rule, so can
> have this as before in pmu-events/Build
>
> HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include
>
> right?
>
> (Indeed I guess that we can get rid of -I$(srctree)/tools/include as well)
hm, the -I.. should stay no? I don't see that
it's being added soem other way
jirka
---
diff --git a/tools/build/Build.include b/tools/build/Build.include
index 2cf3b1bde86e..c2a95ab47379 100644
--- a/tools/build/Build.include
+++ b/tools/build/Build.include
@@ -99,7 +99,7 @@ cxx_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(CXXFLAGS) -D"BUILD_STR(s)=\#s" $(CXX
###
## HOSTCC C flags
-host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
+host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
# output directory for tests below
TMPOUT = .tmp_$$$$
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 0ae2e3d8b832..374f65b52157 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -17,6 +17,7 @@ detected = $(shell echo "$(1)=y" >> $(OUTPUT).config-detected)
detected_var = $(shell echo "$(1)=$($(1))" >> $(OUTPUT).config-detected)
CFLAGS := $(EXTRA_CFLAGS) $(filter-out -Wnested-externs,$(EXTRA_WARNINGS))
+HOSTCFLAGS := $(filter-out -Wnested-externs,$(EXTRA_WARNINGS))
include $(srctree)/tools/scripts/Makefile.arch
@@ -211,6 +212,7 @@ endif
ifneq ($(WERROR),0)
CORE_CFLAGS += -Werror
CXXFLAGS += -Werror
+ HOSTCFLAGS += -Werror
endif
ifndef DEBUG
@@ -292,6 +294,9 @@ CXXFLAGS += -ggdb3
CXXFLAGS += -funwind-tables
CXXFLAGS += -Wno-strict-aliasing
+HOSTCFLAGS += -Wall
+HOSTCFLAGS += -Wextra
+
# Enforce a non-executable stack, as we may regress (again) in the future by
# adding assembler files missing the .GNU-stack linker note.
LDFLAGS += -Wl,-z,noexecstack
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 7df13e74450c..118bcdc70bb4 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -226,7 +226,7 @@ else
endif
export srctree OUTPUT RM CC CXX LD AR CFLAGS CXXFLAGS V BISON FLEX AWK
-export HOSTCC HOSTLD HOSTAR
+export HOSTCC HOSTLD HOSTAR HOSTCFLAGS
include $(srctree)/tools/build/Makefile.include
Em Mon, Oct 25, 2021 at 01:40:44PM +0200, Jiri Olsa escreveu:
> On Fri, Oct 22, 2021 at 10:42:11AM +0100, John Garry wrote:
> > On 21/10/2021 13:48, Jiri Olsa wrote:
> > > > +HOSTCFLAGS += -Wall
> > > > +HOSTCFLAGS += -Wextra
> > > > +
> > > > # Enforce a non-executable stack, as we may regress (again) in the future by
> > > > # adding assembler files missing the .GNU-stack linker note.
> > > > LDFLAGS += -Wl,-z,noexecstack
> > > > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > > > index 7df13e74450c..118bcdc70bb4 100644
> > > > --- a/tools/perf/Makefile.perf
> > > > +++ b/tools/perf/Makefile.perf
> > > > @@ -226,7 +226,7 @@ else
> > > > endif
> > > > export srctree OUTPUT RM CC CXX LD AR CFLAGS CXXFLAGS V BISON FLEX AWK
> > > > -export HOSTCC HOSTLD HOSTAR
> > > > +export HOSTCC HOSTLD HOSTAR HOSTCFLAGS
> > > > include $(srctree)/tools/build/Makefile.include
> > > > diff --git a/tools/perf/pmu-events/Build b/tools/perf/pmu-events/Build
> > > > index a055dee6a46a..d5c287f069a2 100644
> > > > --- a/tools/perf/pmu-events/Build
> > > > +++ b/tools/perf/pmu-events/Build
> > > > @@ -1,7 +1,7 @@
> > > > hostprogs := jevents
> > > > jevents-y += json.o jsmn.o jevents.o
> > > > -HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include
> > > > +HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include $(HOSTCFLAGS)
> > > so the the host cflags are made of:
> > >
> > > host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
> > >
> >
> > ok, so IIRC, then the rule for building .o from .c in
> > tools/build/Makefile.build will pick up HOSTCFLAGS through this variable, so
> > we then don't need to explicitly mention it in the per-target rule, so can
> > have this as before in pmu-events/Build
> >
> > HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include
> >
> > right?
> >
> > (Indeed I guess that we can get rid of -I$(srctree)/tools/include as well)
>
> hm, the -I.. should stay no? I don't see that
> it's being added soem other way
>
> jirka
>
Probably this change from KBUILD_HOSTCFLAGS back to HOSTCFLAGS should
come with this;
Cc: Laura Abbott <[email protected]>
Cc: Masahiro Yamada <[email protected]>
Fixes: 96f14fe738b69dd9 ("kbuild: Rename HOSTCFLAGS to KBUILD_HOSTCFLAGS")
Right?
- Arnaldo
> ---
> diff --git a/tools/build/Build.include b/tools/build/Build.include
> index 2cf3b1bde86e..c2a95ab47379 100644
> --- a/tools/build/Build.include
> +++ b/tools/build/Build.include
> @@ -99,7 +99,7 @@ cxx_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(CXXFLAGS) -D"BUILD_STR(s)=\#s" $(CXX
> ###
> ## HOSTCC C flags
>
> -host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
> +host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
>
> # output directory for tests below
> TMPOUT = .tmp_$$$$
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index 0ae2e3d8b832..374f65b52157 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -17,6 +17,7 @@ detected = $(shell echo "$(1)=y" >> $(OUTPUT).config-detected)
> detected_var = $(shell echo "$(1)=$($(1))" >> $(OUTPUT).config-detected)
>
> CFLAGS := $(EXTRA_CFLAGS) $(filter-out -Wnested-externs,$(EXTRA_WARNINGS))
> +HOSTCFLAGS := $(filter-out -Wnested-externs,$(EXTRA_WARNINGS))
>
> include $(srctree)/tools/scripts/Makefile.arch
>
> @@ -211,6 +212,7 @@ endif
> ifneq ($(WERROR),0)
> CORE_CFLAGS += -Werror
> CXXFLAGS += -Werror
> + HOSTCFLAGS += -Werror
> endif
>
> ifndef DEBUG
> @@ -292,6 +294,9 @@ CXXFLAGS += -ggdb3
> CXXFLAGS += -funwind-tables
> CXXFLAGS += -Wno-strict-aliasing
>
> +HOSTCFLAGS += -Wall
> +HOSTCFLAGS += -Wextra
> +
> # Enforce a non-executable stack, as we may regress (again) in the future by
> # adding assembler files missing the .GNU-stack linker note.
> LDFLAGS += -Wl,-z,noexecstack
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 7df13e74450c..118bcdc70bb4 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -226,7 +226,7 @@ else
> endif
>
> export srctree OUTPUT RM CC CXX LD AR CFLAGS CXXFLAGS V BISON FLEX AWK
> -export HOSTCC HOSTLD HOSTAR
> +export HOSTCC HOSTLD HOSTAR HOSTCFLAGS
>
> include $(srctree)/tools/build/Makefile.include
>
--
- Arnaldo
On 25/10/2021 17:20, Arnaldo Carvalho de Melo wrote:
> Em Mon, Oct 25, 2021 at 01:40:44PM +0200, Jiri Olsa escreveu:
>> On Fri, Oct 22, 2021 at 10:42:11AM +0100, John Garry wrote:
>>> On 21/10/2021 13:48, Jiri Olsa wrote:
>>>>> +HOSTCFLAGS += -Wall
>>>>> +HOSTCFLAGS += -Wextra
>>>>> +
>>>>> # Enforce a non-executable stack, as we may regress (again) in the future by
>>>>> # adding assembler files missing the .GNU-stack linker note.
>>>>> LDFLAGS += -Wl,-z,noexecstack
>>>>> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
>>>>> index 7df13e74450c..118bcdc70bb4 100644
>>>>> --- a/tools/perf/Makefile.perf
>>>>> +++ b/tools/perf/Makefile.perf
>>>>> @@ -226,7 +226,7 @@ else
>>>>> endif
>>>>> export srctree OUTPUT RM CC CXX LD AR CFLAGS CXXFLAGS V BISON FLEX AWK
>>>>> -export HOSTCC HOSTLD HOSTAR
>>>>> +export HOSTCC HOSTLD HOSTAR HOSTCFLAGS
>>>>> include $(srctree)/tools/build/Makefile.include
>>>>> diff --git a/tools/perf/pmu-events/Build b/tools/perf/pmu-events/Build
>>>>> index a055dee6a46a..d5c287f069a2 100644
>>>>> --- a/tools/perf/pmu-events/Build
>>>>> +++ b/tools/perf/pmu-events/Build
>>>>> @@ -1,7 +1,7 @@
>>>>> hostprogs := jevents
>>>>> jevents-y += json.o jsmn.o jevents.o
>>>>> -HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include
>>>>> +HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include $(HOSTCFLAGS)
>>>> so the the host cflags are made of:
>>>>
>>>> host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
>>>>
>>>
>>> ok, so IIRC, then the rule for building .o from .c in
>>> tools/build/Makefile.build will pick up HOSTCFLAGS through this variable, so
>>> we then don't need to explicitly mention it in the per-target rule, so can
>>> have this as before in pmu-events/Build
>>>
>>> HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include
>>>
>>> right?
>>>
>>> (Indeed I guess that we can get rid of -I$(srctree)/tools/include as well)
>>
>> hm, the -I.. should stay no? I don't see that
>> it's being added soem other way
>>
>> jirka
>>
>
> Probably this change from KBUILD_HOSTCFLAGS back to HOSTCFLAGS should
> come with this;
>
> Cc: Laura Abbott <[email protected]>
> Cc: Masahiro Yamada <[email protected]>
> Fixes: 96f14fe738b69dd9 ("kbuild: Rename HOSTCFLAGS to KBUILD_HOSTCFLAGS")
>
> Right?
Maybe, but then renaming back from KBUILD_HOSTCFLAGS -> HOSTCFLAGS seems
odd as a fix
Anyway, now that this original series is in perf/core, I'll send patches
on top with this change, cc'ing Laura and Masahiro
Thanks!
Em Thu, Oct 28, 2021 at 09:23:58AM +0100, John Garry escreveu:
> On 25/10/2021 17:20, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Oct 25, 2021 at 01:40:44PM +0200, Jiri Olsa escreveu:
> > > On Fri, Oct 22, 2021 at 10:42:11AM +0100, John Garry wrote:
> > > > On 21/10/2021 13:48, Jiri Olsa wrote:
> > > > ok, so IIRC, then the rule for building .o from .c in
> > > > tools/build/Makefile.build will pick up HOSTCFLAGS through this variable, so
> > > > we then don't need to explicitly mention it in the per-target rule, so can
> > > > have this as before in pmu-events/Build
> > > > HOSTCFLAGS_jevents.o = -I$(srctree)/tools/include
> > > > right?
> > > > (Indeed I guess that we can get rid of -I$(srctree)/tools/include as well)
> > > hm, the -I.. should stay no? I don't see that
> > > it's being added soem other way
> > Probably this change from KBUILD_HOSTCFLAGS back to HOSTCFLAGS should
> > come with this;
> > Cc: Laura Abbott <[email protected]>
> > Cc: Masahiro Yamada <[email protected]>
> > Fixes: 96f14fe738b69dd9 ("kbuild: Rename HOSTCFLAGS to KBUILD_HOSTCFLAGS")
> > Right?
> Maybe, but then renaming back from KBUILD_HOSTCFLAGS -> HOSTCFLAGS seems odd
> as a fix
> Anyway, now that this original series is in perf/core,
Nope, just this one landed:
commit 342cb7ebf5e29fff4dc09ab2c8f37d710f8f5206
Author: John Garry <[email protected]>
Date: Thu Oct 21 17:16:44 2021 +0800
perf jevents: Fix some would-be warnings
---
The one you asked to wait for further discussion wasn't merged.
- Arnaldo
> I'll send patches on top with this change, cc'ing Laura and Masahiro
On 28/10/2021 12:17, Arnaldo Carvalho de Melo wrote:
>> Maybe, but then renaming back from KBUILD_HOSTCFLAGS -> HOSTCFLAGS seems odd
>> as a fix
>
>> Anyway, now that this original series is in perf/core,
> Nope, just this one landed:
>
> commit 342cb7ebf5e29fff4dc09ab2c8f37d710f8f5206
> Author: John Garry<[email protected]>
> Date: Thu Oct 21 17:16:44 2021 +0800
>
> perf jevents: Fix some would-be warnings
>
> ---
>
> The one you asked to wait for further discussion wasn't merged.
ah, at I glance I saw my name and this patch and assumed both were in.
OK, so I'll put together a new series soon and post that.
Thanks