Fix NativeAOT hex config parser to handle 0x/0X prefix - #127644

Merged
MichalStrehovsky merged 5 commits into
mainfrom
copilot/fix-hex-parser-prefix
May 2, 2026
Merged

Fix NativeAOT hex config parser to handle 0x/0X prefix#127644
MichalStrehovsky merged 5 commits into
mainfrom
copilot/fix-hex-parser-prefix

Conversation

CopilotAI commented May 1, 2026

Copy link
Copy Markdown
Contributor

NativeAOT's RhConfig::Environment::TryGetIntegerValue had a hand-rolled hex parser that rejected the 0x/0X prefix — returning a parse error when it encountered x. This meant env vars like DOTNET_GCHeapHardLimit=0xC0000000 silently failed to parse, leaving the hard limit unset. With GCLargePages=2 also set, the GC would then return CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT and fail initialization. CoreCLR's equivalent uses strtoul(..., 16) which handles the prefix natively.

Description

  • src/coreclr/nativeaot/Runtime/RhConfig.cpp — In TryGetIntegerValue, skip a leading 0x/0X prefix when parsing in hex mode, before entering the digit loop. Additionally, return false (parse error) when the value is exactly "0x" or "0X" with no hex digits following the prefix, matching CoreCLR's strtoul behavior:
uint32_t startIndex = 0;
if (!decimal && cchResult >= 2 && buffer[0] == '0' && (buffer[1] == 'x' || buffer[1] == 'X'))
{
startIndex = 2;
if (startIndex == cchResult)
returnfalse; // parse error - hex prefix without any digits
}
for (uint32_t i = startIndex; i < cchResult; i++)

This aligns NativeAOT's config parsing with CoreCLR's strtoul-based behavior and fixes the Collect_Aggressive_LargePages test failure under NativeAOT.

Original prompt

Problem

NativeAOT's RhConfig::Environment::TryGetIntegerValue in src/coreclr/nativeaot/Runtime/RhConfig.cpp uses a hand-rolled hex parser that does not handle the 0x or 0X prefix. This causes config values like DOTNET_GCHeapHardLimit=0xC0000000 to fail to parse, because when the parser encounters the x character it returns false (parse error).

CoreCLR's equivalent code (CLRConfigNoCache::TryAsInteger in src/coreclr/inc/clrconfignocache.h) uses strtoul(_value, &endPtr, radix) which natively handles the 0x prefix when radix is 16.

This causes the test Collect_Aggressive_LargePages added in PR #127290 to fail under NativeAOT: the GCHeapHardLimit fails to parse, so no hard limit is set, but GCLargePages=2 succeeds → the GC returns CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT and the process exits with -1.

Fix

In src/coreclr/nativeaot/Runtime/RhConfig.cpp, in the TryGetIntegerValue method, when parsing in hex mode (i.e., decimal is false), skip a leading 0x or 0X prefix before entering the digit-parsing loop. This matches the behavior of strtoul with radix 16 that CoreCLR uses.

Specifically, after reading the environment variable into buffer and before the parsing loop, add:

uint32_t startIndex = 0;
if (!decimal && cchResult >= 2 && buffer[0] == '0' && (buffer[1] == 'x' || buffer[1] == 'X'))
{
startIndex = 2;
}

Then change the loop from for (uint32_t i = 0; ...) to for (uint32_t i = startIndex; ...).

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: ```
16:22:36.657 Running test: GC\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd

Return code: 1
Raw output file: C:\h\w\B1EC0A05\w\B5C209C7\uploads\API\GC\Collect_Aggressive_LargePages\output.txt
Raw output:
BEGIN EXECUTION
call C:\h\w\B1EC0A05\p\nativeaottest.cmd C:\h\w\B1EC0A05\w\B5C209C7\e\GC\API\GC\Collect_Aggressive_LargePages\ Collect_Aggressive_LargePages.dll
Expected: 100
Actual: -1
END EXECUTION - FAILED
FAILED
Test Harness Exitcode is : 1
To run the test:
Set up CORE_ROOT and run.

C:\h\w\B1EC0A05\w\B5C209C7\e\GC\GC\native....\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd
Xunit.Sdk.TrueException:
Return code: 1
Raw output file: C:\h\w\B1EC0A05\w\B5C209C7\uploads\API\GC\Collect_Aggressive_LargePages\output.txt
Raw output:
BEGIN EXECUTION
call C:\h\w\B1EC0A05\p\nativeaottest.cmd C:\h\w\B1EC0A05\w\B5C209C7\e\GC\API\GC\Collect_Aggressive_LargePages\ Collect_Aggressive_LargePages.dll
Expected: 100
Actual: -1
END EXECUTION - FAILED
FAILED
Test Harness Exitcode is : 1
To run the test:
Set up CORE_ROOT and run.
C:\h\w\B1EC0A05\w\B5C209C7\e\GC\GC\native....\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd
at Xunit.Assert.True(Nullable`1, String)
at TestLibrary.OutOfProcessTest.RunOutOfProcessTest(String, String)
at Program.<

$>g__TestExecutor26|0_27(StreamWriter, StreamWriter, Program.<>c__DisplayClass0_0&)
16:22:36.755 Failed test: GC\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd

the test added here is failing with native AOT. why? Assistant: The test fails on NativeAOT because **NativeAOT's config reader cannot parse the `0x` prefix** in the `DOTNET_GCHeapHardLimit=0xC0000000` environment variable.
Looking at NativeAOT's `RhConfig::Environment::TryGetIntegerValue` in:
```c++ name=src/coreclr/nativeaot/Runtime/RhConfig.cpp url=https://github.com/dotnet/runtime/blob/bfaabae213cf02d343846d253b7149a03819680d/src/coreclr/nativeaot/Runtime/RhConfig.cpp#L58-L84
uint64_t uiResult = 0;
for (uint32_t i = 0; i < cchResult; i++)
{
TCHAR ch = buffer[i];
// ...
else
{
uiResult *= 16;
if ((ch >= '0') && (ch <= '9'))
uiResult += ch - '0';
else if ((ch >= 'a') && (ch <= 'f'))
uiResult += (ch - 'a') + 10;
else if ((ch >= 'A') && (ch <= 'F'))
uiResult += (ch - 'A') + 10;
else
return false; // parse error <-- 'x' hits this
}
}

When the value is 0xC0000000, the parser reads hex digits character-by-character. It handles 0, but then encounters x, which isn't a valid hex digit — so it returns false (parse error). This means GCHeapHardLimit is never set.

The cascade:

  1. DOTNET_GCHeapHardLimit=0xC0000000 fails to parse → no hard limit is set
  2. DOTNET_GCLargePages=2 is read successfully (it's a simple 2, valid hex) → use_large_pages_p = true
  3. The GC hits this check in interface.cpp: if ((!gc_heap::heap_hard_limit) && gc_heap::use_large_pages_p) → returns CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT
  4. GC initialization fails →...

This pull request was created from Copilot chat.

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/b2cf8499-44ac-4e9a-995b-c72b36e081a2
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotMay 1, 2026 11:14
CopilotAI changed the title [WIP] Fix hex parser to handle 0x prefix in TryGetIntegerValueFix NativeAOT hex config parser to handle 0x/0X prefixMay 1, 2026
CopilotAI requested a review from MichalStrehovskyMay 1, 2026 11:15
CopilotAI review requested due to automatic review settings May 1, 2026 11:21
@MichalStrehovsky
MichalStrehovsky marked this pull request as ready for review May 1, 2026 11:21
@MichalStrehovsky

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

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

CopilotAI 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.

Pull request overview

This pull request fixes NativeAOT configuration parsing so hexadecimal environment variable values can include a 0x/0X prefix (matching the commonly accepted format and aligning more closely with CoreCLR behavior), preventing GC configuration knobs like DOTNET_GCHeapHardLimit=0x... from being ignored under NativeAOT.

Changes:

  • NativeAOT RhConfig::Environment::TryGetIntegerValue now skips a leading 0x/0X prefix when parsing hex values from environment variables.
  • ILCompiler now always adds ManagedDataDescriptorProvider (previously gated behind --debug).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

FileDescription
src/coreclr/nativeaot/Runtime/RhConfig.cppAdds logic to skip 0x/0X prefix in the hand-rolled hex parser for env var config values.
src/coreclr/tools/aot/ILCompiler/Program.csRemoves the --debug condition and unconditionally roots ManagedDataDescriptorProvider (managed cDAC descriptor emission).

Comment threadsrc/coreclr/nativeaot/Runtime/RhConfig.cpp
Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #127644

Note

This review was generated by Copilot using multi-model analysis (Claude Opus 4.6, Claude Sonnet 4.5, GPT-5.3-Codex).

Holistic Assessment

Motivation: The hex prefix fix addresses a real inconsistency — strtoull (used for embedded config values) already handles 0x prefixes, but the manual environment variable parser did not, causing values like DOTNET_GCHeapCount=0x10 to fail. The ManagedDataDescriptorProvider change enables cDAC diagnostics without requiring --debug.

Approach: The hex prefix fix is minimal and correct for the common case. The ManagedDataDescriptorProvider change is a one-line behavioral shift with broader implications that should be explained.

Summary: ⚠️ Needs Human Review. The hex parsing fix is correct but has an edge case worth considering ("0x" alone). The unconditional cDAC metadata change has binary size implications and the two changes appear unrelated — a maintainer should confirm the coupling is intentional and the size impact is acceptable.


Detailed Findings

✅ Correctness — Hex prefix parsing fix (RhConfig.cpp:59-63)

The fix correctly strips the 0x/0X prefix before the hex digit parsing loop. Key observations:

  • Guard cchResult >= 2 prevents out-of-bounds access on buffer[1]
  • Aligns the environment variable path with the embedded config path (line 140) which uses strtoull and already handles 0x natively
  • All existing callers (EventPipe, GC config, dump type) benefit from this fix

⚠️ Edge Case — Bare "0x" input parses as valid zero (RhConfig.cpp:59-93) [advisory, not merge-blocking]

Flagged by all 3 models.

When buffer is exactly "0x" (cchResult == 2), startIndex is set to 2, the loop body never executes, and uiResult = 0 is returned as valid. This is consistent with the strtoull(embeddedValue, NULL, 16) path at line 140, which also returns 0 for "0x" when endptr is NULL. However, it means a bare "0x" is accepted rather than rejected as a parse error.

In practice this is negligible risk (no user would set an env var to exactly "0x"), but if the intent is to reject this case, add:

if (startIndex >= cchResult)
returnfalse;

⚠️ Binary Size Impact — Unconditional ManagedDataDescriptorProvider (Program.cs:260) [needs maintainer judgment]

Flagged by all 3 models.

Previously, cDAC type descriptors were only emitted with --debug/-g. Now every NativeAOT binary includes the DotNetManagedContractDescriptor JSON blob (header + JSON for [DataContract]-annotated types with EETypes). The size impact depends on the number of qualifying types — likely small (few KB) but non-zero.

Questions for maintainer:

  • Is the intent to enable post-mortem/cDAC debugging without debug symbols? If so, a brief comment or commit message explaining the rationale would help future readers.
  • Has the size impact been measured for representative apps?

⚠️ Missing Tests — No regression tests for either behavior change

Flagged by all 3 models.

Neither change includes tests:

  • The hex prefix fix should be verifiable through environment variable integration tests (e.g., DOTNET_GCHeapCount=0x10 produces expected behavior)
  • The unconditional cDAC descriptor could be verified by checking the symbol exists in non-debug NativeAOT output

Given this is native runtime code, integration-level testing may be more appropriate than unit tests. A maintainer can judge whether existing test infrastructure covers these paths.

💡 PR Scope — Two unrelated changes in one PR

The hex prefix fix (C++ runtime bug fix) and the unconditional cDAC metadata (C# compiler behavior change) appear orthogonal. Splitting would ease backporting the bug fix independently. However, if the author (a core NativeAOT maintainer) has a reason for coupling them, that context should be documented.

Generated by Code Review for issue #127644 ·

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

CopilotAI review requested due to automatic review settings May 1, 2026 20:47
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/875e04c9-b166-49fd-aefe-adaf5f6bd0b4
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>

CopilotAI 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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment threadsrc/coreclr/nativeaot/Runtime/RhConfig.cpp
@MichalStrehovsky
MichalStrehovsky merged commit b745957 into mainMay 2, 2026
109 checks passed
@MichalStrehovsky
MichalStrehovsky deleted the copilot/fix-hex-parser-prefix branch May 2, 2026 02:57
MichalStrehovsky added a commit that referenced this pull request May 22, 2026
…values (#128462)
PR #127644 made `0x`/`0X` valid for non-decimal NativeAOT config
parsing, but `CONFIG_VAL_MAXLEN` still capped input at 16 chars,
rejecting full-width values like `0xFFFFFFFFFFFFFFFF`. This change
aligns the textual length limit with the accepted syntax while
preserving existing overlong-value rejection behavior.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jun 1, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@MichalStrehovsky@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Fix NativeAOT hex config parser to handle 0x/0X prefix - #127644

Merged
MichalStrehovsky merged 5 commits into
mainfrom
copilot/fix-hex-parser-prefix
May 2, 2026
Merged

Fix NativeAOT hex config parser to handle 0x/0X prefix#127644
MichalStrehovsky merged 5 commits into
mainfrom
copilot/fix-hex-parser-prefix

Conversation

CopilotAI commented May 1, 2026

Copy link
Copy Markdown
Contributor

NativeAOT's RhConfig::Environment::TryGetIntegerValue had a hand-rolled hex parser that rejected the 0x/0X prefix — returning a parse error when it encountered x. This meant env vars like DOTNET_GCHeapHardLimit=0xC0000000 silently failed to parse, leaving the hard limit unset. With GCLargePages=2 also set, the GC would then return CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT and fail initialization. CoreCLR's equivalent uses strtoul(..., 16) which handles the prefix natively.

Description

  • src/coreclr/nativeaot/Runtime/RhConfig.cpp — In TryGetIntegerValue, skip a leading 0x/0X prefix when parsing in hex mode, before entering the digit loop. Additionally, return false (parse error) when the value is exactly "0x" or "0X" with no hex digits following the prefix, matching CoreCLR's strtoul behavior:
uint32_t startIndex = 0;
if (!decimal && cchResult >= 2 && buffer[0] == '0' && (buffer[1] == 'x' || buffer[1] == 'X'))
{
startIndex = 2;
if (startIndex == cchResult)
returnfalse; // parse error - hex prefix without any digits
}
for (uint32_t i = startIndex; i < cchResult; i++)

This aligns NativeAOT's config parsing with CoreCLR's strtoul-based behavior and fixes the Collect_Aggressive_LargePages test failure under NativeAOT.

Original prompt

Problem

NativeAOT's RhConfig::Environment::TryGetIntegerValue in src/coreclr/nativeaot/Runtime/RhConfig.cpp uses a hand-rolled hex parser that does not handle the 0x or 0X prefix. This causes config values like DOTNET_GCHeapHardLimit=0xC0000000 to fail to parse, because when the parser encounters the x character it returns false (parse error).

CoreCLR's equivalent code (CLRConfigNoCache::TryAsInteger in src/coreclr/inc/clrconfignocache.h) uses strtoul(_value, &endPtr, radix) which natively handles the 0x prefix when radix is 16.

This causes the test Collect_Aggressive_LargePages added in PR #127290 to fail under NativeAOT: the GCHeapHardLimit fails to parse, so no hard limit is set, but GCLargePages=2 succeeds → the GC returns CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT and the process exits with -1.

Fix

In src/coreclr/nativeaot/Runtime/RhConfig.cpp, in the TryGetIntegerValue method, when parsing in hex mode (i.e., decimal is false), skip a leading 0x or 0X prefix before entering the digit-parsing loop. This matches the behavior of strtoul with radix 16 that CoreCLR uses.

Specifically, after reading the environment variable into buffer and before the parsing loop, add:

uint32_t startIndex = 0;
if (!decimal && cchResult >= 2 && buffer[0] == '0' && (buffer[1] == 'x' || buffer[1] == 'X'))
{
startIndex = 2;
}

Then change the loop from for (uint32_t i = 0; ...) to for (uint32_t i = startIndex; ...).

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: ```
16:22:36.657 Running test: GC\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd

