[PATCH net] scripts/pahole-flags.sh: Make sure pahole --version works

From: Saeed Mahameed <saeedm@nvidia.com>

I had a broken pahole and it's been driving me crazy to see tons of the
following error messages on every build.

pahole: symbol lookup error: pahole: undefined symbol: btf_gen_floats
scripts/pahole-flags.sh: line 12: [: : integer expression expected
scripts/pahole-flags.sh: line 16: [: : integer expression expected

Address this by redirecting pahole --version stderr to devnull,
and validate stdout has a non empty string, otherwise exit silently.

Fixes: 9741e07ece7c ("kbuild: Unify options for BTF generation for vmlinux and modules")
CC: Andrii Nakryiko <andrii@kernel.org>
CC: Jiri Olsa <redacted>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 scripts/pahole-flags.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/pahole-flags.sh b/scripts/pahole-flags.sh
index e6093adf4c06..b3b53f890d40 100755
--- a/scripts/pahole-flags.sh
+++ b/scripts/pahole-flags.sh

@@ -7,7 +7,8 @@ if ! [ -x "$(command -v ${PAHOLE})" ]; then
 	exit 0
 fi
 
-pahole_ver=$(${PAHOLE} --version | sed -E 's/v([0-9]+)\.([0-9]+)/\1\2/')
+pahole_ver=$(${PAHOLE} --version 2>/dev/null | sed -E 's/v([0-9]+)\.([0-9]+)/\1\2/')
+[ -z "${pahole_ver}" ] && exit 0
 
 if [ "${pahole_ver}" -ge "118" ] && [ "${pahole_ver}" -le "121" ]; then
 	# pahole 1.18 through 1.21 can't handle zero-sized per-CPU vars

-- 
2.33.1

On 12/31/21 8:56 AM, Saeed Mahameed wrote:

From: Saeed Mahameed <saeedm@nvidia.com>

I had a broken pahole and it's been driving me crazy to see tons of the
following error messages on every build.

pahole: symbol lookup error: pahole: undefined symbol: btf_gen_floats
scripts/pahole-flags.sh: line 12: [: : integer expression expected
scripts/pahole-flags.sh: line 16: [: : integer expression expected

Address this by redirecting pahole --version stderr to devnull,
and validate stdout has a non empty string, otherwise exit silently.

I'll leave this up to Andrii, but broken pahole version sounds like it would
have been better to fix the local pahole installation instead [rather than the
kernel having to guard against it, especially if it's driving you crazy]?

I could image that silent exit on empty version string due to broken pahole
deployment might rather waste developer's time to then go and debug why btf
wasn't generated..

On Wed, Jan 05, 2022 at 02:42:01PM +0100, Daniel Borkmann wrote:

On 12/31/21 8:56 AM, Saeed Mahameed wrote:

quoted

From: Saeed Mahameed <saeedm@nvidia.com>

I had a broken pahole and it's been driving me crazy to see tons of the
following error messages on every build.

pahole: symbol lookup error: pahole: undefined symbol: btf_gen_floats
scripts/pahole-flags.sh: line 12: [: : integer expression expected
scripts/pahole-flags.sh: line 16: [: : integer expression expected

Address this by redirecting pahole --version stderr to devnull,
and validate stdout has a non empty string, otherwise exit silently.

I'll leave this up to Andrii, but broken pahole version sounds like it would
have been better to fix the local pahole installation instead [rather than the
kernel having to guard against it, especially if it's driving you crazy]?

Already did :)

I could image that silent exit on empty version string due to broken pahole
deployment might rather waste developer's time to then go and debug why btf
wasn't generated..

Good point, I was mainly thinking about developers who are not familiar with btf
and who have no time debugging irrelevant build issues, but up to you, I
personally like silent build scripts.

quoted

Fixes: 9741e07ece7c ("kbuild: Unify options for BTF generation for vmlinux and modules")
CC: Andrii Nakryiko <andrii@kernel.org>
CC: Jiri Olsa <redacted>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 scripts/pahole-flags.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/pahole-flags.sh b/scripts/pahole-flags.sh
index e6093adf4c06..b3b53f890d40 100755
--- a/scripts/pahole-flags.sh
+++ b/scripts/pahole-flags.sh

@@ -7,7 +7,8 @@ if ! [ -x "$(command -v ${PAHOLE})" ]; then
 	exit 0
 fi
-pahole_ver=$(${PAHOLE} --version | sed -E 's/v([0-9]+)\.([0-9]+)/\1\2/')
+pahole_ver=$(${PAHOLE} --version 2>/dev/null | sed -E 's/v([0-9]+)\.([0-9]+)/\1\2/')
+[ -z "${pahole_ver}" ] && exit 0
 if [ "${pahole_ver}" -ge "118" ] && [ "${pahole_ver}" -le "121" ]; then
 	# pahole 1.18 through 1.21 can't handle zero-sized per-CPU vars

On Wed, Jan 5, 2022 at 2:14 PM Saeed Mahameed [off-list ref] wrote:

On Wed, Jan 05, 2022 at 02:42:01PM +0100, Daniel Borkmann wrote:

quoted

On 12/31/21 8:56 AM, Saeed Mahameed wrote:

quoted

From: Saeed Mahameed <saeedm@nvidia.com>

I had a broken pahole and it's been driving me crazy to see tons of the
following error messages on every build.

pahole: symbol lookup error: pahole: undefined symbol: btf_gen_floats
scripts/pahole-flags.sh: line 12: [: : integer expression expected
scripts/pahole-flags.sh: line 16: [: : integer expression expected

Address this by redirecting pahole --version stderr to devnull,
and validate stdout has a non empty string, otherwise exit silently.

I'll leave this up to Andrii, but broken pahole version sounds like it would
have been better to fix the local pahole installation instead [rather than the
kernel having to guard against it, especially if it's driving you crazy]?

Already did :)

quoted

I could image that silent exit on empty version string due to broken pahole
deployment might rather waste developer's time to then go and debug why btf
wasn't generated..

Good point, I was mainly thinking about developers who are not familiar with btf
and who have no time debugging irrelevant build issues, but up to you, I
personally like silent build scripts.

Sorry, trying to understand. If you didn't use BTF (and thus
CONFIG_DEBUG_INFO_BTF is not set), is pahole still being called? If
yes, we might want to address that, I suppose.

But if you have a broken pahole that emits something to stderr
(undefined symbol in shared library), then I agree with Daniel that we
shouldn't be working around that in Linux build script.

quoted

quoted

Fixes: 9741e07ece7c ("kbuild: Unify options for BTF generation for vmlinux and modules")
CC: Andrii Nakryiko <andrii@kernel.org>
CC: Jiri Olsa <redacted>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 scripts/pahole-flags.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/pahole-flags.sh b/scripts/pahole-flags.sh
index e6093adf4c06..b3b53f890d40 100755
--- a/scripts/pahole-flags.sh
+++ b/scripts/pahole-flags.sh

@@ -7,7 +7,8 @@ if ! [ -x "$(command -v ${PAHOLE})" ]; then
     exit 0
 fi
-pahole_ver=$(${PAHOLE} --version | sed -E 's/v([0-9]+)\.([0-9]+)/\1\2/')
+pahole_ver=$(${PAHOLE} --version 2>/dev/null | sed -E 's/v([0-9]+)\.([0-9]+)/\1\2/')
+[ -z "${pahole_ver}" ] && exit 0
 if [ "${pahole_ver}" -ge "118" ] && [ "${pahole_ver}" -le "121" ]; then
     # pahole 1.18 through 1.21 can't handle zero-sized per-CPU vars