patchGNU nano - Patches: patch #9623, handle most crashes, and avoid...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #9623: handle most crashes, and avoid these handlers getting called twice

Submitter:  easyaspi314 <easyaspi314>
Submitted:  Tue 24 Apr 2018 05:03:35 PM UTC
   
 
Priority:  3 - Low Status:  Done
Privacy:  Public Assigned to:  bens
Open/Closed:  Closed Release:  None

Jump to the original submission

Wed 16 May 2018 07:37:23 AM UTC, comment #10: 

The essential patch went in in commit e889c28a.  Released in 2.9.7.  Thanks for the patch.

Benno Schulenberg <bens>
Group administrator
Mon 30 Apr 2018 03:22:07 PM UTC, comment #9: 

Okay.  Then I'm sending it on to the devel list, to see if anyone there has comments.

Benno Schulenberg <bens>
Group administrator
Mon 30 Apr 2018 02:25:44 PM UTC, comment #8: 

Looks good.

Now that I think about it, yeah, we don't need the fire once flag on the hup/term signals, the crashes would be caught by the segv and abrt handlers.

easyaspi314 <easyaspi314>
Mon 30 Apr 2018 01:06:07 PM UTC, comment #7: 

Making a patch is easier when you make your changes on a branch.  So normally I do:

git checkout -b some-distinctive-branch-name
<make changes>
git commit -as
git format-patch master

I've reduced your patch to the minimum required changes -- I don't think it is necessary to reset the handlers for HUP and TERM to their defaults when those signals fire.  Please check if you're okay with it.  I've also adjusted the Author line to match the Signed-off line.

(file #44061)

Benno Schulenberg <bens>
Group administrator
Sun 29 Apr 2018 12:15:40 PM UTC, comment #6: 

I'm not good at git, but I think I did this right.


$ git commit -s
$ git format-patch -n HEAD^


(file #44058)

easyaspi314 <easyaspi314>
Sun 29 Apr 2018 09:27:45 AM UTC, comment #5: 

Can you give me a complete version of the patch?  With commit message and signed off.

Benno Schulenberg <bens>
Group administrator
Wed 25 Apr 2018 11:56:32 AM UTC, comment #4: 

Okay.  I'll merge this stuff when 2.9.6 is out -- I want to have it installed locally for a while before releasing it.

Benno Schulenberg <bens>
Group administrator
Tue 24 Apr 2018 07:36:17 PM UTC, comment #3: 

Oops, stupid formatter.


int main(void) {
    int *i = (int *)0x0;
    return *i;
}


easyaspi314 <easyaspi314>
Tue 24 Apr 2018 07:34:44 PM UTC, comment #2: 

At least not on my Mac.

$ cat dummy_speller.c
int main(void) {
    int i = (int )0x0;
    return *i;
}
$ clang dummy_speller.c -o dummy_speller
$ ./dummy_speller
[1]    10082 segmentation fault  ./dummy_speller
(crash reporter pops up)
$ ./nano/src/nano -s ./dummy_speller
When I run the spellchecker, it says
[ Spell checking failed: Error invoking "./dummy_speller" ]
and I get the crash reporter.

Running on Termux on my LG G3 (Android, armv7a) has the same result, although Android lacks a crash reporter.

I get the same result with GCC 7, so I think we're ok.

easyaspi314 <easyaspi314>
Tue 24 Apr 2018 06:50:16 PM UTC, comment #1: 

Cool.

But... in 'man 2 sigaction' it says: "A child created via fork(2) inherits a copy of its parent's signal dispositions."  Does that mean that if a child process (like the speller program or the linter) would segfault, that it then prints "Sorry! GNU nano crashed! ..."?

Benno Schulenberg <bens>
Group administrator
Tue 24 Apr 2018 05:03:35 PM UTC, original submission:  

Currently, if nano crashes, you lose everything and you have to reset the terminal.

This makes it so that nano dies when there is a SIGSEGV or SIGABRT.

It will die with the message

"Sorry! GNU nano crashed! (code <signal code>)  Please report a bug."

On debug builds, I disabled these handlers so they are easier to debug.

I also added the flag SA_RESETHAND on the SIGHUP, SIGTERM, SIGABRT and SIGSEGV handlers. This means that if nano theoretically segfaults in die(), we don't get an infinite loop.

easyaspi314 <easyaspi314>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #44007:  crashhandler.patch added by easyaspi314 (2KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by bens (Posted a comment)
  • -email is unavailable- added by easyaspi314 (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-05-16 bens StatusIn Progress Done
        Open/ClosedOpen Closed
    2018-04-30 bens Attached File- Added 0001-signals-upon-a-crash-save-changed-buffers-and-reset-.patch, #44061
    2018-04-29 easyaspi314 Attached File- Added 0001-Handle-most-crashes-on-release-builds-avoid-these-ha.patch, #44058
    2018-04-29 bens StatusNone In Progress
    2018-04-25 bens Summarymost crashes, and avoid these handlers getting called twice handle most crashes, and avoid these handlers getting called twice
    2018-04-25 bens Priority5 - Normal 3 - Low
        Assigned toNone bens
        SummaryHandle most crashes in non-debug builds, avoid die handlers firing twice. most crashes, and avoid these handlers getting called twice
    2018-04-24 easyaspi314 Attached File- Added crashhandler.patch, #44007

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code