bugGNU Octave - Bugs: bug #64446, Sparse QR segfaults with...

 
 

bug #64446: Sparse QR segfaults with SuiteSparse 7 on 32-bit architectures

Submitter:  Sébastien Villemot <svillemot>
Submitted:  Wed 19 Jul 2023 07:05:47 AM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Segfault, Bus Error, etc.
Status:  Confirmed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * 8.2.0 Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 04 Aug 2023 05:53:58 PM UTC, comment #12: 

Thank you for putting some weight on the issue.

Closing this as an upstream problem.

Markus Mützel <mmuetzel>
Group administrator
Wed 02 Aug 2023 04:41:30 PM UTC, comment #11: 

SuiteSparse upstream has committed a fix that, according to my testing, fixes the present issue:
https://github.com/DrTimothyAldenDavis/SuiteSparse/commit/97d3b1037d2430bea5526f3c0c33310e3016e4a3

No change in Octave source code is needed.

So the present bug report can be closed now, or when there is a new SuiteSparse release, as you prefer.

Sébastien Villemot <svillemot>
Fri 21 Jul 2023 08:08:04 AM UTC, comment #10: 


commentaire #9 :

> comment #8:
> > You mean there may be conversions between integer types and pointer types within SuiteSparse? That would indeed be a rather serious design issue.
>
> Yeah. I tried to point that out multiple times since SuiteSparse 6 was released. But upstream didn't react on that point ever since...


Indeed at this point I don’t really understand why the 64-bit SPQR doesn’t work on 32-bit architectures. The Octave code seems to do the right thing (i.e. convert integers to SuiteSparse_long, which is now int64_t). So the problem seems internal to SPQR/CHOLMOD, and that would indeed be consistent with an integer-to-pointer conversion issue. Adding a 32-bit SPQR will not solve that (though of course it will provide a solution for 32-bit Octave).

Sébastien Villemot <svillemot>
Fri 21 Jul 2023 08:00:18 AM UTC, comment #9: 

comment #8:

> You mean there may be conversions between integer types and pointer types within SuiteSparse? That would indeed be a rather serious design issue.


Yeah. I tried to point that out multiple times since SuiteSparse 6 was released. But upstream didn't react on that point ever since...

Markus Mützel <mmuetzel>
Group administrator
Fri 21 Jul 2023 07:34:30 AM UTC, comment #8: 


commentaire #7 :

> I'm not 100% sure either about the extent of the breakage. But I believe that something is going wrong if the indexing type is larger than `sizeof(void*)`.


You mean there may be conversions between integer types and pointer types within SuiteSparse? That would indeed be a rather serious design issue.

Sébastien Villemot <svillemot>
Thu 20 Jul 2023 07:48:04 AM UTC, comment #7: 

Thank you for pointing to those additional changes. I pushed them to the stable branch here:
https://hg.savannah.gnu.org/hgweb/octave/rev/591c39fb7bca

I'm not 100% sure either about the extent of the breakage. But I believe that something is going wrong if the indexing type is larger than `sizeof(void*)`.

When SuiteSparse will provide SPQR functions that use 32-bit integers for indexing, we'll likely need to adapt Octave's code to start using those. But we'll likely need to wait until a version of SuiteSparse with those additional functions will be released.

Markus Mützel <mmuetzel>
Group administrator
Wed 19 Jul 2023 01:16:02 PM UTC, comment #6: 

As a temporary solution, I’ve disabled SPQR usage for 32-bit architectures in Debian.

Note that I had to apply the following patch to make it compile, that you probably want in the hg repository:
https://salsa.debian.org/pkg-octave-team/octave/-/blob/debian/latest/debian/patches/suitesparse7-part2.patch

I’m not sure I understand your last point about the extent of the breakage. For sure there is an ABI change in many suitesparse functions for 32-bit architectures, but that’s normally not a problem if the source code uses the correct types. Anyways, that discussion is probably off-topic for the present bug report (but I’d still be interested in understanding it better, perhaps in another venue).

Also, my understanding is that suitesparse upstream is working on a fix for this issue (providing a 32-bit SPQR flavour). Maybe that will call for a change in the Octave source code to make use of it. Feel free to either close the present bug report, or to keep it open to track this issue until a proper fix is on the way.

Sébastien Villemot <svillemot>
Wed 19 Jul 2023 11:53:21 AM UTC, comment #5: 

