On 8/15/23 4:20 PM, Eduard Zingerman wrote:
Hi Yonghong, Jose,
I've noticed today that LLVM main started producing an error when
compiling selftest test_skeleton.c:
progs/test_skeleton.c:46:20: error: 'in_dynarr_sz' causes a section type conflict with 'in_dynarr'
46 | const volatile int in_dynarr_sz SEC(".rodata.dyn");
| ^
progs/test_skeleton.c:47:20: note: declared here
47 | const volatile int in_dynarr[4] SEC(".rodata.dyn") = { -1, -2, -3, -4 };
| ^
1 error generated.
CLNG-BPF [test_maps] test_sk_storage_trace_itself.bpf.o
make: *** [Makefile:594: /home/eddy/work/bpf-next/tools/testing/selftests/bpf/test_skeleton.bpf.o] Error 1
The code in question looks as follows:
...
const volatile int in_dynarr_sz SEC(".rodata.dyn");
const volatile int in_dynarr[4] SEC(".rodata.dyn") = { -1, -2, -3, -4 };
...
In fact, it could be simplified to the following example:
#define SEC(n) __attribute__((section(n)))
const int with_init SEC("foo") = 1;
const int no_init SEC("foo");
And error is reported for x86 build as well:
$ clang -c t.c -o /dev/null
t.c:4:11: error: 'no_init' causes a section type conflict with 'with_init'
4 | const int no_init SEC("foo");
| ^
t.c:3:11: note: declared here
3 | const int with_init SEC("foo") = 1;
| ^
1 error generated.
The error occurs because clang infers "read only" attribute for
section "foo" when `with_init` is processed and "read/write"
attributes for section "foo" when `no_init` is processed.
The attributes do not match and error is reported.
(See Sema::UnifySection, `diag::err_section_conflict` diagnostic).
The culprit is revision [1] which landed today. The main focus of that
revision is C++ and handling of structure fields marked as `mutable`.
However, it also adds a new requirement: for global value to be
considered "read only" it must have an initializer
(the `var->hasInit()` check in [2]).
GCC can handle the example above w/o any issues.
The relevant part of the C standard [3] is "6.7.3 Type qualifiers",
but it does not discuss sections, the only section-related sentence
that I found is:
160) The implementation can place a const object that is not
volatile in a read-only region of storage. Moreover, the
implementation need not allocate storage for such an object if
its address is never used.
Which does not make example at hand invalid.
Although `const` values w/o initializer do seem strange they might
have some sense if, say, linker materializes these definitions with
something useful.
Thus, it appears to me that:
- test_skeleton.c is ok and should not be changed;
- revision [1] introduced a bug and I should bring it up with upstream.
Thanks Eduard.
Please go ahead bringing the issue to upstream as a comment on [1].
Although the issue can be trivial fixed by add '= 0' initialization,
it will be still good to clarify with upstream.
I remember that we discussed 'const volatile' thing with gcc as well
and agreed it should be put into .rodata section. But I lost that
email communication. I am not sure how initialization will change this.
What do you think?
Thanks,
Eduard
[1] https://reviews.llvm.org/D156726
[2] https://github.com/llvm/llvm-project/compare/main...llvm-premerge-tests:llvm-project:phab-diff-550097#diff-edac6256ac508912a16d0165b2f8cf37123dc2f40a147dca49a34c33f1db13ddR14366
[3] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3088.pdf
