[BUG] Posix time_t isn't always long

[kukrimate]

Describe the bug

Cython/Includes/posix/types.pxd hard-codes time_t as long, which isn't correct on 32-bit Linux using 64-bit time_t.

This results in an incorrect struct stat layout specified by Cython/Includes/posix/stat.pxd, leading to much sadness, undefined behavior and broken programs.

[scoder]

Thanks for the report. Since these are extern definitions, Cython does not use them directly but only the declared typedef names, i.e. time_t in this case, and leaves the rest to the C compiler, which sees the correct, platform-specific definitions.

Could you clarify where exactly this hurts? Do you have a short code example that exposes this?

[kukrimate]

@scoder Sorry I don't have a reduced example currently, but I can try producing one if desired.

The failure I was seeing in Ubuntu, which now switched to 64-bit time_t on 32-bit ARM is pyfuse3. pyfuse3 seems to rely on the struct stat definition mentioned above.

It reads struct stat's incorrectly unless I patch cython to use time_t as long long on armhf

[scoder]

The error that you see might actually be due to code in pyfuse3, not Cython. They define access macros that explicitly return long:

cdef extern from "macros.c" nogil:
    long GET_BIRTHTIME(struct_stat* buf)
    long GET_ATIME_NS(struct_stat* buf)
    long GET_CTIME_NS(struct_stat* buf)
    long GET_MTIME_NS(struct_stat* buf)
    long GET_BIRTHTIME_NS(struct_stat* buf)

https://github.com/libfuse/pyfuse3/blob/9cd9699a8c2f4a83f5228feb1211585eaf49a621/src/pyfuse3/macros.pxd#L14-L19
This makes Cython generate code that uses long internally, e.g. for temporary variables.

That said, long long seems a better typedef for time_t, so I'll change the declaration in Cython. This probably has no direct impact on pyfuse3, though.

[scoder]

That said, long long seems a better typedef for time_t

Hmm. I take that back. The comment at the top of posix/types.pxd actually suggests that we might end up with a (slight) performance and usability regression if we replace long by long long, due to generally larger types being handled and potential changes to the conversion functions being used.

cython/Cython/Includes/posix/types.pxd

Lines 1 to 11 in 16994b9

	# Note that the actual size of these types is system-dependent, and
	# can't be detected at C compile time. However, the generated C code
	# will correctly use the actual size of these types *except* for
	# determining promotion in binary arithmetic expressions involving
	# mixed types. In this case, operands are promoted to the declared
	# larger type, with a bias towards typedef types. Thus, with the
	# declarations below, long + time_t will result in a time_t whereas
	# long long + time_t will result in a long long which should be
	# acceptable for either 32-bit or 64-bit signed time_t (though admittedly
	# the POSIX standard doesn't even specify that time_t must be an integral
	# type).

So, not sure what to do here. The declarations are actually ok and work, but the question is what users expect. If they want to be platform independent, they need to either use time_t and friends in their code, or use long long to be on the safe side.

To me, it seems best to use time_t also in user code. That's what the typedef is there for, and it's explicitly marked as platform specific in the Posix specs. Relying on it being either long or long long seems to produce more problems than it solves.

[kukrimate]

long long is also problematic because some platforms (i386) on some distros still use 32-bit time_t but long long is still 64-bit on those platforms, so I don't think that long long is an universally valid solution.

[scoder]

Anyway, could you please report the issue to pyfuse3, so that they can solve it on their side?

[kukrimate]

So do you believe this is only related to the wrong return types of the pyfuse3 macros?

Is the struct defined in Cython/Includes/posix/stat.pxd never used as a layout for accessing platform provided struct stats?

My errors seems to have went away by changing the cython time_t type without touching pyfuse3 yesterday.

[scoder]

So do you believe this is only related to the wrong return types of the pyfuse3 macros?

No, I only saw the wrong definitions and considered them a clear source of bugs. I did not validate that they actually have a problematic effect in the current implementation.

Is the struct defined in Cython/Includes/posix/stat.pxd never used as a layout for accessing platform provided struct stats?

Struct access is always resolved at C compile time, which sees the platform specific definition. Cython does not even need to know all fields in the struct, it's happy if you declare those that the code uses.

My errors seems to have went away by changing the cython time_t type without touching pyfuse3 yesterday.

I cannot comment on this because I do not know exactly how pyfuse3 uses the declarations.