Skip to content

deps(mimalloc): set MI_OVERRIDE=OFF on Windows#29467

Merged
dylan-conway merged 2 commits into
mainfrom
claude/interesting-mahavira-c5f1c5
Apr 18, 2026
Merged

deps(mimalloc): set MI_OVERRIDE=OFF on Windows#29467
dylan-conway merged 2 commits into
mainfrom
claude/interesting-mahavira-c5f1c5

Conversation

@dylan-conway

Copy link
Copy Markdown
Member

Summary

Windows debug builds fail to link since the dev3 mimalloc bump (#29420 / #29435):

lld-link: error: duplicate symbol: _expand
>>> defined at mimalloc-debug.lib(alloc.c.obj)
>>> defined at libucrtd.lib(expand.obj)

Root cause: mimalloc.ts never set MI_OVERRIDE for Windows — only a comment claiming the upstream default was "no override". The actual default is ON. Pre-dev3 this was harmless because alloc-override.c's _MSC_VER block was an empty comment ("cannot override malloc unless using a dll"). Upstream microsoft/mimalloc#1259 / #1263 filled it with real CRT symbol definitions (_expand, _msize, _msize_base, _free_base, free), so the static lib now exports them and collides with the debug CRT.

Fix: explicit MI_OVERRIDE=OFF on Windows. Bun links the static CRT and calls mi_* directly; nothing routes through CRT malloc, so override has no benefit there. This restores the effective pre-dev3 behavior.

Not stale cache: dep_configure already uses cmake --fresh, so the cache was correctly regenerated — it got ON because that's the real default.

Why CI didn't catch it: all ci-* profiles use buildType: "Release" (/MTlibucrt.lib). The duplicate only fires under /MTd because libucrtd.lib's expand.obj is pulled in for its debug-heap symbols. CI never builds Windows debug.

Test plan

  • bunx tsc --noEmit -p scripts/build/tsconfig.json
  • bun scripts/build.ts --configure-only
  • Windows debug: bun bd --version1.3.13-debug (previously failed at link)
  • Verified build/debug/deps/mimalloc/CMakeCache.txt shows MI_OVERRIDE:BOOL=OFF after reconfigure
  • CI (release Windows — also flips ON→OFF; expected no-op since pre-dev3 override did nothing on Windows static)

mimalloc's CMake defaults MI_OVERRIDE to ON. The Windows branch in
mimalloc.ts was a comment ("use mimalloc's defaults — no override"),
which was wrong about the default and set nothing.

This was harmless before dev3 because alloc-override.c's _MSC_VER block
was a no-op ("cannot override malloc unless using a dll"). Upstream
microsoft/mimalloc#1259/#1263 filled it in with real CRT symbol
definitions (_expand/_msize/free), so the static lib now exports them
and lld-link fails Windows debug builds with:

  duplicate symbol: _expand
  defined in mimalloc-debug.lib(alloc.c.obj) and libucrtd.lib(expand.obj)

Bun links the static CRT and calls mi_* directly on Windows; nothing
routes through CRT malloc, so override has no benefit. Setting OFF
restores the effective pre-dev3 behavior.

CI didn't catch this because all ci-* profiles use buildType: Release,
which links /MT (libucrt) — its expand.obj isn't pulled into the link.
Only /MTd (libucrtd) drags expand.obj in for its debug-heap symbols.
@robobun

robobun commented Apr 18, 2026

Copy link
Copy Markdown
Collaborator
Updated 7:56 PM PT - Apr 18th, 2026

@dylan-conway, your commit 1432ab3 has 3 failures in Build #46361 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 29467

That installs a local version of the PR into your bun-29467 executable, so you can run:

bun-29467 --bun

@coderabbitai

coderabbitai Bot commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2f6bc007-161e-47e4-8695-8f79cecb6041

📥 Commits

Reviewing files that changed from the base of the PR and between 684dd94 and 1432ab3.

📒 Files selected for processing (1)
  • scripts/build/deps/mimalloc.ts

Walkthrough

Added an explicit Windows branch in the mimalloc build script that sets MI_OVERRIDE = "OFF" for Windows targets; other existing cfg.asan, cfg.darwin, and cfg.linux branches remain unchanged and subsequent debug/valgrind flag logic is unaffected.

Changes

Cohort / File(s) Summary
Windows CMake configuration
scripts/build/deps/mimalloc.ts
Introduced an explicit cfg.windows branch that sets args.MI_OVERRIDE = "OFF" for Windows builds. Existing cfg.asan, cfg.darwin, and cfg.linux logic unchanged; debug/valgrind/flag handling also unchanged.
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and concisely summarizes the main change: setting MI_OVERRIDE=OFF on Windows to fix debug build linking failures.
Description check ✅ Passed The description comprehensively explains what the PR does and how it was verified, exceeding the template requirements with detailed context on root cause, fix rationale, and testing.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — simple, targeted build fix with clear root-cause analysis; the only finding is a minor doc nit in the comment matrix.

Extended reasoning...

Overview

Single-file change to scripts/build/deps/mimalloc.ts: adds an explicit else if (cfg.windows) branch setting MI_OVERRIDE=OFF, and removes a stale/incorrect comment that claimed the Windows default was already "no override". Touches only the build-script layer, not runtime code.

Security risks

None. This is a CMake flag passed during dependency configuration. It controls whether mimalloc replaces the system malloc/free on Windows — explicitly disabling it (which matches the pre-dev3 effective behavior) has no security implications.

Level of scrutiny

Low. The change is mechanical: one new else if branch, one removed stale comment, well-explained rationale. The author verified locally (CMakeCache shows MI_OVERRIDE:BOOL=OFF) and the PR description traces the root cause in detail. The fix is consistent with the existing pattern for macOS/ASAN.

Other factors

The sole bug report is a documentation nit (decision matrix comment omits the new Windows row) — non-blocking and does not affect correctness. No prior reviews on this PR.

Comment thread scripts/build/deps/mimalloc.ts

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — my prior inline comment was a false positive; the Windows row is already present in the decision matrix. Straightforward, well-documented fix.

Extended reasoning...

Overview

Single-file change to scripts/build/deps/mimalloc.ts: adds an explicit MI_OVERRIDE=OFF CMake argument for Windows builds and adds a Windows row to the override decision matrix comment. The old code had an incorrect comment claiming Windows used "no override" via mimalloc defaults, when the actual default is ON.

Security risks

None. This is a build configuration change affecting a memory allocator compile-time flag. It has no effect on runtime security boundaries, authentication, or data handling.

Level of scrutiny

Low. The change is mechanical: one new else if (cfg.windows) branch setting a single CMake boolean, plus a documentation update to keep the matrix in sync. The root cause (upstream mimalloc dev3 filling in alloc-override.c with real CRT symbols) is well-researched and documented. The fix (explicit OFF) is the obvious correct response.

Other factors

My previous inline comment (flagging a missing Windows row in the decision matrix) was a false positive — the PR already adds that row. No bugs were found by the bug hunting system. The test plan is thorough: TypeScript type-check, configure-only dry run, and a verified CMakeCache.txt showing MI_OVERRIDE:BOOL=OFF after reconfigure. CI coverage gap (release-only Windows CI) is acknowledged and explained.

@dylan-conway dylan-conway merged commit 40ffda1 into main Apr 18, 2026
58 of 61 checks passed
@dylan-conway dylan-conway deleted the claude/interesting-mahavira-c5f1c5 branch April 18, 2026 22:06
structwafel pushed a commit to structwafel/bun that referenced this pull request Apr 25, 2026
## Summary

Windows debug builds fail to link since the dev3 mimalloc bump (oven-sh#29420 /
oven-sh#29435):

```
lld-link: error: duplicate symbol: _expand
>>> defined at mimalloc-debug.lib(alloc.c.obj)
>>> defined at libucrtd.lib(expand.obj)
```

**Root cause:** `mimalloc.ts` never set `MI_OVERRIDE` for Windows — only
a comment claiming the upstream default was "no override". The actual
default is `ON`. Pre-dev3 this was harmless because `alloc-override.c`'s
`_MSC_VER` block was an empty comment ("cannot override malloc unless
using a dll"). Upstream
[microsoft/mimalloc#1259](microsoft/mimalloc#1259)
/ oven-sh#1263 filled it with real CRT symbol definitions (`_expand`, `_msize`,
`_msize_base`, `_free_base`, `free`), so the static lib now exports them
and collides with the debug CRT.

**Fix:** explicit `MI_OVERRIDE=OFF` on Windows. Bun links the static CRT
and calls `mi_*` directly; nothing routes through CRT malloc, so
override has no benefit there. This restores the effective pre-dev3
behavior.

**Not stale cache:** `dep_configure` already uses `cmake --fresh`, so
the cache was correctly regenerated — it got `ON` because that's the
real default.

**Why CI didn't catch it:** all `ci-*` profiles use `buildType:
"Release"` (`/MT` → `libucrt.lib`). The duplicate only fires under
`/MTd` because `libucrtd.lib`'s `expand.obj` is pulled in for its
debug-heap symbols. CI never builds Windows debug.

## Test plan

- [x] `bunx tsc --noEmit -p scripts/build/tsconfig.json`
- [x] `bun scripts/build.ts --configure-only`
- [x] Windows debug: `bun bd --version` → `1.3.13-debug` (previously
failed at link)
- [x] Verified `build/debug/deps/mimalloc/CMakeCache.txt` shows
`MI_OVERRIDE:BOOL=OFF` after reconfigure
- [ ] CI (release Windows — also flips ON→OFF; expected no-op since
pre-dev3 override did nothing on Windows static)
xhjkl pushed a commit to xhjkl/bun that referenced this pull request May 14, 2026
## Summary

Windows debug builds fail to link since the dev3 mimalloc bump (oven-sh#29420 /
oven-sh#29435):

```
lld-link: error: duplicate symbol: _expand
>>> defined at mimalloc-debug.lib(alloc.c.obj)
>>> defined at libucrtd.lib(expand.obj)
```

**Root cause:** `mimalloc.ts` never set `MI_OVERRIDE` for Windows — only
a comment claiming the upstream default was "no override". The actual
default is `ON`. Pre-dev3 this was harmless because `alloc-override.c`'s
`_MSC_VER` block was an empty comment ("cannot override malloc unless
using a dll"). Upstream
[microsoft/mimalloc#1259](microsoft/mimalloc#1259)
/ oven-sh#1263 filled it with real CRT symbol definitions (`_expand`, `_msize`,
`_msize_base`, `_free_base`, `free`), so the static lib now exports them
and collides with the debug CRT.

**Fix:** explicit `MI_OVERRIDE=OFF` on Windows. Bun links the static CRT
and calls `mi_*` directly; nothing routes through CRT malloc, so
override has no benefit there. This restores the effective pre-dev3
behavior.

**Not stale cache:** `dep_configure` already uses `cmake --fresh`, so
the cache was correctly regenerated — it got `ON` because that's the
real default.

**Why CI didn't catch it:** all `ci-*` profiles use `buildType:
"Release"` (`/MT` → `libucrt.lib`). The duplicate only fires under
`/MTd` because `libucrtd.lib`'s `expand.obj` is pulled in for its
debug-heap symbols. CI never builds Windows debug.

## Test plan

- [x] `bunx tsc --noEmit -p scripts/build/tsconfig.json`
- [x] `bun scripts/build.ts --configure-only`
- [x] Windows debug: `bun bd --version` → `1.3.13-debug` (previously
failed at link)
- [x] Verified `build/debug/deps/mimalloc/CMakeCache.txt` shows
`MI_OVERRIDE:BOOL=OFF` after reconfigure
- [ ] CI (release Windows — also flips ON→OFF; expected no-op since
pre-dev3 override did nothing on Windows static)
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