-
Notifications
You must be signed in to change notification settings - Fork 417
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
argument parsers improvements #4279
Open
geyslan
wants to merge
14
commits into
aquasecurity:main
Choose a base branch
from
geyslan:parse-args-optm-cont
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
+1,200
−1,147
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as outdated.
This comment was marked as outdated.
geyslan
force-pushed
the
parse-args-optm-cont
branch
10 times, most recently
from
September 16, 2024 22:03
a349ec7
to
58a8fb4
Compare
This comment was marked as duplicate.
This comment was marked as duplicate.
After all refactor done, I pretend to split it in different files. I also have an idea to try to reduce ParseArgs() logic, but gonna play with it only after finishing the whole data parsers. |
geyslan
force-pushed
the
parse-args-optm-cont
branch
from
September 17, 2024 14:59
58a8fb4
to
7eb0e8f
Compare
geyslan
force-pushed
the
parse-args-optm-cont
branch
from
September 27, 2024 13:57
7eb0e8f
to
32999c7
Compare
Try as much as possible to use values defined in the C system, avoiding entering incorrect values.
Iterate slice instead of conditional branch. Return string only instead of the Argument type since it's the only value used. | BenchmarkParseCloneFlags-32 | Before | After | |-----------------------------|----------|--------| | Execution Time (ns/op) | 1117 | 696.7 | | Memory Allocated (B/op) | 1192 | 736 | | Allocations per Operation | 27 | 25 |
When possible iterates slice instead of conditional branch. Return string only instead of the Argument type since it's the only value used. Fix logic, since it wasn't printing O_RDONLY flag alone.
Iterate slice instead of conditional branch. Return string only instead of the Argument type since it's the only value used. Fix logic, since it wasn't printing F_OK flag alone.
Introduce ParseFaccessatFlag to parse faccessat flags. Introduce ParseFchmodatFlag to parse fchmodat flags. Iterate slice instead of conditional branch. Return string only instead of the Argument type since it's the only value used. Fix ParseExecFlag (now ParseExecveatFlag) to check only related flags.
Add missing flags to ParseCapability: CAP_PERFMON, CAP_BPF and CAP_CHECKPOINT_RESTORE Use slice instead of maps. This allows for direct access to values via index. Return string only instead of the Argument type since it's the only value used.
Add missing flags to ParsePrctlOption: PR_SET_IO_FLUSHER, PR_GET_IO_FLUSHER, PR_SET_SYSCALL_USER_DISPATCH, PR_PAC_SET_ENABLED_KEYS, PR_PAC_GET_ENABLED_KEYS, PR_SCHED_CORE, PR_SME_SET_VL, PR_SME_GET_VL, PR_SET_MDWE, PR_GET_MDWE, PR_SET_MEMORY_MERGE and PR_GET_MEMORY_MERGE. Iterate slice instead of fetching a map. Return string only instead of the Argument type since it's the only value used.
Add missing flags to ParseBPFCmd: BPF_PROG_BIND_MAP, BPF_TOKEN_CREATE Use slice instead of maps. This allows for direct access to values via index. Return string only instead of the Argument type since it's the only value used.
geyslan
force-pushed
the
parse-args-optm-cont
branch
from
October 17, 2024 12:32
32999c7
to
6e04d2f
Compare
Add missing flags to ParsePtraceRequestArgument: PTRACE_GET_THREAD_AREA, PTRACE_SET_THREAD_AREA, PTRACE_ARCH_PRCTL, PTRACE_SYSEMU, PTRACE_SYSEMU_SINGLESTEP, PTRACE_SINGLEBLOCK, PTRACE_GET_RSEQ_CONFIGURATION, PTRACE_SET_SYSCALL_USER_DISPATCH_CONFIG and PTRACE_GET_SYSCALL_USER_DISPATCH_CONFIG. Iterate slice instead of fetching a map. Return string only instead of the Argument type since it's the only value used.
Use slice instead of maps. This allows for direct access to values via index. Return string only instead of the Argument type since it's the only value used.
Add missing flag to ParseSocketDomainArgument: AF_MCTP Use slice instead of maps. This allows for direct access to values via index. Return string only instead of the Argument type since it's the only value used.
syscalls with dirfd arg now parse for special case AT_FDCWD when ParseArgumentsFDs is true.
geyslan
force-pushed
the
parse-args-optm-cont
branch
from
October 17, 2024 13:39
6e04d2f
to
62541c2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
1. Explain what the PR does
This is a sequence of the effort already started by #4200. As it was getting huge, the continuation will come after.
Improvements include reducing the size of the parser logic by using slices instead of maps whenever possible. This allows for direct access to values via index, and when direct access isn't feasible, iterating through slices is generally more efficient than fetching values from a map.
Return string only instead of the Argument type since it's the only value used.
This PR reduces the size of the final binary by ~56KB.
62541c2 chore: add todo comment
5d200ec feat: parse dirfd for special case AT_FDCWD
ced30bc perf: chore: add flag to ParseSocketDomainArgument
ecb756c perf: chore: improve ParseSocketcallCall
c36ce9c perf: chore: add flags ParsePtraceRequestArgument
1a9dea4 perf: chore: add missing flags to ParseBPFCmd
5358fcd perf: chore: add missing flags to ParsePrctlOption
e076a4a perf: chore: add missing flags to ParseCapability
397600d feat: perf: fix: AT flags parsing
8313914 perf: fix: improve ParseAccessMode
d70102e perf: fix: improve ParseOpenFlagArgument
1307fb5 perf: improve ParseCloneFlags
4f7adae chore: use system values from C includes
6182f4d chore: interface is used internally
5d200ec feat: parse dirfd for special case AT_FDCWD
ced30bc perf: chore: add flag to ParseSocketDomainArgument
ecb756c perf: chore: improve ParseSocketcallCall
c36ce9c perf: chore: add flags ParsePtraceRequestArgument
1a9dea4 perf: chore: add missing flags to ParseBPFCmd
5358fcd perf: chore: add missing flags to ParsePrctlOption
e076a4a perf: chore: add missing flags to ParseCapability
397600d feat: perf: fix: AT flags parsing
8313914 perf: fix: improve ParseAccessMode
d70102e perf: fix: improve ParseOpenFlagArgument
1307fb5 perf: improve ParseCloneFlags
4f7adae chore: use system values from C includes
2. Explain how to test it
3. Other comments