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.

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

Show error message when processes are killed by SIGSEGV/SIGILL/etc #12874

Closed
tgross35 opened this issue May 15, 2024 · 3 comments · Fixed by #13034
Closed

Show error message when processes are killed by SIGSEGV/SIGILL/etc #12874

tgross35 opened this issue May 15, 2024 · 3 comments · Fixed by #13034
Labels
enhancement New feature or request error-handling How errors in externals/nu code are caught or handled programmatically (see also unhelpful-error) external-commands Issues related to external commands
Milestone

Comments

@tgross35
Copy link

tgross35 commented May 15, 2024

Related problem

Nushell currently does not print anything when a program is terminated by a signal:

> echo "int main() { asm("ud2"); }" out> sigill.c
> gcc sigill.c -o sigill.out
> ./sigill.out
> # *crickets*
> echo "int main() { int v = *((int *)0x0); }" out> sigsegv.c
> gcc sigsegv.c -o sigsegv.out
> ./sigsegv.out
> # *crickets*

This could be okay if you have the exit code displayed as part of the prompt, but if you don't then it looks like everything completed successfully.

By contrast, Bash tells you what went wrong:

$ ./sigill.out
Illegal instruction
$ ./sigsegv.out
Segmentation fault

Describe the solution you'd like

If a signal is the reason a program exits prematurely, print the associated comment so the user is aware of the problem (available at https://man7.org/linux/man-pages/man7/signal.7.html "Standard signals" table), or the same for the Windows equivalent. This can be retrieved on nix with ExitStatusExt::signal.

Describe alternatives you've considered

No response

Additional context and details

No response

@tgross35 tgross35 added enhancement New feature or request needs-triage An issue that hasn't had any proper look labels May 15, 2024
@sholderbach
Copy link
Member

Related challenge #12369 (not a duplicate)

@IanManske built some parts to potentially fix this in #12774 if I recall, I tried some things in #12370 but that did not work yet.

@sholderbach sholderbach added external-commands Issues related to external commands error-handling How errors in externals/nu code are caught or handled programmatically (see also unhelpful-error) and removed needs-triage An issue that hasn't had any proper look labels May 15, 2024
@sholderbach
Copy link
Member

With #12774 we now properly pull out the signal but don't report the error as mentioned here (I think we had code for that at some point.) The $env.LAST_EXIT_CODE contains the signal value in negated form (which I think is not the conventional form. If I recall on Linux it is signal value + 128?)

@tgross35
Copy link
Author

Does nushell not use std::process::Command? If so, the signal is available with .signal() under the ExitSignalExt trait (which calls libc::WIFSIGNALED(self.0).then(|| libc::WTERMSIG(self.0)) internally)

fdncred added a commit that referenced this issue Jun 7, 2024
<!--
if this PR closes one or more issues, you can automatically link the PR
with
them by using one of the [*linking
keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword),
e.g.

- this PR should close #12874
- fixes #12874
I want to fix the issue which is induced by the fix for
#12369. after this pr. This pr
induced a new error for unix system, in order to show coredump messages

you can also mention related issues, PRs or discussions!
-->

# Description
<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->

- this PR should close #12874
- fixes #12874
I want to fix the issue which is induced by the fix for
#12369. after this pr. This pr
induced a new error for unix system, in order to show coredump messages

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

after fix for 12874, coredump message is messing, so I want to fix it

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use toolkit.nu; toolkit test stdlib"` to run the
tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->


![image](https://github.com/nushell/nushell/assets/60290287/6d8ab756-3031-4212-a5f5-5f71be3857f9)

---------

Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com>
@hustcer hustcer added this to the v0.95.0 milestone Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request error-handling How errors in externals/nu code are caught or handled programmatically (see also unhelpful-error) external-commands Issues related to external commands
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants