warning: ‘fmpz_mod_mat_mul’ accessing 40 bytes in a region of size 32

[GiacomoPope]

Noticed in the CI that there's something wrong about the mat_mul operations (https://github.com/flintlib/python-flint/actions/runs/10402835971/job/28808112409?pr=182)

For example

 In function ‘__pyx_pf_5flint_5types_12fmpz_mod_mat_12fmpz_mod_mat_36_pow’,
    inlined from ‘__pyx_pw_5flint_5types_12fmpz_mod_mat_12fmpz_mod_mat_37_pow’ at src/flint/types/fmpz_mod_mat.c:11474:13:
src/flint/types/fmpz_mod_mat.c:1330:51: warning: ‘fmpz_mod_mat_mul’ accessing 40 bytes in a region of size 32 [-Wstringop-overflow=]
 1330 |     #define compat_fmpz_mod_mat_mul(C, A, B, ctx) fmpz_mod_mat_mul(C, A, B)
      |                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~
src/flint/types/fmpz_mod_mat.c:11676:7: note: in expansion of macro ‘compat_fmpz_mod_mat_mul’
11676 |       compat_fmpz_mod_mat_mul(__pyx_v_res->val, __pyx_v_res->val, __pyx_v_tmp->val, __pyx_v_self->ctx->val);
      |       ^~~~~~~~~~~~~~~~~~~~~~~
src/flint/types/fmpz_mod_mat.c:1330:51: note: referencing argument 1 of type ‘fmpz_mod_mat_struct[1]’
 1330 |     #define compat_fmpz_mod_mat_mul(C, A, B, ctx) fmpz_mod_mat_mul(C, A, B)
      |                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~
src/flint/types/fmpz_mod_mat.c:11676:7: note: in expansion of macro ‘compat_fmpz_mod_mat_mul’
11676 |       compat_fmpz_mod_mat_mul(__pyx_v_res->val, __pyx_v_res->val, __pyx_v_tmp->val, __pyx_v_self->ctx->val);
      |       ^~~~~~~~~~~~~~~~~~~~~~~
src/flint/types/fmpz_mod_mat.c:1330:51: warning: ‘fmpz_mod_mat_mul’ reading 40 bytes from a region of size 32 [-Wstringop-overread]
 1330 |     #define compat_fmpz_mod_mat_mul(C, A, B, ctx) fmpz_mod_mat_mul(C, A, B)
      |                                                   ^~~~~~~~~~~~~~~~~~~~~~~~~

[GiacomoPope]

Here we have:

cdef extern from "flint/fmpz_mod_mat.h":
    ctypedef struct fmpz_mod_mat_struct:
        fmpz_mat_t mat
        fmpz_t mod
    ctypedef fmpz_mod_mat_struct fmpz_mod_mat_t[1]

and in the C source: https://github.com/flintlib/flint/blob/05340d2a7349761aff2cd3370810c93e2174ce46/src/fmpz_mod_types.h#L36

typedef fmpz_mat_struct fmpz_mod_mat_struct;

typedef fmpz_mod_mat_struct fmpz_mod_mat_t[1];

So maybe we want instead

cdef extern from "flint/fmpz_mod_mat.h":
    ctypedef fmpz_mat_struct fmpz_mod_mat_struct:
    ctypedef fmpz_mod_mat_struct fmpz_mod_mat_t[1]

[GiacomoPope]

oh i see there's this change in 3.1.0 versus earlier versions of flint -- so maybe that's the issue?

[oscarbenjamin]

It is probably better to correct the struct definition but I don't know if that would break under older Flint. It might be that we can just make it an opaque struct. I assume we don't need to access the mat or mod members directly anywhere.

I have been seeing warnings like that for a long time coming from different parts of the codebase but I don't see them now when building with latest flint. The CI job you point to is using Flint 3.0.1 though.

[GiacomoPope]

We're not able to do two different struct definitions for different flint, right?

[oscarbenjamin]

We can have different struct definitions but it needs to be done in the C preprocessor just like the other #define:

python-flint/src/flint/flintlib/fmpz_mod_mat.pxd

Lines 21 to 30 in 5445684

	cdef extern from *:
	"""
	/*
	* fmpz_mod_mat function signatures were changed in FLINT 3.1.0
	*/
	#if __FLINT_RELEASE >= 30100 /* Flint 3.1.0 or later */
	#define compat_fmpz_mod_mat_init(mat, rows, cols, ctx) fmpz_mod_mat_init(mat, rows, cols, ctx)
	#define compat_fmpz_mod_mat_init_set(mat, src, ctx) fmpz_mod_mat_init_set(mat, src, ctx)
	#define compat_fmpz_mod_mat_clear(mat, ctx) fmpz_mod_mat_clear(mat, ctx)

From Cython's perspective the struct would be opaque but we could also use the preprocessor to define macros to access the elements if needed.

[GiacomoPope]

so could we do:

cdef extern from "flint/fmpz_mod_mat.h":
    ctypedef struct fmpz_mod_mat_struct_new:
        fmpz_mat_t mat
        fmpz_t mod
    ctypedef fmpz_mod_mat_struct_newfmpz_mod_mat_new_t[1]

    ctypedef fmpz_mat_struct fmpz_mod_mat_struct_old:
    ctypedef fmpz_mod_mat_struct fmpz_mod_mat_old_t[1]

Then use #define fmpz_mod_mat_struct_old fmpz_mod_mat_struct etc?

[oscarbenjamin]

That might work.

I was imagining something more like

cdef extern from *
    """
    #if FLINTVER < 3.1
    # define compat_fmpz_mod_mat_struct ...
    """
    ctypedef compat_fmpz_mod_mat_struct

If we need something to be different at the C level for different Flint versions it needs to be handled by the C preprocessor and it all needs to be opaque from Cython's perspective.