Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

Remove .text section #224

Merged
merged 3 commits into from
Nov 16, 2021
Merged

Remove .text section #224

merged 3 commits into from
Nov 16, 2021

Conversation

rhdxmr
Copy link
Collaborator

@rhdxmr rhdxmr commented Nov 15, 2021

  • Remove redbpf-macros/src/mem.rs not to include memcpy, memset, memcmp in the .text section
  • Strip .text section from the relocatable elf file

I was unconvinced why the memcmp, memcpy, memset, memmove, bcmp are defined in the redbpf-macros. Also, to strip .text section it is better to not have those definitions in .text section.

Definitions of memcpy, memset, memcmp, memmove, bcmp are
removed. Without these definitions all example probes and foniod probes
are compiled successfully with llvm11, llvm12 and llvm13. And those
probes can pass the BPF verifier. And it is unclear why these
definitions should be included by `program!` macro.

Signed-off-by: Junyeong Jeong <rhdxmr@gmail.com>
The .text section is ignored by redbpf when parsing the elf relocatable
file and tc utility fails at loading the elf relocatable file if .text
exists. So it is better to remove .text section.

Signed-off-by: Junyeong Jeong <rhdxmr@gmail.com>
Now users don't need to remove .text section in person because redbpf
handles it under the hood.

Signed-off-by: Junyeong Jeong <rhdxmr@gmail.com>
@rhdxmr rhdxmr requested a review from rsdy November 15, 2021 18:07
@rhdxmr
Copy link
Collaborator Author

rhdxmr commented Nov 15, 2021

I tried to remove .text section to solve the problem that is reported by this issue

It is evident that all new users struggle with the .text section when they try using tc action.

@rsdy
Copy link
Collaborator

rsdy commented Nov 16, 2021

Is it worth cutting a release with this? Seems like this is a great improvement for many people.

@rsdy rsdy merged commit 120d06a into main Nov 16, 2021
@rsdy rsdy deleted the remove-text-sec branch November 16, 2021 11:34
@rhdxmr
Copy link
Collaborator Author

rhdxmr commented Nov 16, 2021

@rsdy
Okay. I'll prepare release note.
And I want one more PR to get merged before release new package.
I'll send new PR in few hours.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants