Skip to content

Centralise fault handling#204

Merged
cgzones merged 1 commit intohtop-dev:masterfrom
BenBE:central-fault-handling
Oct 12, 2020
Merged

Centralise fault handling#204
cgzones merged 1 commit intohtop-dev:masterfrom
BenBE:central-fault-handling

Conversation

@BenBE
Copy link
Copy Markdown
Member

@BenBE BenBE commented Oct 3, 2020

This should be done as all platforms essentially did the same anyway and there was nothing platform specific.

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Oct 3, 2020

This pull request introduces 1 alert when merging 0672aaa into 4b14ab9 - view on LGTM.com

new alerts:

  • 1 for Implicit function declaration

Copy link
Copy Markdown
Member

@cgzones cgzones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, an example (with sanitizers):

FATAL PROGRAM ERROR DETECTED
============================
Please report this issue at https://htop.dev including the following information:

- Your htop version (htop --version)
- Your OS and kernel version (uname -a)
- Your distribution and release (lsb_release -a)
- Likely steps to reproduce (How did it happened?)
- Backtrace of the issue (see below)

Error information:
------------------
A signal 11 was received.

Backtrace information:
----------------------
The following function calls were active when the issue was detected:
---
./htop(backtrace+0x5b)[0x45d8cb]
./htop(CRT_handleSIGSEGV+0xa7)[0x4dc717]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x14140)[0x7f0cbeccd140]
/lib/x86_64-linux-gnu/libc.so.6(_IO_str_overflow+0x121)[0x7f0cbeb4b3a1]
/lib/x86_64-linux-gnu/libc.so.6(_IO_default_xsputn+0x71)[0x7f0cbeb49c61]
/lib/x86_64-linux-gnu/libc.so.6(+0x69e3c)[0x7f0cbeb30e3c]
/lib/x86_64-linux-gnu/libc.so.6(vsprintf+0x84)[0x7f0cbeb3e264]
./htop(vsprintf+0xa7)[0x4451b7]
./htop(sprintf+0xc4)[0x446254]
./htop(EnvScreen_draw+0x21)[0x5401f1]
./htop(InfoScreen_run+0x23b)[0x541eab]
./htop[0x53ea9c]
./htop[0x4dd767]
./htop(ScreenManager_run+0x70d)[0x5145ad]
./htop(main+0x6c0)[0x4e9490]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xea)[0x7f0cbeaedcca]
./htop(_start+0x2a)[0x4267ca]
---

To make the above information more practical to work with,
you should provide a disassembly of your binary.
This can usually be done by running the following command:

   objdump -d -S -w `which htop` > ~/htop.objdump

Please include the generated file in your report.

Running this program with debug symbols or inside a debugger may provide further insights.

Thank you for helping to improve htop!

htop 3.0.2 aborting.

AddressSanitizer:DEADLYSIGNAL
=================================================================
==7330==ERROR: AddressSanitizer: SEGV on unknown address 0x03e800001ca2 (pc 0x7f0cbeb4b3a1 bp 0x000000000074 sp 0x7ffe4f2dde90 T0)
==7330==The signal is caused by a WRITE memory access.
    #0 0x7f0cbeb4b3a1 in _IO_str_overflow libio/strops.c:133:28
    #1 0x7f0cbeb49c60 in __GI__IO_default_xsputn libio/genops.c:399:24
    #2 0x7f0cbeb49c60 in _IO_default_xsputn libio/genops.c:370:1
    #3 0x7f0cbeb30e3b in __vfprintf_internal stdio-common/vfprintf-internal.c:1373:3
    #4 0x7f0cbeb3e263 in __vsprintf_internal libio/iovsprintf.c:96:9
    #5 0x7f0cbeb3e263 in vsprintf libio/iovsprintf.c:105:10
    #6 0x4451b6 in vsprintf (/home/christian/Coding/workspaces/htop/htop+0x4451b6)
    #7 0x446253 in sprintf (/home/christian/Coding/workspaces/htop/htop+0x446253)
    #8 0x5401f0 in EnvScreen_draw /home/christian/Coding/workspaces/htop/EnvScreen.c:36:4
    #9 0x541eaa in InfoScreen_run /home/christian/Coding/workspaces/htop/InfoScreen.c:75:4
    #10 0x53ea9b in actionShowEnvScreen /home/christian/Coding/workspaces/htop/Action.c:527:4
    #11 0x4dd766 in MainPanel_eventHandler /home/christian/Coding/workspaces/htop/MainPanel.c:80:19
    #12 0x5145ac in ScreenManager_run /home/christian/Coding/workspaces/htop/ScreenManager.c:227:19
    #13 0x4e948f in main /home/christian/Coding/workspaces/htop/htop.c:266:4
    #14 0x7f0cbeaedcc9 in __libc_start_main csu/../csu/libc-start.c:308:16
    #15 0x4267c9 in _start (/home/christian/Coding/workspaces/htop/htop+0x4267c9)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV libio/strops.c:133:28 in _IO_str_overflow
==7330==ABORTING

