2023-10-19 21:17:54

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 1/1] tools build: Fix llvm feature detection, still used by bpftool

When removing the BPF event for perf a feature test that checks if the
llvm devel files are availabe was removed but that is also used by
bpftool.

bpftool uses it to decide what kind of disassembly it will use: llvm or
binutils based.

Removing the tools/build/feature/test-llvm.cpp file made bpftool to
always fallback to binutils disassembly, even with the llvm devel files
installed, fix it by restoring just that small test-llvm.cpp test file.

Fixes: 56b11a2126bf2f42 ("perf bpf: Remove support for embedding clang for compiling BPF events (-e foo.c)")
Reported-by: Manu Bretelle <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Andrii Nakryiko <[email protected]>
Cc: Anshuman Khandual <[email protected]>
Cc: Carsten Haitzler <[email protected]>
Cc: Eduard Zingerman <[email protected]>
Cc: Fangrui Song <[email protected]>
Cc: He Kuang <[email protected]>
Cc: Ian Rogers <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: James Clark <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Kan Liang <[email protected]>
Cc: Leo Yan <[email protected]>
Cc: [email protected]
Cc: Madhavan Srinivasan <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Nathan Chancellor <[email protected]>
Cc: Naveen N. Rao <[email protected]>
Cc: Nick Desaulniers <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Quentin Monnet <[email protected]>
Cc: Ravi Bangoria <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Tiezhu Yang <[email protected]>
Cc: Tom Rix <[email protected]>
Cc: Wang Nan <[email protected]>
Cc: Wang ShaoBo <[email protected]>
Cc: Yang Jihong <[email protected]>
Cc: Yonghong Song <[email protected]>
Cc: YueHaibing <[email protected]>
Link: https://lore.kernel.org/lkml/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/build/feature/test-llvm.cpp | 14 ++++++++++++++
1 file changed, 14 insertions(+)
create mode 100644 tools/build/feature/test-llvm.cpp

diff --git a/tools/build/feature/test-llvm.cpp b/tools/build/feature/test-llvm.cpp
new file mode 100644
index 0000000000000000..88a3d1bdd9f6978e
--- /dev/null
+++ b/tools/build/feature/test-llvm.cpp
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "llvm/Support/ManagedStatic.h"
+#include "llvm/Support/raw_ostream.h"
+#define NUM_VERSION (((LLVM_VERSION_MAJOR) << 16) + (LLVM_VERSION_MINOR << 8) + LLVM_VERSION_PATCH)
+
+#if NUM_VERSION < 0x030900
+# error "LLVM version too low"
+#endif
+int main()
+{
+ llvm::errs() << "Hello World!\n";
+ llvm::llvm_shutdown();
+ return 0;
+}
--
2.41.0


2023-10-19 21:58:38

by Ian Rogers

[permalink] [raw]
Subject: Re: [PATCH 1/1] tools build: Fix llvm feature detection, still used by bpftool

On Thu, Oct 19, 2023 at 2:17 PM Arnaldo Carvalho de Melo
<[email protected]> wrote:
>
> When removing the BPF event for perf a feature test that checks if the
> llvm devel files are availabe was removed but that is also used by
> bpftool.
>
> bpftool uses it to decide what kind of disassembly it will use: llvm or
> binutils based.
>
> Removing the tools/build/feature/test-llvm.cpp file made bpftool to
> always fallback to binutils disassembly, even with the llvm devel files
> installed, fix it by restoring just that small test-llvm.cpp test file.
>
> Fixes: 56b11a2126bf2f42 ("perf bpf: Remove support for embedding clang for compiling BPF events (-e foo.c)")
> Reported-by: Manu Bretelle <[email protected]>
> Cc: Adrian Hunter <[email protected]>
> Cc: Alexander Shishkin <[email protected]>
> Cc: Andi Kleen <[email protected]>
> Cc: Andrii Nakryiko <[email protected]>
> Cc: Anshuman Khandual <[email protected]>
> Cc: Carsten Haitzler <[email protected]>
> Cc: Eduard Zingerman <[email protected]>
> Cc: Fangrui Song <[email protected]>
> Cc: He Kuang <[email protected]>
> Cc: Ian Rogers <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: James Clark <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> Cc: Kan Liang <[email protected]>
> Cc: Leo Yan <[email protected]>
> Cc: [email protected]
> Cc: Madhavan Srinivasan <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Cc: Nathan Chancellor <[email protected]>
> Cc: Naveen N. Rao <[email protected]>
> Cc: Nick Desaulniers <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Quentin Monnet <[email protected]>
> Cc: Ravi Bangoria <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Tiezhu Yang <[email protected]>
> Cc: Tom Rix <[email protected]>
> Cc: Wang Nan <[email protected]>
> Cc: Wang ShaoBo <[email protected]>
> Cc: Yang Jihong <[email protected]>
> Cc: Yonghong Song <[email protected]>
> Cc: YueHaibing <[email protected]>
> Link: https://lore.kernel.org/lkml/[email protected]
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>

