declare the last arg to GAP_CallFunc3Args volatile

[dimpase]

This appears to fix #37026 (segfaults in src/sage/libs/gap/element.pyx with Python 3.12 and gcc 13.2.1)

See also #36407 (comment)

The corresponding gcc 13.2.1's bug (or feature) is being dealt with here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114872

📝 Checklist

• The title is concise and informative.
• The description explains in detail what this PR is about.
• I have linked a relevant issue or discussion.
• I have created tests covering the changes.
• I have updated the documentation and checked the documentation preview.

⌛ Dependencies

[dimpase]

CI seems to be shot, but the tests for element.pyx work, and also libgap.AbelianGroup(0,0,0) does not cause a crash any more.

[tornaria]

Since sig_GAP_Enter() calls sig_error(), shouldn't it be called after the call to sig_on()?

[dimpase]

I agree that this ought to be fixed, but it's a different bug. Flipping these orders don't cure the issue at hand, that volatile declaration is still needed. I'm not sure it's a Sage or a gcc bug.

[tornaria]

Ok, I was just wondering. Since I don't have gcc 14 I can't reproduce myself. Since we don't know what is the cause of the problem, we don't know whether the volatile fixes the issue or we just push around the optimizer to avoid it. The nesting of setjmp() (one for gap, one for sig_on) seems to complicate things.

Looking at the assembler, it seems that gcc is using %ebx for both t in #define sig_GAP_Enter() {int t = GAP_Enter(); if (!t) sig_error();} and for __pyx_t_6 (which contains the third parameter). Making the third parameter volatile avoids the conflict t, but maybe it's better to mark t volatile instead (we don't know if another gcc will place the second parameter in %ebx, etc). Does that work?

I.e.,

--- a/src/sage/libs/gap/gap_includes.pxd
+++ b/src/sage/libs/gap/gap_includes.pxd
@@ -28,7 +28,7 @@ cdef extern from "gap/calls.h" nogil:
 
 cdef extern from "gap/libgap-api.h" nogil:
     """
-    #define sig_GAP_Enter()  {int t = GAP_Enter(); if (!t) sig_error();}
+    #define sig_GAP_Enter()  {volatile int t = GAP_Enter(); if (!t) sig_error();}
     """
     ctypedef void (*GAP_CallbackFunc)()
     void GAP_Initialize(int argc, char ** argv,

[dimpase]

it's actually gcc 13.2.1 which gives this trouble on Gentoo, not gcc 14.

[tornaria]

Ah.... I'm on 13.2.0

[dimpase]

Looking at the assembler, it seems that gcc is using %ebx for both t in #define sig_GAP_Enter() {int t = GAP_Enter(); if (!t) sig_error();} and for __pyx_t_6 (which contains the third parameter). Making the third parameter volatile avoids the conflict t, but maybe it's better to mark t volatile instead (we don't know if another gcc will place the second parameter in %ebx, etc). Does that work?

no, it doesn't. And, after all, t in int t = GAP_Enter(); gets out of the way almost immediately, before the list for arguments for GAP_CallFunc3Args is even created.

[mkoeppe]

CI seems to be shot

@dimpase The bullet: #36982

[github-actions]

Documentation preview for this PR (built with commit 72e6b66; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

[mkoeppe]

LGTM based on the GCC bugzilla discussion cited.
If nothing else, it's a harmless change.
Let's merge it.

[dimpase]

This is certainly a minimal hotfix, and we should fully expect more trouble of this sort as C/C++ compilers optimise more, and ref-counted GAP objects we use end up needing post-processing after a longjmp from libgap. Postprocessing which can be invalidated by a longjmp, if the respective handle is held in a non-volatile local variable.

A better, more efficient way, would be having a non-ref-counted version of GAP objects, which don't need this flaky post-processing, and can be used as headache-free automatic variables.