Skip to content

NativeAOT: allow full-width 0x/0X-prefixed 64-bit hex env config values#128462

Open
Copilot wants to merge 2 commits into
mainfrom
copilot/fix-nativeaot-config-max-length
Open

NativeAOT: allow full-width 0x/0X-prefixed 64-bit hex env config values#128462
Copilot wants to merge 2 commits into
mainfrom
copilot/fix-nativeaot-config-max-length

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 21, 2026

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.

  • Config length limit update

    • Increased CONFIG_VAL_MAXLEN in src/coreclr/nativeaot/Runtime/RhConfig.h from 16 to 18 to allow 0x/0X + 16 hex digits.
  • Comment accuracy

    • Updated nearby comments in RhConfig.h and RhConfig.cpp to describe max textual integer length semantics, not just raw hex digit count.
  • Behavioral impact

    • Accepted now: 0xFFFFFFFFFFFFFFFF, 0XFFFFFFFFFFFFFFFF
    • Unchanged: inputs longer than prefix + 16 hex digits are still rejected by the existing buffer-length check (cchResult >= cchBuffer).
// RhConfig.h
#define CONFIG_VAL_MAXLEN 18              // "0x" prefix + 64-bit uint in hex

// RhConfig.cpp
TCHAR buffer[CONFIG_VAL_MAXLEN + 1]; // Max textual integer value plus a nul terminator.
Original prompt

Fix NativeAOT's environment config value maximum length after PR #127644 added support for 0x/0X prefixes in hex values.

Context:

  • PR Fix NativeAOT hex config parser to handle 0x/0X prefix #127644 changed RhConfig::Environment::TryGetIntegerValue in src/coreclr/nativeaot/Runtime/RhConfig.cpp to accept and skip a leading 0x/0X prefix when parsing non-decimal values.
  • CONFIG_VAL_MAXLEN is currently defined in src/coreclr/nativeaot/Runtime/RhConfig.h as 16 with the comment //64 bit uint in hex.
  • TryGetIntegerValue uses TCHAR buffer[CONFIG_VAL_MAXLEN + 1] and rejects values where PalGetEnvironmentVariable returns cchResult >= cchBuffer.

Problem:

  • A full-width 64-bit hex value without prefix is 16 characters, e.g. FFFFFFFFFFFFFFFF, and still fits.
  • A full-width 64-bit hex value with prefix is 18 characters, e.g. 0xFFFFFFFFFFFFFFFF.
  • Since prefixed hex values are now accepted syntax, CONFIG_VAL_MAXLEN == 16 incorrectly rejects valid full-width prefixed 64-bit values before parsing.

Task:

  • Update the NativeAOT config parser length limit so values with a 0x/0X prefix plus up to 16 hex digits are accepted.
  • Prefer a clear implementation, such as defining the textual max length as 18 or separating max hex digits from max prefix length if that reads better.
  • Update nearby comments so they accurately describe the allowed input length.
  • Add or update tests if an appropriate NativeAOT config parsing test location exists. At minimum, ensure the change is covered by existing relevant tests or add targeted coverage for 0xFFFFFFFFFFFFFFFF / 0XFFFFFFFFFFFFFFFF if feasible.
  • Keep behavior for overlong values unchanged: values longer than prefix + 16 hex digits should still be rejected/truncated handling should remain safe.

Repository: dotnet/runtime
Related PR: #127644

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

User: is value of CONFIG_VAL_MAXLEN still correct after this PR merged?

Assistant: No — not if 0x/0X is now considered valid syntax for 64-bit hex config values.

CONFIG_VAL_MAXLEN is still:

#define CONFIG_VAL_MAXLEN 16              //64 bit uint in hex

and the buffer is:

TCHAR buffer[CONFIG_VAL_MAXLEN + 1]; // hex digits plus a nul terminator.
const uint32_t cchBuffer = ARRAY_SIZE(buffer);
uint32_t cchResult = PalGetEnvironmentVariable(variableName, buffer, cchBuffer);
if (cchResult == 0 || cchResult >= cchBuffer)
    return false;

After the PR, a value like 0xFFFFFFFFFFFFFFFF is valid syntax conceptually — 16 hex digits for a full uint64_t, plus the 0x prefix — but it is 18 characters long. With CONFIG_VAL_MAXLEN == 16, the buffer only accepts up to 16 characters plus NUL, so this full-width prefixed value will be rejected before parsing.

So the constant should probably become something like:

#define CONFIG_VAL_MAXLEN 18 // "0x" prefix + 64-bit uint in hex

or split into clearer constants, e.g. max hex digits = 16 and max textual length = 18.

The merged PR fixes common values like 0xC0000000, but it leaves this max-length edge case inconsistent with the comment and with the newly accepted prefix syntax.

User: make a PR with the fix

Assistant: I’m waiting for your confirmation to proceed with creating the PR.

This pull request was created from Copilot chat.

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

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/9942cd0a-d0b4-4b91-baba-7ff32bbc30f9

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot May 21, 2026 22:53
Copilot AI changed the title [WIP] Fix NativeAOT's environment config value maximum length NativeAOT: allow full-width 0x/0X-prefixed 64-bit hex env config values May 21, 2026
Copilot AI requested a review from MichalStrehovsky May 21, 2026 22:54
@MichalStrehovsky MichalStrehovsky marked this pull request as ready for review May 22, 2026 00:48
Copilot AI review requested due to automatic review settings May 22, 2026 00:48
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates NativeAOT’s environment-variable integer config parsing to accept full-width 64-bit hex values when using the 0x/0X prefix, by increasing the fixed input buffer length and clarifying related comments.

Changes:

  • Increase CONFIG_VAL_MAXLEN from 16 to 18 to allow "0x" + 16 hex digits.
  • Update the TryGetIntegerValue local buffer comment to describe a textual-length bound rather than “hex digits”.

Reviewed changes

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

File Description
src/coreclr/nativeaot/Runtime/RhConfig.h Increases the max textual length constant for env integer config values to allow 0x/0X + 16 hex digits.
src/coreclr/nativeaot/Runtime/RhConfig.cpp Updates the local buffer comment for env integer parsing to reflect the new textual max length semantics.
Comments suppressed due to low confidence (1)

src/coreclr/nativeaot/Runtime/RhConfig.cpp:55

  • This change adjusts the accepted textual length for environment-provided integer config values, but I couldn’t find any targeted test that covers a full-width prefixed value like 0xFFFFFFFFFFFFFFFF/0XFFFFFFFFFFFFFFFF (and optionally an overlong value to confirm rejection). Adding a small NativeAOT-covered test case would help prevent regressions in this parsing edge case.
    TCHAR buffer[CONFIG_VAL_MAXLEN + 1]; // Max textual integer value plus a nul terminator.
    const uint32_t cchBuffer = ARRAY_SIZE(buffer);
    uint32_t cchResult = PalGetEnvironmentVariable(variableName, buffer, cchBuffer);
    if (cchResult == 0 || cchResult >= cchBuffer)
        return false;

class RhConfig
{
#define CONFIG_VAL_MAXLEN 16 //64 bit uint in hex
#define CONFIG_VAL_MAXLEN 18 // "0x" prefix + 64-bit uint in hex
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants