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

Submitted by:  easyaspi314 <easyaspi314>
Submitted on:  Tue 24 Apr 2018 05:03:35 PM UTC  
 
Priority:  3 - Low Status:  Done
Privacy:  Public Assigned to:  Benno Schulenberg <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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>

 

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)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    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.5