Skip to content

Add ReportSystemAllocations option, default to VLD 2.5.10 leak attrib…#64

Merged
anporumb merged 18 commits into
masterfrom
anporumb/immediate-caller-and-api-pair-tracking
Jun 5, 2026
Merged

Add ReportSystemAllocations option, default to VLD 2.5.10 leak attrib…#64
anporumb merged 18 commits into
masterfrom
anporumb/immediate-caller-and-api-pair-tracking

Conversation

@anporumb

@anporumb anporumb commented Jun 3, 2026

Copy link
Copy Markdown

This PR has two intertwined goals:

  1. Revive the VLD 2.5.10 "immediate caller is the leak owner" default behind a new opt-in ini key, restoring the historically quieterleak report.
  2. Bring native ARM64 to behavioural parity with x64, so VLD on ARM64 reports the same leaks, the same way, with no architecture-specific surprises.

Along the way it also unifies the dynamic-DLL patching mechanism across all architectures, upgrades the bundled dbghelp.dll to a currentWindows SDK redistributable on every platform, and documents the provenance of every bundled binary.

What the user sees

New ini option: ReportSystemAllocations (default no)

ReportSystemAllocations = no ; default: VLD 2.5.10 behaviour
ReportSystemAllocations = yes ; opt-in to the always-stack-walk behaviour

  • no (default). When an allocation's immediate caller is a non-reporting CRT/system module (ucrtbase.dll, ws2_32.dll, etc.), theallocation is excluded at capture time. This is the VLD 2.5.10 behaviour: fewer system/CRT-internal allocations are surfaced as leaks.
  • yes. VLD walks the call stack from the allocation site and, if it finds a user module higher in the chain, attributes the allocation to that user code. This is the post-2.5.10 always-stack-walk behaviour. It recovers real leaks routed through an unhooked CRT but alsosurfaces more CRT/system noise (which can be trimmed with IgnoreModulesList).

Documented in vld.ini next to the existing options.

ARM64 native is now a first-class target

ARM64 used to be effectively a stub: FindRealCode was a no-op, dynamic-module IAT patching was not installed, and dynamic_app's "0 leaks"was achieved by accident (user-DLL IATs were never patched, so the 2.5.15 stack walk happened to find dynamic.dll from the unhooked CRTframe). With ReportSystemAllocations defaulting to no, that accidental path stops working, so ARM64 needed real implementations.

