Skip to content

Fix mimalloc build#821

Merged
saghul merged 3 commits intosaghul:masterfrom
lal12:fix_mimalloc_build
Mar 5, 2026
Merged

Fix mimalloc build#821
saghul merged 3 commits intosaghul:masterfrom
lal12:fix_mimalloc_build

Conversation

@lal12
Copy link
Copy Markdown
Contributor

@lal12 lal12 commented Mar 3, 2026

See #815

@lal12 lal12 force-pushed the fix_mimalloc_build branch 2 times, most recently from daac570 to 7ca1cef Compare March 3, 2026 20:58
@lal12 lal12 marked this pull request as ready for review March 3, 2026 21:15
@lal12
Copy link
Copy Markdown
Contributor Author

lal12 commented Mar 3, 2026

CI hangs in some timeout. Probably not related to MR. Can you re trigger CI run?

I don't think I can...

@lal12 lal12 force-pushed the fix_mimalloc_build branch from 8931214 to 7ca1cef Compare March 4, 2026 11:50
@lal12
Copy link
Copy Markdown
Contributor Author

lal12 commented Mar 4, 2026

No idea what the issues is.
After hours of getting it to build on windows, it doesn't run into a timeout.
Since it also is the same code as before #810, I assume whatever the issue is was there before...

I could exclude the case from the build matrix 🤷

@saghul
Copy link
Copy Markdown
Owner

saghul commented Mar 4, 2026

Let me take a stab at it tonight.

@saghul
Copy link
Copy Markdown
Owner

saghul commented Mar 4, 2026

See the failures in #823

It's due to quickjs-ng/quickjs@7304c3d which means we have some mismatched allocation thing going on, and likely what breaks the test is an underflow.

I'm digging into it.

@saghul saghul force-pushed the fix_mimalloc_build branch 2 times, most recently from cce2d18 to f6a2da4 Compare March 4, 2026 23:36
@saghul saghul force-pushed the fix_mimalloc_build branch from f6a2da4 to 2e50e72 Compare March 4, 2026 23:38
@saghul saghul merged commit a1432bd into saghul:master Mar 5, 2026
30 checks passed
@saghul
Copy link
Copy Markdown
Owner

saghul commented Mar 5, 2026

Forgot to leave this here, since it ended up being late yesterday 😅

The 1st step to figuring it out was to split the ffi tests into individual files. I have done this several times already since it really helps narrow down failures.

Then I thought I'd see if Claude could figure it out before busting out my Windows machine. Right on the money:

On Windows x86_64 (LLP64 data model), sizeof(long) = 4, but ffi_call requires the return buffer to be at least sizeof(ffi_arg)
  bytes (which is 8 on x86_64). When the return type is sint or float (4 bytes), the buffer is only 4 bytes, but ffi_call writes 8
   bytes into it — a heap buffer overflow.

  - Debug mode catches it: MSVC debug CRT adds guard bytes around allocations. The overflow corrupts them, and on free the CRT
  triggers _ASSERTE which shows a modal dialog box. In CI/headless, the dialog blocks forever → timeout.
  - mimalloc masks it: no debug guard bytes, so the overflow goes undetected.
  - Release mode gets lucky: the standard CRT doesn't check for overflows.

  The two failing tests both call C functions returning int (4 bytes). Tests returning pointers/structs/size_t/int64 (all >= 8
  bytes) allocate enough space and pass fine.

That explains the timeout, and all is good now!

@lal12
Copy link
Copy Markdown
Contributor Author

lal12 commented Mar 5, 2026

Ah thanks for the explanation. I wonder why it didn't occur on my win VM.
But who knows it was an awful experience getting it to build there at all.
E.g. had to reduce src/cacert.h to even get it build, compiler always complained about not enough heap (on a 24GB RAM VM). I mean how can you blow up your AST and symbols to have issues with a 250Kb source file 🤦‍♂️

Anyway thanks for the work!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants