Bug 1178241

Summary: Building cilium on x86_64 fails after LLVM 11 update
Product: [openSUSE] openSUSE Tumbleweed Reporter: Aaron Puchert <aaronpuchert>
Component: KubicAssignee: Michał Rostecki <mrostecki>
Status: RESOLVED FIXED QA Contact: E-mail List <qa-bugs>
Severity: Normal    
Priority: P5 - None CC: dmueller, mrostecki
Version: Current   
Target Milestone: ---   
Hardware: Other   
OS: Other   
Whiteboard:
Found By: --- Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---
Bug Depends on:    
Bug Blocks: 1184920    

Description Aaron Puchert 2020-10-28 22:54:28 UTC
There is a new default warning, and cilium builds with -Werror:

/home/abuild/rpmbuild/BUILD/go/src/github.com/cilium/cilium/bpf/lib/dbg.h:168:13: error: cast to smaller integer type '__u32' (aka 'unsigned int') from '__u32 *' (aka 'unsigned int *') [-Werror,-Wpointer-to-int-cast]
                .source = EVENT_SOURCE,
                          ^~~~~~~~~~~~
bpf_lxc.c:21:22: note: expanded from macro 'EVENT_SOURCE'
#define EVENT_SOURCE LXC_ID
                     ^~~~~~
/home/abuild/rpmbuild/BUILD/go/src/github.com/cilium/cilium/bpf/lxc_config.h:28:16: note: expanded from macro 'LXC_ID'
#define LXC_ID fetch_u32(LXC_ID)
               ^~~~~~~~~~~~~~~~~
/home/abuild/rpmbuild/BUILD/go/src/github.com/cilium/cilium/bpf/lib/utils.h:130:22: note: expanded from macro 'fetch_u32'
#define fetch_u32(x) __fetch(x)
                     ^~~~~~~~~~
/home/abuild/rpmbuild/BUILD/go/src/github.com/cilium/cilium/bpf/lib/utils.h:122:21: note: expanded from macro '__fetch'
# define __fetch(x) (__u32)(&(x))
                    ^~~~~~~~~~~~~

There are more occurrences. The warning seems valid to me: pointers on x86_64 are 64-bit wide, so the conversion to a 32-bit integer is lossy. If this is intended, convert to uintptr_t first, then to __u32:

# define __fetch(x) (__u32)(uintptr_t)(&(x))

That might require #include <stdint.h>.

Generally only conversions to uintptr_t are guaranteed to have no information loss, i.e. you can convert them back to a pointer and get the original pointer back.

LLVM 11 is currently staged in K, so you can see the full log here:
https://build.opensuse.org/package/live_build_log/openSUSE:Factory:Staging:K/cilium/standard/x86_64
Comment 1 Aaron Puchert 2020-10-28 23:02:30 UTC
To quote C11 [1], 6.3.2.3.6:

> Any pointer type may be converted to an integer type. Except as previously
> specified, the result is implementation-defined. If the result cannot be
> represented in the integer type, the behavior is undefined. The result need
> not be in the range of values of any integer type.

So this is warning about undefined behavior.

However, 7.20.1.4 comes to the rescue:

> The following type designates a signed integer type with the property that any
> valid pointer to void can be converted to this type, then converted back to
> pointer to void, and the result will compare equal to the original pointer:
>     intptr_t
> The following type designates an unsigned integer type with the property that
> any valid pointer to void can be converted to this type, then converted back
> to pointer to void, and the result will compare equal to the original pointer:
>     uintptr_t
> These types are optional.

[1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf
Comment 2 Michał Rostecki 2020-10-30 16:06:01 UTC
Cilium doesn't use stdint.h on purpose - we don't want to have a dependency on glibc.

But there is already a fix which does:

# define __fetch(X)		(__u32)(__u64)(&(X))

https://github.com/cilium/cilium/commit/e43c2fff3749614a0d5908df6f15db6ea96aed94

Let me push an update.
Comment 3 Michał Rostecki 2020-10-30 20:28:10 UTC
Fix here: https://build.opensuse.org/request/show/845100
Comment 4 Aaron Puchert 2020-12-13 00:23:41 UTC
Fix landed. (Quite some time ago in fact.)
Comment 5 Aaron Puchert 2021-04-22 21:51:14 UTC
Could you bring this fix to openSUSE:Backports:SLE-15-SP3/cilium as well?

It's currently broken because we updated to LLVM 11 by default (bug 1184920).
Comment 6 Aaron Puchert 2021-04-23 15:38:32 UTC
(In reply to Aaron Puchert from comment #5)
> Could you bring this fix to openSUSE:Backports:SLE-15-SP3/cilium as well?
> 
> It's currently broken because we updated to LLVM 11 by default (bug 1184920).
Fixed by Max Lin via https://build.opensuse.org/request/show/887868.