Add support for compiling BPF object file from multiple input files #1063
Conversation
Introduce the BpfObjBuilder type which encapsulates the logic for compiling a .bpf.c file into an object file. Right now this file duplicates a bunch of the logic from SkeletonBuilder::build(), but eventually it hopefully will replace it. Having this type available will help us untangle the mess that is our build_single(), build(), compile_one(), compile() functions. It will also be developed into an entity that is able to produce an object file from multiple input sources. Signed-off-by: Daniel Müller <[email protected]>
Move the logic for dealing with the __TARGET_ARCH definition from the compile_one() function into BpfObjBuilder. Note that this change is not semantic-preserving as-is, because we have an additional compile_one() invocation that is now lacking this logic. However, that will be replaced subsequently and until then this is not a major issue. Signed-off-by: Daniel Müller <[email protected]>
Merge the core functionality provided by the compile_one() function into the BpfObjBuilder. Adjust the compile() function to use BpfObjBuilder instead of relying on compile_one(). In so doing we fix the lack of disabling of stack protector logic on this path, while consolidating more of the necessary "build" infrastructure inside of BpfObjBuilder. Signed-off-by: Daniel Müller <[email protected]>
The compile() function no longer does anything with the project's target directory. Hence, remove the logic surrounding it. Signed-off-by: Daniel Müller <[email protected]>
Inline the compile() function body directly into the build() function. compile() is just not sufficiently telling in any way and we do not want callers of it to proliferate. Signed-off-by: Daniel Müller <[email protected]>
Rename the build() function to build_project(), because that's what it does. Signed-off-by: Daniel Müller <[email protected]>
Introduce the BpfObjBuilder::with_compiler_args() helper for invoking a block of code with the final set of compiler arguments. Signed-off-by: Daniel Müller <[email protected]>
Building a BPF object file already invokes the libbpf linker in order to strip DWARF debug information from the resulting file. In the future we would like to link multiple files instead of only a single one, so refactor the code in such a way that we have a dedicated link step in BpfObjBuilder::build(). Signed-off-by: Daniel Müller <[email protected]>
Add support for building a BPF object file from multiple input files, by first compiling each separately and the linking everything together. Signed-off-by: Daniel Müller <[email protected]>
danielocfb
force-pushed
the
topic/link-multi
branch
from
8225b2e to
016326e
Compare
There was a problem hiding this comment.
LGTM, I think there is just one potential issue with the API: compiler args. It seems like currently you only allow to specify compiler args for all .bpf.c files and there is no way to augment it with some extra stuff for individual files.
This seems like something that can be added in the future, treating current compiler_args as "common compiler args", so doesn't seem blocking. But I wanted to point it out anyways.
|
Thanks for the review. Fair point, though can you think of any concrete examples where this is a blocker? I mean obviously we can construct something, but I am more asking about partical experience where this would be an unsolvable limitation. We could rework the API to add individual files (similar to what we do with the |
|
Typically (and by "typically" it's still pretty rare, but still...) it would be the need to do some file-specific |
|
from API perspective, having common compiler args like you do makes sense and is convenient for 99% of cases. Perhaps the solution here is to have one extra API when adding individual file to also specify compiler args (or just change existing API to access Option[] argument for extra args, which would require all those None`s in common case, so I don't know). |
|
Okay, yeah, I will keep that in mind for when/if we expose this functionality publicly. Until then it is certainly a non-issue. |
d-e-s-o
force-pushed
the
topic/link-multi
branch
from
016326e to
07231df
Compare
Please see individual commits for descriptions. This is internal logic only at this point.