Adding ExitCodeObserver and ExitSignalObserver

[riesentoaster]

These observers allow observing the exit code and exit signal of a process, specifically in CommandExecutor. This is helpful if the standard CrashFeedback is not specific enough.

[tokatoka]

nice

[tokatoka]

This is not your fault,
but can you change this exitkind::oom (and all occurences of exitkind::oom) to something else?
for example exitkind::sigkill

exit code 9 is not oom. there's no way to tell if the process exited out of oom (unless we type dmesg

[riesentoaster]

I was wondering what was up with that but didn't want to touch it without understanding what was actually happening.

If this gets touched already: Does it make sense to have an exit kind for SIGKILL only? Or do we want further types for certain signals? Or just remove it altogether and use an ExitSignalObserver whenever we need the functionality? What do you think?

[tokatoka]

imo just changing this to ExitKind::SIGKILL is fine. it represents an error sent by the kernel (but not necessarily OOM)
ideally we could separate it into SIGSEGV, SIGABRT, SIGTERM... (if you want to do it)

[riesentoaster]

I've looked into it a bit more deeply and I think the best way would just be to remove ExitKind::Oom without replacement. If a user wants more advanced handling of exits, they have to write a custom Feedback implementation regardless, and they can just use a ExitSignalObserver to get the specific signal.

The issue is that a SIGSEGV etc. is also a crash, so a user may be confused if it isn't reported as such.

I'm more so thinking about what signals may not indicate a crash and maybe handle them differently (or just report them as ExitKind::Ok. I don't know Unix well enough to make this a final list, but maybe something like this:

• SIGTERM
• SIGINT
• SIGUSR1
• SIGUSR2
• SIGCONT
• SIGALRM

Any other signal? How do we handle them?

[tokatoka]

I would call them maybe ExitKind::Interrupted.

[riesentoaster]

I had to re-add ExitKind::Oom, because libafl_libfuzzer uses it — I added a disclaimer to the definition.

In the future, I'd prefer if all features are implemented for all possible combinations, or at least have compile-time errors it not — it's a recipe for weird behaviour for people new to libafl. Not sure how this would be done here and I feel like it doesn't have much to do with this PR, so I'm not going to make those changes now. This is something I've stumbled across at multiple places within the code base, so it may be good to fundamentally think about this at some point.

[tokatoka]

can you fix the clippy errors too?

[riesentoaster]

(I'm so sorry, I messed up my git history. Should hopefully be better now.)

[tokatoka]

thank you 👍

[tokatoka]

Sorry. I have another idea about this.
IMO i don't think we should add these two methods into ObserverTuples

    /// Runs `observe_exit_code` for all exit code observers in the list
    fn observe_exit_code(&mut self, exit_code: i32);
    /// Runs `observe_exit_signal` for all exit signal observers in the list
    fn observe_exit_signal(&mut self, exit_signal: i32);

(Okay there is fn observe_stdout(&mut self, stdout: &[u8]); and fn observe_stderr(&mut self, stderr: &[u8]);
but IMO there should be removed too)

What I would do is
instead of doing

        if self.observers.observes_exit_signal() {

can you

1. When instantiate this command executor, give the Handle or name of the exit code observer
2. When you want to check the exit signal, search it by executor.observers.match_name
   ?