diff mbox series

[bpf-next] selftest/bpf: fix compilation on clang 11

Message ID 20201125035255.17970-1-andreimatei1@gmail.com (mailing list archive)
State Accepted
Delegated to: BPF
Headers show
Series [bpf-next] selftest/bpf: fix compilation on clang 11 | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for bpf-next
netdev/subject_prefix success Link
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning CHECK: Please don't use multiple blank lines
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Andrei Matei Nov. 25, 2020, 3:52 a.m. UTC
Before this patch, profiler.inc.h wouldn't compile with clang-11 (before
the __builtin_preserve_enum_value LLVM builtin was introduced in
https://reviews.llvm.org/D83242).
Another test that uses this builtin (test_core_enumval) is conditionally
skipped if the compiler is too old. In that spirit, this patch inhibits
part of populate_cgroup_info(), which needs this CO-RE builtin. The
selftests build again on clang-11.  The affected test (the profiler
test) doesn't pass on clang-11 because it's missing
https://reviews.llvm.org/D85570, but at least the test suite as a whole
compiles. The test's expected failure is already called out in the
README.

Signed-off-by: Andrei Matei <andreimatei1@gmail.com>
---
 tools/testing/selftests/bpf/progs/profiler.inc.h | 3 +++
 1 file changed, 3 insertions(+)

Comments

Yonghong Song Nov. 25, 2020, 6:28 a.m. UTC | #1
On 11/24/20 7:52 PM, Andrei Matei wrote:
> Before this patch, profiler.inc.h wouldn't compile with clang-11 (before
> the __builtin_preserve_enum_value LLVM builtin was introduced in
> https://reviews.llvm.org/D83242 ).
> Another test that uses this builtin (test_core_enumval) is conditionally
> skipped if the compiler is too old. In that spirit, this patch inhibits
> part of populate_cgroup_info(), which needs this CO-RE builtin. The
> selftests build again on clang-11.  The affected test (the profiler
> test) doesn't pass on clang-11 because it's missing
> https://reviews.llvm.org/D85570 , but at least the test suite as a whole
> compiles. The test's expected failure is already called out in the
> README.
> 
> Signed-off-by: Andrei Matei <andreimatei1@gmail.com>

Thanks for the fix! This change won't impact correctness as
profiler.c mostly to test verifier.

Acked-by: Yonghong Song <yhs@fb.com>
Florian Lehner Nov. 25, 2020, 8:20 p.m. UTC | #2
On Tue, Nov 24, 2020 at 10:52:55PM -0500, Andrei Matei wrote:
> Before this patch, profiler.inc.h wouldn't compile with clang-11 (before
> the __builtin_preserve_enum_value LLVM builtin was introduced in
> https://reviews.llvm.org/D83242).
> Another test that uses this builtin (test_core_enumval) is conditionally
> skipped if the compiler is too old. In that spirit, this patch inhibits
> part of populate_cgroup_info(), which needs this CO-RE builtin. The
> selftests build again on clang-11.  The affected test (the profiler
> test) doesn't pass on clang-11 because it's missing
> https://reviews.llvm.org/D85570, but at least the test suite as a whole
> compiles. The test's expected failure is already called out in the
> README.
> 
> Signed-off-by: Andrei Matei <andreimatei1@gmail.com>

Tested-by: Florian Lehner <dev@der-flo.net>

Thanks for the fix!

> ---
>  tools/testing/selftests/bpf/progs/profiler.inc.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/progs/profiler.inc.h b/tools/testing/selftests/bpf/progs/profiler.inc.h
> index 30982a7e4d0f..b79d7f688655 100644
> --- a/tools/testing/selftests/bpf/progs/profiler.inc.h
> +++ b/tools/testing/selftests/bpf/progs/profiler.inc.h
> @@ -256,6 +256,8 @@ static INLINE void* populate_cgroup_info(struct cgroup_data_t* cgroup_data,
>  		BPF_CORE_READ(task, nsproxy, cgroup_ns, root_cset, dfl_cgrp, kn);
>  	struct kernfs_node* proc_kernfs = BPF_CORE_READ(task, cgroups, dfl_cgrp, kn);
>  
> +
> +#if __has_builtin(__builtin_preserve_enum_value)
>  	if (ENABLE_CGROUP_V1_RESOLVER && CONFIG_CGROUP_PIDS) {
>  		int cgrp_id = bpf_core_enum_value(enum cgroup_subsys_id___local,
>  						  pids_cgrp_id___local);
> @@ -275,6 +277,7 @@ static INLINE void* populate_cgroup_info(struct cgroup_data_t* cgroup_data,
>  			}
>  		}
>  	}
> +#endif
>  
>  	cgroup_data->cgroup_root_inode = get_inode_from_kernfs(root_kernfs);
>  	cgroup_data->cgroup_proc_inode = get_inode_from_kernfs(proc_kernfs);
> -- 
> 2.27.0
>
Daniel Borkmann Nov. 25, 2020, 11:37 p.m. UTC | #3
On 11/25/20 4:52 AM, Andrei Matei wrote:
> Before this patch, profiler.inc.h wouldn't compile with clang-11 (before
> the __builtin_preserve_enum_value LLVM builtin was introduced in
> https://reviews.llvm.org/D83242).
> Another test that uses this builtin (test_core_enumval) is conditionally
> skipped if the compiler is too old. In that spirit, this patch inhibits
> part of populate_cgroup_info(), which needs this CO-RE builtin. The
> selftests build again on clang-11.  The affected test (the profiler
> test) doesn't pass on clang-11 because it's missing
> https://reviews.llvm.org/D85570, but at least the test suite as a whole
> compiles. The test's expected failure is already called out in the
> README.
> 
> Signed-off-by: Andrei Matei <andreimatei1@gmail.com>

Looks good, applied, thanks!
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/progs/profiler.inc.h b/tools/testing/selftests/bpf/progs/profiler.inc.h
index 30982a7e4d0f..b79d7f688655 100644
--- a/tools/testing/selftests/bpf/progs/profiler.inc.h
+++ b/tools/testing/selftests/bpf/progs/profiler.inc.h
@@ -256,6 +256,8 @@  static INLINE void* populate_cgroup_info(struct cgroup_data_t* cgroup_data,
 		BPF_CORE_READ(task, nsproxy, cgroup_ns, root_cset, dfl_cgrp, kn);
 	struct kernfs_node* proc_kernfs = BPF_CORE_READ(task, cgroups, dfl_cgrp, kn);
 
+
+#if __has_builtin(__builtin_preserve_enum_value)
 	if (ENABLE_CGROUP_V1_RESOLVER && CONFIG_CGROUP_PIDS) {
 		int cgrp_id = bpf_core_enum_value(enum cgroup_subsys_id___local,
 						  pids_cgrp_id___local);
@@ -275,6 +277,7 @@  static INLINE void* populate_cgroup_info(struct cgroup_data_t* cgroup_data,
 			}
 		}
 	}
+#endif
 
 	cgroup_data->cgroup_root_inode = get_inode_from_kernfs(root_kernfs);
 	cgroup_data->cgroup_proc_inode = get_inode_from_kernfs(proc_kernfs);