Sorry for the breakage.

Reviewed-by: Ian Rogers <[email protected]>

Thanks,
Ian

> ---
> tools/build/feature/test-llvm.cpp | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
> create mode 100644 tools/build/feature/test-llvm.cpp
>
> diff --git a/tools/build/feature/test-llvm.cpp b/tools/build/feature/test-llvm.cpp
> new file mode 100644
> index 0000000000000000..88a3d1bdd9f6978e
> --- /dev/null
> +++ b/tools/build/feature/test-llvm.cpp
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "llvm/Support/ManagedStatic.h"
> +#include "llvm/Support/raw_ostream.h"
> +#define NUM_VERSION (((LLVM_VERSION_MAJOR) << 16) + (LLVM_VERSION_MINOR << 8) + LLVM_VERSION_PATCH)
> +
> +#if NUM_VERSION < 0x030900
> +# error "LLVM version too low"
> +#endif
> +int main()
> +{
> + llvm::errs() << "Hello World!\n";
> + llvm::llvm_shutdown();
> + return 0;
> +}
> --
> 2.41.0
>

2023-10-19 22:01:49

by Quentin Monnet

[permalink] [raw]
Subject: Re: [PATCH 1/1] tools build: Fix llvm feature detection, still used by bpftool

On 19/10/2023 22:17, Arnaldo Carvalho de Melo wrote:
> When removing the BPF event for perf a feature test that checks if the
> llvm devel files are availabe was removed but that is also used by
> bpftool.
>
> bpftool uses it to decide what kind of disassembly it will use: llvm or
> binutils based.
>
> Removing the tools/build/feature/test-llvm.cpp file made bpftool to
> always fallback to binutils disassembly, even with the llvm devel files
> installed, fix it by restoring just that small test-llvm.cpp test file.
>
> Fixes: 56b11a2126bf2f42 ("perf bpf: Remove support for embedding clang for compiling BPF events (-e foo.c)")
> Reported-by: Manu Bretelle <[email protected]>
> Cc: Adrian Hunter <[email protected]>
> Cc: Alexander Shishkin <[email protected]>
> Cc: Andi Kleen <[email protected]>
> Cc: Andrii Nakryiko <[email protected]>
> Cc: Anshuman Khandual <[email protected]>
> Cc: Carsten Haitzler <[email protected]>
> Cc: Eduard Zingerman <[email protected]>
> Cc: Fangrui Song <[email protected]>
> Cc: He Kuang <[email protected]>
> Cc: Ian Rogers <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: James Clark <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> Cc: Kan Liang <[email protected]>
> Cc: Leo Yan <[email protected]>
> Cc: [email protected]
> Cc: Madhavan Srinivasan <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Cc: Nathan Chancellor <[email protected]>
> Cc: Naveen N. Rao <[email protected]>
> Cc: Nick Desaulniers <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Quentin Monnet <[email protected]>
> Cc: Ravi Bangoria <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Tiezhu Yang <[email protected]>
> Cc: Tom Rix <[email protected]>
> Cc: Wang Nan <[email protected]>
> Cc: Wang ShaoBo <[email protected]>
> Cc: Yang Jihong <[email protected]>
> Cc: Yonghong Song <[email protected]>
> Cc: YueHaibing <[email protected]>
> Link: https://lore.kernel.org/lkml/[email protected]
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
> ---
> tools/build/feature/test-llvm.cpp | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
> create mode 100644 tools/build/feature/test-llvm.cpp
>
> diff --git a/tools/build/feature/test-llvm.cpp b/tools/build/feature/test-llvm.cpp
> new file mode 100644
> index 0000000000000000..88a3d1bdd9f6978e
> --- /dev/null
> +++ b/tools/build/feature/test-llvm.cpp
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "llvm/Support/ManagedStatic.h"
> +#include "llvm/Support/raw_ostream.h"
> +#define NUM_VERSION (((LLVM_VERSION_MAJOR) << 16) + (LLVM_VERSION_MINOR << 8) + LLVM_VERSION_PATCH)
> +
> +#if NUM_VERSION < 0x030900
> +# error "LLVM version too low"
> +#endif
> +int main()
> +{
> + llvm::errs() << "Hello World!\n";
> + llvm::llvm_shutdown();
> + return 0;
> +}

