Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Logging errors results in abnormal termination (abort) #3163

Closed
ajor opened this issue May 13, 2024 · 2 comments · Fixed by #3164
Closed

Logging errors results in abnormal termination (abort) #3163

ajor opened this issue May 13, 2024 · 2 comments · Fixed by #3164
Assignees
@jordalgo
Labels
bug Something isn't working reliability Correctness and polish work
Milestone
v0.21.0

Comments

@ajor
Copy link
Member

ajor commented May 13, 2024

Since #3091, errors which trigger bpftrace to terminate do so by calling abort() and core-dumping. This is a bad user experience.

Repro

Trigger an error:

# bpftrace -e 'kretprobe:_raw_spin_lock {}'
Attaching 1 probe...
ERROR: kretprobe:_raw_spin_lock can't be used as it might lock up your system.
Aborted

Expected Behaviour

bpftrace should exit by returning from main with a non-zero error code.
@ajor ajor added bug Something isn't working reliability Correctness and polish work labels May 13, 2024
@ajor ajor added this to the v0.21.0 milestone May 13, 2024
@jordalgo
Copy link
Contributor

@ajor Is the only issue here is the calling of abort? Can we just call exit(1) instead in the LOG(FATAL) path as this is dedicated to user errors?

@ajor
Copy link
Member Author

ajor commented May 14, 2024

I think throwing an exception from the LOG(FATAL) path and catching it in main() would be more future-proof: it'll perform call destructors for all living objects so any RAII cleanups will run, and it'll work almost without changes for bpftrace-as-a-library (we can create a no-throwing wrapper around the library later).

jordalgo pushed a commit to jordalgo/bpftrace that referenced this issue May 15, 2024
Remove LOG(FATAL)
186ba5a 
Replace these with either LOG(BUG),
LOG(ERROR) and exit(1) in main.cpp, or
`throw std::runtime_error`.

This is to avoid calling `abort` for user
errors and removes the confusion between
`LOG(ERROR)` and `LOG(FATAL)`.

Issue: bpftrace#3163
jordalgo pushed a commit to jordalgo/bpftrace that referenced this issue May 15, 2024
Remove LOG(FATAL)
7ba631f 
Replace these with either LOG(BUG),
LOG(ERROR) and exit(1) in main.cpp, or
`throw std::runtime_error`.

This is to avoid calling `abort` for user
errors and removes the confusion between
`LOG(ERROR)` and `LOG(FATAL)`.

Issue: bpftrace#3163
@jordalgo jordalgo mentioned this issue May 15, 2024
Merged
3 tasks
jordalgo pushed a commit to jordalgo/bpftrace that referenced this issue May 15, 2024
Remove LOG(FATAL)
fb3f0cd 
Replace these with either LOG(BUG),
LOG(ERROR) and exit(1) in main.cpp, or
`throw std::runtime_error`.

This is to avoid calling `abort` for user
errors and removes the confusion between
`LOG(ERROR)` and `LOG(FATAL)`.

Issue: bpftrace#3163
@ajor ajor linked a pull request May 15, 2024 that will close this issue
Remove LOG(FATAL) #3164
Merged
3 tasks
jordalgo pushed a commit to jordalgo/bpftrace that referenced this issue May 18, 2024
Remove LOG(FATAL)
c33a8de 
Replace these with either LOG(BUG),
LOG(ERROR) and exit(1) in main.cpp, or
`throw FatalUserException`.

This is to avoid calling `abort` for user
errors and removes the confusion between
`LOG(ERROR)` and `LOG(FATAL)`.

Issue: bpftrace#3163
jordalgo pushed a commit to jordalgo/bpftrace that referenced this issue May 18, 2024
Remove LOG(FATAL)
71adb27 
Replace these with either LOG(BUG),
LOG(ERROR) and exit(1) in main.cpp, or
`throw FatalUserException`.

This is to avoid calling `abort` for user
errors and removes the confusion between
`LOG(ERROR)` and `LOG(FATAL)`.

Issue: bpftrace#3163
jordalgo pushed a commit to jordalgo/bpftrace that referenced this issue May 21, 2024
Remove LOG(FATAL)
c28f07d 
Replace these with either LOG(BUG),
LOG(ERROR) and exit(1) in main.cpp, or
`throw FatalUserException`.

This is to avoid calling `abort` for user
errors and removes the confusion between
`LOG(ERROR)` and `LOG(FATAL)`.

Issue: bpftrace#3163
jordalgo pushed a commit to jordalgo/bpftrace that referenced this issue May 21, 2024
Remove LOG(FATAL)
a0fee2f 
Replace these with either LOG(BUG),
LOG(ERROR) and exit(1) in main.cpp, or
`throw FatalUserException`.

This is to avoid calling `abort` for user
errors and removes the confusion between
`LOG(ERROR)` and `LOG(FATAL)`.

Issue: bpftrace#3163
jordalgo pushed a commit to jordalgo/bpftrace that referenced this issue May 23, 2024
Remove LOG(FATAL)
b01d47e 
Replace these with either LOG(BUG),
LOG(ERROR) and exit(1) in main.cpp, or
`throw FatalUserException`.

This is to avoid calling `abort` for user
errors and removes the confusion between
`LOG(ERROR)` and `LOG(FATAL)`.

Issue: bpftrace#3163
jordalgo added a commit that referenced this issue May 23, 2024
@jordalgo
Remove LOG(FATAL) (#3164)
c316904 
Replace these with either LOG(BUG),
LOG(ERROR) and exit(1) in main.cpp, or
`throw FatalUserException`.

This is to avoid calling `abort` for user
errors and removes the confusion between
`LOG(ERROR)` and `LOG(FATAL)`.

Issue: #3163

Co-authored-by: Jordan Rome <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jordalgo jordalgo

Labels
bug Something isn't working reliability Correctness and polish work
Projects
None yet
Milestone
v0.21.0
Development

Successfully merging a pull request may close this issue.

Remove LOG(FATAL) jordalgo/bpftrace
2 participants
@ajor @jordalgo