FYI.The latest llvm trunk will not emit errors for static variables. The patch also gives detailed information how to relate a particular static variable to a particular relocation. https://reviews.llvm.org/rL354954 Thanks, Yonghong On Fri, Feb 15, 2019 at 12:18 PM Y Song <ys114321@xxxxxxxxx> wrote: > > On Thu, Feb 14, 2019 at 11:16 PM Joe Stringer <joe@xxxxxxxxxxx> wrote: > > > > On Thu, 14 Feb 2019 at 21:39, Y Song <ys114321@xxxxxxxxx> wrote: > > > > > > On Mon, Feb 11, 2019 at 4:48 PM Joe Stringer <joe@xxxxxxxxxxx> wrote: > > > > > > > > Support loads of static 32-bit data when BPF writers make use of > > > > convenience macros for accessing static global data variables. A later > > > > patch in this series will demonstrate its usage in a selftest. > > > > > > > > As of LLVM-7, this technique only works with 32-bit data, as LLVM will > > > > complain if this technique is attempted with data of other sizes: > > > > > > > > LLVM ERROR: Unsupported relocation: try to compile with -O2 or above, > > > > or check your static variable usage > > > > > > A little bit clarification from compiler side. > > > The above compiler error is to prevent people use static variables since current > > > kernel/libbpf does not handle this. The compiler only warns if .bss or > > > .data section > > > has more than one definitions. The first definition always has section offset 0 > > > and the compiler did not warn. > > > > Ah, interesting. I observed that warning when I attempted to define > > global variables of multiple sizes, and I thought also with sizes > > other than 32-bit. This clarifies things a bit, thanks. > > > > For the .bss my observation was that if you had a definition like: > > > > static int a = 0; > > > > Then this will be placed into .bss, hence why I looked into the > > approach from this patch for patch 3 as well. > > > > > The restriction is a little strange. To only work with 32-bit data is > > > not a right > > > statement. The following are some examples. > > > > > > The following static variable definitions will succeed: > > > static int a; /* one in .bss */ > > > static long b = 2; /* one in .data */ > > > > > > The following definitions will fail as both in .bss. > > > static int a; > > > static int b; > > > > > > The following definitions will fail as both in .data: > > > static char a = 2; > > > static int b = 3; > > > > Are there type restrictions or something? I've been defining multiple > > There is no type restrictions. > -bash-4.4$ cat g.c > struct t { > int a; > char b; > long c; > }; > static volatile struct t v; > int test() { return v.a + v.b; } > -bash-4.4$ clang -O2 -g -c -target bpf g.c > -bash-4.4$ > > > static uint32_t and using them per the approach in this patch series > > without hitting this compiler assertion. > > -bash-4.4$ cat g1.c > static volatile int a; > static volatile int b; > int test() { return a + b; } > -bash-4.4$ clang -O2 -g -c -target bpf g1.c > fatal error: error in backend: Unsupported relocation: try to compile > with -O2 or above, or check your static variable > usage > -bash-4.4$ > > > > > > Using global variables can prevent compiler errors. > > > maps are defined as globals and the compiler does not > > > check whether a particular global variable is defining a map or not. > > > > > > If you just use static variable like below > > > static int a = 2; > > > without potential assignment to a, the compiler will replace variable > > > a with 2 at compile time. > > > An alternative is to define like below > > > static volatile int a = 2; > > > You can get a "load" for variable "a" in the bpf load even if there is > > > no assignment to a. > > > > I'll take a closer look at this too. > > > > > Maybe now is the time to remove the compiler assertions as > > > libbpf/kernel starts to > > > handle static variables? > > > > I don't understand why those assertions exists in this form. It > > already allows code which will not load with libbpf (ie generate any > > .data/.bss), does it help prevent unexpected situations for > > developers? > > The error is introduced by the following llvm commit: > commit 39184e407cd937f2f20d3f61eec205925bae7b13 > Author: Yonghong Song <yhs@xxxxxx> > Date: Wed Aug 22 21:21:03 2018 +0000 > > bpf: fix an assertion in BPFAsmBackend applyFixup() > > Fix bug https://bugs.llvm.org/show_bug.cgi?id=38643 > > In BPFAsmBackend applyFixup(), there is an assertion for FixedValue to be 0. > This may not be true, esp. for optimiation level 0. > For example, in the above bug, for the following two > static variables: > @bpf_map_lookup_elem = internal global i8* (i8*, i8*)* > inttoptr (i64 1 to i8* (i8*, i8*)*), align 8 > @bpf_map_update_elem = internal global i32 (i8*, i8*, i8*, i64)* > inttoptr (i64 2 to i32 (i8*, i8*, i8*, i64)*), align 8 > > The static variable @bpf_map_update_elem will have a symbol > offset of 8 and a FK_SecRel_8 with FixupValue 8 will cause > the assertion if llvm is built with -DLLVM_ENABLE_ASSERTIONS=ON. > > The above relocations will not exist if the program is compiled > with optimization level -O1 and above as the compiler optimizes > those static variables away. In the below error message, -O2 > is suggested as this is the common practice. > > Note that FixedValue = 0 in applyFixup() does exist and is valid, > e.g., for the global variable my_map in the above bug. The bpf > loader will process them properly for map_id's before loading > the program into the kernel. > > The static variables, which are not optimized away by compiler, > may have FK_SecRel_8 relocation with non-zero FixedValue. > > The patch removed the offending assertion and will issue > a hard error as below if the FixedValue in applyFixup() > is not 0. > $ llc -march=bpf -filetype=obj fixup.ll > LLVM ERROR: Unsupported relocation: try to compile with -O2 or above, > or check your static variable usage > > Its main purpose is to fix a behavior difference with and without > -DLLVM_ENABLE_ASSERTIONS=ON. The patch generated an error > regardless whether the compiler time assertion is turned on or not. > > It does not catch all the cases e.g., only one static variable is defined, > which needs fine tuning as there are legitimate cases (e.g., in some dwarf > sessions) where the Fixup is valid with FixedValue = 0. > > If you try to use more than onee static variables, the compiler will > print an error and let you know your potential issues. > > The question is since we are on the path to allow static variables > in the bpf programs for later patching, we probably should remove > this compiler fatal error?