Acked-by: Quentin Monnet <[email protected]>

Thanks Arnaldo, Manu!

2023-10-19 22:50:47

by Quentin Monnet

[permalink] [raw]
Subject: Re: [PATCH 1/1] tools build: Fix llvm feature detection, still used by bpftool

On 19/10/2023 23:47, Andrii Nakryiko wrote:
> On Thu, Oct 19, 2023 at 2:17 PM Arnaldo Carvalho de Melo
> <[email protected]> wrote:
>>
>> When removing the BPF event for perf a feature test that checks if the
>> llvm devel files are availabe was removed but that is also used by
>> bpftool.
>>
>> bpftool uses it to decide what kind of disassembly it will use: llvm or
>> binutils based.
>>
>> Removing the tools/build/feature/test-llvm.cpp file made bpftool to
>> always fallback to binutils disassembly, even with the llvm devel files
>> installed, fix it by restoring just that small test-llvm.cpp test file.
>>
>> Fixes: 56b11a2126bf2f42 ("perf bpf: Remove support for embedding clang for compiling BPF events (-e foo.c)")
>
> Should we route this through the bpf-next tree to get the fix for
> bpftool into Github mirror ASAP?

It shouldn't be necessary. The GitHub mirror for bpftool uses its own
feature detection mechanism, and is not affected here. This patch won't
even make it to the mirror.

Quentin

2023-10-19 22:52:32

by Manu Bretelle

[permalink] [raw]
Subject: Re: [PATCH 1/1] tools build: Fix llvm feature detection, still used by bpftool

On Thu, Oct 19, 2023 at 06:17:37PM -0300, Arnaldo Carvalho de Melo wrote:
> When removing the BPF event for perf a feature test that checks if the
> llvm devel files are availabe was removed but that is also used by
> bpftool.
>
> bpftool uses it to decide what kind of disassembly it will use: llvm or
> binutils based.
>
> Removing the tools/build/feature/test-llvm.cpp file made bpftool to
> always fallback to binutils disassembly, even with the llvm devel files
> installed, fix it by restoring just that small test-llvm.cpp test file.
>
> Fixes: 56b11a2126bf2f42 ("perf bpf: Remove support for embedding clang for compiling BPF events (-e foo.c)")
> Reported-by: Manu Bretelle <[email protected]>
> Cc: Adrian Hunter <[email protected]>
> Cc: Alexander Shishkin <[email protected]>
> Cc: Andi Kleen <[email protected]>
> Cc: Andrii Nakryiko <[email protected]>
> Cc: Anshuman Khandual <[email protected]>
> Cc: Carsten Haitzler <[email protected]>
> Cc: Eduard Zingerman <[email protected]>
> Cc: Fangrui Song <[email protected]>
> Cc: He Kuang <[email protected]>
> Cc: Ian Rogers <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: James Clark <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> Cc: Kan Liang <[email protected]>
> Cc: Leo Yan <[email protected]>
> Cc: [email protected]
> Cc: Madhavan Srinivasan <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Cc: Nathan Chancellor <[email protected]>
> Cc: Naveen N. Rao <[email protected]>
> Cc: Nick Desaulniers <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Quentin Monnet <[email protected]>
> Cc: Ravi Bangoria <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Tiezhu Yang <[email protected]>
> Cc: Tom Rix <[email protected]>
> Cc: Wang Nan <[email protected]>
> Cc: Wang ShaoBo <[email protected]>
> Cc: Yang Jihong <[email protected]>
> Cc: Yonghong Song <[email protected]>
> Cc: YueHaibing <[email protected]>
> Link: https://lore.kernel.org/lkml/[email protected]
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
> ---
> tools/build/feature/test-llvm.cpp | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
> create mode 100644 tools/build/feature/test-llvm.cpp
>
> diff --git a/tools/build/feature/test-llvm.cpp b/tools/build/feature/test-llvm.cpp
> new file mode 100644
> index 0000000000000000..88a3d1bdd9f6978e
> --- /dev/null
> +++ b/tools/build/feature/test-llvm.cpp
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "llvm/Support/ManagedStatic.h"
> +#include "llvm/Support/raw_ostream.h"
> +#define NUM_VERSION (((LLVM_VERSION_MAJOR) << 16) + (LLVM_VERSION_MINOR << 8) + LLVM_VERSION_PATCH)
> +
> +#if NUM_VERSION < 0x030900
> +# error "LLVM version too low"
> +#endif
> +int main()
> +{
> + llvm::errs() << "Hello World!\n";
> + llvm::llvm_shutdown();
> + return 0;
> +}

