Skip to content

Fix Linux madvise() advice values incorrectly combined via bitwise OR in GCToOSInterface::VirtualReset#126966

Open
Copilot wants to merge 6 commits intomainfrom
copilot/fix-madvise-advice-values
Open

Fix Linux madvise() advice values incorrectly combined via bitwise OR in GCToOSInterface::VirtualReset#126966
Copilot wants to merge 6 commits intomainfrom
copilot/fix-madvise-advice-values

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 15, 2026

main PR

Description

VirtualReset was combining MADV_FREE (8) and MADV_DONTDUMP (16) via bitwise OR and passing the result (24) to a single madvise() call. madvise() takes a single advice constant — not a bitmask — so this was either rejected as EINVAL (kernels < 5.18) or silently matched MADV_DONTNEED_LOCKED (kernels ≥ 5.18), a completely different operation.

Fix: Issue two separate madvise() calls, and use posix_madvise(POSIX_MADV_DONTNEED) as a proper fallback only when MADV_FREE is not available:

// Before (broken)
int madviseFlags = 0;
madviseFlags |= MADV_DONTDUMP;  // 16
madviseFlags |= MADV_FREE;      // 8
st = madvise(address, size, madviseFlags);  // passes 24 — invalid

// After (fixed)
#ifdef MADV_DONTDUMP
st = madvise(address, size, MADV_DONTDUMP);  // coredump hint (independent)
#endif

#ifdef MADV_FREE
st = madvise(address, size, MADV_FREE);      // primary: page reclaim
#elif defined(HAVE_POSIX_MADVISE)
st = posix_madvise(address, size, POSIX_MADV_DONTNEED);  // fallback when MADV_FREE unavailable
#endif

The posix_madvise(POSIX_MADV_DONTNEED) path is an #elif chained to #ifdef MADV_FREE, making it a proper fallback that only executes when MADV_FREE is unavailable. The guard uses MADV_FREE directly (the kernel header constant) rather than HAVE_MADV_FREE, which was never defined by the GC unix CMake build system. The previous #if defined(HAVE_POSIX_MADVISE) && !defined(MADV_DONTDUMP) condition was semantically unrelated to that fallback decision. This matches the pattern already used by VirtualReserveInner and VirtualCommitInner in the same file. Introduced by #95643.

Customer Impact

On all Linux builds where both MADV_DONTDUMP and MADV_FREE are defined (i.e., all modern glibc/x86-64 Linux):

  • Kernels < 5.18: GC never successfully resets pages to the OS — memory pressure behavior is broken and MADV_DONTDUMP is never applied (reset memory appears in coredumps).
  • Kernels ≥ 5.18: MADV_DONTNEED_LOCKED is invoked instead — immediately discarding pages including locked pages, which is semantically incorrect and potentially dangerous.

Additionally, because HAVE_MADV_FREE was never defined by the build system, the MADV_FREE reclaim path was silently compiled out on all platforms, falling through to posix_madvise(POSIX_MADV_DONTNEED) instead. Using MADV_FREE directly as the preprocessor guard restores the intended behavior.

Regression

Yes — introduced in .NET 9 by #95643 (an optimization to reduce syscall count).

Testing

The bug is a straightforward misuse of a syscall interface. The fix has been manually verified against the Linux madvise(2) man page and kernel headers. No existing automated test infrastructure covers madvise() advice values directly.

Risk

Low. The change is a minimal, targeted fix: replace one combined madvise() call with two separate calls, restructure the posix_madvise fallback as a proper #elif, and use the MADV_FREE kernel constant directly as the preprocessor guard. No logic changes, no new APIs, no structural changes.

Package authoring no longer needed in .NET 9

IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version.
Keep in mind that we still need package authoring in .NET 8 and older versions.

Copilot AI requested review from Copilot and removed request for Copilot April 15, 2026 18:46
Copilot AI requested review from Copilot and removed request for Copilot April 15, 2026 18:47
Copilot AI requested review from Copilot and removed request for Copilot April 15, 2026 18:50
Copilot AI changed the title [WIP] Fix madvise() advice values combined via bitwise OR Fix Linux madvise() advice values incorrectly combined via bitwise OR in GCToOSInterface::VirtualReset Apr 15, 2026
Copilot AI requested a review from janvorli April 15, 2026 18:52
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

Comment thread src/coreclr/gc/unix/gcenv.unix.cpp Outdated
Copilot AI requested review from Copilot and removed request for Copilot April 15, 2026 22:32
Copilot AI requested a review from janvorli April 15, 2026 22:33
@janvorli janvorli requested a review from jkotas April 15, 2026 22:35
Copy link
Copy Markdown
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM

@jkotas jkotas marked this pull request as ready for review April 15, 2026 23:23
Copilot AI review requested due to automatic review settings April 15, 2026 23:23
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes Linux GCToOSInterface::VirtualReset to avoid passing an invalid/incorrectly-combined advice value to madvise(), ensuring the GC’s “reset pages” hint behaves correctly across kernel versions.

Changes:

  • Replace the single madvise() call (with OR-combined advice values) with separate calls for MADV_DONTDUMP and the reclaim hint (MADV_FREE or POSIX fallback).
  • Restructure the POSIX fallback as a proper #elif so it is only used when the MADV_FREE path is unavailable.

Comment thread src/coreclr/gc/unix/gcenv.unix.cpp Outdated
Copilot AI requested a review from janvorli April 16, 2026 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Linux madvise() advice values incorrectly combined via bitwise OR in GCToOSInterface::VirtualReset

4 participants