Make BTF for data sections optional #675
Conversation
The default behaviour of podman is to drop all stdout/err it generates into the journal, which gets rather noisy. Signed-off-by: Timo Beckers <[email protected]>
|
I'll try to review this Wednesday afternoon after #647. |
| volatile int ctx = 0; | ||
|
|
||
| // 32 bytes wide results in a .rodata.cst32 section. | ||
| #define values \ |
There was a problem hiding this comment.
Could we just fake a .rodata.foo via explicit section annotations? Might be more robust on newer versions of LLVM as well.
There was a problem hiding this comment.
I've tried all possible ways I could think of to force this to be emitted to a particular section, but haven't managed. Since it's not a real symbol, clang doesn't support setting the section attribute on it.
| @@ -131,6 +131,12 @@ func TestLoadCollectionSpec(t *testing.T) { | |||
| SectionName: "static", | |||
| License: "MIT", | |||
| }, | |||
| "anon_const": { | |||
There was a problem hiding this comment.
Seems like there is no fix for the problem here, this means that the tests will fail on this commit? If yes, that's problematic when trying to bisect problems.
There was a problem hiding this comment.
Okay, didn't know we really cared about that, will respin the set to take this into account.
|
|
||
| copy(cpy[v.Offset:v.Offset+v.Size], b) | ||
|
|
||
| replaced[vname] = true |
There was a problem hiding this comment.
Should we allow replacing the same variable in multiple rodata sections? Seems wonky.
There was a problem hiding this comment.
What's the reasoning behind not allowing it? Detecting compiler bugs? Technically, the same var being present in multiple rodata sections would also require having 2 equally-named ELF symbols with global vis, which is wonky indeed. So, we error here instead?
There was a problem hiding this comment.
Done, added a check that makes sure we don't replace the same var twice.
There was a problem hiding this comment.
I guess this could also happen when the caller copies .rodata from a CollectionSpec and adds it under a different key in Maps.
LoadCollectionSpecFromReader marks all .rodata* sections as data sections, but loadDataSections() only freezes '.rodata'. Signed-off-by: Timo Beckers <[email protected]>
https://lore.kernel.org/bpf/CAK3+h2wcDceeGyFVDU3n7kPm=zgp7r1q4WK0=abxBsj9pyFN-g@mail.gmail.com If an anonymous value or constant (e.g. array declared in a macro) ends up in a data section, it will not be accompanied by debug info, resulting in no BTF being emitted for the value. If the section consists of only anonymous values, there could be no BTF for the section as a whole. A workaround was added to LLVM to always emit an empty BTF Datasec for .rodata specifically, but this doesn't work for special sections like .rodata.cst32 and friends. Signed-off-by: Timo Beckers <[email protected]>
Symbols for anonymous constants are somewhat irregular on older versions
of LLVM. This commit relaxes the constraints on symbols those compilers
emit map relocations against.
LLVM 7:
1: 0000000000000000 0 NOTYPE LOCAL DEFAULT 16 .Lconstinit.1
LLVM 9:
1: 0000000000000000 32 OBJECT LOCAL DEFAULT 22 .Lconstinit.1
LLVM 14 has these symbols correctly sanitized.
Signed-off-by: Timo Beckers <[email protected]>
Instead of only looking for constants in .rodata, iterate over all maps with an .rodata prefix. Moved the datasec bytes and BTF info extraction into a method on MapSpec. Lifted patchValue back into RewriteConstants. Signed-off-by: Timo Beckers <[email protected]>
This exercises support for constants in custom .rodata* sections. Signed-off-by: Timo Beckers <[email protected]>
ti-mo
force-pushed
the
tb/datasec-optional-btf
branch
from
f960522 to
352d2b3
Compare
In the past we had the limitation that string sections like `.rodata.str1.1` could not not be loaded since clang/LLVM doesn't provide BTF information for such sections. In cilium#675 we lifted the requirement to have BTF information for global data sections. This means that we can now also allow string sections to be loaded as global data. All facilities to do so are already in place, this commit just removes the check for references to sections with the `SHF_STRINGS` flag set. Fixes: cilium#741 Signed-off-by: Dylan Reimerink <[email protected]>
In the past we had the limitation that string sections like `.rodata.str1.1` could not not be loaded since clang/LLVM doesn't provide BTF information for such sections. In cilium#675 we lifted the requirement to have BTF information for global data sections. This means that we can now also allow string sections to be loaded as global data. All facilities to do so are already in place, this commit just removes the check for references to sections with the `SHF_STRINGS` flag set. Fixes: cilium#741 Signed-off-by: Dylan Reimerink <[email protected]>
In the past we had the limitation that string sections like `.rodata.str1.1` could not not be loaded since clang/LLVM doesn't provide BTF information for such sections. In cilium#675 we lifted the requirement to have BTF information for global data sections. This means that we can now also allow string sections to be loaded as global data. All facilities to do so are already in place, this commit just removes the check for references to sections with the `SHF_STRINGS` flag set. Fixes: cilium#741 Signed-off-by: Dylan Reimerink <[email protected]>
In the past we had the limitation that string sections like `.rodata.str1.1` could not not be loaded since clang/LLVM doesn't provide BTF information for such sections. In #675 we lifted the requirement to have BTF information for global data sections. This means that we can now also allow string sections to be loaded as global data. All facilities to do so are already in place, this commit just removes the check for references to sections with the `SHF_STRINGS` flag set. Fixes: #741 Signed-off-by: Dylan Reimerink <[email protected]>
This is a workaround for a compiler bug/limitation when working with arrays declared as preprocessor macros.
This bit us on two occasions:
Discussion on the list:
https://lore.kernel.org/bpf/CAK3+h2wcDceeGyFVDU3n7kPm=zgp7r1q4WK0=abxBsj9pyFN-g@mail.gmail.com
Best reviewed per commit.