lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Mon, 24 Feb 2020 15:00:05 +0800
From:   He Zhe <zhe.he@...driver.com>
To:     Sam Lunt <samueljlunt@...il.com>
CC:     <linux-kernel@...r.kernel.org>, <peterz@...radead.org>,
        <mingo@...hat.com>, <acme@...nel.org>, <mark.rutland@....com>,
        <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...hat.com>,
        <namhyung@...nel.org>, <trivial@...nel.org>
Subject: Re: [PATCH] perf: Support Python 3.8+ in Makefile



On 2/23/20 12:38 AM, Sam Lunt wrote:
> On Sun, Feb 16, 2020 at 8:24 PM He Zhe <zhe.he@...driver.com> wrote:
>> On 2/1/20 2:11 AM, Sam Lunt wrote:
>>> Python 3.8 changed the output of 'python-config --ldflags' to no longer
>>> include the '-lpythonX.Y' flag (this apparently fixed an issue loading
>>> modules with a statically linked Python executable).  The libpython
>>> feature check in linux/build/feature fails if the Python library is not
>>> included in FEATURE_CHECK_LDFLAGS-libpython variable.
>>>
>>> This adds a check in the Makefile to determine if PYTHON_CONFIG accepts
>>> the '--embed' flag and passes that flag alongside '--ldflags' if so.
>>>
>>> tools/perf is the only place the libpython feature check is used.
>>>
>>> Signed-off-by: Sam Lunt <samuel.j.lunt@...il.com>
>>> ---
>>>  tools/perf/Makefile.config | 11 ++++++++++-
>>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
>>> index c90f4146e5a2..ccf99351f058 100644
>>> --- a/tools/perf/Makefile.config
>>> +++ b/tools/perf/Makefile.config
>>> @@ -228,8 +228,17 @@ strip-libs  = $(filter-out -l%,$(1))
>>>
>>>  PYTHON_CONFIG_SQ := $(call shell-sq,$(PYTHON_CONFIG))
>>>
>>> +# Python 3.8 changed the output of `python-config --ldflags` to not include the
>>> +# '-lpythonX.Y' flag unless '--embed' is also passed. The feature check for
>>> +# libpython fails if that flag is not included in LDFLAGS
>>> +ifeq ($(shell $(PYTHON_CONFIG_SQ) --ldflags --embed 2>&1 1>/dev/null; echo $$?), 0)
>>> +  PYTHON_CONFIG_LDFLAGS := --ldflags --embed
>>> +else
>>> +  PYTHON_CONFIG_LDFLAGS := --ldflags
>>> +endif
>>> +
>>>  ifdef PYTHON_CONFIG
>>> -  PYTHON_EMBED_LDOPTS := $(shell $(PYTHON_CONFIG_SQ) --ldflags 2>/dev/null)
>>> +  PYTHON_EMBED_LDOPTS := $(shell $(PYTHON_CONFIG_SQ) $(PYTHON_CONFIG_LDFLAGS) 2>/dev/null)
>> I met the same problem. Would the following change be more simple and clear?
>>
>> -  PYTHON_EMBED_LDOPTS := $(shell $(PYTHON_CONFIG_SQ) --ldflags 2>/dev/null)
>> +  PYTHON_EMBED_LDOPTS := $(shell $(PYTHON_CONFIG_SQ) --ldflags --embed 2>/dev/null || $(PYTHON_CONFIG_SQ) --ldflags 2>/dev/null)
> That change is nearly equivalent to the change I'd suggested, just
> squashed into one line. I think it's certainly more terse, but I'm not
> sure it's any clearer.
>
> It's also making the implicit assumption that PYTHON_CONFIG_SQ does
> not print anything to stdout when it exits with a non-zero return
> code. I think that's probably a safe assumption, but it seems more
> robust not to make that assumption at all.

That said, such assumption was also made in the "else" case of your change and
in the original line. If we wanted that robustness, we'd better give a stricter
condition and a warning like below.

+ifeq ($(shell $(PYTHON_CONFIG_SQ) --ldflags --embed 2>&1 1>/dev/null; echo $$?), 0)
+  PYTHON_CONFIG_LDFLAGS := --ldflags --embed
+else ifeq ($(shell $(PYTHON_CONFIG_SQ) --ldflags 2>&1 1>/dev/null; echo $$?), 0)
+  PYTHON_CONFIG_LDFLAGS := --ldflags
+else
+  $(warning Failed to get python ldflags.)
+endif

Given that such kind of assumptions spread across kernel makefiles, the
one-liner might be good enough.


Regards,
Zhe

>
> Best,
> Sam

Powered by blists - more mailing lists