Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
gpshead merged 14 commits intopython:mainfrompicnixz:fix/hacl/dynamic-linker-131298
Apr 20, 2025

Conversation

picnixz
Copy link
Member

@picnixzpicnixz commentedApr 12, 2025
edited
Loading

In#130157, I actually statically linked HACL* modules but this has issues withfreeze.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 doesNOT simplify the build process (configure & co are still messy IMO but I've explained the whys in#132438 (comment))

cc@msprotz

@bedevere-bot

This comment was marked as resolved.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelApr 12, 2025
@picnixz
Copy link
MemberAuthor

I'm leaving my dev session for now so I'll be back tomorrow if needs arise.

@picnixz
Copy link
MemberAuthor

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
Copy link
Contributor

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!

@picnixzpicnixz changed the titlegh-131298: simplify HACL* build for MD5, SHA1, SHA2 and SHA3 modulesgh-131298: eliminate HACL* static libraries for cryptographic modulesApr 13, 2025
@picnixz
Copy link
MemberAuthor

Ok, it's not really a simplification of the build, but I managed to eliminate static libraries.

@picnixzpicnixzforce-pushed thefix/hacl/dynamic-linker-131298 branch from5113a98 to8d86ff6CompareApril 13, 2025 12:29
@picnixz
Copy link
MemberAuthor

!buildbot FIPS

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@picnixz for commit8d86ff6 🤖

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:FIPS

The builders matched are:

  • AMD64 CentOS9 FIPS No Builtin Hashes PR
  • AMD64 CentOS9 FIPS Only Blake2 Builtin Hash PR
  • AMD64 RHEL8 FIPS No Builtin Hashes PR
  • AMD64 RHEL8 FIPS Only Blake2 Builtin Hash PR

@picnixzpicnixzforce-pushed thefix/hacl/dynamic-linker-131298 branch from8d86ff6 tob7d509cCompareApril 13, 2025 12:45
@picnixzpicnixz marked this pull request as draftApril 13, 2025 12:45
@picnixzpicnixzforce-pushed thefix/hacl/dynamic-linker-131298 branch fromb7d509c to3f0b41eCompareApril 13, 2025 12:46
@picnixz
Copy link
MemberAuthor

Ok, so for WASI I don't know what to do :D

@picnixz
Copy link
MemberAuthor

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?

@picnixzpicnixz marked this pull request as ready for reviewApril 13, 2025 16:22
@picnixz

This comment was marked as resolved.

@picnixz

This comment was marked as resolved.

@picnixz
Copy link
MemberAuthor

Ok, so here is how I eventually fix:

  • HACL* can be built statically or dynamically. In other words, it's not possible to just useSetup.stdlib.in. Why? if I were to specify a.o file, it would actually create rules automatically and I won't be able to customize the HACL* build itself.
  • So, what I do is the following: I just say "ok build this extension module but don't bother about the dependencies, I'll tell you which ones are dependencies".

For regular builds, there are two kinds of dependencies:

  • the Makefile rule prequisites.
  • the actual files to pass to the linker when building the final *.so or *.a file.

For instance, to build the HACL*-based MD5 module I first need to build the HACL* MD5 part and then buildmd5module.o itself, and finally link_md5.so.

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$(LIBHACL_MD5_LIB_STATIC) rule may not necessarily be used later but it's the rule needed to build statically the HACL* MD5 part. It's as if I was building a separate project.

Then, I haveexplicitly 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.o files but I also need them to be built separately, which is why they are marked asLDEPS. TheLDEPS variable is only used when I want to indicate additional linkerrule prerequisites. Example:

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.oModules/_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 forModules/_md5$(EXT_SUFFIX) areautomatically created bymakesetup and I cannot just change them. I need to tellmakesetup that I have an extra prerequisite for that rule but I cannot tell it viaSetup.stdlib.in. So I introduced theLDEPS variable instead. Note that it can be different from theLDFLAGS variable (in this case, it's not).


Now what was the issue with WASI? well... before, I didn't add the.a file as arule prequisite (namely, I forgot to also include it in theDEPS variable). Because of that, when executing thelibpython rule, there was a missing prequisite, which is why the static HACL* library was never built, leading to a missing file error. It's because LIBPYTHON was built before the extension modules were built.

gpshead reacted with thumbs up emoji

@picnixz
Copy link
MemberAuthor

It works!!

@picnixzpicnixz added the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelApr 19, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@picnixz for commitc87bb27 🤖

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.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelApr 19, 2025
@picnixzpicnixz self-assigned thisApr 19, 2025
@picnixz
Copy link
MemberAuthor

picnixz commentedApr 20, 2025
edited
Loading

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

@gpsheadgpshead added the buildThe build process and cross-build labelApr 20, 2025
@gpsheadgpsheadenabled auto-merge (squash)April 20, 2025 17:40
@gpsheadgpshead merged commit5f2ba15 intopython:mainApr 20, 2025
128 of 131 checks passed
@picnixzpicnixz deleted the fix/hacl/dynamic-linker-131298 branchApril 24, 2025 21:01
asottile added a commit to asottile/cpython that referenced this pull requestApr 26, 2025
@asottile
Copy link
Contributor

looks likeModules/Setup was missed as part of this patch -- I opened#133012 to update that as well!

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@gpsheadgpsheadgpshead approved these changes

@erlend-aaslanderlend-aaslandAwaiting requested review from erlend-aaslanderlend-aasland is a code owner

@corona10corona10Awaiting requested review from corona10corona10 is a code owner

@sethmlarsonsethmlarsonAwaiting requested review from sethmlarsonsethmlarson is a code owner

Assignees

@picnixzpicnixz

Labels
buildThe build process and cross-buildskip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

7 participants
@picnixz@bedevere-bot@msprotz@hoodmane@brettcannon@asottile@gpshead

[8]ページ先頭

©2009-2025 Movatter.jp