Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Widespread error: implicit conversion loses integer precision: '__u64' (aka 'unsigned long long') to '__u32' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]" subsys=datapath-loader when compiling with Clang 19 #35162

Open
3 tasks done
jingyuanliang opened this issue Oct 2, 2024 · 6 comments
Labels
area/llvm Requires upstream work in LLVM. kind/community-report This was reported by a user in the Cilium community, eg via Slack. kind/enhancement This would improve or streamline existing functionality. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.

Comments

@jingyuanliang
Copy link
Contributor

jingyuanliang commented Oct 2, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Version

equal or higher than v1.16.0 and lower than v1.17.0

What happened?

Clang 19 introduced:

The -Wshorten-64-to-32 diagnostic is now grouped under -Wimplicit-int-conversion instead
of -Wconversion. Fixes #GH69444.

which causes the current code to fail compilation with the flags currently in use.

How can we reproduce the issue?

Run tests on a system with Clang 19 installed, such as running make -C bpf/tests all, but this also affects unit tests like:

=== RUN   TestCompile
=== RUN   TestCompile/obj:bpf_lxc.o
time="2024-10-01T22:39:35Z" level=error msg="Failed to compile bpf_lxc.o: exit status 1" compiler-pid=66176 subsys=datapath-loader
time="2024-10-01T22:39:35Z" level=warning msg="In file included from ../../../bpf/bpf_lxc.c:20:" subsys=datapath-loader
time="2024-10-01T22:39:35Z" level=warning msg="In file included from ../../../bpf/lib/auth.h:7:" subsys=datapath-loader
time="2024-10-01T22:39:35Z" level=warning msg="../../../bpf/lib/common.h:202:22: error: implicit conversion loses integer precision: 'const __u64' (aka 'const unsigned long long') to '__u32' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]" subsys=datapath-loader
time="2024-10-01T22:39:35Z" level=warning msg="  202 |                 ctx_pull_data(ctx, tot_len);" subsys=datapath-loader
time="2024-10-01T22:39:35Z" level=warning msg="      |                 ~~~~~~~~~~~~~      ^~~~~~~" subsys=datapath-loader
time="2024-10-01T22:39:35Z" level=warning msg="In file included from ../../../bpf/bpf_lxc.c:20:" subsys=datapath-loader

Cilium Version

1.16.2

Kernel Version

N/A

Kubernetes Version

N/a

Regression

No response

Sysdump

No response

Relevant log output

No response

Anything else?

Regardless how we handle this, we should publish the "highest tested" clang version, instead of a broad "clang LLVM >= 10.0".

Cilium Users Document

  • Are you a user of Cilium? Please add yourself to the Users doc

Code of Conduct

  • I agree to follow this project's Code of Conduct
@jingyuanliang jingyuanliang added kind/bug This is a bug in the Cilium logic. kind/community-report This was reported by a user in the Cilium community, eg via Slack. needs/triage This issue requires triaging to establish severity and next steps. labels Oct 2, 2024
@bowei
Copy link
Member

bowei commented Oct 2, 2024

It would help if you edited the above description with the specific commands run to hit the issue. I'm assuming this is some form of git clone; ... ; make test.

@bowei
Copy link
Member

bowei commented Oct 2, 2024

Also, did you patch the repo to update to clang-19? Post the patch as well if that is the case.

@jingyuanliang
Copy link
Contributor Author

It would help if you edited the above description with the specific commands run to hit the issue. I'm assuming this is some form of git clone; ... ; make test.

done

@jingyuanliang
Copy link
Contributor Author

Also, did you patch the repo to update to clang-19? Post the patch as well if that is the case.

Didn't patch the repo. Those tests (running from make) take clang from the running system.

@bowei
Copy link
Member

bowei commented Oct 2, 2024

You should give the clang --version and Linux distribution used.

@julianwiedmann julianwiedmann added kind/enhancement This would improve or streamline existing functionality. area/llvm Requires upstream work in LLVM. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. and removed kind/bug This is a bug in the Cilium logic. needs/triage This issue requires triaging to establish severity and next steps. labels Oct 3, 2024
@julianwiedmann
Copy link
Member

as bowei suggested, Cilium currently doesn't support LLVM 19. There's ongoing work to bump us from 17 to 18, maybe you'd like to contribute there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/llvm Requires upstream work in LLVM. kind/community-report This was reported by a user in the Cilium community, eg via Slack. kind/enhancement This would improve or streamline existing functionality. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
None yet
Development

No branches or pull requests

3 participants