Also be aware when switching to the SuiteSparse 7:
It's not only SPQR that is affected. For all libraries, the `_l_` interface is not usable on 32-bit platforms afaict. By `_l_` interface I mean the part of the API that used to use `long` but is now using `int64_t` for indices. Most of those functions contain `_l_` in their name.
Other libraries have most functions in variants that use either `int32_t` or `int64_t`. For those libraries, "only" half the interface is unusable on 32-bit platforms.
SPQR is "more affected" because it only has an interface using `int64_t`.

Markus Mützel <mmuetzel>
Group administrator
Wed 19 Jul 2023 10:21:44 AM UTC, comment #4: 

Realized after hitting the "Submit" button that my comment was unclear on the first affected version:
SPQR from SuiteSparse 5 or earlier can be used on 32-bit platforms.
SPQR from SuiteSparse 6 or later can't be used on 32-bit platforms.

(That is at least my current understanding.)

Markus Mützel <mmuetzel>
Group administrator
Wed 19 Jul 2023 10:19:22 AM UTC, comment #3: 

My understanding is that SPQR from SuiteSparse after version 6 is unusable on 32-bit platforms. I don't think there is anything that Octave can do about this.

The only solution that I can see if you need to use SuiteSparse 7 is to not use SPQR on 32-bit platforms.

Markus Mützel <mmuetzel>
Group administrator
Wed 19 Jul 2023 07:36:35 AM UTC, comment #2: 

Thanks for clarifying. Unfortunately pinning SuiteSparse to version 5 on some architectures is not an option for Debian.

I’m afraid this is a blocker for Debian. Unless someone provides a real fix, the only “solution” that I can think of is to disable the use of SPQR by Octave on 32-bit architectures, but maybe this will have unintended consequences.

Sébastien Villemot <svillemot>
Wed 19 Jul 2023 07:22:53 AM UTC, comment #1: 


> Note that between SuiteSparse 5 and 7, the SuiteSparse_long type has changed on 32-bit architectures. It used to be a long (i.e. 32-bit), and it is now an int64_t (i.e. 64-bit). I don’t know if that is related to the issue. There is however already some code in sparse-qr.cc to deal with the different possible sizes of SuiteSparse_long, so maybe this is a red herring.


That is likely the cause of the issue. IIUC, that change causes those libraries from SuiteSparse that don't have an explicit interface for 32-bit integers to become unusable on 32-bit platforms. (I might be wrong though).
See also this report for SuiteSparse:
https://github.com/DrTimothyAldenDavis/SuiteSparse/issues/221

For the SuiteSparse libraries distributed by MSYS2, this was "resolved" by pinning SuiteSparse to version 5 for 32-bit:
https://github.com/msys2/MINGW-packages/pull/14663

Markus Mützel <mmuetzel>
Group administrator
Wed 19 Jul 2023 07:05:47 AM UTC, original submission:  

Debian is currently transitioning to SuiteSparse 7, and the Octave 8.2.0 testsuite now fails on 32-bit architectures.
See for example:
https://buildd.debian.org/status/fetch.php?pkg=octave&arch=i386&ver=8.2.0-4&stamp=1689712650&file=log

More precisely, there is now a segmentation fault in the tests for the “qr” function. The segfault is triggered at line 523 of liboctave/numeric/sparse-qr.cc. I attach a backtrace obtained by running “test qr” on i386.

Note that in Debian, Octave 8.2.0 has been patched to compile against SuiteSparse 7 (hg ID 134152cf1a3f has been backported).

I tried to reproduce the problem on the default branch, but unfortunately that branch fails to build on i386 for an unrelated reason (some issue around time_t types).

Note that between SuiteSparse 5 and 7, the SuiteSparse_long type has changed on 32-bit architectures. It used to be a long (i.e. 32-bit), and it is now an int64_t (i.e. 64-bit). I don’t know if that is related to the issue. There is however already some code in sparse-qr.cc to deal with the different possible sizes of SuiteSparse_long, so maybe this is a red herring.


Sébastien Villemot <svillemot>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #54939:  suitesparse7.log added by svillemot (18KiB - text/x-log)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by svillemot (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 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-08-04 mmuetzel Open/ClosedOpen Closed
    2023-07-20 mmuetzel StatusNone Confirmed
        Operating SystemGNU/Linux Any
    2023-07-19 svillemot Attached File- Added suitesparse7.log, #54939

    Back to the top

    Powered by Savane 3.13-aa77.
    Corresponding source code