[PATCH 1/1 fyi] perf python: Fix splitting CC into compiler and options

[PATCH 1/1 fyi] perf python: Fix splitting CC into compiler and options

[Arnaldo Carvalho de Melo]

Just fyi, I'm carrying this in the perf tools tree.

- Arnaldo

----

Noticed this build failure on archlinux:base when building with clang:

  clang-14: error: optimization flag '-ffat-lto-objects' is not supported [-Werror,-Wignored-optimization-argument]

In tools/perf/util/setup.py we check if clang supports that option, but
since commit 3cad53a6f9cdbafa ("perf python: Account for multiple words
in CC") this got broken as in the common case where CC="clang":

  >>> cc="clang"
  >>> print(cc.split()[0])
  clang
  >>> option="-ffat-lto-objects"
  >>> print(str(cc.split()[1:]) + option)
  []-ffat-lto-objects
  >>>

And then the Popen will call clang with that bogus option name that in
turn will not produce the b"unknown argument" or b"is not supported"
that this function uses to detect if the option is not available and
thus later on clang will be called with an unknown/unsupported option.

Fix it by looking if really there are options in the provided CC
variable, and if so override 'cc' with the first token and append the
options to the 'option' variable.

Fixes: 3cad53a6f9cdbafa ("perf python: Account for multiple words in CC")
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Fangrui Song <maskray@google.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: John Keeping <john@metanate.com>
Cc: Khem Raj <raj.khem@gmail.com>
Cc: Leo Yan <leo.yan@linaro.org>
Cc: Michael Petlan <mpetlan@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Sedat Dilek <sedat.dilek@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/setup.py | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/setup.py b/tools/perf/util/setup.py
index 4f265d0222c454e2..c294db713677c0c2 100644
--- a/tools/perf/util/setup.py
+++ b/tools/perf/util/setup.py
@@ -3,11 +3,20 @@ from subprocess import Popen, PIPE
 from re import sub
 
 cc = getenv("CC")
-cc_is_clang = b"clang version" in Popen([cc.split()[0], "-v"], stderr=PIPE).stderr.readline()
+
+# Check if CC has options, as is the case in yocto, where it uses CC="cc --sysroot..."
+cc_tokens = cc.split()
+if len(cc_tokens) > 1:
+    cc = cc_tokens[0]
+    cc_options = " ".join([str(e) for e in cc_tokens[1:]]) + " "
+else:
+    cc_options = ""
+
+cc_is_clang = b"clang version" in Popen([cc, "-v"], stderr=PIPE).stderr.readline()
 src_feature_tests  = getenv('srctree') + '/tools/build/feature'
 
 def clang_has_option(option):
-    cc_output = Popen([cc.split()[0], str(cc.split()[1:]) + option, path.join(src_feature_tests, "test-hello.c") ], stderr=PIPE).stderr.readlines()
+    cc_output = Popen([cc, cc_options + option, path.join(src_feature_tests, "test-hello.c") ], stderr=PIPE).stderr.readlines()
     return [o for o in cc_output if ((b"unknown argument" in o) or (b"is not supported" in o))] == [ ]
 
 if cc_is_clang:
-- 
2.38.1

Re: [PATCH 1/1 fyi] perf python: Fix splitting CC into compiler and options

[Nick Desaulniers]

On Thu, Dec 22, 2022 at 6:34 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Just fyi, I'm carrying this in the perf tools tree.
>
> - Arnaldo
>
> ----
>
> Noticed this build failure on archlinux:base when building with clang:
>
>   clang-14: error: optimization flag '-ffat-lto-objects' is not supported [-Werror,-Wignored-optimization-argument]
>
> In tools/perf/util/setup.py we check if clang supports that option, but
> since commit 3cad53a6f9cdbafa ("perf python: Account for multiple words
> in CC") this got broken as in the common case where CC="clang":
>
>   >>> cc="clang"
>   >>> print(cc.split()[0])
>   clang
>   >>> option="-ffat-lto-objects"
>   >>> print(str(cc.split()[1:]) + option)
>   []-ffat-lto-objects
>   >>>
>
> And then the Popen will call clang with that bogus option name that in
> turn will not produce the b"unknown argument" or b"is not supported"
> that this function uses to detect if the option is not available and
> thus later on clang will be called with an unknown/unsupported option.
>
> Fix it by looking if really there are options in the provided CC
> variable, and if so override 'cc' with the first token and append the
> options to the 'option' variable.
>
> Fixes: 3cad53a6f9cdbafa ("perf python: Account for multiple words in CC")
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Fangrui Song <maskray@google.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Ian Rogers <irogers@google.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: John Keeping <john@metanate.com>
> Cc: Khem Raj <raj.khem@gmail.com>
> Cc: Leo Yan <leo.yan@linaro.org>
> Cc: Michael Petlan <mpetlan@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Sedat Dilek <sedat.dilek@gmail.com>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/util/setup.py | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/setup.py b/tools/perf/util/setup.py
> index 4f265d0222c454e2..c294db713677c0c2 100644
> --- a/tools/perf/util/setup.py
> +++ b/tools/perf/util/setup.py
> @@ -3,11 +3,20 @@ from subprocess import Popen, PIPE
>  from re import sub
>
>  cc = getenv("CC")
> -cc_is_clang = b"clang version" in Popen([cc.split()[0], "-v"], stderr=PIPE).stderr.readline()
> +
> +# Check if CC has options, as is the case in yocto, where it uses CC="cc --sysroot..."
> +cc_tokens = cc.split()
> +if len(cc_tokens) > 1:
> +    cc = cc_tokens[0]

What if someone is using `CC="cache clang"`? Then cc is set to `cache`
and the cc_is_clang check below will fail.

> +    cc_options = " ".join([str(e) for e in cc_tokens[1:]]) + " "
> +else:
> +    cc_options = ""
> +
> +cc_is_clang = b"clang version" in Popen([cc, "-v"], stderr=PIPE).stderr.readline()
>  src_feature_tests  = getenv('srctree') + '/tools/build/feature'
>
>  def clang_has_option(option):
> -    cc_output = Popen([cc.split()[0], str(cc.split()[1:]) + option, path.join(src_feature_tests, "test-hello.c") ], stderr=PIPE).stderr.readlines()
> +    cc_output = Popen([cc, cc_options + option, path.join(src_feature_tests, "test-hello.c") ], stderr=PIPE).stderr.readlines()
>      return [o for o in cc_output if ((b"unknown argument" in o) or (b"is not supported" in o))] == [ ]
>
>  if cc_is_clang:
> --
> 2.38.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 1/1 fyi] perf python: Fix splitting CC into compiler and options

[Nick Desaulniers]

On Thu, Dec 22, 2022 at 9:45 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Thu, Dec 22, 2022 at 6:34 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Just fyi, I'm carrying this in the perf tools tree.
> >
> > - Arnaldo
> >
> > ----
> >
> > Noticed this build failure on archlinux:base when building with clang:
> >
> >   clang-14: error: optimization flag '-ffat-lto-objects' is not supported [-Werror,-Wignored-optimization-argument]
> >
> > In tools/perf/util/setup.py we check if clang supports that option, but
> > since commit 3cad53a6f9cdbafa ("perf python: Account for multiple words
> > in CC") this got broken as in the common case where CC="clang":
> >
> >   >>> cc="clang"
> >   >>> print(cc.split()[0])
> >   clang
> >   >>> option="-ffat-lto-objects"
> >   >>> print(str(cc.split()[1:]) + option)
> >   []-ffat-lto-objects
> >   >>>
> >
> > And then the Popen will call clang with that bogus option name that in
> > turn will not produce the b"unknown argument" or b"is not supported"
> > that this function uses to detect if the option is not available and
> > thus later on clang will be called with an unknown/unsupported option.
> >
> > Fix it by looking if really there are options in the provided CC
> > variable, and if so override 'cc' with the first token and append the
> > options to the 'option' variable.
> >
> > Fixes: 3cad53a6f9cdbafa ("perf python: Account for multiple words in CC")
> > Cc: Adrian Hunter <adrian.hunter@intel.com>
> > Cc: Fangrui Song <maskray@google.com>
> > Cc: Florian Fainelli <f.fainelli@gmail.com>
> > Cc: Ian Rogers <irogers@google.com>
> > Cc: Jiri Olsa <jolsa@kernel.org>
> > Cc: John Keeping <john@metanate.com>
> > Cc: Khem Raj <raj.khem@gmail.com>
> > Cc: Leo Yan <leo.yan@linaro.org>
> > Cc: Michael Petlan <mpetlan@redhat.com>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Cc: Nathan Chancellor <nathan@kernel.org>
> > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > Cc: Sedat Dilek <sedat.dilek@gmail.com>
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > ---
> >  tools/perf/util/setup.py | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/util/setup.py b/tools/perf/util/setup.py
> > index 4f265d0222c454e2..c294db713677c0c2 100644
> > --- a/tools/perf/util/setup.py
> > +++ b/tools/perf/util/setup.py
> > @@ -3,11 +3,20 @@ from subprocess import Popen, PIPE
> >  from re import sub
> >
> >  cc = getenv("CC")
> > -cc_is_clang = b"clang version" in Popen([cc.split()[0], "-v"], stderr=PIPE).stderr.readline()
> > +
> > +# Check if CC has options, as is the case in yocto, where it uses CC="cc --sysroot..."
> > +cc_tokens = cc.split()
> > +if len(cc_tokens) > 1:
> > +    cc = cc_tokens[0]
>
> What if someone is using `CC="cache clang"`? Then cc is set to `cache`
> and the cc_is_clang check below will fail.

s/cache/ccache/g

>
> > +    cc_options = " ".join([str(e) for e in cc_tokens[1:]]) + " "
> > +else:
> > +    cc_options = ""
> > +
> > +cc_is_clang = b"clang version" in Popen([cc, "-v"], stderr=PIPE).stderr.readline()
> >  src_feature_tests  = getenv('srctree') + '/tools/build/feature'
> >
> >  def clang_has_option(option):
> > -    cc_output = Popen([cc.split()[0], str(cc.split()[1:]) + option, path.join(src_feature_tests, "test-hello.c") ], stderr=PIPE).stderr.readlines()
> > +    cc_output = Popen([cc, cc_options + option, path.join(src_feature_tests, "test-hello.c") ], stderr=PIPE).stderr.readlines()
> >      return [o for o in cc_output if ((b"unknown argument" in o) or (b"is not supported" in o))] == [ ]
> >
> >  if cc_is_clang:
> > --
> > 2.38.1
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers



-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 1/1 fyi] perf python: Fix splitting CC into compiler and options

[Arnaldo Carvalho de Melo]

On December 22, 2022 2:45:57 PM GMT-03:00, Nick Desaulniers <ndesaulniers@google.com> wrote:
>On Thu, Dec 22, 2022 at 6:34 AM Arnaldo Carvalho de Melo
><acme@kernel.org> wrote:
>>
>> Just fyi, I'm carrying this in the perf tools tree.
>>
>> - Arnaldo
>>
>> ----
>>
>> Noticed this build failure on archlinux:base when building with clang:
>>
>>   clang-14: error: optimization flag '-ffat-lto-objects' is not supported [-Werror,-Wignored-optimization-argument]
>>
>> In tools/perf/util/setup.py we check if clang supports that option, but
>> since commit 3cad53a6f9cdbafa ("perf python: Account for multiple words
>> in CC") this got broken as in the common case where CC="clang":
>>
>>   >>> cc="clang"
>>   >>> print(cc.split()[0])
>>   clang
>>   >>> option="-ffat-lto-objects"
>>   >>> print(str(cc.split()[1:]) + option)
>>   []-ffat-lto-objects
>>   >>>
>>
>> And then the Popen will call clang with that bogus option name that in
>> turn will not produce the b"unknown argument" or b"is not supported"
>> that this function uses to detect if the option is not available and
>> thus later on clang will be called with an unknown/unsupported option.
>>
>> Fix it by looking if really there are options in the provided CC
>> variable, and if so override 'cc' with the first token and append the
>> options to the 'option' variable.
>>
>> Fixes: 3cad53a6f9cdbafa ("perf python: Account for multiple words in CC")
>> Cc: Adrian Hunter <adrian.hunter@intel.com>
>> Cc: Fangrui Song <maskray@google.com>
>> Cc: Florian Fainelli <f.fainelli@gmail.com>
>> Cc: Ian Rogers <irogers@google.com>
>> Cc: Jiri Olsa <jolsa@kernel.org>
>> Cc: John Keeping <john@metanate.com>
>> Cc: Khem Raj <raj.khem@gmail.com>
>> Cc: Leo Yan <leo.yan@linaro.org>
>> Cc: Michael Petlan <mpetlan@redhat.com>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Nathan Chancellor <nathan@kernel.org>
>> Cc: Nick Desaulniers <ndesaulniers@google.com>
>> Cc: Sedat Dilek <sedat.dilek@gmail.com>
>> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>> ---
>>  tools/perf/util/setup.py | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/util/setup.py b/tools/perf/util/setup.py
>> index 4f265d0222c454e2..c294db713677c0c2 100644
>> --- a/tools/perf/util/setup.py
>> +++ b/tools/perf/util/setup.py
>> @@ -3,11 +3,20 @@ from subprocess import Popen, PIPE
>>  from re import sub
>>
>>  cc = getenv("CC")
>> -cc_is_clang = b"clang version" in Popen([cc.split()[0], "-v"], stderr=PIPE).stderr.readline()
>> +
>> +# Check if CC has options, as is the case in yocto, where it uses CC="cc --sysroot..."
>> +cc_tokens = cc.split()
>> +if len(cc_tokens) > 1:
>> +    cc = cc_tokens[0]
>
>What if someone is using `CC="cache clang"`? Then cc is set to `cache`
>and the cc_is_clang check below will fail.

Agreed, but this is a preexisting bug, let's fix this with a follow-up patch,

- Arnaldo 


>
>> +    cc_options = " ".join([str(e) for e in cc_tokens[1:]]) + " "
>> +else:
>> +    cc_options = ""
>
>> +cc_is_clang = b"clang version" in Popen([cc, "-v"], stderr=PIPE).stderr.readline()
>>  src_feature_tests  = getenv('srctree') + '/tools/build/feature'
>>
>>  def clang_has_option(option):
>> -    cc_output = Popen([cc.split()[0], str(cc.split()[1:]) + option, path.join(src_feature_tests, "test-hello.c") ], stderr=PIPE).stderr.readlines()
>> +    cc_output = Popen([cc, cc_options + option, path.join(src_feature_tests, "test-hello.c") ], stderr=PIPE).stderr.readlines()
>>      return [o for o in cc_output if ((b"unknown argument" in o) or (b"is not supported" in o))] == [ ]
>>
>>  if cc_is_clang:
>> --
>> 2.38.1
>>
>
>

Re: [PATCH 1/1 fyi] perf python: Fix splitting CC into compiler and options

[Nick Desaulniers]

On Thu, Dec 22, 2022 at 1:00 PM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
>
>
> On December 22, 2022 2:45:57 PM GMT-03:00, Nick Desaulniers <ndesaulniers@google.com> wrote:
> >On Thu, Dec 22, 2022 at 6:34 AM Arnaldo Carvalho de Melo
> ><acme@kernel.org> wrote:
> >>
> >> Just fyi, I'm carrying this in the perf tools tree.
> >>
> >> - Arnaldo
> >>
> >> ----
> >>
> >> Noticed this build failure on archlinux:base when building with clang:
> >>
> >>   clang-14: error: optimization flag '-ffat-lto-objects' is not supported [-Werror,-Wignored-optimization-argument]
> >>
> >> In tools/perf/util/setup.py we check if clang supports that option, but
> >> since commit 3cad53a6f9cdbafa ("perf python: Account for multiple words
> >> in CC") this got broken as in the common case where CC="clang":
> >>
> >>   >>> cc="clang"
> >>   >>> print(cc.split()[0])
> >>   clang
> >>   >>> option="-ffat-lto-objects"
> >>   >>> print(str(cc.split()[1:]) + option)
> >>   []-ffat-lto-objects
> >>   >>>
> >>
> >> And then the Popen will call clang with that bogus option name that in
> >> turn will not produce the b"unknown argument" or b"is not supported"
> >> that this function uses to detect if the option is not available and
> >> thus later on clang will be called with an unknown/unsupported option.
> >>
> >> Fix it by looking if really there are options in the provided CC
> >> variable, and if so override 'cc' with the first token and append the
> >> options to the 'option' variable.
> >>
> >> Fixes: 3cad53a6f9cdbafa ("perf python: Account for multiple words in CC")
> >> Cc: Adrian Hunter <adrian.hunter@intel.com>
> >> Cc: Fangrui Song <maskray@google.com>
> >> Cc: Florian Fainelli <f.fainelli@gmail.com>
> >> Cc: Ian Rogers <irogers@google.com>
> >> Cc: Jiri Olsa <jolsa@kernel.org>
> >> Cc: John Keeping <john@metanate.com>
> >> Cc: Khem Raj <raj.khem@gmail.com>
> >> Cc: Leo Yan <leo.yan@linaro.org>
> >> Cc: Michael Petlan <mpetlan@redhat.com>
> >> Cc: Namhyung Kim <namhyung@kernel.org>
> >> Cc: Nathan Chancellor <nathan@kernel.org>
> >> Cc: Nick Desaulniers <ndesaulniers@google.com>
> >> Cc: Sedat Dilek <sedat.dilek@gmail.com>
> >> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> >> ---
> >>  tools/perf/util/setup.py | 13 +++++++++++--
> >>  1 file changed, 11 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/tools/perf/util/setup.py b/tools/perf/util/setup.py
> >> index 4f265d0222c454e2..c294db713677c0c2 100644
> >> --- a/tools/perf/util/setup.py
> >> +++ b/tools/perf/util/setup.py
> >> @@ -3,11 +3,20 @@ from subprocess import Popen, PIPE
> >>  from re import sub
> >>
> >>  cc = getenv("CC")
> >> -cc_is_clang = b"clang version" in Popen([cc.split()[0], "-v"], stderr=PIPE).stderr.readline()
> >> +
> >> +# Check if CC has options, as is the case in yocto, where it uses CC="cc --sysroot..."
> >> +cc_tokens = cc.split()
> >> +if len(cc_tokens) > 1:
> >> +    cc = cc_tokens[0]
> >
> >What if someone is using `CC="cache clang"`? Then cc is set to `cache`
> >and the cc_is_clang check below will fail.
>
> Agreed, but this is a preexisting bug, let's fix this with a follow-up patch,

Ack; it is.

>
> - Arnaldo
>
>
> >
> >> +    cc_options = " ".join([str(e) for e in cc_tokens[1:]]) + " "
> >> +else:
> >> +    cc_options = ""
> >
> >> +cc_is_clang = b"clang version" in Popen([cc, "-v"], stderr=PIPE).stderr.readline()
> >>  src_feature_tests  = getenv('srctree') + '/tools/build/feature'
> >>
> >>  def clang_has_option(option):
> >> -    cc_output = Popen([cc.split()[0], str(cc.split()[1:]) + option, path.join(src_feature_tests, "test-hello.c") ], stderr=PIPE).stderr.readlines()
> >> +    cc_output = Popen([cc, cc_options + option, path.join(src_feature_tests, "test-hello.c") ], stderr=PIPE).stderr.readlines()
> >>      return [o for o in cc_output if ((b"unknown argument" in o) or (b"is not supported" in o))] == [ ]
> >>
> >>  if cc_is_clang:
> >> --
> >> 2.38.1
> >>
> >
> >



-- 
Thanks,
~Nick Desaulniers