bugGNU Octave - Bugs: bug #60311, Static variable in eigs may cause...

 
 

bug #60311: Static variable in eigs may cause SIGSEGV after exit

Submitter:  None
Submitted:  Tue 30 Mar 2021 07:44:04 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Segfault, Bus Error, etc.
Status:  Fixed Assigned to:  None
Originator Name:  Octave User Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * dev
Operating System:  * Any Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Thu 01 Apr 2021 05:55:14 PM UTC, comment #5: 

Reinhard sent a private message in which he allowed to add him to the contributors list.

I made a few minor changes to the other patch (mostly whitespaces and line breaks of lines longer than 80 characters), added him to the list of contributors, added a commit message, and pushed the patch on default here:
https://hg.savannah.gnu.org/hgweb/octave/rev/7b160bf6b897

Thank you again for your contribution.

Marking as ready for test.

Markus Mützel <mmuetzel>
Group administrator
Thu 01 Apr 2021 05:09:51 PM UTC, comment #4: 

Your patch didn't apply on the stable branch. I used a slightly different version, added a commit message and pushed the change here:
https://hg.savannah.gnu.org/hgweb/octave/rev/a11714e03733

I used your version while merging to default:
https://hg.savannah.gnu.org/hgweb/octave/rev/b99d87eafd4e


I'll have a look at the other patch that removes the static variables on the default branch next.


Would you like to be added to the list of contributors that appears at the beginning of the manual? Which name should I use?

Markus Mützel <mmuetzel>
Group administrator
Wed 31 Mar 2021 06:55:05 PM UTC, comment #3: 

Dear Markus Mützel,

I have attached two patches as you requested.

Best regards,
Octave User


(file #51166, file #51167)

Anonymous
Wed 31 Mar 2021 05:22:32 PM UTC, comment #2: 

Markus: Yes, we do aim for binary compatibility throughout a major release series, but if there are serious problems that can't be fixed without making some change in a public interface, then I'd rather have the fix than binary compatibility.  This is also a fairly obscure interface so I don't think it will cause trouble for most people.  It's not like we are changing something that would affect everyone who writes code to create a .oct file.

I also have some pending changes for anonymous functions and handles to nested functions that I would like to apply for the next stable release that will cause binary compatibility issues (but again, in fairly obscure interfaces) that very few (if any) people would be using directly.

John W. Eaton <jwe>
Group administrator
Wed 31 Mar 2021 04:53:14 PM UTC, comment #1: 

Thanks for your patch. I haven't tested it yet. But it looks good to me in principle.

Afaict, reducing the number of static variables is something jwe is also interested in.

Since this fixes a potential segmentation fault, I'd like to push on stable. I'm not sure this is possible with the current patch though.
Does changing a typedef in a distributed header from a function pointer to a std::function constitute an API change? (I'm afraid it might.)

If it does, we should probably first fix the SIGSEGV on stable by resetting the static variable `eigs_fcn` like you suggested. And then get rid of the static variables on default in a second patch.

Also the construct with the functional shadowing a local function seems a bit odd to me.
Would it be possible to use a different name for the functional to make it clearer what is used where?

Could you please provide patches for these steps?

Markus Mützel <mmuetzel>
Group administrator
Tue 30 Mar 2021 07:44:04 PM UTC, original submission:  

Dear GNU-Octave developers,

I have found out, that the static variable "eigs_fcn" in "__eigs__.cc" may hold an invalid references to user variables after exit has been called. As a consequence, you get a SIGSEGV. This can happen for example if anonymous callback functions are passed to eigs which point to Octave class objects (e.g. those inside a @class_name directory).

Unfortunately this issue is not easy to reproduce with a few lines of code. However, I have attached a patch which fixes the problem. I tried to touch as few lines as possible. Please let me know if you want any changes for that patch!

In addition to avoiding a SIGSEGV, that patch will also reduce memory consumption in case of large matrices referenced by anonymous functions. Such matrices will not be released after eigs returns, because of the static variable which is still holding a reference to the callback function. Of course, you could also reset that static variable and it would fix the SIGSEGV as well.

Best regards,
Octave User

Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #51166:  fix-eigs-sigsegv-after-exit_v1.patch added by None (7KiB - text/x-patch)
file #51167:  fix-eigs-sigsegv-after-exit_v2.patch added by None (390B - text/x-patch)
file #51163:  fix-eigs-sigsegv-after-exit.patch added by None (5KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Updated the item)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by None (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.

    Only group members can vote.

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-04-05 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2021-04-01 mmuetzel StatusIn Progress Ready For Test
    2021-04-01 mmuetzel StatusNeed Info In Progress
    2021-03-31 None Attached File- Added fix-eigs-sigsegv-after-exit_v1.patch, #51166
        Attached File- Added fix-eigs-sigsegv-after-exit_v2.patch, #51167
    2021-03-31 mmuetzel StatusNone Need Info
        Operating SystemGNU/Linux Any
    2021-03-30 None Attached File- Added fix-eigs-sigsegv-after-exit.patch, #51163

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code