Make dn_simdhash OOM safe

[kg]

Like its predecessor ghashtable, dn_simdhash was not written to handle malloc failures. Thanks to @davidwrighton for reminding me that this needed to be fixed.

Note that the 'ght compatible' version of simdhash asserts on malloc failure, since there's no way to retrofit it to allow the caller to do anything else - all the existing call sites in mono would become wrong if we did that.

The 'new' operation now returns NULL on malloc failure.
The add operations now return an enum where a positive value indicates success, a negative value indicates OOM, and a 0 indicates failure.
The ensure capacity operation now returns a success bool, where 0 means malloc failed and 1 means that the hash is big enough to hold the capacity you requested.

This PR also adds the new dn_simdhash_ght_try_insert API which allows you to sense OOM when using the ght compatible variant.

[Copilot]

Pull Request Overview

This PR updates dn_simdhash to handle allocation failures by returning error codes rather than asserting or leaking memory. Key changes include:

• Introducing dn_simdhash_add_result to report add operation outcomes including OOM.
• Changing ensure_capacity_internal and ensure_capacity to return a success flag.
• Propagating and asserting on OOM in all call sites (specializations, ght-compatible, and the CoreCLR stackmap).

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
run-benchmark.ps1	Added dn-simdhash-ght-compatible.c to the compile command
src/native/containers/dn-simdhash.h	Added dn_simdhash_add_result enum and updated capacity APIs
src/native/containers/dn-simdhash.c	Checked malloc return values and propagated OOM in constructors and capacity logic
src/native/containers/dn-simdhash-specialization.h	Changed TRY_ADD functions to return dn_simdhash_add_result
src/native/containers/dn-simdhash-specialization-declarations.h	Updated declarations to use dn_simdhash_add_result
src/native/containers/dn-simdhash-ght-compatible.c	Added allocation checks and assertions for ght-compatible API
src/coreclr/interpreter/stackmap.cpp	Assert on creation and add return codes in the shared stack map

Comments suppressed due to low confidence (3)

src/native/containers/dn-simdhash.h:56

• Add unit tests that simulate allocation failures to verify that DN_SIMDHASH_OUT_OF_MEMORY is correctly returned from both add and ensure_capacity operations.

typedef enum dn_simdhash_add_result {

src/native/containers/dn-simdhash.h:56

• [nitpick] Include comments explaining each dn_simdhash_add_result enumerator value (e.g., DN_SIMDHASH_ADD_FAILED vs DN_SIMDHASH_OUT_OF_MEMORY) to clarify their distinct semantics for API consumers.

typedef enum dn_simdhash_add_result {

src/native/containers/simdhash-benchmark/run-benchmark.ps1:1

• The added line has an unexpected leading '1 ' before the '+'. It should start with '+cl ...' without the extra '1 ' to correctly include the new source file in the compile command.

cl /GS- /O2 /std:c17 ./*.c ../dn-simdhash-u32-ptr.c ../dn-simdhash.c ../dn-vector.c ../dn-simdhash-string-ptr.c ../dn-simdhash-ght-compatible.c /DNO_CONFIG_H /DSIZEOF_VOID_P=8 /DNDEBUG /Fe:all-measurements.exe

[kg]

Updated to add a (speculative) dn_simdhash_ght_try_insert API that lets you explicitly specify which insert mode you want and sense whether it OOMed.

[kg]

Since we had a conflict in the compiler code I took the opportunity to refactor a bit, there's a failures.h where NOMEM lives now (so files other than compiler can use it) and a simdhash.h that contains the ptr_ptr_holder and result check helpers.

[janvorli]

LGTM modulo the comment on assertAbort.

[kg]

Should I complusthrow instead of assertabort?

…

-kg

Message ID: ***@***.***>

[janvorli]

Should I complusthrow instead of assertabort?

I cannot see your code using assertAbort in release builds. Or do you mean the abort in the new assertXXXX functions you've added? I guess those ones should use NO_WAY instead, unless the failures would represent a bug in our code and not something that can happen due to the state of the system even if our code is correct.

[kg]

Should I complusthrow instead of assertabort?

I cannot see your code using assertAbort in release builds. Or do you mean the abort in the new assertXXXX functions you've added? I guess those ones should use NO_WAY instead, unless the failures would represent a bug in our code and not something that can happen due to the state of the system even if our code is correct.

They represent a bug in our code.

[janvorli]

In that case, I'd keep them as asserts (debug only checks).

[janvorli]

But NO_WAY doesn't hurt, so please feel free to keep it as you've made it in the latest commit.