Thanks for the quick turnaround!

2023-10-19 22:54:23

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH 1/1] tools build: Fix llvm feature detection, still used by bpftool

On Thu, Oct 19, 2023 at 2:17 PM Arnaldo Carvalho de Melo
<[email protected]> wrote:
>
> When removing the BPF event for perf a feature test that checks if the
> llvm devel files are availabe was removed but that is also used by
> bpftool.
>
> bpftool uses it to decide what kind of disassembly it will use: llvm or
> binutils based.
>
> Removing the tools/build/feature/test-llvm.cpp file made bpftool to
> always fallback to binutils disassembly, even with the llvm devel files
> installed, fix it by restoring just that small test-llvm.cpp test file.
>
> Fixes: 56b11a2126bf2f42 ("perf bpf: Remove support for embedding clang for compiling BPF events (-e foo.c)")

Should we route this through the bpf-next tree to get the fix for
bpftool into Github mirror ASAP?

> Reported-by: Manu Bretelle <[email protected]>
> Cc: Adrian Hunter <[email protected]>
> Cc: Alexander Shishkin <[email protected]>
> Cc: Andi Kleen <[email protected]>
> Cc: Andrii Nakryiko <[email protected]>
> Cc: Anshuman Khandual <[email protected]>
> Cc: Carsten Haitzler <[email protected]>
> Cc: Eduard Zingerman <[email protected]>
> Cc: Fangrui Song <[email protected]>
> Cc: He Kuang <[email protected]>
> Cc: Ian Rogers <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: James Clark <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> Cc: Kan Liang <[email protected]>
> Cc: Leo Yan <[email protected]>
> Cc: [email protected]
> Cc: Madhavan Srinivasan <[email protected]>
> Cc: Mark Rutland <[email protected]>
> Cc: Namhyung Kim <[email protected]>
> Cc: Nathan Chancellor <[email protected]>
> Cc: Naveen N. Rao <[email protected]>
> Cc: Nick Desaulniers <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Quentin Monnet <[email protected]>
> Cc: Ravi Bangoria <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Tiezhu Yang <[email protected]>
> Cc: Tom Rix <[email protected]>
> Cc: Wang Nan <[email protected]>
> Cc: Wang ShaoBo <[email protected]>
> Cc: Yang Jihong <[email protected]>
> Cc: Yonghong Song <[email protected]>
> Cc: YueHaibing <[email protected]>
> Link: https://lore.kernel.org/lkml/[email protected]
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
> ---
> tools/build/feature/test-llvm.cpp | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
> create mode 100644 tools/build/feature/test-llvm.cpp
>
> diff --git a/tools/build/feature/test-llvm.cpp b/tools/build/feature/test-llvm.cpp
> new file mode 100644
> index 0000000000000000..88a3d1bdd9f6978e
> --- /dev/null
> +++ b/tools/build/feature/test-llvm.cpp
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "llvm/Support/ManagedStatic.h"
> +#include "llvm/Support/raw_ostream.h"
> +#define NUM_VERSION (((LLVM_VERSION_MAJOR) << 16) + (LLVM_VERSION_MINOR << 8) + LLVM_VERSION_PATCH)
> +
> +#if NUM_VERSION < 0x030900
> +# error "LLVM version too low"
> +#endif
> +int main()
> +{
> + llvm::errs() << "Hello World!\n";
> + llvm::llvm_shutdown();
> + return 0;
> +}
> --
> 2.41.0
>