Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In Emscripten, sizeof(time_t) != sizeof(int) #10293

Closed
hoodmane opened this issue Jan 30, 2024 · 8 comments
Closed

In Emscripten, sizeof(time_t) != sizeof(int) #10293

hoodmane opened this issue Jan 30, 2024 · 8 comments
Labels
waiting-on-reporter Issue is waiting on a reply from the reporter. It will be automatically cloesd if there is no reply.

Comments

@hoodmane
Copy link
Contributor

In asn1.py, cryptography defines time_t to be int...:
https://github.com/pyca/cryptography/blob/main/src/_cffi_src/openssl/asn1.py#L12

I'm not 100% sure what int... means, since the documentation of cffi doesn't directly suggest that the parser would accept this and I haven't looked at the source code. But if it's any of void *, int *, or int these are all 32 bits in Emscripten, but time_t is 64 bits.

Applying the following patch gets it building again, but I'm not sure how we can upstream this:

--- a/src/_cffi_src/openssl/asn1.py
+++ b/src/_cffi_src/openssl/asn1.py
@@ -9,7 +9,7 @@ INCLUDES = """
 """
 
 TYPES = """
-typedef int... time_t;
+typedef long long time_t;
 
 typedef ... ASN1_INTEGER;
@alex
Copy link
Member

alex commented Jan 30, 2024

int... means an integral type of a width cffi determines at compile time

@alex alex added the waiting-on-reporter Issue is waiting on a reply from the reporter. It will be automatically cloesd if there is no reply. label Jan 30, 2024
@hoodmane
Copy link
Contributor Author

int... means an integral type of a width cffi determines at compile time

How does it determine the size?

@alex
Copy link
Member

alex commented Jan 30, 2024

cffi ultimately generates C code for its modules, so it happens via that process -- I can't tell you with more specificity than that without diving into cffi's own code

(https://github.com/python-cffi/cffi/blob/39f99876c82676f1fb430fdf797e0cd221e62e0b/src/cffi/cparser.py#L244)

@hoodmane
Copy link
Contributor Author

Seems to get converted to __dotdotdotint__ here:
https://github.com/python-cffi/cffi/blob/main/src/cffi/cparser.py?plain=1#L244
and the parser seems to emit model.UnknownIntegerType here:
https://github.com/python-cffi/cffi/blob/main/src/cffi/cparser.py?plain=1#L997-L1000
but then there are only three references to UnknownIntegerType. It seems to be the important logic is in build_backend_type which raises NotImplementedError:
https://github.com/python-cffi/cffi/blob/main/src/cffi/model.py?plain=1#L188-L190

So probably build_backend_type is monkey patched when we compile...?

@hoodmane
Copy link
Contributor Author

Or some transform that happens replaces UnknownIntegerType with a concrete so build_backend_type is never invoked?

@alex
Copy link
Member

alex commented Jan 30, 2024

Based on a very quick grep (I don't know the cffi source code that well unfortunately), I see _emit_bytecode_UnknownIntegerType which I suspect is part of the answer.

I think more abstractly, it sounds like the emscripten build process somehow doesn't support this feature of cffi well, possibly. It may make sense to build a minimal reproducer that demonstrates the issue, and bring it to the cffi folks.

@hoodmane
Copy link
Contributor Author

hoodmane commented Jan 30, 2024

Actually I looked again and

import cffi
ffi = cffi.FFI()
ffi.cdef("""
   typedef int... time_t; 
   void g(time_t x);
""")
ffi.set_source("x.c", "void f(time_t b) { g(x); }")
ffi.emit_c_code("abc.c")

emits something like:

static void _cffi_d_g(time_t x0)
{
  g(x0);
}

Which just uses time_t there and it finds out the correct value of time_t from the headers just like it should do. So my apologies, I think this is all working correctly.

The actual problem is in rust-lang/libc which says that time_t has size long which is wrong:
rust-lang/libc#3569

Overriding the libc crate with my patched version fixes the build:

[patch.crates-io]
libc = { git = 'https://github.com/hoodmane/libc.git', branch = 'emscripten-time_t-64-bit' }

Thanks for the help @alex!

@alex
Copy link
Member

alex commented Jan 30, 2024

Glad we figured it out. Will follow on the rust issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
waiting-on-reporter Issue is waiting on a reply from the reporter. It will be automatically cloesd if there is no reply.
Development

No branches or pull requests

2 participants