Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-131298: eliminate HACL* static libraries for cryptographic modules#132438
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
picnixz commented Apr 12, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as resolved.
This comment was marked as resolved.
picnixz commented Apr 12, 2025
I'm leaving my dev session for now so I'll be back tomorrow if needs arise. |
picnixz commented Apr 12, 2025
Mmh, I'll need some configure hacks when only BLAKE2 is present and other hash functions are disabled (otherwise the build bot will never build and it's not very useful). |
msprotz commented Apr 12, 2025
I don't think there's any issue with shared objects containing simd instructions, as long as the files are compiled with compiler flags consistent with their suffix (e.g. -mavx2 only for Hacl_*_Simd256) prior to linking into a shared object (which we already do properly). Thanks for looking into this! |
picnixz commented Apr 13, 2025
Ok, it's not really a simplification of the build, but I managed to eliminate static libraries. |
5113a98 to 8d86ff6Comparepicnixz commented Apr 13, 2025
!buildbot FIPS |
bedevere-bot commented Apr 13, 2025
🤖 New build scheduled with the buildbot fleet by @picnixz for commit 8d86ff6 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F132438%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
8d86ff6 to b7d509cCompareb7d509c to 3f0b41eComparepicnixz commented Apr 13, 2025
Ok, so for WASI I don't know what to do :D |
picnixz commented Apr 13, 2025
Ok, so nothing was fixed. I give up. I don't know how to do it for WASI. Maybe static linking is the only way to do it? |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
picnixz commented Apr 19, 2025
Ok, so here is how I eventually fix:
For regular builds, there are two kinds of dependencies:
For instance, to build the HACL*-based MD5 module I first need to build the HACL* MD5 part and then build We have: Modules/_hacl/Hacl_Hash_MD5.o: $(srcdir)/Modules/_hacl/Hacl_Hash_MD5.c $(LIBHACL_MD5_HEADERS)$(CC) -c $(LIBHACL_CFLAGS) -o $@$(srcdir)/Modules/_hacl/Hacl_Hash_MD5.c $(LIBHACL_MD5_LIB_STATIC): $(LIBHACL_MD5_OBJS) -rm -f $@$(AR)$(ARFLAGS)$@$(LIBHACL_MD5_OBJS)This step builds the HACL* part. The Then, I have explicitly defined: MODULE__MD5_DEPS=$(srcdir)/Modules/hashlib.h $(LIBHACL_MD5_HEADERS)$(LIBHACL_MD5_LIB_SHARED)MODULE__MD5_LDEPS=$(LIBHACL_MD5_LIB_SHARED)The first variable is for everything related to rule dependencies. In this example, to build the md5 (cpython) module, I need the HACL* library and some other stuff. When calling the linker, I only need to pass the Modules/md5module.o: $(srcdir)/Modules/md5module.c $(MODULE__MD5_DEPS)$(MODULE_DEPS_SHARED)$(PYTHON_HEADERS)$(CC)$(MODULE__MD5_CFLAGS)$(PY_STDMODULE_CFLAGS)$(CCSHARED) -c $(srcdir)/Modules/md5module.c -o Modules/md5module.o Modules/_md5$(EXT_SUFFIX): Modules/md5module.o $(MODULE__MD5_LDEPS)$(BLDSHARED) Modules/md5module.o $(MODULE__MD5_LDFLAGS)$(LIBPYTHON) -o Modules/_md5$(EXT_SUFFIX)Note that the rules for Now what was the issue with WASI? well... before, I didn't add the |
picnixz commented Apr 19, 2025
It works!! |
bedevere-bot commented Apr 19, 2025
🤖 New build scheduled with the buildbot fleet by @picnixz for commit c87bb27 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F132438%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
picnixz commented Apr 20, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
All failures are unrelated or already known. So I think we're good with this change. I'll now actually check if freeze.py works (I actually didn't test this...). The following works: $ ./configure -q --prefix="$(pwd)/build" --libdir="$(pwd)/build/lib" --exec-prefix="$(pwd)/build"&& make -j12 && make install $ echo'print("Hello world")'>> hello.py $ ./python ./Tools/freeze/freeze.py -o frozenhello hello.py $ make -C frozenhello |
5f2ba15 into python:mainUh oh!
There was an error while loading. Please reload this page.
looks like this was missed in python#132438
asottile commented Apr 26, 2025
looks like |
- python/cpython#133027 - python/cpython#133366 - python/cpython#133284 - python/cpython#133398 - python/cpython#131298 - python/cpython#132438 - python/cpython#133012 --------- Co-authored-by: Wingy <git@wingysam.xyz> Co-authored-by: Geoffrey Thomas <geofft@ldpreload.com>
In #130157, I actually statically linked HACL* modules but this has issues with
freeze.py. Instead, I'm reverting back to dynamic linking when possible. .However, I don't know how we can make a dynamic linking for BLAKE2 and for HMAC (considering the underlying objects need to be compiled with possible SIMD instructions, I don't think it's possible to provide a shared library, or am I wrong here? honestly, I'm not really good at linking issues :D)This supersedes #119320 but this does NOT simplify the build process (configure & co are still messy IMO but I've explained the whys in #132438 (comment))
cc @msprotz