This PR adds, for ARM64 only:

  • FindRealCode thunk decoding so the IAT patcher resolves ARM64 import veneers to their terminal bodies.
  • Arm64CaptureKernelbaseHeapProcs / Arm64LookupKernelbaseHeapProc to capture the real kernelbase bodies for HeapCreate / HeapAlloc / HeapReAlloc / HeapFree / HeapDestroy. On ARM64 these kernel32 exports are fast-forward dispatch thunks whose target slot resolves lazily(kernel32 on ARM64 Windows ships as an ARM64X binary, hence the mechanism). Without this capture, calling the kernel32 export from insideVLD's hook at process detach (under the loader lock) would either recurse back into the hook or spin on an unresolved dispatch thunk.
  • Arm64TeardownReportScope — a process-teardown guard that disables dbghelp symbol resolution during ~VisualLeakDetector's leak report on ARM64. The bundled ARM64 dbghelp.dll reliably spins forever inside SymFromInlineContextW / SymFindFileInPathW when called under theloader lock at shutdown. Address-only frames are emitted instead, mirroring what x64 dbghelp returns for the same inputs. The destructorpre-warms the suppression cache (CallStack::m_status and blockinfo_t::reported) by calling GetLeaksCount() BEFORE the scope is activated,so suppression by IgnoreFunctionsList/IgnoreModulesList/SkipCrtStartupLeaks still works correctly under the scope.
  • A handful of small ARM64-only patch-site guards (e.g. PatchImport must not record VLD's own hook as the saved "real" address when anARM64 fast-forward dispatch veneer routes the resolution through an already-patched IAT slot).

x64 was carefully kept byte-identical with master where the comment didn't lie about the code.

Unified dynamic-module patching on LdrRegisterDllNotification

The historical x64 design installed a machine-code detour on ntdll's internal LdrpCallInitRoutine (the routine that invokes DllMain) sothat newly mapped DLLs could be IAT-patched between "imports snapped" and "DllMain runs". That detour was a fixed-pattern byte scanner: it broke on each new ntdll release and never worked on ARM64.

This PR deletes the detour entirely:

  • Removed: LdrpCallInitRoutine detour installer, NtDllPatch / NtDllRestore / NtDllFindDetourAddress / NtDllFindDetourParam /NtDllFindDetourCall, the NTDLL_LDR_PATCH struct, and the patch global.
  • Replaced by: a single LdrRegisterDllNotification LOADED/UNLOADED callback installed in the VLD constructor on every architecture. TheLOADED notification fires inside LdrpDoPostSnapWork — exactly where the detour used to interpose — so timing, race profile, and featureset are equivalent, just without hot-patching ntdll.
  • Cleanup side benefits: removed the fallback GetProcAddress and unused GetModuleHandleA in PatchImport that would have beenloader-lock-reentrant under the parallel loader.
  • The LdrpCallInitRoutine detour was also the cause of two long-standing local-only test failures in dynamic_app and vld_unload on theVS-Insiders-18.7 toolchain (the byte-pattern scanner misread the new ILT thunk shape). Those tests now pass cleanly on every gate leg.

Net deletion: ~150-200 lines from src/vld.cpp.

Bundled dbghelp.dll upgraded on every architecture

┌───────┬─────────────────┬─────────────────────┬───────────────────────────────────────────────────────────────┐
│ Arch  │ Before          │ After               │ Source                                                        │
├───────┼─────────────────┼─────────────────────┼───────────────────────────────────────────────────────────────┤
│ x64   │ 10.0.17022.1001 │ 10.0.26100.8249     │ Windows 10 SDK 10.0.26100.0, Debuggers component              │
├───────┼─────────────────┼─────────────────────┼───────────────────────────────────────────────────────────────┤
│ x86   │ 10.0.16299.15   │ 10.0.26100.5074     │ (SDK no longer ships x86 redist) Windows 11 25H2 SysWOW64     │
├───────┼─────────────────┼─────────────────────┼───────────────────────────────────────────────────────────────┤
│ ARM64 │ 10.0.26100.6584 │ 10.0.26100.8249     │ Windows 10 SDK 10.0.26100.0, Debuggers component (ARM64 host) │
└───────┴─────────────────┴─────────────────────┴───────────────────────────────────────────────────────────────┘

A previous attempt to bump x64/x86 in this branch was reverted because ignore_functions_test and ignore_modules_getaddrinfo_test brokeunder the new dbghelp's name-resolution shape. With the rest of this PR in place — particularly the LdrRegisterDllNotification unification and the new IAT-patching path — those tests pass on every gate leg with the new bundled dbghelp.

Every dbghelp folder under setup/dbghelp/ and lib/dbghelp/ now has a SOURCE.md that pins the SDK release, the file-version stamp bakedinto the redistributable, and the byte size, plus a step-by-step "How to update" recipe.

Tests added

┌─────────────────────────┬────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Test                    │ What it guards                                                                                             │
├─────────────────────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ dtor_suppression_test   │ Verifies that IgnoreFunctionsList suppression still works under Arm64TeardownReportScope, even when user   │
│                         │ code never calls a VLD API (so the suppression cache pre-warm in the destructor is the only thing keeping  │
│                         │ suppression correct). Fails without the destructor pre-warm.                                               │
├─────────────────────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ dyn_symbol_test         │ Loads a DLL via LoadLibrary after VLD has started, leaks from named functions inside it, and asserts that  │
│                         │ the leak report shows resolved frame names (dynamic.dll!LeakMemoryMalloc) rather than address-only frames. │
│                         │ Fails without RegisterLoadedModule's SymLoadModuleExW call.                                                │
└─────────────────────────┴────────────────────────────────────────────────────────────────────────────────────────────────────────────┘

Three existing tests that exercise the always-stack-walk behaviour (getaddrinfo_leaks_test, getaddrinfo_missing_free_test,ignore_modules_getaddrinfo_test) opt in with ReportSystemAllocations = yes so they continue to validate the post-2.5.10 attribution path.

Reviewer's tour

  • src/vld.cpp is where the bulk of the design change lives. Start at VldDllLoadNotification (LOADED/UNLOADED callback), thenRegisterLoadedModule / UnregisterLoadedModule (the per-DLL classification + dbghelp pre-registration), then the ARM64-onlyArm64TeardownReportScope and the ~VisualLeakDetector pre-warm.
  • src/utility.cpp carries the ARM64-specific kernel32 fast-forward dispatch capture and the FindRealCode thunk decoder, plus aself-cleanup fix in VLDVirtualProtect for the rare partial-failure case.
  • src/vld_hooks.cpp carries the ARM64-only heap-hook indirection through the captured terminal bodies (so VLD's own pass-through doesn'trecurse).
  • vld.ini and vld_def.h cover the new ReportSystemAllocations option and VLD_OPT_REPORT_SYSTEM_ALLOCS flag.
  • The SOURCE.md files under setup/dbghelp/ and lib/dbghelp/ are pure documentation but are worth a glance, particularly the explicit note that lib/dbghelp/include/DbgHelp.h and lib/dbghelp/lib/{x64,Win32}/DbgHelp.Lib predate the current SDK and were inherited from upstream2.5.15.

Validation

  • Local x64 Debug ctest: 17/17 PASS. (Previously local x64 had two pre-existing failures on the VS-Insiders-18.7 toolchain — both gonenow that the byte-pattern detour is deleted.)
  • ARM64 ctest on the ARM64 dev box: 17/17 PASS.
  • CI gate, all six legs (x86 Debug + RelWithDebInfo, x64 Debug + RelWithDebInfo, ARM64 Debug + RelWithDebInfo): GREEN on the latestcommit (d54e223).
  • A multi-model code-review pass (GPT-5.5, GPT-5.3-codex, GPT-5.4, MAI-Code-1-Flash) was run against the diff; every finding was eitherproven-and-fixed-with-a-regression-test or proven-stale-and-removed. The destructor pre-warm and the dynamic-DLL SymLoadModuleExWpre-registration both come from that review pass.

Out of scope (follow-ups)

  • The bundled DbgHelp.h and the x64/Win32 DbgHelp.Lib import libraries are still inherited from the upstream 2.5.15 release commit(4d6e7da). They were NOT updated in this PR because their structure layouts are pinned by the static_assert sizeof(IMAGEHLP_MODULE64) == 3264 in src/vld.cpp and refreshing them touches both the bundled DLLs and the assertion in lock-step. Their SOURCE.md files call this outexplicitly.

…ution

VLD 2.5.15 changed CaptureContext::IsExcludedModule() so that when an
allocation's immediate caller is a non-reporting CRT/system module it walks
the call stack to attribute the allocation to a user module higher in the
chain. VLD 2.5.10 instead excluded such allocations based solely on the
immediate caller. The 2.5.15 behavior surfaces more CRT/system-internal
allocations (e.g. WSAStartup bookkeeping, printf stdio buffers).

This adds a vld.ini option, StackWalkCrtAllocations, that gates the stack
walk. The default is "no", which restores the VLD 2.5.10 immediate-caller
behavior. Set it to "yes" to keep the VLD 2.5.15 stack-walk behavior.

Changes:
- vld_def.h: new VLD_OPT_STACKWALK_CRT_ALLOCS (0x20000) flag.
- vld.cpp: parse StackWalkCrtAllocations in configure() (default "no"); gate
  the stack walk in IsExcludedModule() behind the flag; add the flag to
  OptionsMask so GetOptions()/VLDGetOptions()/VLDSetOptions() observe it.
- vld.ini: document the new option.
- CHANGES.txt / version.txt: 2.5.16.
- Three getaddrinfo tests validate the 2.5.15 stack-walk behavior, so their
  vld.ini files opt in with StackWalkCrtAllocations = yes.

Verified with a leak battery: default (no) reports 3 user leaks (matches
2.5.10); opt-in (yes) reports 6 (matches pristine 2.5.15). Full test suite
builds; getaddrinfo and core tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@anporumb

anporumb commented Jun 3, 2026

Copy link
Copy Markdown
Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

On ARM64, FindRealCode was a no-op, so PatchImport's normalized
func == import comparison failed whenever one side pointed at a
linker-emitted thunk. As a result operator new (and other imports)
were not patched in dynamically loaded user DLLs on Debug/incremental
builds, so the malloc-level immediate caller resolved to ucrtbase (a
CRT module with reportLeaks=FALSE). With the new default
StackWalkCrtAllocations=no (2.5.10 immediate-caller behavior), those
DLL allocations were excluded, undercounting leaks. This regressed the
dynamic_app and vld_unload gate tests on ARM64 while x64 stayed green.

Decode the common MSVC ARM64 veneer shapes so the real function body is
reached, mirroring the x86/x64 logic:
  - B imm26 incremental-linking thunk (analog of E9 jmp rel32)
  - ADRP Xn; LDR Xn,[Xn,#imm12]; BR Xn import stub (analog of FF25)
  - ADRP Xn; ADD Xn,Xn,#imm12; BR Xn
  - ADRP Xn; ADD Xn,Xn,#imm12; LDR Xn,[Xn]; BR Xn
  - LDR Xn, literal; BR Xn
The scratch register field is decoded (x16 or x17) rather than
hard-coded. Resolution is iterative with a depth bound and a self-loop
guard. Un-gate VLDVirtualProtect/VLDVirtualRestore so they are compiled
on ARM64 as well.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@anporumb

anporumb commented Jun 3, 2026

Copy link
Copy Markdown
Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

ARM64 Windows now matches x64 leak attribution and teardown. All changes are
guarded with #if defined(_M_ARM64); the x64 code path is unchanged. Full ctest
passes 15/15 on ARM64 hardware.

Fixes (ARM64-only):
- RestoreImport teardown hang: on ARM64X, kernel32 heap exports are
  fast-forward dispatch thunks whose slot is never resolved. Capture the real
  kernelbase/ntdll terminal bodies at DLL attach (after a heap warm-up resolves
  kernelbase forwarders) and restore module IATs to those instead of the
  unresolved kernel32 thunk. New Arm64CaptureKernelbaseHeapProcs /
  Arm64LookupKernelbaseHeapProc in utility.cpp.
- Process-exit access violation: unregister the LDR DLL load notification in
  the destructor so the loader does not call back into an unloaded vld_arm64
  during the CRT exit path.
- _HeapDestroy/_HeapAlloc/_HeapReAlloc infinite recursion: these hooks have a
  NULL patch-table original and called the kernel32 API directly, which on
  ARM64X routes back through kernel32's patched dispatch slot into the same
  hook. Call the captured terminal body instead. No re-entry of _RtlAllocateHeap
  and no double-counting (verified by leak counts).
- Teardown symbolization: the bundled ARM64 dbghelp spins forever resolving
  symbols under the loader lock at teardown. Emit address-only frames and skip
  SymCleanup during the automatic leak report (x64 parity); leak suppression is
  decided earlier while modules are stable.
- PatchImport store-time guard: never record VLD's own code as the call-through
  original when an ARM64X veneer routes a resolution into VLD's module.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@anporumb

anporumb commented Jun 4, 2026

Copy link
Copy Markdown
Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

On ARM64 PatchImport runs from the LDR DLL-load notification
(VldDllLoadNotification) under the parallel loader. Two hooked calls in
PatchImport re-enter LdrLockLoaderLock there and deadlock against a
concurrent loader worker that owns the lock:
 - the fallback GetProcAddress (utility.cpp ~801), and
 - a dead GetModuleHandleA whose result is unused (utility.cpp ~823).

Skip both on ARM64. The saved real _RGetProcAddress already resolves every
patch-table import, so the fallback is unnecessary, and the GetModuleHandleA
result was never used. x64 keeps the original code path unchanged via
#if !defined(_M_ARM64) guards. Also corrects the vld.cpp notification comment
that incorrectly claimed the loader lock is held recursively in the callback.

Validated on ARM64: full dynamic_app suite passes deterministically (1152
leaks, no hang) and full ctest is 15/15.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@anporumb

anporumb commented Jun 4, 2026

Copy link
Copy Markdown
Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Fixes ARM64-only +1 off-by-one in dynamic_app MultithreadLoadingTests
(1153 vs 1152 expected). Root cause: dynamic.dll has a static MemoryLeak
ml in Allocs.cpp whose ctor calls malloc(7) during CRT init. On x64 the
LdrpCallInitRoutine detour calls RefreshModules before PatchCurrentModule,
which puts dynamic.dll into m_loadedModules with VLD_MODULE_EXCLUDED set
because the module does not import g_vld. The hooked malloc therefore
sees an excluded module and skips tracking. On ARM64 the LDR LOADED
notification only called PatchCurrentModule; dynamic.dll was not in
m_loadedModules, so isModuleExcluded returned false and the static-init
allocation was tracked, producing one extra reported leak per test run.

Mirror the x64 behavior without invoking the loader-lock-unsafe paths
(EnumerateLoadedModulesW64, dbghelp SymLoadModuleEx, GetProcAddress) that
would deadlock the parallel loader on ARM64:

* Add FindImportByExportAddress in utility.cpp/h (ARM64 only) that walks
  the IAT looking for a pre-resolved export address instead of calling
  GetProcAddress on the loading thread.

* Add VisualLeakDetector::Arm64RegisterLoadedModule that computes module
  flags by mirroring attachToLoadedModules' classification (FindImport
  against g_vld, m_patchTable lookup, ForceIncludeModules /
  IgnoreModulesList) and inserts the entry into m_loadedModules under
  m_modulesLock. Updates m_patchTable[].moduleBase for matching CRT
  modules. Does not call dbghelp or any loader-lock-reentrant API.

* VldDllLoadNotification calls Arm64RegisterLoadedModule BEFORE
  PatchCurrentModule so DllMain / static initializers see the proper
  EXCLUDED flag. Switches the patching-reentry flag to RAII so any
  early return keeps the flag balanced.

All additions are wrapped in #if defined(_M_ARM64); x64 binaries remain
byte-identical (verified by build).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@anporumb

anporumb commented Jun 4, 2026

Copy link
Copy Markdown
Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@anporumb

anporumb commented Jun 4, 2026

Copy link
Copy Markdown
Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Two minimal cleanups after auditing the branch against master to reduce the
change footprint to what is strictly necessary:

* PatchImport store-time guard (src/utility.cpp): wrap the
  "GetCallingModule(func) != vldModule" check in #if defined(_M_ARM64). The
  guard fixes an ARM64X-only recursion (kernel32 dispatch thunk routes a
  module's import through VLD's own ILT thunk, so FindRealCode resolves into
  VLD and "original" would point back at the hook). x64 has no fast-forward
  thunk chain that can route through VLD, so the previous unconditional store
  is preserved there and x64 is byte-identical with upstream for this hunk.
  The vldModule lookup is also ARM64-only now.

* VldDllLoadNotification (src/vld.cpp): drop the RAII PatchingGuard wrapper
  and go back to the simple "patching = true; ...; patching = false;" pair.
  The function has no early-return path between set and clear, so the RAII
  is unnecessary protection that just adds code.

ARM64 ctest 15/15 still passes; x64 build still clean.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@anporumb

anporumb commented Jun 4, 2026

Copy link
Copy Markdown
Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

The old name described the implementation (a stack walk through CRT
allocations) rather than what the user actually gets from the option. The new
name describes the user-visible effect: when enabled, VLD reports allocations
made by system/CRT modules on behalf of the calling code.

Renamed in all user-facing surfaces and corresponding internals:
- src/vld_def.h: VLD_OPT_STACKWALK_CRT_ALLOCS -> VLD_OPT_REPORT_SYSTEM_ALLOCS
- src/vld.cpp: configure() option key, OptionsMask, IsExcludedModule gate,
  inline comment
- vld.ini: doc block reworded around "report system allocations" and option
  name updated
- CHANGES.txt: 2.5.16 release note updated
- Three opt-in test ini files (getaddrinfo_leaks_test,
  getaddrinfo_missing_free_test, ignore_modules_getaddrinfo_test)

ARM64 ctest 15/15 passes; x64 builds clean. No behavior change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@anporumb

anporumb commented Jun 4, 2026

Copy link
Copy Markdown
Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

ARM64, ARM64X, and ARM64EC are distinct things, and four of our comments
conflated them. VLD here is built as native ARM64 (_M_ARM64); kernel32 on
Windows ARM64 is an ARM64X PE (a single image bundling both ARM64 and ARM64EC
implementations). The fast-forward dispatch thunks that cause the heap-hook
recursion live in kernel32 because kernel32 is ARM64X. They are not a property
of the runtime mode we are in.

Fix four comments that read as if we are "on ARM64X":
- src/utility.cpp Arm64CaptureKernelbaseHeapProcs preamble
- src/vld_hooks.cpp _HeapDestroy / _HeapAlloc / _HeapReAlloc preambles

The corrected phrasing is "On ARM64, kernel32's ... is/are ARM64X ..."
(ARM64 = our runtime mode; ARM64X stays as the adjective that explains why
kernel32's exports are dispatch thunks). The three other places that already
used ARM64X correctly as an adjective on the dispatch mechanism
(utility.cpp:898, utility.h:169, vld.cpp:535) are left alone.

Comments only. No behavior change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@anporumb

anporumb commented Jun 4, 2026

Copy link
Copy Markdown
Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

…tures

x86:   10.0.16299.15        ->  10.0.26100.5074
x64:   10.0.17022.1001      ->  10.0.26100.8249
ARM64: 10.0.26100.6584      ->  10.0.26100.8249

Source for each:
* x86   - C:\Windows\SysWOW64\dbghelp.dll (system, redistributable)
* x64   - C:\Program Files (x86)\Windows Kits\10\Debuggers\x64\dbghelp.dll
* ARM64 - C:\Program Files (x86)\Windows Kits\10\Debuggers\arm64\dbghelp.dll

The new DLLs import the same set of system API-set DLLs (api-ms-win-*)
with a few additions (ms-win-core-com, ms-win-core-threadpool,
ms-win-core-util, ms-win-crt-locale, OLEAUT32) that all resolve from the
running OS. No companion DLLs (dbgcore, symsrv, srcsrv) need to be
shipped alongside.

Validation on ARM64 hardware:
* Full ctest 15/15 PASS with the new bundled dbghelp.
* The bundled ARM64 dbghelp's loader-lock deferred-symbol-load spin is
  NOT fixed in 10.0.26100.8249 - the isolated
  --gtest_filter=*MultithreadLoadingTests* repro still hangs 5/5 (same as
  with the previous 10.0.26100.6584 version). The ARM64 teardown-report
  guards in vld.cpp and callstack.cpp remain necessary.

Updated files:
* setup/dbghelp/x86/dbghelp.dll
* setup/dbghelp/x64/dbghelp.dll
* setup/dbghelp/arm64/dbghelp.dll
* lib/dbghelp/lib/arm64/dbghelp.dll (kept in sync with setup/)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@anporumb

anporumb commented Jun 4, 2026

Copy link
Copy Markdown
Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

…eline

The previous commit (3030436) updated bundled dbghelp.dll for all three
architectures. The ARM64 update (10.0.26100.6584 -> 10.0.26100.8249) is
fine: full ARM64 ctest still passes 15/15.

The x86 and x64 updates broke two tests on x64 in CI (build 166770957):
* ignore_functions_test - all three subtests fail with "expected 0/1/2
  leaks, got 4". The new x64 dbghelp resolves the static-leak helper names
  ("GetOSVersion", "SomeOtherString", "abcdefg", ...) in a form that the
  exact-match IgnoreFunctionsList no longer recognizes, so the
  intentionally-ignored allocations are now reported as leaks.
* ignore_modules_getaddrinfo_test - one subtest fails for the same family
  of reason (the test relies on dbghelp-resolved module attribution for an
  IgnoreModulesList match).

Tightening or expanding the ini lists to cover both old and new dbghelp
name shapes is out of scope here, so just keep the previously shipped x86
and x64 dbghelp - that pair is what upstream master uses and what the CI
tests were calibrated against.

Net change vs upstream master: ARM64 dbghelp upgraded to 10.0.26100.8249;
x86 and x64 left untouched.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ormatting

Removes references to the review-time bug clusters (A, B, ...) from the

two regression tests' source comments. Those tags only made sense during

the original code-review thread and would be opaque to anyone reading the

code later. Replaces them with descriptions of what the test actually

guards.

Also adds a blank line in ignore_modules_getaddrinfo_test/vld.ini before

the ReportSystemAllocations block so the explanatory comment is clearly

separated from the preceding option.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@anporumb

anporumb commented Jun 5, 2026

Copy link
Copy Markdown
Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

After unifying the loader-notification path on LdrRegisterDllNotification,
distinguishing ARM64 from ARM64X, and renaming StackWalkCrtAllocations to
ReportSystemAllocations, several comments, docstrings, identifiers and ini
files were still describing the old world. This commit synchronizes them
with the current code without changing behavior.

Stale "LdrpCallInitRoutine detour" comments:
  * src/vld.cpp _LdrLoadDll - was 22-line comment describing re-enumeration
    of all modules after each LdrLoadDll. The hook is now a pure pass-through,
    documented as such.
  * src/vld.cpp PatchCurrentModule - doc now says it is called from
    VldDllLoadNotification before DllMain, not from a deleted detour.
  * src/vld.cpp isModuleIgnored - removed the obsolete race-condition
    rationale (the LDR LOADED callback inserts every dynamic DLL into
    m_loadedModules before its DllMain runs).
  * src/vld.cpp UnregisterLoadedModule - "next RefreshModules" reworded to
    "explicit VLDRefreshModules call (if the host code makes one)".
  * src/vld.cpp parallel-loader paragraph - "post-LdrLoadDll hook" reframed
    as the historical alternative design.
  * src/tests/CMakeLists.txt dyn_symbol_test block - rewritten to describe
    the shared LDR LOADED callback.
  * src/tests/dyn_symbol_test/dyn_symbol_test.cpp intro comment - same.

ARM64 vs ARM64X terminology cleanup:
  * The ARM64X label is the kernel32 binary's mechanism, not anything VLD
    builds against. Every code-flow comment now says "fast-forward dispatch
    thunk" / "kernel32 dispatch slot" instead of "ARM64X dispatch slot".
  * Two paragraphs (src/vld.cpp _CRT_INIT pre-warm, src/utility.cpp top of
    file) retain a single parenthetical explanation of why kernel32 has the
    forwarders, with ARM64X named explicitly there as the proper Windows term.
  * Files touched: src/vld.cpp, src/utility.cpp, src/utility.h,
    src/vld_hooks.cpp.

ReportSystemAllocations version-label drift:
  * The 2.5.10 reference is real and kept (last public version with the
    immediate-caller default). The 2.5.15 reference was opaque (no public
    2.5.15 release in CHANGES.txt) and has been replaced everywhere with
    the descriptive label "always-stack-walk behavior".
  * Files touched: src/vld.cpp, src/vld_def.h, vld.ini,
    src/tests/getaddrinfo_leaks_test/vld.ini,
    src/tests/getaddrinfo_missing_free_test/vld.ini,
    src/tests/ignore_modules_getaddrinfo_test/vld.ini.

Naming collision with deleted NtDllPatch detour:
  * The IAT-patch tables for ntdll were lowercase ntdllPatch (file-scope
    moduleentry_t array in vld.cpp) and m_ntdllPatch (patchentry_t member
    of VisualLeakDetector). Both stayed in use after the unrelated NtDllPatch
    machine-code detour was deleted, but the visual similarity was
    confusing. Renamed to ntdllModuleEntry and m_ntdllImportPatch
    respectively (vld.cpp, vldint.h, dllspatches.cpp).

Top-level vld.ini polish:
  * ReportSystemAllocations Valid Values now uses the same unquoted
    "yes, no" style as every other option block in the file.

Test ini comment consistency:
  * dyn_symbol_test/vld.ini no longer claims this is ARM64-only (it is
    now a unified-path regression on every architecture).
  * The three getaddrinfo / ignore_modules_getaddrinfo tests all describe
    their ReportSystemAllocations opt-in with the same wording.

CHANGES.txt:
  * Fixed the release date for 2.5.16 (5 June 2026, not 3 June).
  * Reworded to drop the "VLD 2.5.15 behavior" phrase (no public 2.5.15
    release).

No code behavior changes; the only non-comment edits are the renames and
the vld.ini style tweak. Verified that the renames build clean on x64
Debug.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@anporumb

anporumb commented Jun 5, 2026

Copy link
Copy Markdown
Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

The original SOURCE.md files said WHERE on disk each bundled file came from
but did not nail down the Windows SDK version that owns those paths, nor
the exact file-version stamp inside each DLL. That made it hard to tell at a
glance whether the file someone copied in last week matches what the repo
shipped a month ago.

For every dbghelp folder, the SOURCE.md now lists:
  * the SDK release version that produced the file (eg "Windows 10 SDK
    10.0.26100.0, Debugging Tools for Windows component"), or, when the
    file predates this repo's current SDK install, "inherited from upstream
    VLD 2.5.15 release commit 4d6e7da";
  * the FileVersion stamp baked into each redistributable DLL
    (10.0.26100.8249 for x64 and ARM64, 10.0.26100.5074 for x86) and the
    exact file size, with a copy-paste PowerShell one-liner to re-read it
    locally;
  * for the bundled header (lib/dbghelp/include/DbgHelp.h) and the x64/Win32
    import libraries (lib/dbghelp/lib/x64/DbgHelp.Lib and
    lib/dbghelp/lib/Win32/DbgHelp.Lib), explicit notes that their SHA256
    hashes do NOT match the current Windows 10 SDK 10.0.26100.0 equivalents
    (sizes recorded), so a future maintainer cannot mistake them for stock
    SDK files;
  * for the x86 runtime DLL, the SysWOW64 fallback is documented as the
    only redistributable source available (the current SDK Debuggers
    component installs only x64) with the OS build that was used as source
    (Windows 11 Enterprise 26200.8390 / 25H2 on this dev machine);
  * the static assertion in src/vld.cpp that ties IMAGEHLP_MODULE64 layout
    between the bundled header and the bundled DLLs;
  * a step-by-step "How to update" recipe in every file.

No code changes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@anporumb

anporumb commented Jun 5, 2026

Copy link
Copy Markdown
Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@anporumb anporumb marked this pull request as ready for review June 5, 2026 12:43
@anporumb anporumb requested a review from a team June 5, 2026 12:43
@anporumb anporumb enabled auto-merge (squash) June 5, 2026 12:43
@anporumb

anporumb commented Jun 5, 2026

Copy link
Copy Markdown
Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

The repo had been carrying the public header (lib/dbghelp/include/DbgHelp.h)
and the x64/Win32 import libraries (lib/dbghelp/lib/x64/DbgHelp.Lib and
lib/dbghelp/lib/Win32/DbgHelp.Lib) inherited verbatim from the upstream
VLD 2.5.15 release commit (4d6e7da). The runtime DLLs under setup/dbghelp/
were updated last week to the Windows 10 SDK 10.0.26100.0 redistributables
but the matching header and link-time libraries had stayed pinned to a much
older SDK release. This commit aligns all three:

  Bundled header:
    lib/dbghelp/include/DbgHelp.h
        before:  109,113 bytes, SHA256 868433...92E5B
                 (upstream VLD 2.5.15, exact source SDK version unknown)
        after:   124,135 bytes, SHA256 55EC5F...D161C
                 (Windows 10 SDK 10.0.26100.0,
                  Include\10.0.26100.0\um\DbgHelp.h)

  x64 import library:
    lib/dbghelp/lib/x64/DbgHelp.Lib
        before:  53,008 bytes
        after:   57,836 bytes
                 (Windows 10 SDK 10.0.26100.0,
                  Lib\10.0.26100.0\um\x64\DbgHelp.Lib)

  x86 import library:
    lib/dbghelp/lib/Win32/DbgHelp.Lib
        before:  56,180 bytes
        after:   60,310 bytes
                 (Windows 10 SDK 10.0.26100.0,
                  Lib\10.0.26100.0\um\x86\DbgHelp.Lib)

  Existing redistributable DLLs (unchanged in this commit, already at the
  same SDK release):
    setup/dbghelp/x64/dbghelp.dll        10.0.26100.8249
    setup/dbghelp/arm64/dbghelp.dll      10.0.26100.8249
    lib/dbghelp/lib/arm64/dbghelp.dll    10.0.26100.8249
    setup/dbghelp/x86/dbghelp.dll        10.0.26100.5074
                                         (system SysWOW64 fallback; SDK no
                                          longer ships x86 redistributable)

ABI compatibility check: the `IMAGEHLP_MODULE64` struct that
src/vld.cpp pins via `static_assert sizeof(IMAGEHLP_MODULE64) == 3264` is
byte-for-byte identical between the 2.5.15 header and the SDK
10.0.26100.0 header, so the assertion still holds without modification.
All dbghelp APIs VLD calls (SymInitializeW, SymCleanup, SymSetOptions,
SymFromAddrW, SymGetLineFromAddrW64, SymGetModuleInfoW64,
SymLoadModuleExW, SymUnloadModule64, StackWalk64,
EnumerateLoadedModulesW64, ImageDirectoryEntryToDataEx) are decades-old
stable exports and resolve cleanly against the new import library.

SOURCE.md updates: lib/dbghelp/include/SOURCE.md and the x64 / Win32
import-lib SOURCE.md files all drop the "inherited from upstream VLD
2.5.15" provenance note and now pin SDK 10.0.26100.0 as the explicit
source, with the new file sizes and SHA256 hashes recorded.

Validated by a clean rebuild of vld on x64 Debug (the static assertion
passes; all dbghelp symbols link) followed by ctest 17/17 PASS locally.
Equivalent ARM64-box and CI gate runs are queued in the same iteration.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@anporumb

anporumb commented Jun 5, 2026

Copy link
Copy Markdown
Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Comment thread lib/dbghelp/include/DbgHelp.h
Comment thread src/utility.cpp
Comment thread src/utility.cpp
Comment thread src/tests/dtor_suppression_test/dtor_suppression_test.cpp Outdated

@dcristoloveanu dcristoloveanu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:shipit:

@anporumb

anporumb commented Jun 5, 2026

Copy link
Copy Markdown
Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Comment thread src/vld.cpp
Comment thread src/vld.cpp
Comment thread CHANGES.txt

@jebrando jebrando left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

:shipit:

@anporumb anporumb merged commit d39bca1 into master Jun 5, 2026
8 checks passed
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.

3 participants