Return code: 1
Raw output file: C:\h\w\B1EC0A05\w\B5C209C7\uploads\API\GC\Collect_Aggressive_LargePages\output.txt
Raw output:
BEGIN EXECUTION
call C:\h\w\B1EC0A05\p\nativeaottest.cmd C:\h\w\B1EC0A05\w\B5C209C7\e\GC\API\GC\Collect_Aggressive_LargePages\ Collect_Aggressive_LargePages.dll
Expected: 100
Actual: -1
END EXECUTION - FAILED
FAILED
Test Harness Exitcode is : 1
To run the test:
Set up CORE_ROOT and run.

C:\h\w\B1EC0A05\w\B5C209C7\e\GC\GC\native....\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd
Xunit.Sdk.TrueException:
Return code: 1
Raw output file: C:\h\w\B1EC0A05\w\B5C209C7\uploads\API\GC\Collect_Aggressive_LargePages\output.txt
Raw output:
BEGIN EXECUTION
call C:\h\w\B1EC0A05\p\nativeaottest.cmd C:\h\w\B1EC0A05\w\B5C209C7\e\GC\API\GC\Collect_Aggressive_LargePages\ Collect_Aggressive_LargePages.dll
Expected: 100
Actual: -1
END EXECUTION - FAILED
FAILED
Test Harness Exitcode is : 1
To run the test:
Set up CORE_ROOT and run.
C:\h\w\B1EC0A05\w\B5C209C7\e\GC\GC\native....\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd
at Xunit.Assert.True(Nullable`1, String)
at TestLibrary.OutOfProcessTest.RunOutOfProcessTest(String, String)
at Program.<

$>g__TestExecutor26|0_27(StreamWriter, StreamWriter, Program.<>c__DisplayClass0_0&)
16:22:36.755 Failed test: GC\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd

the test added here is failing with native AOT. why? Assistant: The test fails on NativeAOT because **NativeAOT's config reader cannot parse the `0x` prefix** in the `DOTNET_GCHeapHardLimit=0xC0000000` environment variable.
Looking at NativeAOT's `RhConfig::Environment::TryGetIntegerValue` in:
```c++ name=src/coreclr/nativeaot/Runtime/RhConfig.cpp url=https://github.com/dotnet/runtime/blob/bfaabae213cf02d343846d253b7149a03819680d/src/coreclr/nativeaot/Runtime/RhConfig.cpp#L58-L84
uint64_t uiResult = 0;
for (uint32_t i = 0; i < cchResult; i++)
{
TCHAR ch = buffer[i];
// ...
else
{
uiResult *= 16;
if ((ch >= '0') && (ch <= '9'))
uiResult += ch - '0';
else if ((ch >= 'a') && (ch <= 'f'))
uiResult += (ch - 'a') + 10;
else if ((ch >= 'A') && (ch <= 'F'))
uiResult += (ch - 'A') + 10;
else
return false; // parse error <-- 'x' hits this
}
}

When the value is 0xC0000000, the parser reads hex digits character-by-character. It handles 0, but then encounters x, which isn't a valid hex digit — so it returns false (parse error). This means GCHeapHardLimit is never set.

The cascade:

  1. DOTNET_GCHeapHardLimit=0xC0000000 fails to parse → no hard limit is set
  2. DOTNET_GCLargePages=2 is read successfully (it's a simple 2, valid hex) → use_large_pages_p = true
  3. The GC hits this check in interface.cpp: if ((!gc_heap::heap_hard_limit) && gc_heap::use_large_pages_p) → returns CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT
  4. GC initialization fails →...

This pull request was created from Copilot chat.

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/b2cf8499-44ac-4e9a-995b-c72b36e081a2
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotMay 1, 2026 11:14
CopilotAI changed the title [WIP] Fix hex parser to handle 0x prefix in TryGetIntegerValueFix NativeAOT hex config parser to handle 0x/0X prefixMay 1, 2026
CopilotAI requested a review from MichalStrehovskyMay 1, 2026 11:15
CopilotAI review requested due to automatic review settings May 1, 2026 11:21
@MichalStrehovsky
MichalStrehovsky marked this pull request as ready for review May 1, 2026 11:21
@MichalStrehovsky

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

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

CopilotAI 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.

Pull request overview

This pull request fixes NativeAOT configuration parsing so hexadecimal environment variable values can include a 0x/0X prefix (matching the commonly accepted format and aligning more closely with CoreCLR behavior), preventing GC configuration knobs like DOTNET_GCHeapHardLimit=0x... from being ignored under NativeAOT.

Changes:

  • NativeAOT RhConfig::Environment::TryGetIntegerValue now skips a leading 0x/0X prefix when parsing hex values from environment variables.
  • ILCompiler now always adds ManagedDataDescriptorProvider (previously gated behind --debug).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

FileDescription
src/coreclr/nativeaot/Runtime/RhConfig.cppAdds logic to skip 0x/0X prefix in the hand-rolled hex parser for env var config values.
src/coreclr/tools/aot/ILCompiler/Program.csRemoves the --debug condition and unconditionally roots ManagedDataDescriptorProvider (managed cDAC descriptor emission).

Comment threadsrc/coreclr/nativeaot/Runtime/RhConfig.cpp
Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #127644

Note

This review was generated by Copilot using multi-model analysis (Claude Opus 4.6, Claude Sonnet 4.5, GPT-5.3-Codex).

Holistic Assessment

Motivation: The hex prefix fix addresses a real inconsistency — strtoull (used for embedded config values) already handles 0x prefixes, but the manual environment variable parser did not, causing values like DOTNET_GCHeapCount=0x10 to fail. The ManagedDataDescriptorProvider change enables cDAC diagnostics without requiring --debug.

Approach: The hex prefix fix is minimal and correct for the common case. The ManagedDataDescriptorProvider change is a one-line behavioral shift with broader implications that should be explained.

Summary: ⚠️ Needs Human Review. The hex parsing fix is correct but has an edge case worth considering ("0x" alone). The unconditional cDAC metadata change has binary size implications and the two changes appear unrelated — a maintainer should confirm the coupling is intentional and the size impact is acceptable.


Detailed Findings

✅ Correctness — Hex prefix parsing fix (RhConfig.cpp:59-63)

The fix correctly strips the 0x/0X prefix before the hex digit parsing loop. Key observations:

  • Guard cchResult >= 2 prevents out-of-bounds access on buffer[1]
  • Aligns the environment variable path with the embedded config path (line 140) which uses strtoull and already handles 0x natively
  • All existing callers (EventPipe, GC config, dump type) benefit from this fix

⚠️ Edge Case — Bare "0x" input parses as valid zero (RhConfig.cpp:59-93) [advisory, not merge-blocking]

Flagged by all 3 models.

When buffer is exactly "0x" (cchResult == 2), startIndex is set to 2, the loop body never executes, and uiResult = 0 is returned as valid. This is consistent with the strtoull(embeddedValue, NULL, 16) path at line 140, which also returns 0 for "0x" when endptr is NULL. However, it means a bare "0x" is accepted rather than rejected as a parse error.

In practice this is negligible risk (no user would set an env var to exactly "0x"), but if the intent is to reject this case, add:

if (startIndex >= cchResult)
returnfalse;

⚠️ Binary Size Impact — Unconditional ManagedDataDescriptorProvider (Program.cs:260) [needs maintainer judgment]

Flagged by all 3 models.

Previously, cDAC type descriptors were only emitted with --debug/-g. Now every NativeAOT binary includes the DotNetManagedContractDescriptor JSON blob (header + JSON for [DataContract]-annotated types with EETypes). The size impact depends on the number of qualifying types — likely small (few KB) but non-zero.

Questions for maintainer:

  • Is the intent to enable post-mortem/cDAC debugging without debug symbols? If so, a brief comment or commit message explaining the rationale would help future readers.
  • Has the size impact been measured for representative apps?

⚠️ Missing Tests — No regression tests for either behavior change

Flagged by all 3 models.

Neither change includes tests:

  • The hex prefix fix should be verifiable through environment variable integration tests (e.g., DOTNET_GCHeapCount=0x10 produces expected behavior)
  • The unconditional cDAC descriptor could be verified by checking the symbol exists in non-debug NativeAOT output

Given this is native runtime code, integration-level testing may be more appropriate than unit tests. A maintainer can judge whether existing test infrastructure covers these paths.

💡 PR Scope — Two unrelated changes in one PR

The hex prefix fix (C++ runtime bug fix) and the unconditional cDAC metadata (C# compiler behavior change) appear orthogonal. Splitting would ease backporting the bug fix independently. However, if the author (a core NativeAOT maintainer) has a reason for coupling them, that context should be documented.

Generated by Code Review for issue #127644 ·

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

CopilotAI review requested due to automatic review settings May 1, 2026 20:47
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/875e04c9-b166-49fd-aefe-adaf5f6bd0b4
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>

CopilotAI 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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment threadsrc/coreclr/nativeaot/Runtime/RhConfig.cpp
@MichalStrehovsky
MichalStrehovsky merged commit b745957 into mainMay 2, 2026
109 checks passed
@MichalStrehovsky
MichalStrehovsky deleted the copilot/fix-hex-parser-prefix branch May 2, 2026 02:57
MichalStrehovsky added a commit that referenced this pull request May 22, 2026
…values (#128462)
PR #127644 made `0x`/`0X` valid for non-decimal NativeAOT config
parsing, but `CONFIG_VAL_MAXLEN` still capped input at 16 chars,
rejecting full-width values like `0xFFFFFFFFFFFFFFFF`. This change
aligns the textual length limit with the accepted syntax while
preserving existing overlong-value rejection behavior.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jun 1, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@MichalStrehovsky@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix NativeAOT hex config parser to handle 0x/0X prefix - #127644

Merged
MichalStrehovsky merged 5 commits into
mainfrom
copilot/fix-hex-parser-prefix
May 2, 2026
Merged

Fix NativeAOT hex config parser to handle 0x/0X prefix#127644
MichalStrehovsky merged 5 commits into
mainfrom
copilot/fix-hex-parser-prefix

Conversation

CopilotAI commented May 1, 2026

Copy link
Copy Markdown
Contributor

NativeAOT's RhConfig::Environment::TryGetIntegerValue had a hand-rolled hex parser that rejected the 0x/0X prefix — returning a parse error when it encountered x. This meant env vars like DOTNET_GCHeapHardLimit=0xC0000000 silently failed to parse, leaving the hard limit unset. With GCLargePages=2 also set, the GC would then return CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT and fail initialization. CoreCLR's equivalent uses strtoul(..., 16) which handles the prefix natively.

Description

  • src/coreclr/nativeaot/Runtime/RhConfig.cpp — In TryGetIntegerValue, skip a leading 0x/0X prefix when parsing in hex mode, before entering the digit loop. Additionally, return false (parse error) when the value is exactly "0x" or "0X" with no hex digits following the prefix, matching CoreCLR's strtoul behavior:
uint32_t startIndex = 0;
if (!decimal && cchResult >= 2 && buffer[0] == '0' && (buffer[1] == 'x' || buffer[1] == 'X'))
{
startIndex = 2;
if (startIndex == cchResult)
returnfalse; // parse error - hex prefix without any digits
}
for (uint32_t i = startIndex; i < cchResult; i++)

This aligns NativeAOT's config parsing with CoreCLR's strtoul-based behavior and fixes the Collect_Aggressive_LargePages test failure under NativeAOT.

Original prompt

Problem

NativeAOT's RhConfig::Environment::TryGetIntegerValue in src/coreclr/nativeaot/Runtime/RhConfig.cpp uses a hand-rolled hex parser that does not handle the 0x or 0X prefix. This causes config values like DOTNET_GCHeapHardLimit=0xC0000000 to fail to parse, because when the parser encounters the x character it returns false (parse error).

CoreCLR's equivalent code (CLRConfigNoCache::TryAsInteger in src/coreclr/inc/clrconfignocache.h) uses strtoul(_value, &endPtr, radix) which natively handles the 0x prefix when radix is 16.

This causes the test Collect_Aggressive_LargePages added in PR #127290 to fail under NativeAOT: the GCHeapHardLimit fails to parse, so no hard limit is set, but GCLargePages=2 succeeds → the GC returns CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT and the process exits with -1.

Fix

In src/coreclr/nativeaot/Runtime/RhConfig.cpp, in the TryGetIntegerValue method, when parsing in hex mode (i.e., decimal is false), skip a leading 0x or 0X prefix before entering the digit-parsing loop. This matches the behavior of strtoul with radix 16 that CoreCLR uses.

Specifically, after reading the environment variable into buffer and before the parsing loop, add:

uint32_t startIndex = 0;
if (!decimal && cchResult >= 2 && buffer[0] == '0' && (buffer[1] == 'x' || buffer[1] == 'X'))
{
startIndex = 2;
}

Then change the loop from for (uint32_t i = 0; ...) to for (uint32_t i = startIndex; ...).

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: ```
16:22:36.657 Running test: GC\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd

Return code: 1
Raw output file: C:\h\w\B1EC0A05\w\B5C209C7\uploads\API\GC\Collect_Aggressive_LargePages\output.txt
Raw output:
BEGIN EXECUTION
call C:\h\w\B1EC0A05\p\nativeaottest.cmd C:\h\w\B1EC0A05\w\B5C209C7\e\GC\API\GC\Collect_Aggressive_LargePages\ Collect_Aggressive_LargePages.dll
Expected: 100
Actual: -1
END EXECUTION - FAILED
FAILED
Test Harness Exitcode is : 1
To run the test:
Set up CORE_ROOT and run.

C:\h\w\B1EC0A05\w\B5C209C7\e\GC\GC\native....\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd
Xunit.Sdk.TrueException:
Return code: 1
Raw output file: C:\h\w\B1EC0A05\w\B5C209C7\uploads\API\GC\Collect_Aggressive_LargePages\output.txt
Raw output:
BEGIN EXECUTION
call C:\h\w\B1EC0A05\p\nativeaottest.cmd C:\h\w\B1EC0A05\w\B5C209C7\e\GC\API\GC\Collect_Aggressive_LargePages\ Collect_Aggressive_LargePages.dll
Expected: 100
Actual: -1
END EXECUTION - FAILED
FAILED
Test Harness Exitcode is : 1
To run the test:
Set up CORE_ROOT and run.
C:\h\w\B1EC0A05\w\B5C209C7\e\GC\GC\native....\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd
at Xunit.Assert.True(Nullable`1, String)
at TestLibrary.OutOfProcessTest.RunOutOfProcessTest(String, String)
at Program.<

$>g__TestExecutor26|0_27(StreamWriter, StreamWriter, Program.<>c__DisplayClass0_0&)
16:22:36.755 Failed test: GC\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd

the test added here is failing with native AOT. why? Assistant: The test fails on NativeAOT because **NativeAOT's config reader cannot parse the `0x` prefix** in the `DOTNET_GCHeapHardLimit=0xC0000000` environment variable.
Looking at NativeAOT's `RhConfig::Environment::TryGetIntegerValue` in:
```c++ name=src/coreclr/nativeaot/Runtime/RhConfig.cpp url=https://github.com/dotnet/runtime/blob/bfaabae213cf02d343846d253b7149a03819680d/src/coreclr/nativeaot/Runtime/RhConfig.cpp#L58-L84
uint64_t uiResult = 0;
for (uint32_t i = 0; i < cchResult; i++)
{
TCHAR ch = buffer[i];
// ...
else
{
uiResult *= 16;
if ((ch >= '0') && (ch <= '9'))
uiResult += ch - '0';
else if ((ch >= 'a') && (ch <= 'f'))
uiResult += (ch - 'a') + 10;
else if ((ch >= 'A') && (ch <= 'F'))
uiResult += (ch - 'A') + 10;
else
return false; // parse error <-- 'x' hits this
}
}

When the value is 0xC0000000, the parser reads hex digits character-by-character. It handles 0, but then encounters x, which isn't a valid hex digit — so it returns false (parse error). This means GCHeapHardLimit is never set.

The cascade:

  1. DOTNET_GCHeapHardLimit=0xC0000000 fails to parse → no hard limit is set
  2. DOTNET_GCLargePages=2 is read successfully (it's a simple 2, valid hex) → use_large_pages_p = true
  3. The GC hits this check in interface.cpp: if ((!gc_heap::heap_hard_limit) && gc_heap::use_large_pages_p) → returns CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT
  4. GC initialization fails →...

This pull request was created from Copilot chat.

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/b2cf8499-44ac-4e9a-995b-c72b36e081a2
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotMay 1, 2026 11:14
CopilotAI changed the title [WIP] Fix hex parser to handle 0x prefix in TryGetIntegerValueFix NativeAOT hex config parser to handle 0x/0X prefixMay 1, 2026
CopilotAI requested a review from MichalStrehovskyMay 1, 2026 11:15
CopilotAI review requested due to automatic review settings May 1, 2026 11:21
@MichalStrehovsky
MichalStrehovsky marked this pull request as ready for review May 1, 2026 11:21
@MichalStrehovsky

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

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

CopilotAI 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.

Pull request overview

This pull request fixes NativeAOT configuration parsing so hexadecimal environment variable values can include a 0x/0X prefix (matching the commonly accepted format and aligning more closely with CoreCLR behavior), preventing GC configuration knobs like DOTNET_GCHeapHardLimit=0x... from being ignored under NativeAOT.

Changes:

  • NativeAOT RhConfig::Environment::TryGetIntegerValue now skips a leading 0x/0X prefix when parsing hex values from environment variables.
  • ILCompiler now always adds ManagedDataDescriptorProvider (previously gated behind --debug).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

FileDescription
src/coreclr/nativeaot/Runtime/RhConfig.cppAdds logic to skip 0x/0X prefix in the hand-rolled hex parser for env var config values.
src/coreclr/tools/aot/ILCompiler/Program.csRemoves the --debug condition and unconditionally roots ManagedDataDescriptorProvider (managed cDAC descriptor emission).

Comment threadsrc/coreclr/nativeaot/Runtime/RhConfig.cpp
Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #127644

Note

This review was generated by Copilot using multi-model analysis (Claude Opus 4.6, Claude Sonnet 4.5, GPT-5.3-Codex).

Holistic Assessment

Motivation: The hex prefix fix addresses a real inconsistency — strtoull (used for embedded config values) already handles 0x prefixes, but the manual environment variable parser did not, causing values like DOTNET_GCHeapCount=0x10 to fail. The ManagedDataDescriptorProvider change enables cDAC diagnostics without requiring --debug.

Approach: The hex prefix fix is minimal and correct for the common case. The ManagedDataDescriptorProvider change is a one-line behavioral shift with broader implications that should be explained.

Summary: ⚠️ Needs Human Review. The hex parsing fix is correct but has an edge case worth considering ("0x" alone). The unconditional cDAC metadata change has binary size implications and the two changes appear unrelated — a maintainer should confirm the coupling is intentional and the size impact is acceptable.


Detailed Findings

✅ Correctness — Hex prefix parsing fix (RhConfig.cpp:59-63)

The fix correctly strips the 0x/0X prefix before the hex digit parsing loop. Key observations:

  • Guard cchResult >= 2 prevents out-of-bounds access on buffer[1]
  • Aligns the environment variable path with the embedded config path (line 140) which uses strtoull and already handles 0x natively
  • All existing callers (EventPipe, GC config, dump type) benefit from this fix

⚠️ Edge Case — Bare "0x" input parses as valid zero (RhConfig.cpp:59-93) [advisory, not merge-blocking]

Flagged by all 3 models.

When buffer is exactly "0x" (cchResult == 2), startIndex is set to 2, the loop body never executes, and uiResult = 0 is returned as valid. This is consistent with the strtoull(embeddedValue, NULL, 16) path at line 140, which also returns 0 for "0x" when endptr is NULL. However, it means a bare "0x" is accepted rather than rejected as a parse error.

In practice this is negligible risk (no user would set an env var to exactly "0x"), but if the intent is to reject this case, add:

if (startIndex >= cchResult)
returnfalse;

⚠️ Binary Size Impact — Unconditional ManagedDataDescriptorProvider (Program.cs:260) [needs maintainer judgment]

Flagged by all 3 models.

Previously, cDAC type descriptors were only emitted with --debug/-g. Now every NativeAOT binary includes the DotNetManagedContractDescriptor JSON blob (header + JSON for [DataContract]-annotated types with EETypes). The size impact depends on the number of qualifying types — likely small (few KB) but non-zero.

Questions for maintainer:

  • Is the intent to enable post-mortem/cDAC debugging without debug symbols? If so, a brief comment or commit message explaining the rationale would help future readers.
  • Has the size impact been measured for representative apps?

⚠️ Missing Tests — No regression tests for either behavior change

Flagged by all 3 models.

Neither change includes tests:

  • The hex prefix fix should be verifiable through environment variable integration tests (e.g., DOTNET_GCHeapCount=0x10 produces expected behavior)
  • The unconditional cDAC descriptor could be verified by checking the symbol exists in non-debug NativeAOT output

Given this is native runtime code, integration-level testing may be more appropriate than unit tests. A maintainer can judge whether existing test infrastructure covers these paths.

💡 PR Scope — Two unrelated changes in one PR

The hex prefix fix (C++ runtime bug fix) and the unconditional cDAC metadata (C# compiler behavior change) appear orthogonal. Splitting would ease backporting the bug fix independently. However, if the author (a core NativeAOT maintainer) has a reason for coupling them, that context should be documented.

Generated by Code Review for issue #127644 ·

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

CopilotAI review requested due to automatic review settings May 1, 2026 20:47
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/875e04c9-b166-49fd-aefe-adaf5f6bd0b4
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>

CopilotAI 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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment threadsrc/coreclr/nativeaot/Runtime/RhConfig.cpp
@MichalStrehovsky
MichalStrehovsky merged commit b745957 into mainMay 2, 2026
109 checks passed
@MichalStrehovsky
MichalStrehovsky deleted the copilot/fix-hex-parser-prefix branch May 2, 2026 02:57
MichalStrehovsky added a commit that referenced this pull request May 22, 2026
…values (#128462)
PR #127644 made `0x`/`0X` valid for non-decimal NativeAOT config
parsing, but `CONFIG_VAL_MAXLEN` still capped input at 16 chars,
rejecting full-width values like `0xFFFFFFFFFFFFFFFF`. This change
aligns the textual length limit with the accepted syntax while
preserving existing overlong-value rejection behavior.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jun 1, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@MichalStrehovsky@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix NativeAOT hex config parser to handle 0x/0X prefix - #127644

Merged
MichalStrehovsky merged 5 commits into
mainfrom
copilot/fix-hex-parser-prefix
May 2, 2026
Merged

Fix NativeAOT hex config parser to handle 0x/0X prefix#127644
MichalStrehovsky merged 5 commits into
mainfrom
copilot/fix-hex-parser-prefix

Conversation

CopilotAI commented May 1, 2026

Copy link
Copy Markdown
Contributor

NativeAOT's RhConfig::Environment::TryGetIntegerValue had a hand-rolled hex parser that rejected the 0x/0X prefix — returning a parse error when it encountered x. This meant env vars like DOTNET_GCHeapHardLimit=0xC0000000 silently failed to parse, leaving the hard limit unset. With GCLargePages=2 also set, the GC would then return CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT and fail initialization. CoreCLR's equivalent uses strtoul(..., 16) which handles the prefix natively.

Description

  • src/coreclr/nativeaot/Runtime/RhConfig.cpp — In TryGetIntegerValue, skip a leading 0x/0X prefix when parsing in hex mode, before entering the digit loop. Additionally, return false (parse error) when the value is exactly "0x" or "0X" with no hex digits following the prefix, matching CoreCLR's strtoul behavior:
uint32_t startIndex = 0;
if (!decimal && cchResult >= 2 && buffer[0] == '0' && (buffer[1] == 'x' || buffer[1] == 'X'))
{
startIndex = 2;
if (startIndex == cchResult)
returnfalse; // parse error - hex prefix without any digits
}
for (uint32_t i = startIndex; i < cchResult; i++)

This aligns NativeAOT's config parsing with CoreCLR's strtoul-based behavior and fixes the Collect_Aggressive_LargePages test failure under NativeAOT.

Original prompt

Problem

NativeAOT's RhConfig::Environment::TryGetIntegerValue in src/coreclr/nativeaot/Runtime/RhConfig.cpp uses a hand-rolled hex parser that does not handle the 0x or 0X prefix. This causes config values like DOTNET_GCHeapHardLimit=0xC0000000 to fail to parse, because when the parser encounters the x character it returns false (parse error).

CoreCLR's equivalent code (CLRConfigNoCache::TryAsInteger in src/coreclr/inc/clrconfignocache.h) uses strtoul(_value, &endPtr, radix) which natively handles the 0x prefix when radix is 16.

This causes the test Collect_Aggressive_LargePages added in PR #127290 to fail under NativeAOT: the GCHeapHardLimit fails to parse, so no hard limit is set, but GCLargePages=2 succeeds → the GC returns CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT and the process exits with -1.

Fix

In src/coreclr/nativeaot/Runtime/RhConfig.cpp, in the TryGetIntegerValue method, when parsing in hex mode (i.e., decimal is false), skip a leading 0x or 0X prefix before entering the digit-parsing loop. This matches the behavior of strtoul with radix 16 that CoreCLR uses.

Specifically, after reading the environment variable into buffer and before the parsing loop, add:

uint32_t startIndex = 0;
if (!decimal && cchResult >= 2 && buffer[0] == '0' && (buffer[1] == 'x' || buffer[1] == 'X'))
{
startIndex = 2;
}

Then change the loop from for (uint32_t i = 0; ...) to for (uint32_t i = startIndex; ...).

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: ```
16:22:36.657 Running test: GC\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd

Return code: 1
Raw output file: C:\h\w\B1EC0A05\w\B5C209C7\uploads\API\GC\Collect_Aggressive_LargePages\output.txt
Raw output:
BEGIN EXECUTION
call C:\h\w\B1EC0A05\p\nativeaottest.cmd C:\h\w\B1EC0A05\w\B5C209C7\e\GC\API\GC\Collect_Aggressive_LargePages\ Collect_Aggressive_LargePages.dll
Expected: 100
Actual: -1
END EXECUTION - FAILED
FAILED
Test Harness Exitcode is : 1
To run the test:
Set up CORE_ROOT and run.

C:\h\w\B1EC0A05\w\B5C209C7\e\GC\GC\native....\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd
Xunit.Sdk.TrueException:
Return code: 1
Raw output file: C:\h\w\B1EC0A05\w\B5C209C7\uploads\API\GC\Collect_Aggressive_LargePages\output.txt
Raw output:
BEGIN EXECUTION
call C:\h\w\B1EC0A05\p\nativeaottest.cmd C:\h\w\B1EC0A05\w\B5C209C7\e\GC\API\GC\Collect_Aggressive_LargePages\ Collect_Aggressive_LargePages.dll
Expected: 100
Actual: -1
END EXECUTION - FAILED
FAILED
Test Harness Exitcode is : 1
To run the test:
Set up CORE_ROOT and run.
C:\h\w\B1EC0A05\w\B5C209C7\e\GC\GC\native....\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd
at Xunit.Assert.True(Nullable`1, String)
at TestLibrary.OutOfProcessTest.RunOutOfProcessTest(String, String)
at Program.<

$>g__TestExecutor26|0_27(StreamWriter, StreamWriter, Program.<>c__DisplayClass0_0&)
16:22:36.755 Failed test: GC\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd

the test added here is failing with native AOT. why? Assistant: The test fails on NativeAOT because **NativeAOT's config reader cannot parse the `0x` prefix** in the `DOTNET_GCHeapHardLimit=0xC0000000` environment variable.
Looking at NativeAOT's `RhConfig::Environment::TryGetIntegerValue` in:
```c++ name=src/coreclr/nativeaot/Runtime/RhConfig.cpp url=https://github.com/dotnet/runtime/blob/bfaabae213cf02d343846d253b7149a03819680d/src/coreclr/nativeaot/Runtime/RhConfig.cpp#L58-L84
uint64_t uiResult = 0;
for (uint32_t i = 0; i < cchResult; i++)
{
TCHAR ch = buffer[i];
// ...
else
{
uiResult *= 16;
if ((ch >= '0') && (ch <= '9'))
uiResult += ch - '0';
else if ((ch >= 'a') && (ch <= 'f'))
uiResult += (ch - 'a') + 10;
else if ((ch >= 'A') && (ch <= 'F'))
uiResult += (ch - 'A') + 10;
else
return false; // parse error <-- 'x' hits this
}
}

When the value is 0xC0000000, the parser reads hex digits character-by-character. It handles 0, but then encounters x, which isn't a valid hex digit — so it returns false (parse error). This means GCHeapHardLimit is never set.

The cascade:

  1. DOTNET_GCHeapHardLimit=0xC0000000 fails to parse → no hard limit is set
  2. DOTNET_GCLargePages=2 is read successfully (it's a simple 2, valid hex) → use_large_pages_p = true
  3. The GC hits this check in interface.cpp: if ((!gc_heap::heap_hard_limit) && gc_heap::use_large_pages_p) → returns CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT
  4. GC initialization fails →...

This pull request was created from Copilot chat.

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/b2cf8499-44ac-4e9a-995b-c72b36e081a2
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotMay 1, 2026 11:14
CopilotAI changed the title [WIP] Fix hex parser to handle 0x prefix in TryGetIntegerValueFix NativeAOT hex config parser to handle 0x/0X prefixMay 1, 2026
CopilotAI requested a review from MichalStrehovskyMay 1, 2026 11:15
CopilotAI review requested due to automatic review settings May 1, 2026 11:21
@MichalStrehovsky
MichalStrehovsky marked this pull request as ready for review May 1, 2026 11:21
@MichalStrehovsky

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

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

CopilotAI 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.

Pull request overview

This pull request fixes NativeAOT configuration parsing so hexadecimal environment variable values can include a 0x/0X prefix (matching the commonly accepted format and aligning more closely with CoreCLR behavior), preventing GC configuration knobs like DOTNET_GCHeapHardLimit=0x... from being ignored under NativeAOT.

Changes:

  • NativeAOT RhConfig::Environment::TryGetIntegerValue now skips a leading 0x/0X prefix when parsing hex values from environment variables.
  • ILCompiler now always adds ManagedDataDescriptorProvider (previously gated behind --debug).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

FileDescription
src/coreclr/nativeaot/Runtime/RhConfig.cppAdds logic to skip 0x/0X prefix in the hand-rolled hex parser for env var config values.
src/coreclr/tools/aot/ILCompiler/Program.csRemoves the --debug condition and unconditionally roots ManagedDataDescriptorProvider (managed cDAC descriptor emission).

Comment threadsrc/coreclr/nativeaot/Runtime/RhConfig.cpp
Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #127644

Note

This review was generated by Copilot using multi-model analysis (Claude Opus 4.6, Claude Sonnet 4.5, GPT-5.3-Codex).

Holistic Assessment

Motivation: The hex prefix fix addresses a real inconsistency — strtoull (used for embedded config values) already handles 0x prefixes, but the manual environment variable parser did not, causing values like DOTNET_GCHeapCount=0x10 to fail. The ManagedDataDescriptorProvider change enables cDAC diagnostics without requiring --debug.

Approach: The hex prefix fix is minimal and correct for the common case. The ManagedDataDescriptorProvider change is a one-line behavioral shift with broader implications that should be explained.

Summary: ⚠️ Needs Human Review. The hex parsing fix is correct but has an edge case worth considering ("0x" alone). The unconditional cDAC metadata change has binary size implications and the two changes appear unrelated — a maintainer should confirm the coupling is intentional and the size impact is acceptable.


Detailed Findings

✅ Correctness — Hex prefix parsing fix (RhConfig.cpp:59-63)

The fix correctly strips the 0x/0X prefix before the hex digit parsing loop. Key observations:

  • Guard cchResult >= 2 prevents out-of-bounds access on buffer[1]
  • Aligns the environment variable path with the embedded config path (line 140) which uses strtoull and already handles 0x natively
  • All existing callers (EventPipe, GC config, dump type) benefit from this fix

⚠️ Edge Case — Bare "0x" input parses as valid zero (RhConfig.cpp:59-93) [advisory, not merge-blocking]

Flagged by all 3 models.

When buffer is exactly "0x" (cchResult == 2), startIndex is set to 2, the loop body never executes, and uiResult = 0 is returned as valid. This is consistent with the strtoull(embeddedValue, NULL, 16) path at line 140, which also returns 0 for "0x" when endptr is NULL. However, it means a bare "0x" is accepted rather than rejected as a parse error.

In practice this is negligible risk (no user would set an env var to exactly "0x"), but if the intent is to reject this case, add:

if (startIndex >= cchResult)
returnfalse;

⚠️ Binary Size Impact — Unconditional ManagedDataDescriptorProvider (Program.cs:260) [needs maintainer judgment]

Flagged by all 3 models.

Previously, cDAC type descriptors were only emitted with --debug/-g. Now every NativeAOT binary includes the DotNetManagedContractDescriptor JSON blob (header + JSON for [DataContract]-annotated types with EETypes). The size impact depends on the number of qualifying types — likely small (few KB) but non-zero.

Questions for maintainer:

  • Is the intent to enable post-mortem/cDAC debugging without debug symbols? If so, a brief comment or commit message explaining the rationale would help future readers.
  • Has the size impact been measured for representative apps?

⚠️ Missing Tests — No regression tests for either behavior change

Flagged by all 3 models.

Neither change includes tests:

  • The hex prefix fix should be verifiable through environment variable integration tests (e.g., DOTNET_GCHeapCount=0x10 produces expected behavior)
  • The unconditional cDAC descriptor could be verified by checking the symbol exists in non-debug NativeAOT output

Given this is native runtime code, integration-level testing may be more appropriate than unit tests. A maintainer can judge whether existing test infrastructure covers these paths.

💡 PR Scope — Two unrelated changes in one PR

The hex prefix fix (C++ runtime bug fix) and the unconditional cDAC metadata (C# compiler behavior change) appear orthogonal. Splitting would ease backporting the bug fix independently. However, if the author (a core NativeAOT maintainer) has a reason for coupling them, that context should be documented.

Generated by Code Review for issue #127644 ·

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

CopilotAI review requested due to automatic review settings May 1, 2026 20:47
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/875e04c9-b166-49fd-aefe-adaf5f6bd0b4
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>

CopilotAI 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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment threadsrc/coreclr/nativeaot/Runtime/RhConfig.cpp
@MichalStrehovsky
MichalStrehovsky merged commit b745957 into mainMay 2, 2026
109 checks passed
@MichalStrehovsky
MichalStrehovsky deleted the copilot/fix-hex-parser-prefix branch May 2, 2026 02:57
MichalStrehovsky added a commit that referenced this pull request May 22, 2026
…values (#128462)
PR #127644 made `0x`/`0X` valid for non-decimal NativeAOT config
parsing, but `CONFIG_VAL_MAXLEN` still capped input at 16 chars,
rejecting full-width values like `0xFFFFFFFFFFFFFFFF`. This change
aligns the textual length limit with the accepted syntax while
preserving existing overlong-value rejection behavior.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jun 1, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@MichalStrehovsky@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Fix NativeAOT hex config parser to handle 0x/0X prefix - #127644

Merged
MichalStrehovsky merged 5 commits into
mainfrom
copilot/fix-hex-parser-prefix
May 2, 2026
Merged

Fix NativeAOT hex config parser to handle 0x/0X prefix#127644
MichalStrehovsky merged 5 commits into
mainfrom
copilot/fix-hex-parser-prefix

Conversation

CopilotAI commented May 1, 2026

Copy link
Copy Markdown
Contributor

NativeAOT's RhConfig::Environment::TryGetIntegerValue had a hand-rolled hex parser that rejected the 0x/0X prefix — returning a parse error when it encountered x. This meant env vars like DOTNET_GCHeapHardLimit=0xC0000000 silently failed to parse, leaving the hard limit unset. With GCLargePages=2 also set, the GC would then return CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT and fail initialization. CoreCLR's equivalent uses strtoul(..., 16) which handles the prefix natively.

Description

  • src/coreclr/nativeaot/Runtime/RhConfig.cpp — In TryGetIntegerValue, skip a leading 0x/0X prefix when parsing in hex mode, before entering the digit loop. Additionally, return false (parse error) when the value is exactly "0x" or "0X" with no hex digits following the prefix, matching CoreCLR's strtoul behavior:
uint32_t startIndex = 0;
if (!decimal && cchResult >= 2 && buffer[0] == '0' && (buffer[1] == 'x' || buffer[1] == 'X'))
{
startIndex = 2;
if (startIndex == cchResult)
returnfalse; // parse error - hex prefix without any digits
}
for (uint32_t i = startIndex; i < cchResult; i++)

This aligns NativeAOT's config parsing with CoreCLR's strtoul-based behavior and fixes the Collect_Aggressive_LargePages test failure under NativeAOT.

Original prompt

Problem

NativeAOT's RhConfig::Environment::TryGetIntegerValue in src/coreclr/nativeaot/Runtime/RhConfig.cpp uses a hand-rolled hex parser that does not handle the 0x or 0X prefix. This causes config values like DOTNET_GCHeapHardLimit=0xC0000000 to fail to parse, because when the parser encounters the x character it returns false (parse error).

CoreCLR's equivalent code (CLRConfigNoCache::TryAsInteger in src/coreclr/inc/clrconfignocache.h) uses strtoul(_value, &endPtr, radix) which natively handles the 0x prefix when radix is 16.

This causes the test Collect_Aggressive_LargePages added in PR #127290 to fail under NativeAOT: the GCHeapHardLimit fails to parse, so no hard limit is set, but GCLargePages=2 succeeds → the GC returns CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT and the process exits with -1.

Fix

In src/coreclr/nativeaot/Runtime/RhConfig.cpp, in the TryGetIntegerValue method, when parsing in hex mode (i.e., decimal is false), skip a leading 0x or 0X prefix before entering the digit-parsing loop. This matches the behavior of strtoul with radix 16 that CoreCLR uses.

Specifically, after reading the environment variable into buffer and before the parsing loop, add:

uint32_t startIndex = 0;
if (!decimal && cchResult >= 2 && buffer[0] == '0' && (buffer[1] == 'x' || buffer[1] == 'X'))
{
startIndex = 2;
}

Then change the loop from for (uint32_t i = 0; ...) to for (uint32_t i = startIndex; ...).

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: ```
16:22:36.657 Running test: GC\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd

Return code: 1
Raw output file: C:\h\w\B1EC0A05\w\B5C209C7\uploads\API\GC\Collect_Aggressive_LargePages\output.txt
Raw output:
BEGIN EXECUTION
call C:\h\w\B1EC0A05\p\nativeaottest.cmd C:\h\w\B1EC0A05\w\B5C209C7\e\GC\API\GC\Collect_Aggressive_LargePages\ Collect_Aggressive_LargePages.dll
Expected: 100
Actual: -1
END EXECUTION - FAILED
FAILED
Test Harness Exitcode is : 1
To run the test:
Set up CORE_ROOT and run.

C:\h\w\B1EC0A05\w\B5C209C7\e\GC\GC\native....\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd
Xunit.Sdk.TrueException:
Return code: 1
Raw output file: C:\h\w\B1EC0A05\w\B5C209C7\uploads\API\GC\Collect_Aggressive_LargePages\output.txt
Raw output:
BEGIN EXECUTION
call C:\h\w\B1EC0A05\p\nativeaottest.cmd C:\h\w\B1EC0A05\w\B5C209C7\e\GC\API\GC\Collect_Aggressive_LargePages\ Collect_Aggressive_LargePages.dll
Expected: 100
Actual: -1
END EXECUTION - FAILED
FAILED
Test Harness Exitcode is : 1
To run the test:
Set up CORE_ROOT and run.
C:\h\w\B1EC0A05\w\B5C209C7\e\GC\GC\native....\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd
at Xunit.Assert.True(Nullable`1, String)
at TestLibrary.OutOfProcessTest.RunOutOfProcessTest(String, String)
at Program.<

$>g__TestExecutor26|0_27(StreamWriter, StreamWriter, Program.<>c__DisplayClass0_0&)
16:22:36.755 Failed test: GC\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd

the test added here is failing with native AOT. why? Assistant: The test fails on NativeAOT because **NativeAOT's config reader cannot parse the `0x` prefix** in the `DOTNET_GCHeapHardLimit=0xC0000000` environment variable.
Looking at NativeAOT's `RhConfig::Environment::TryGetIntegerValue` in:
```c++ name=src/coreclr/nativeaot/Runtime/RhConfig.cpp url=https://github.com/dotnet/runtime/blob/bfaabae213cf02d343846d253b7149a03819680d/src/coreclr/nativeaot/Runtime/RhConfig.cpp#L58-L84
uint64_t uiResult = 0;
for (uint32_t i = 0; i < cchResult; i++)
{
TCHAR ch = buffer[i];
// ...
else
{
uiResult *= 16;
if ((ch >= '0') && (ch <= '9'))
uiResult += ch - '0';
else if ((ch >= 'a') && (ch <= 'f'))
uiResult += (ch - 'a') + 10;
else if ((ch >= 'A') && (ch <= 'F'))
uiResult += (ch - 'A') + 10;
else
return false; // parse error <-- 'x' hits this
}
}

When the value is 0xC0000000, the parser reads hex digits character-by-character. It handles 0, but then encounters x, which isn't a valid hex digit — so it returns false (parse error). This means GCHeapHardLimit is never set.

The cascade:

  1. DOTNET_GCHeapHardLimit=0xC0000000 fails to parse → no hard limit is set
  2. DOTNET_GCLargePages=2 is read successfully (it's a simple 2, valid hex) → use_large_pages_p = true
  3. The GC hits this check in interface.cpp: if ((!gc_heap::heap_hard_limit) && gc_heap::use_large_pages_p) → returns CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT
  4. GC initialization fails →...

This pull request was created from Copilot chat.

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/b2cf8499-44ac-4e9a-995b-c72b36e081a2
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotMay 1, 2026 11:14
CopilotAI changed the title [WIP] Fix hex parser to handle 0x prefix in TryGetIntegerValueFix NativeAOT hex config parser to handle 0x/0X prefixMay 1, 2026
CopilotAI requested a review from MichalStrehovskyMay 1, 2026 11:15
CopilotAI review requested due to automatic review settings May 1, 2026 11:21
@MichalStrehovsky
MichalStrehovsky marked this pull request as ready for review May 1, 2026 11:21
@MichalStrehovsky

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

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

CopilotAI 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.

Pull request overview

This pull request fixes NativeAOT configuration parsing so hexadecimal environment variable values can include a 0x/0X prefix (matching the commonly accepted format and aligning more closely with CoreCLR behavior), preventing GC configuration knobs like DOTNET_GCHeapHardLimit=0x... from being ignored under NativeAOT.

Changes:

  • NativeAOT RhConfig::Environment::TryGetIntegerValue now skips a leading 0x/0X prefix when parsing hex values from environment variables.
  • ILCompiler now always adds ManagedDataDescriptorProvider (previously gated behind --debug).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

FileDescription
src/coreclr/nativeaot/Runtime/RhConfig.cppAdds logic to skip 0x/0X prefix in the hand-rolled hex parser for env var config values.
src/coreclr/tools/aot/ILCompiler/Program.csRemoves the --debug condition and unconditionally roots ManagedDataDescriptorProvider (managed cDAC descriptor emission).

Comment threadsrc/coreclr/nativeaot/Runtime/RhConfig.cpp
Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #127644

Note

This review was generated by Copilot using multi-model analysis (Claude Opus 4.6, Claude Sonnet 4.5, GPT-5.3-Codex).

Holistic Assessment

Motivation: The hex prefix fix addresses a real inconsistency — strtoull (used for embedded config values) already handles 0x prefixes, but the manual environment variable parser did not, causing values like DOTNET_GCHeapCount=0x10 to fail. The ManagedDataDescriptorProvider change enables cDAC diagnostics without requiring --debug.

Approach: The hex prefix fix is minimal and correct for the common case. The ManagedDataDescriptorProvider change is a one-line behavioral shift with broader implications that should be explained.

Summary: ⚠️ Needs Human Review. The hex parsing fix is correct but has an edge case worth considering ("0x" alone). The unconditional cDAC metadata change has binary size implications and the two changes appear unrelated — a maintainer should confirm the coupling is intentional and the size impact is acceptable.


Detailed Findings

✅ Correctness — Hex prefix parsing fix (RhConfig.cpp:59-63)

The fix correctly strips the 0x/0X prefix before the hex digit parsing loop. Key observations:

  • Guard cchResult >= 2 prevents out-of-bounds access on buffer[1]
  • Aligns the environment variable path with the embedded config path (line 140) which uses strtoull and already handles 0x natively
  • All existing callers (EventPipe, GC config, dump type) benefit from this fix

⚠️ Edge Case — Bare "0x" input parses as valid zero (RhConfig.cpp:59-93) [advisory, not merge-blocking]

Flagged by all 3 models.

When buffer is exactly "0x" (cchResult == 2), startIndex is set to 2, the loop body never executes, and uiResult = 0 is returned as valid. This is consistent with the strtoull(embeddedValue, NULL, 16) path at line 140, which also returns 0 for "0x" when endptr is NULL. However, it means a bare "0x" is accepted rather than rejected as a parse error.

In practice this is negligible risk (no user would set an env var to exactly "0x"), but if the intent is to reject this case, add:

if (startIndex >= cchResult)
returnfalse;

⚠️ Binary Size Impact — Unconditional ManagedDataDescriptorProvider (Program.cs:260) [needs maintainer judgment]

Flagged by all 3 models.

Previously, cDAC type descriptors were only emitted with --debug/-g. Now every NativeAOT binary includes the DotNetManagedContractDescriptor JSON blob (header + JSON for [DataContract]-annotated types with EETypes). The size impact depends on the number of qualifying types — likely small (few KB) but non-zero.

Questions for maintainer:

  • Is the intent to enable post-mortem/cDAC debugging without debug symbols? If so, a brief comment or commit message explaining the rationale would help future readers.
  • Has the size impact been measured for representative apps?

⚠️ Missing Tests — No regression tests for either behavior change

Flagged by all 3 models.

Neither change includes tests:

  • The hex prefix fix should be verifiable through environment variable integration tests (e.g., DOTNET_GCHeapCount=0x10 produces expected behavior)
  • The unconditional cDAC descriptor could be verified by checking the symbol exists in non-debug NativeAOT output

Given this is native runtime code, integration-level testing may be more appropriate than unit tests. A maintainer can judge whether existing test infrastructure covers these paths.

💡 PR Scope — Two unrelated changes in one PR

The hex prefix fix (C++ runtime bug fix) and the unconditional cDAC metadata (C# compiler behavior change) appear orthogonal. Splitting would ease backporting the bug fix independently. However, if the author (a core NativeAOT maintainer) has a reason for coupling them, that context should be documented.

Generated by Code Review for issue #127644 ·

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

CopilotAI review requested due to automatic review settings May 1, 2026 20:47
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/875e04c9-b166-49fd-aefe-adaf5f6bd0b4
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>

CopilotAI 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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment threadsrc/coreclr/nativeaot/Runtime/RhConfig.cpp
@MichalStrehovsky
MichalStrehovsky merged commit b745957 into mainMay 2, 2026
109 checks passed
@MichalStrehovsky
MichalStrehovsky deleted the copilot/fix-hex-parser-prefix branch May 2, 2026 02:57
MichalStrehovsky added a commit that referenced this pull request May 22, 2026
…values (#128462)
PR #127644 made `0x`/`0X` valid for non-decimal NativeAOT config
parsing, but `CONFIG_VAL_MAXLEN` still capped input at 16 chars,
rejecting full-width values like `0xFFFFFFFFFFFFFFFF`. This change
aligns the textual length limit with the accepted syntax while
preserving existing overlong-value rejection behavior.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jun 1, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@MichalStrehovsky@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix NativeAOT hex config parser to handle 0x/0X prefix - #127644

Merged
MichalStrehovsky merged 5 commits into
mainfrom
copilot/fix-hex-parser-prefix
May 2, 2026
Merged

Fix NativeAOT hex config parser to handle 0x/0X prefix#127644
MichalStrehovsky merged 5 commits into
mainfrom
copilot/fix-hex-parser-prefix

Conversation

CopilotAI commented May 1, 2026

Copy link
Copy Markdown
Contributor

NativeAOT's RhConfig::Environment::TryGetIntegerValue had a hand-rolled hex parser that rejected the 0x/0X prefix — returning a parse error when it encountered x. This meant env vars like DOTNET_GCHeapHardLimit=0xC0000000 silently failed to parse, leaving the hard limit unset. With GCLargePages=2 also set, the GC would then return CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT and fail initialization. CoreCLR's equivalent uses strtoul(..., 16) which handles the prefix natively.

Description

  • src/coreclr/nativeaot/Runtime/RhConfig.cpp — In TryGetIntegerValue, skip a leading 0x/0X prefix when parsing in hex mode, before entering the digit loop. Additionally, return false (parse error) when the value is exactly "0x" or "0X" with no hex digits following the prefix, matching CoreCLR's strtoul behavior:
uint32_t startIndex = 0;
if (!decimal && cchResult >= 2 && buffer[0] == '0' && (buffer[1] == 'x' || buffer[1] == 'X'))
{
startIndex = 2;
if (startIndex == cchResult)
returnfalse; // parse error - hex prefix without any digits
}
for (uint32_t i = startIndex; i < cchResult; i++)

This aligns NativeAOT's config parsing with CoreCLR's strtoul-based behavior and fixes the Collect_Aggressive_LargePages test failure under NativeAOT.

Original prompt

Problem

NativeAOT's RhConfig::Environment::TryGetIntegerValue in src/coreclr/nativeaot/Runtime/RhConfig.cpp uses a hand-rolled hex parser that does not handle the 0x or 0X prefix. This causes config values like DOTNET_GCHeapHardLimit=0xC0000000 to fail to parse, because when the parser encounters the x character it returns false (parse error).

CoreCLR's equivalent code (CLRConfigNoCache::TryAsInteger in src/coreclr/inc/clrconfignocache.h) uses strtoul(_value, &endPtr, radix) which natively handles the 0x prefix when radix is 16.

This causes the test Collect_Aggressive_LargePages added in PR #127290 to fail under NativeAOT: the GCHeapHardLimit fails to parse, so no hard limit is set, but GCLargePages=2 succeeds → the GC returns CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT and the process exits with -1.

Fix

In src/coreclr/nativeaot/Runtime/RhConfig.cpp, in the TryGetIntegerValue method, when parsing in hex mode (i.e., decimal is false), skip a leading 0x or 0X prefix before entering the digit-parsing loop. This matches the behavior of strtoul with radix 16 that CoreCLR uses.

Specifically, after reading the environment variable into buffer and before the parsing loop, add:

uint32_t startIndex = 0;
if (!decimal && cchResult >= 2 && buffer[0] == '0' && (buffer[1] == 'x' || buffer[1] == 'X'))
{
startIndex = 2;
}

Then change the loop from for (uint32_t i = 0; ...) to for (uint32_t i = startIndex; ...).

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: ```
16:22:36.657 Running test: GC\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd

Return code: 1
Raw output file: C:\h\w\B1EC0A05\w\B5C209C7\uploads\API\GC\Collect_Aggressive_LargePages\output.txt
Raw output:
BEGIN EXECUTION
call C:\h\w\B1EC0A05\p\nativeaottest.cmd C:\h\w\B1EC0A05\w\B5C209C7\e\GC\API\GC\Collect_Aggressive_LargePages\ Collect_Aggressive_LargePages.dll
Expected: 100
Actual: -1
END EXECUTION - FAILED
FAILED
Test Harness Exitcode is : 1
To run the test:
Set up CORE_ROOT and run.

C:\h\w\B1EC0A05\w\B5C209C7\e\GC\GC\native....\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd
Xunit.Sdk.TrueException:
Return code: 1
Raw output file: C:\h\w\B1EC0A05\w\B5C209C7\uploads\API\GC\Collect_Aggressive_LargePages\output.txt
Raw output:
BEGIN EXECUTION
call C:\h\w\B1EC0A05\p\nativeaottest.cmd C:\h\w\B1EC0A05\w\B5C209C7\e\GC\API\GC\Collect_Aggressive_LargePages\ Collect_Aggressive_LargePages.dll
Expected: 100
Actual: -1
END EXECUTION - FAILED
FAILED
Test Harness Exitcode is : 1
To run the test:
Set up CORE_ROOT and run.
C:\h\w\B1EC0A05\w\B5C209C7\e\GC\GC\native....\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd
at Xunit.Assert.True(Nullable`1, String)
at TestLibrary.OutOfProcessTest.RunOutOfProcessTest(String, String)
at Program.<

$>g__TestExecutor26|0_27(StreamWriter, StreamWriter, Program.<>c__DisplayClass0_0&)
16:22:36.755 Failed test: GC\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd

the test added here is failing with native AOT. why? Assistant: The test fails on NativeAOT because **NativeAOT's config reader cannot parse the `0x` prefix** in the `DOTNET_GCHeapHardLimit=0xC0000000` environment variable.
Looking at NativeAOT's `RhConfig::Environment::TryGetIntegerValue` in:
```c++ name=src/coreclr/nativeaot/Runtime/RhConfig.cpp url=https://github.com/dotnet/runtime/blob/bfaabae213cf02d343846d253b7149a03819680d/src/coreclr/nativeaot/Runtime/RhConfig.cpp#L58-L84
uint64_t uiResult = 0;
for (uint32_t i = 0; i < cchResult; i++)
{
TCHAR ch = buffer[i];
// ...
else
{
uiResult *= 16;
if ((ch >= '0') && (ch <= '9'))
uiResult += ch - '0';
else if ((ch >= 'a') && (ch <= 'f'))
uiResult += (ch - 'a') + 10;
else if ((ch >= 'A') && (ch <= 'F'))
uiResult += (ch - 'A') + 10;
else
return false; // parse error <-- 'x' hits this
}
}

When the value is 0xC0000000, the parser reads hex digits character-by-character. It handles 0, but then encounters x, which isn't a valid hex digit — so it returns false (parse error). This means GCHeapHardLimit is never set.

The cascade:

  1. DOTNET_GCHeapHardLimit=0xC0000000 fails to parse → no hard limit is set
  2. DOTNET_GCLargePages=2 is read successfully (it's a simple 2, valid hex) → use_large_pages_p = true
  3. The GC hits this check in interface.cpp: if ((!gc_heap::heap_hard_limit) && gc_heap::use_large_pages_p) → returns CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT
  4. GC initialization fails →...

This pull request was created from Copilot chat.

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/b2cf8499-44ac-4e9a-995b-c72b36e081a2
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotMay 1, 2026 11:14
CopilotAI changed the title [WIP] Fix hex parser to handle 0x prefix in TryGetIntegerValueFix NativeAOT hex config parser to handle 0x/0X prefixMay 1, 2026
CopilotAI requested a review from MichalStrehovskyMay 1, 2026 11:15
CopilotAI review requested due to automatic review settings May 1, 2026 11:21
@MichalStrehovsky
MichalStrehovsky marked this pull request as ready for review May 1, 2026 11:21
@MichalStrehovsky

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

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

CopilotAI 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.

Pull request overview

This pull request fixes NativeAOT configuration parsing so hexadecimal environment variable values can include a 0x/0X prefix (matching the commonly accepted format and aligning more closely with CoreCLR behavior), preventing GC configuration knobs like DOTNET_GCHeapHardLimit=0x... from being ignored under NativeAOT.

Changes:

  • NativeAOT RhConfig::Environment::TryGetIntegerValue now skips a leading 0x/0X prefix when parsing hex values from environment variables.
  • ILCompiler now always adds ManagedDataDescriptorProvider (previously gated behind --debug).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

FileDescription
src/coreclr/nativeaot/Runtime/RhConfig.cppAdds logic to skip 0x/0X prefix in the hand-rolled hex parser for env var config values.
src/coreclr/tools/aot/ILCompiler/Program.csRemoves the --debug condition and unconditionally roots ManagedDataDescriptorProvider (managed cDAC descriptor emission).

Comment threadsrc/coreclr/nativeaot/Runtime/RhConfig.cpp
Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #127644

Note

This review was generated by Copilot using multi-model analysis (Claude Opus 4.6, Claude Sonnet 4.5, GPT-5.3-Codex).

Holistic Assessment

Motivation: The hex prefix fix addresses a real inconsistency — strtoull (used for embedded config values) already handles 0x prefixes, but the manual environment variable parser did not, causing values like DOTNET_GCHeapCount=0x10 to fail. The ManagedDataDescriptorProvider change enables cDAC diagnostics without requiring --debug.

Approach: The hex prefix fix is minimal and correct for the common case. The ManagedDataDescriptorProvider change is a one-line behavioral shift with broader implications that should be explained.

Summary: ⚠️ Needs Human Review. The hex parsing fix is correct but has an edge case worth considering ("0x" alone). The unconditional cDAC metadata change has binary size implications and the two changes appear unrelated — a maintainer should confirm the coupling is intentional and the size impact is acceptable.


Detailed Findings

✅ Correctness — Hex prefix parsing fix (RhConfig.cpp:59-63)

The fix correctly strips the 0x/0X prefix before the hex digit parsing loop. Key observations:

  • Guard cchResult >= 2 prevents out-of-bounds access on buffer[1]
  • Aligns the environment variable path with the embedded config path (line 140) which uses strtoull and already handles 0x natively
  • All existing callers (EventPipe, GC config, dump type) benefit from this fix

⚠️ Edge Case — Bare "0x" input parses as valid zero (RhConfig.cpp:59-93) [advisory, not merge-blocking]

Flagged by all 3 models.

When buffer is exactly "0x" (cchResult == 2), startIndex is set to 2, the loop body never executes, and uiResult = 0 is returned as valid. This is consistent with the strtoull(embeddedValue, NULL, 16) path at line 140, which also returns 0 for "0x" when endptr is NULL. However, it means a bare "0x" is accepted rather than rejected as a parse error.

In practice this is negligible risk (no user would set an env var to exactly "0x"), but if the intent is to reject this case, add:

if (startIndex >= cchResult)
returnfalse;

⚠️ Binary Size Impact — Unconditional ManagedDataDescriptorProvider (Program.cs:260) [needs maintainer judgment]

Flagged by all 3 models.

Previously, cDAC type descriptors were only emitted with --debug/-g. Now every NativeAOT binary includes the DotNetManagedContractDescriptor JSON blob (header + JSON for [DataContract]-annotated types with EETypes). The size impact depends on the number of qualifying types — likely small (few KB) but non-zero.

Questions for maintainer:

  • Is the intent to enable post-mortem/cDAC debugging without debug symbols? If so, a brief comment or commit message explaining the rationale would help future readers.
  • Has the size impact been measured for representative apps?

⚠️ Missing Tests — No regression tests for either behavior change

Flagged by all 3 models.

Neither change includes tests:

  • The hex prefix fix should be verifiable through environment variable integration tests (e.g., DOTNET_GCHeapCount=0x10 produces expected behavior)
  • The unconditional cDAC descriptor could be verified by checking the symbol exists in non-debug NativeAOT output

Given this is native runtime code, integration-level testing may be more appropriate than unit tests. A maintainer can judge whether existing test infrastructure covers these paths.

💡 PR Scope — Two unrelated changes in one PR

The hex prefix fix (C++ runtime bug fix) and the unconditional cDAC metadata (C# compiler behavior change) appear orthogonal. Splitting would ease backporting the bug fix independently. However, if the author (a core NativeAOT maintainer) has a reason for coupling them, that context should be documented.

Generated by Code Review for issue #127644 ·

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

CopilotAI review requested due to automatic review settings May 1, 2026 20:47
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/875e04c9-b166-49fd-aefe-adaf5f6bd0b4
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>

CopilotAI 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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment threadsrc/coreclr/nativeaot/Runtime/RhConfig.cpp
@MichalStrehovsky
MichalStrehovsky merged commit b745957 into mainMay 2, 2026
109 checks passed
@MichalStrehovsky
MichalStrehovsky deleted the copilot/fix-hex-parser-prefix branch May 2, 2026 02:57
MichalStrehovsky added a commit that referenced this pull request May 22, 2026
…values (#128462)
PR #127644 made `0x`/`0X` valid for non-decimal NativeAOT config
parsing, but `CONFIG_VAL_MAXLEN` still capped input at 16 chars,
rejecting full-width values like `0xFFFFFFFFFFFFFFFF`. This change
aligns the textual length limit with the accepted syntax while
preserving existing overlong-value rejection behavior.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jun 1, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@MichalStrehovsky@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix NativeAOT hex config parser to handle 0x/0X prefix - #127644

Merged
MichalStrehovsky merged 5 commits into
mainfrom
copilot/fix-hex-parser-prefix
May 2, 2026
Merged

Fix NativeAOT hex config parser to handle 0x/0X prefix#127644
MichalStrehovsky merged 5 commits into
mainfrom
copilot/fix-hex-parser-prefix

Conversation

CopilotAI commented May 1, 2026

Copy link
Copy Markdown
Contributor

NativeAOT's RhConfig::Environment::TryGetIntegerValue had a hand-rolled hex parser that rejected the 0x/0X prefix — returning a parse error when it encountered x. This meant env vars like DOTNET_GCHeapHardLimit=0xC0000000 silently failed to parse, leaving the hard limit unset. With GCLargePages=2 also set, the GC would then return CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT and fail initialization. CoreCLR's equivalent uses strtoul(..., 16) which handles the prefix natively.

Description

  • src/coreclr/nativeaot/Runtime/RhConfig.cpp — In TryGetIntegerValue, skip a leading 0x/0X prefix when parsing in hex mode, before entering the digit loop. Additionally, return false (parse error) when the value is exactly "0x" or "0X" with no hex digits following the prefix, matching CoreCLR's strtoul behavior:
uint32_t startIndex = 0;
if (!decimal && cchResult >= 2 && buffer[0] == '0' && (buffer[1] == 'x' || buffer[1] == 'X'))
{
startIndex = 2;
if (startIndex == cchResult)
returnfalse; // parse error - hex prefix without any digits
}
for (uint32_t i = startIndex; i < cchResult; i++)

This aligns NativeAOT's config parsing with CoreCLR's strtoul-based behavior and fixes the Collect_Aggressive_LargePages test failure under NativeAOT.

Original prompt

Problem

NativeAOT's RhConfig::Environment::TryGetIntegerValue in src/coreclr/nativeaot/Runtime/RhConfig.cpp uses a hand-rolled hex parser that does not handle the 0x or 0X prefix. This causes config values like DOTNET_GCHeapHardLimit=0xC0000000 to fail to parse, because when the parser encounters the x character it returns false (parse error).

CoreCLR's equivalent code (CLRConfigNoCache::TryAsInteger in src/coreclr/inc/clrconfignocache.h) uses strtoul(_value, &endPtr, radix) which natively handles the 0x prefix when radix is 16.

This causes the test Collect_Aggressive_LargePages added in PR #127290 to fail under NativeAOT: the GCHeapHardLimit fails to parse, so no hard limit is set, but GCLargePages=2 succeeds → the GC returns CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT and the process exits with -1.

Fix

In src/coreclr/nativeaot/Runtime/RhConfig.cpp, in the TryGetIntegerValue method, when parsing in hex mode (i.e., decimal is false), skip a leading 0x or 0X prefix before entering the digit-parsing loop. This matches the behavior of strtoul with radix 16 that CoreCLR uses.

Specifically, after reading the environment variable into buffer and before the parsing loop, add:

uint32_t startIndex = 0;
if (!decimal && cchResult >= 2 && buffer[0] == '0' && (buffer[1] == 'x' || buffer[1] == 'X'))
{
startIndex = 2;
}

Then change the loop from for (uint32_t i = 0; ...) to for (uint32_t i = startIndex; ...).

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: ```
16:22:36.657 Running test: GC\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd

Return code: 1
Raw output file: C:\h\w\B1EC0A05\w\B5C209C7\uploads\API\GC\Collect_Aggressive_LargePages\output.txt
Raw output:
BEGIN EXECUTION
call C:\h\w\B1EC0A05\p\nativeaottest.cmd C:\h\w\B1EC0A05\w\B5C209C7\e\GC\API\GC\Collect_Aggressive_LargePages\ Collect_Aggressive_LargePages.dll
Expected: 100
Actual: -1
END EXECUTION - FAILED
FAILED
Test Harness Exitcode is : 1
To run the test:
Set up CORE_ROOT and run.

C:\h\w\B1EC0A05\w\B5C209C7\e\GC\GC\native....\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd
Xunit.Sdk.TrueException:
Return code: 1
Raw output file: C:\h\w\B1EC0A05\w\B5C209C7\uploads\API\GC\Collect_Aggressive_LargePages\output.txt
Raw output:
BEGIN EXECUTION
call C:\h\w\B1EC0A05\p\nativeaottest.cmd C:\h\w\B1EC0A05\w\B5C209C7\e\GC\API\GC\Collect_Aggressive_LargePages\ Collect_Aggressive_LargePages.dll
Expected: 100
Actual: -1
END EXECUTION - FAILED
FAILED
Test Harness Exitcode is : 1
To run the test:
Set up CORE_ROOT and run.
C:\h\w\B1EC0A05\w\B5C209C7\e\GC\GC\native....\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd
at Xunit.Assert.True(Nullable`1, String)
at TestLibrary.OutOfProcessTest.RunOutOfProcessTest(String, String)
at Program.<

$>g__TestExecutor26|0_27(StreamWriter, StreamWriter, Program.<>c__DisplayClass0_0&)
16:22:36.755 Failed test: GC\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd

the test added here is failing with native AOT. why? Assistant: The test fails on NativeAOT because **NativeAOT's config reader cannot parse the `0x` prefix** in the `DOTNET_GCHeapHardLimit=0xC0000000` environment variable.
Looking at NativeAOT's `RhConfig::Environment::TryGetIntegerValue` in:
```c++ name=src/coreclr/nativeaot/Runtime/RhConfig.cpp url=https://github.com/dotnet/runtime/blob/bfaabae213cf02d343846d253b7149a03819680d/src/coreclr/nativeaot/Runtime/RhConfig.cpp#L58-L84
uint64_t uiResult = 0;
for (uint32_t i = 0; i < cchResult; i++)
{
TCHAR ch = buffer[i];
// ...
else
{
uiResult *= 16;
if ((ch >= '0') && (ch <= '9'))
uiResult += ch - '0';
else if ((ch >= 'a') && (ch <= 'f'))
uiResult += (ch - 'a') + 10;
else if ((ch >= 'A') && (ch <= 'F'))
uiResult += (ch - 'A') + 10;
else
return false; // parse error <-- 'x' hits this
}
}

When the value is 0xC0000000, the parser reads hex digits character-by-character. It handles 0, but then encounters x, which isn't a valid hex digit — so it returns false (parse error). This means GCHeapHardLimit is never set.

The cascade:

  1. DOTNET_GCHeapHardLimit=0xC0000000 fails to parse → no hard limit is set
  2. DOTNET_GCLargePages=2 is read successfully (it's a simple 2, valid hex) → use_large_pages_p = true
  3. The GC hits this check in interface.cpp: if ((!gc_heap::heap_hard_limit) && gc_heap::use_large_pages_p) → returns CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT
  4. GC initialization fails →...

This pull request was created from Copilot chat.

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/b2cf8499-44ac-4e9a-995b-c72b36e081a2
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotMay 1, 2026 11:14
CopilotAI changed the title [WIP] Fix hex parser to handle 0x prefix in TryGetIntegerValueFix NativeAOT hex config parser to handle 0x/0X prefixMay 1, 2026
CopilotAI requested a review from MichalStrehovskyMay 1, 2026 11:15
CopilotAI review requested due to automatic review settings May 1, 2026 11:21
@MichalStrehovsky
MichalStrehovsky marked this pull request as ready for review May 1, 2026 11:21
@MichalStrehovsky

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

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

CopilotAI 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.

Pull request overview

This pull request fixes NativeAOT configuration parsing so hexadecimal environment variable values can include a 0x/0X prefix (matching the commonly accepted format and aligning more closely with CoreCLR behavior), preventing GC configuration knobs like DOTNET_GCHeapHardLimit=0x... from being ignored under NativeAOT.

Changes:

  • NativeAOT RhConfig::Environment::TryGetIntegerValue now skips a leading 0x/0X prefix when parsing hex values from environment variables.
  • ILCompiler now always adds ManagedDataDescriptorProvider (previously gated behind --debug).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

FileDescription
src/coreclr/nativeaot/Runtime/RhConfig.cppAdds logic to skip 0x/0X prefix in the hand-rolled hex parser for env var config values.
src/coreclr/tools/aot/ILCompiler/Program.csRemoves the --debug condition and unconditionally roots ManagedDataDescriptorProvider (managed cDAC descriptor emission).

Comment threadsrc/coreclr/nativeaot/Runtime/RhConfig.cpp
Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #127644

Note

This review was generated by Copilot using multi-model analysis (Claude Opus 4.6, Claude Sonnet 4.5, GPT-5.3-Codex).

Holistic Assessment

Motivation: The hex prefix fix addresses a real inconsistency — strtoull (used for embedded config values) already handles 0x prefixes, but the manual environment variable parser did not, causing values like DOTNET_GCHeapCount=0x10 to fail. The ManagedDataDescriptorProvider change enables cDAC diagnostics without requiring --debug.

Approach: The hex prefix fix is minimal and correct for the common case. The ManagedDataDescriptorProvider change is a one-line behavioral shift with broader implications that should be explained.

Summary: ⚠️ Needs Human Review. The hex parsing fix is correct but has an edge case worth considering ("0x" alone). The unconditional cDAC metadata change has binary size implications and the two changes appear unrelated — a maintainer should confirm the coupling is intentional and the size impact is acceptable.


Detailed Findings

✅ Correctness — Hex prefix parsing fix (RhConfig.cpp:59-63)

The fix correctly strips the 0x/0X prefix before the hex digit parsing loop. Key observations:

  • Guard cchResult >= 2 prevents out-of-bounds access on buffer[1]
  • Aligns the environment variable path with the embedded config path (line 140) which uses strtoull and already handles 0x natively
  • All existing callers (EventPipe, GC config, dump type) benefit from this fix

⚠️ Edge Case — Bare "0x" input parses as valid zero (RhConfig.cpp:59-93) [advisory, not merge-blocking]

Flagged by all 3 models.

When buffer is exactly "0x" (cchResult == 2), startIndex is set to 2, the loop body never executes, and uiResult = 0 is returned as valid. This is consistent with the strtoull(embeddedValue, NULL, 16) path at line 140, which also returns 0 for "0x" when endptr is NULL. However, it means a bare "0x" is accepted rather than rejected as a parse error.

In practice this is negligible risk (no user would set an env var to exactly "0x"), but if the intent is to reject this case, add:

if (startIndex >= cchResult)
returnfalse;

⚠️ Binary Size Impact — Unconditional ManagedDataDescriptorProvider (Program.cs:260) [needs maintainer judgment]

Flagged by all 3 models.

Previously, cDAC type descriptors were only emitted with --debug/-g. Now every NativeAOT binary includes the DotNetManagedContractDescriptor JSON blob (header + JSON for [DataContract]-annotated types with EETypes). The size impact depends on the number of qualifying types — likely small (few KB) but non-zero.

Questions for maintainer:

  • Is the intent to enable post-mortem/cDAC debugging without debug symbols? If so, a brief comment or commit message explaining the rationale would help future readers.
  • Has the size impact been measured for representative apps?

⚠️ Missing Tests — No regression tests for either behavior change

Flagged by all 3 models.

Neither change includes tests:

  • The hex prefix fix should be verifiable through environment variable integration tests (e.g., DOTNET_GCHeapCount=0x10 produces expected behavior)
  • The unconditional cDAC descriptor could be verified by checking the symbol exists in non-debug NativeAOT output

Given this is native runtime code, integration-level testing may be more appropriate than unit tests. A maintainer can judge whether existing test infrastructure covers these paths.

💡 PR Scope — Two unrelated changes in one PR

The hex prefix fix (C++ runtime bug fix) and the unconditional cDAC metadata (C# compiler behavior change) appear orthogonal. Splitting would ease backporting the bug fix independently. However, if the author (a core NativeAOT maintainer) has a reason for coupling them, that context should be documented.

Generated by Code Review for issue #127644 ·

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

CopilotAI review requested due to automatic review settings May 1, 2026 20:47
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/875e04c9-b166-49fd-aefe-adaf5f6bd0b4
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>

CopilotAI 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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment threadsrc/coreclr/nativeaot/Runtime/RhConfig.cpp
@MichalStrehovsky
MichalStrehovsky merged commit b745957 into mainMay 2, 2026
109 checks passed
@MichalStrehovsky
MichalStrehovsky deleted the copilot/fix-hex-parser-prefix branch May 2, 2026 02:57
MichalStrehovsky added a commit that referenced this pull request May 22, 2026
…values (#128462)
PR #127644 made `0x`/`0X` valid for non-decimal NativeAOT config
parsing, but `CONFIG_VAL_MAXLEN` still capped input at 16 chars,
rejecting full-width values like `0xFFFFFFFFFFFFFFFF`. This change
aligns the textual length limit with the accepted syntax while
preserving existing overlong-value rejection behavior.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jun 1, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@MichalStrehovsky@jkotas
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Fix NativeAOT hex config parser to handle 0x/0X prefix - #127644

Merged
MichalStrehovsky merged 5 commits into
mainfrom
copilot/fix-hex-parser-prefix
May 2, 2026
Merged

Fix NativeAOT hex config parser to handle 0x/0X prefix#127644
MichalStrehovsky merged 5 commits into
mainfrom
copilot/fix-hex-parser-prefix

Conversation

CopilotAI commented May 1, 2026

Copy link
Copy Markdown
Contributor

NativeAOT's RhConfig::Environment::TryGetIntegerValue had a hand-rolled hex parser that rejected the 0x/0X prefix — returning a parse error when it encountered x. This meant env vars like DOTNET_GCHeapHardLimit=0xC0000000 silently failed to parse, leaving the hard limit unset. With GCLargePages=2 also set, the GC would then return CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT and fail initialization. CoreCLR's equivalent uses strtoul(..., 16) which handles the prefix natively.

Description

  • src/coreclr/nativeaot/Runtime/RhConfig.cpp — In TryGetIntegerValue, skip a leading 0x/0X prefix when parsing in hex mode, before entering the digit loop. Additionally, return false (parse error) when the value is exactly "0x" or "0X" with no hex digits following the prefix, matching CoreCLR's strtoul behavior:
uint32_t startIndex = 0;
if (!decimal && cchResult >= 2 && buffer[0] == '0' && (buffer[1] == 'x' || buffer[1] == 'X'))
{
startIndex = 2;
if (startIndex == cchResult)
returnfalse; // parse error - hex prefix without any digits
}
for (uint32_t i = startIndex; i < cchResult; i++)

This aligns NativeAOT's config parsing with CoreCLR's strtoul-based behavior and fixes the Collect_Aggressive_LargePages test failure under NativeAOT.

Original prompt

Problem

NativeAOT's RhConfig::Environment::TryGetIntegerValue in src/coreclr/nativeaot/Runtime/RhConfig.cpp uses a hand-rolled hex parser that does not handle the 0x or 0X prefix. This causes config values like DOTNET_GCHeapHardLimit=0xC0000000 to fail to parse, because when the parser encounters the x character it returns false (parse error).

CoreCLR's equivalent code (CLRConfigNoCache::TryAsInteger in src/coreclr/inc/clrconfignocache.h) uses strtoul(_value, &endPtr, radix) which natively handles the 0x prefix when radix is 16.

This causes the test Collect_Aggressive_LargePages added in PR #127290 to fail under NativeAOT: the GCHeapHardLimit fails to parse, so no hard limit is set, but GCLargePages=2 succeeds → the GC returns CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT and the process exits with -1.

Fix

In src/coreclr/nativeaot/Runtime/RhConfig.cpp, in the TryGetIntegerValue method, when parsing in hex mode (i.e., decimal is false), skip a leading 0x or 0X prefix before entering the digit-parsing loop. This matches the behavior of strtoul with radix 16 that CoreCLR uses.

Specifically, after reading the environment variable into buffer and before the parsing loop, add:

uint32_t startIndex = 0;
if (!decimal && cchResult >= 2 && buffer[0] == '0' && (buffer[1] == 'x' || buffer[1] == 'X'))
{
startIndex = 2;
}

Then change the loop from for (uint32_t i = 0; ...) to for (uint32_t i = startIndex; ...).

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: ```
16:22:36.657 Running test: GC\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd

Return code: 1
Raw output file: C:\h\w\B1EC0A05\w\B5C209C7\uploads\API\GC\Collect_Aggressive_LargePages\output.txt
Raw output:
BEGIN EXECUTION
call C:\h\w\B1EC0A05\p\nativeaottest.cmd C:\h\w\B1EC0A05\w\B5C209C7\e\GC\API\GC\Collect_Aggressive_LargePages\ Collect_Aggressive_LargePages.dll
Expected: 100
Actual: -1
END EXECUTION - FAILED
FAILED
Test Harness Exitcode is : 1
To run the test:
Set up CORE_ROOT and run.

C:\h\w\B1EC0A05\w\B5C209C7\e\GC\GC\native....\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd
Xunit.Sdk.TrueException:
Return code: 1
Raw output file: C:\h\w\B1EC0A05\w\B5C209C7\uploads\API\GC\Collect_Aggressive_LargePages\output.txt
Raw output:
BEGIN EXECUTION
call C:\h\w\B1EC0A05\p\nativeaottest.cmd C:\h\w\B1EC0A05\w\B5C209C7\e\GC\API\GC\Collect_Aggressive_LargePages\ Collect_Aggressive_LargePages.dll
Expected: 100
Actual: -1
END EXECUTION - FAILED
FAILED
Test Harness Exitcode is : 1
To run the test:
Set up CORE_ROOT and run.
C:\h\w\B1EC0A05\w\B5C209C7\e\GC\GC\native....\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd
at Xunit.Assert.True(Nullable`1, String)
at TestLibrary.OutOfProcessTest.RunOutOfProcessTest(String, String)
at Program.<

$>g__TestExecutor26|0_27(StreamWriter, StreamWriter, Program.<>c__DisplayClass0_0&)
16:22:36.755 Failed test: GC\API\GC\Collect_Aggressive_LargePages\Collect_Aggressive_LargePages.cmd

the test added here is failing with native AOT. why? Assistant: The test fails on NativeAOT because **NativeAOT's config reader cannot parse the `0x` prefix** in the `DOTNET_GCHeapHardLimit=0xC0000000` environment variable.
Looking at NativeAOT's `RhConfig::Environment::TryGetIntegerValue` in:
```c++ name=src/coreclr/nativeaot/Runtime/RhConfig.cpp url=https://github.com/dotnet/runtime/blob/bfaabae213cf02d343846d253b7149a03819680d/src/coreclr/nativeaot/Runtime/RhConfig.cpp#L58-L84
uint64_t uiResult = 0;
for (uint32_t i = 0; i < cchResult; i++)
{
TCHAR ch = buffer[i];
// ...
else
{
uiResult *= 16;
if ((ch >= '0') && (ch <= '9'))
uiResult += ch - '0';
else if ((ch >= 'a') && (ch <= 'f'))
uiResult += (ch - 'a') + 10;
else if ((ch >= 'A') && (ch <= 'F'))
uiResult += (ch - 'A') + 10;
else
return false; // parse error <-- 'x' hits this
}
}

When the value is 0xC0000000, the parser reads hex digits character-by-character. It handles 0, but then encounters x, which isn't a valid hex digit — so it returns false (parse error). This means GCHeapHardLimit is never set.

The cascade:

  1. DOTNET_GCHeapHardLimit=0xC0000000 fails to parse → no hard limit is set
  2. DOTNET_GCLargePages=2 is read successfully (it's a simple 2, valid hex) → use_large_pages_p = true
  3. The GC hits this check in interface.cpp: if ((!gc_heap::heap_hard_limit) && gc_heap::use_large_pages_p) → returns CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT
  4. GC initialization fails →...

This pull request was created from Copilot chat.

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/b2cf8499-44ac-4e9a-995b-c72b36e081a2
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
CopilotAI requested review from Copilot and removed request for CopilotMay 1, 2026 11:14
CopilotAI changed the title [WIP] Fix hex parser to handle 0x prefix in TryGetIntegerValueFix NativeAOT hex config parser to handle 0x/0X prefixMay 1, 2026
CopilotAI requested a review from MichalStrehovskyMay 1, 2026 11:15
CopilotAI review requested due to automatic review settings May 1, 2026 11:21
@MichalStrehovsky
MichalStrehovsky marked this pull request as ready for review May 1, 2026 11:21
@MichalStrehovsky

Copy link
Copy Markdown
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines

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

CopilotAI 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.

Pull request overview

This pull request fixes NativeAOT configuration parsing so hexadecimal environment variable values can include a 0x/0X prefix (matching the commonly accepted format and aligning more closely with CoreCLR behavior), preventing GC configuration knobs like DOTNET_GCHeapHardLimit=0x... from being ignored under NativeAOT.

Changes:

  • NativeAOT RhConfig::Environment::TryGetIntegerValue now skips a leading 0x/0X prefix when parsing hex values from environment variables.
  • ILCompiler now always adds ManagedDataDescriptorProvider (previously gated behind --debug).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

FileDescription
src/coreclr/nativeaot/Runtime/RhConfig.cppAdds logic to skip 0x/0X prefix in the hand-rolled hex parser for env var config values.
src/coreclr/tools/aot/ILCompiler/Program.csRemoves the --debug condition and unconditionally roots ManagedDataDescriptorProvider (managed cDAC descriptor emission).

Comment threadsrc/coreclr/nativeaot/Runtime/RhConfig.cpp
Comment threadsrc/coreclr/tools/aot/ILCompiler/Program.cs
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Copilot Code Review — PR #127644

Note

This review was generated by Copilot using multi-model analysis (Claude Opus 4.6, Claude Sonnet 4.5, GPT-5.3-Codex).

Holistic Assessment

Motivation: The hex prefix fix addresses a real inconsistency — strtoull (used for embedded config values) already handles 0x prefixes, but the manual environment variable parser did not, causing values like DOTNET_GCHeapCount=0x10 to fail. The ManagedDataDescriptorProvider change enables cDAC diagnostics without requiring --debug.

Approach: The hex prefix fix is minimal and correct for the common case. The ManagedDataDescriptorProvider change is a one-line behavioral shift with broader implications that should be explained.

Summary: ⚠️ Needs Human Review. The hex parsing fix is correct but has an edge case worth considering ("0x" alone). The unconditional cDAC metadata change has binary size implications and the two changes appear unrelated — a maintainer should confirm the coupling is intentional and the size impact is acceptable.


Detailed Findings

✅ Correctness — Hex prefix parsing fix (RhConfig.cpp:59-63)

The fix correctly strips the 0x/0X prefix before the hex digit parsing loop. Key observations:

  • Guard cchResult >= 2 prevents out-of-bounds access on buffer[1]
  • Aligns the environment variable path with the embedded config path (line 140) which uses strtoull and already handles 0x natively
  • All existing callers (EventPipe, GC config, dump type) benefit from this fix

⚠️ Edge Case — Bare "0x" input parses as valid zero (RhConfig.cpp:59-93) [advisory, not merge-blocking]

Flagged by all 3 models.

When buffer is exactly "0x" (cchResult == 2), startIndex is set to 2, the loop body never executes, and uiResult = 0 is returned as valid. This is consistent with the strtoull(embeddedValue, NULL, 16) path at line 140, which also returns 0 for "0x" when endptr is NULL. However, it means a bare "0x" is accepted rather than rejected as a parse error.

In practice this is negligible risk (no user would set an env var to exactly "0x"), but if the intent is to reject this case, add:

if (startIndex >= cchResult)
returnfalse;

⚠️ Binary Size Impact — Unconditional ManagedDataDescriptorProvider (Program.cs:260) [needs maintainer judgment]

Flagged by all 3 models.

Previously, cDAC type descriptors were only emitted with --debug/-g. Now every NativeAOT binary includes the DotNetManagedContractDescriptor JSON blob (header + JSON for [DataContract]-annotated types with EETypes). The size impact depends on the number of qualifying types — likely small (few KB) but non-zero.

Questions for maintainer:

  • Is the intent to enable post-mortem/cDAC debugging without debug symbols? If so, a brief comment or commit message explaining the rationale would help future readers.
  • Has the size impact been measured for representative apps?

⚠️ Missing Tests — No regression tests for either behavior change

Flagged by all 3 models.

Neither change includes tests:

  • The hex prefix fix should be verifiable through environment variable integration tests (e.g., DOTNET_GCHeapCount=0x10 produces expected behavior)
  • The unconditional cDAC descriptor could be verified by checking the symbol exists in non-debug NativeAOT output

Given this is native runtime code, integration-level testing may be more appropriate than unit tests. A maintainer can judge whether existing test infrastructure covers these paths.

💡 PR Scope — Two unrelated changes in one PR

The hex prefix fix (C++ runtime bug fix) and the unconditional cDAC metadata (C# compiler behavior change) appear orthogonal. Splitting would ease backporting the bug fix independently. However, if the author (a core NativeAOT maintainer) has a reason for coupling them, that context should be documented.

Generated by Code Review for issue #127644 ·

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

CopilotAI review requested due to automatic review settings May 1, 2026 20:47
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/875e04c9-b166-49fd-aefe-adaf5f6bd0b4
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>

CopilotAI 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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment threadsrc/coreclr/nativeaot/Runtime/RhConfig.cpp
@MichalStrehovsky
MichalStrehovsky merged commit b745957 into mainMay 2, 2026
109 checks passed
@MichalStrehovsky
MichalStrehovsky deleted the copilot/fix-hex-parser-prefix branch May 2, 2026 02:57
MichalStrehovsky added a commit that referenced this pull request May 22, 2026
…values (#128462)
PR #127644 made `0x`/`0X` valid for non-decimal NativeAOT config
parsing, but `CONFIG_VAL_MAXLEN` still capped input at 16 chars,
rejecting full-width values like `0xFFFFFFFFFFFFFFFF`. This change
aligns the textual length limit with the accepted syntax while
preserving existing overlong-value rejection behavior.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jun 1, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@MichalStrehovsky@jkotas