CRT.c Outdated
fprintf(stderr, "\n\n"
"FATAL PROGRAM ERROR DETECTED\n"
"============================\n"
"Please report this issue at https://htop.dev including the following information:\n"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or make https://htop.dev/new-issue point there. That's more flexible and allows for us to change where reports drop; even for ancient™ versions.

@BenBE BenBE force-pushed the central-fault-handling branch from 0672aaa to 1993beb Compare October 3, 2020 17:00
@cgzones
Copy link
Copy Markdown
Member

cgzones commented Oct 3, 2020

It seems on FreeBSD backtrace requires linking with -lexecinfo.
[Maybe also do a function-check in configure.ac and use backtrace only if available.] nvm, its already in a HAVE_EXECINFO_H block

@BenBE
Copy link
Copy Markdown
Member Author

BenBE commented Oct 3, 2020

It seems on FreeBSD backtrace requires linking with -lexecinfo.
[Maybe also do a function-check in configure.ac and use backtrace only if available.] nvm, its already in a HAVE_EXECINFO_H block

Didn't do an in-depth comparison of the configure.ac stuff, but comparing the 14 files I removed, there were essentially two versions: The Linux+Backtrace one and the UnsupportedPlatform one. On Darwin (MacOS) the HAVE_EXECINFO_H header check was missing for #include <execinfo.h>. Apart from this these files were the same.

@BenBE BenBE force-pushed the central-fault-handling branch 2 times, most recently from d7b11b8 to a86f5a4 Compare October 3, 2020 19:23
@fasterit
Copy link
Copy Markdown
Member

fasterit commented Oct 4, 2020

"Please report this issue at https://htop.dev" -> "Please check whether this issue has already been reported at https://htop.dev. If the same problem has not been reported before, please create a new issue with the following information:"
(trying to avoid a flood of duplicates if we publish a version that fails for many people)

@BenBE BenBE force-pushed the central-fault-handling branch 2 times, most recently from fd90937 to b54fac1 Compare October 5, 2020 19:42
@BenBE
Copy link
Copy Markdown
Member Author

BenBE commented Oct 5, 2020

I changed the error link to https://htop.dev/issues. I'd suggest pointing this to https://github.com/htop-dev/htop/issues on our server.

@BenBE BenBE force-pushed the central-fault-handling branch from b54fac1 to 923f57b Compare October 6, 2020 12:12
@cgzones
Copy link
Copy Markdown
Member

cgzones commented Oct 6, 2020

A further step will be to install a handler for SIGABRT and then call abort() insead of err() in XAlloc.c, to have stack-traces on Xfunction failures.

@BenBE BenBE force-pushed the central-fault-handling branch 2 times, most recently from b4036d2 to 226268c Compare October 6, 2020 17:04
@BenBE BenBE force-pushed the central-fault-handling branch 2 times, most recently from 42c490d to a351318 Compare October 6, 2020 17:35
Copy link
Copy Markdown
Member

@cgzones cgzones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@fasterit @natoscott can you create a site redirection for https://htop.dev/issues ?

xSnprinf failure:

FATAL PROGRAM ERROR DETECTED
============================
Please check at https://htop.dev/issues whether this issue has already been reported.
If no similar issue has been reported before, please create a new issue with the following information:

- Your htop version (htop --version)
- Your OS and kernel version (uname -a)
- Your distribution and release (lsb_release -a)
- Likely steps to reproduce (How did it happened?)
- Backtrace of the issue (see below)

Error information:
------------------
A signal 6 was received.

Backtrace information:
----------------------
The following function calls were active when the issue was detected:
---
./htop(backtrace+0x5b)[0x45d8db]
./htop(CRT_handleSIGSEGV+0x175)[0x4eb025]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x14140)[0x7f8bbc8ba140]
/lib/x86_64-linux-gnu/libc.so.6(gsignal+0x141)[0x7f8bbc6efdb1]
/lib/x86_64-linux-gnu/libc.so.6(abort+0x123)[0x7f8bbc6d9537]
./htop(fail+0x1d)[0x54301d]
./htop[0x54343c]
./htop(OpenFilesScreen_scan+0xb1)[0x50d1b1]
./htop(InfoScreen_run+0x1bc)[0x4fd88c]
./htop[0x4d58bc]
./htop[0x501c07]
./htop(ScreenManager_run+0x69b)[0x528edb]
./htop(main+0x6df)[0x4f792f]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xea)[0x7f8bbc6dacca]
./htop(_start+0x2a)[0x4267da]
---

To make the above information more practical to work with,
you should provide a disassembly of your binary.
This can usually be done by running the following command:

   objdump -d -S -w `which htop` > ~/htop.objdump

Please include the generated file in your report.

Running this program with debug symbols or inside a debugger may provide further insights.

Thank you for helping to improve htop!

htop 3.0.3-dev aborting.

Aborted

Copy link
Copy Markdown
Member

@cgzones cgzones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

double post ...

This should be done as all platforms essentially did the same anyway and there was nothing platform specific.
@BenBE BenBE force-pushed the central-fault-handling branch from a351318 to 738db1b Compare October 7, 2020 11:19
@cgzones cgzones merged commit 6014800 into htop-dev:master Oct 12, 2020
@cgzones
Copy link
Copy Markdown
Member

cgzones commented Oct 12, 2020

Thanks, applied.

@BenBE BenBE deleted the central-fault-handling branch October 13, 2020 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants