Ok so I need to debug the krmlrenamings bug and review this PR. I should be able to tackle this early next week. Let me know if this is more pressing and I'll see what I can do. Thanks!

Oh for me this is absolutely not pressing.

Have a great weekend!

@msprotz:

• sync with hacl-star/hacl-star@322f6d5
• remove FStar_UInt128.h (not needed anymore because fstar_uint128_{gcc64,msvc.h} are gone)
• revert interim fixes in blake2module.c
• regen sbom

(I personally don't mind it, and I think it's better to make the script simple for consuming new versions of HACL*, but just want to check you're aware of this)

Regarding the "extra cruft": when I started, I could not use the "uncrufting" out of the box and it was easier for me to "just go with what comes from hacl-star".

As you mention, it is easier for people like me who are no hacl-star experts to sync with upstream if there is less "uncrufting" happening. At least I'd like to keep the folder structure closer - creating a "public" stub types.h and everywhere patching it in instead of #include "krml/internal/types.h" seems too much to me.

But that's just my take on it - happy to adopt all changes you and the other reviewers suggest in refresh.sh.

Do you e.g. mean the call to Hacl_Hash_SHA3_digest here

Hacl_Hash_SHA3_digest already previously had a return type of Hacl_Streaming_Types_error_code

What about code parts in blake2module.c

        case Blake2b_256:
            digest_length = Hacl_Hash_Blake2b_Simd256_digest(self->blake2b_256_state, digest);
            break;

Can they fail, too, and if so, how do they signal their failure?

Sorry for so many questions here, I am totally new to this part of the Python code (TBH, this code is quite new anyway :).

I am just the guy who stumbled over blake2module.c not compiling with clang-cl on Windows for older clangs and then found out that, coincidentally, around the same time this came up in #130213 for older clangs on Linux, too.

Since then I tried to help and learn - but that could go over my head now :)

🤖 New build scheduled with the buildbot fleet by @gpshead for commit 427dfab 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F130960%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

🤖 New build scheduled with the buildbot fleet by @gpshead for commit 427dfab 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F130960%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

re: blurb to add the news:

"update to latest hacl-star to fix build issues with older clang compilers"
but where would it belong? In Misc/NEWS.d/next/Build?

yep, Build seems fine. as would Library. don't overthink it. the important thing is that we have a news entry. some thing cross categories.

@chris-eibl no worries! I'm already super grateful with your huge help so far... let me recap -- to be clear, I think all of this can be dealt with later:

For Hacl_Streaming_HMAC:

• malloc may return OutOfMemory, InvalidAlgorithm (e.g. requesting Blake2b_256 on an ARM machine), or Success
• reset may return InvalidLength (if trying to reset the state with a key of different length, this is not supported), or Success
• update: MaximumLengthExceeded or Success
• digest: OutOfMemory or Success
• copy: may return NULL (indicates out of memory)

For all hash algorithms (NEW with this PR):

• malloc, malloc_with_params_and_key, malloc_with_key: may return NULL (out of memory)
• copy: may return NULL (out of memory)

For SHA3/Keccak only:

• digest may return InvalidAlgorithm (if the algorithm is shake)
• squeeze may return InvalidAlgorithm (if the algorithm is not shake)

I think all of these can be handled as a followup, I just thought it would be good to have it in writing here so that you can decide which of these are worth checking for. The reason I brought up other hash algorithms is that, since you requested (or maybe @picnixz ?) proper out of memory handling in HACL*, we now may return NULL for other algorithms (like hash algorithms), meaning that this PR will introduce new possibly-NULL return values as a side-effect of updating the vendored copy of HACL*.

For the record, Python ignores MaximumLengthExceeded on the basis that this cannot happen in practice.

(I requested it and in my HMAC PR I'm handling and converting possible return codes from HACL*)

Is there anything left do be done in this PR?
E.g. #130960 (comment)?
Seems this would be handled in #130960 (comment)?

Is this feasible for alpha 7 (2025-04-08, the last alpha)?
If not, shall we revisit the "workarounds" #130332 and #130447 or is there anyway no pressure because still possible after alpha 7, e.g. because treated as "bug, because failing to build"?

Seems this would be handled in #130960 (comment)?

Yes.

Is this feasible for alpha 7 (2025-04-08, the last alpha)?

I think we can still ship something just before the beta (so until May). But the HMAC PR is ready technically, and everything is completed on my side (tests pass on my machine and everything builds correctly).

is there anyway no pressure because still possible after alpha 7, e.g. because treated as "bug, because failing to build"?

It would be treated as a bugfix and not a new feature so no worries here I think.

I've actually just observed that:

For all hash algorithms (NEW with this PR):
malloc, malloc_with_params_and_key, malloc_with_key: may return NULL (out of memory)
copy: may return NULL (out of memory)

A follow-up PR could be done to raise/check that we raise PyErr_NoMemory() in existing hash functions.

A follow-up PR

Shall we create an issue so that this does not get lost? What is the common approach here in such cases?

Shall we create an issue so that this does not get lost? What is the common approach here in such cases?

Yes, create a new issue from my comment for instance. The title can be like "Handle NULL returned by HACL* allocation functions"