Skip to content

SPMI: Fix invalid format string and allow parsing full uint32 range#125874

Open
jakobbotsch wants to merge 3 commits intodotnet:mainfrom
jakobbotsch:spmi-fix-hex-parse
Open

SPMI: Fix invalid format string and allow parsing full uint32 range#125874
jakobbotsch wants to merge 3 commits intodotnet:mainfrom
jakobbotsch:spmi-fix-hex-parse

Conversation

@jakobbotsch
Copy link
Member

Noticed that e.g. DOTNET_JitHashBreak=9b29601d hits the error path below, which hits an assert due to %ws. Fix the format string, and then also fix the parsing so that it works properly when the upper bit is set.

PTAL @dotnet/jit-contrib

Copilot AI review requested due to automatic review settings March 20, 2026 22:27
@github-actions github-actions bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 20, 2026
@jakobbotsch jakobbotsch requested a review from a team March 20, 2026 22:28
@dotnet-policy-service
Copy link
Contributor

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

Copy link
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 fixes SuperPMI JIT host configuration parsing/logging so hex config values like DOTNET_JitHashBreak=9b29601d no longer hit an assert and can be accepted across the full uint32 range.

Changes:

  • Fix LogWarning format specifiers to correctly print UTF-8 char* strings (%s instead of %ws).
  • Relax hex parsing range check from INT_MAX to UINT_MAX to allow values with the high bit set.

Copilot AI review requested due to automatic review settings March 20, 2026 22:34
Copy link
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

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

Comments suppressed due to low confidence (1)

src/coreclr/tools/superpmi/superpmi/jithost.cpp:128

  • Allowing values up to UINT_MAX means longResult can be > INT_MAX, but the code then does result = static_cast<int>(longResult). Converting an out-of-range unsigned value to int is implementation-defined, and it’s easy to lose the intended 32-bit bit-pattern on non-two’s-complement/odd platforms. If the goal is to accept any 32-bit value (e.g. method hashes with the high bit set), consider parsing into a uint32_t and then copying/bit-casting into the int result with a static_assert(sizeof(int) == sizeof(uint32_t)), so the bit pattern is preserved deterministically.
    unsigned long longResult = strtoul(stringValue, &endPtr, 16);
    bool          succeeded  = (errno != ERANGE) && (endPtr != stringValue) && (longResult <= UINT_MAX);
    if (!succeeded)
    {
        LogWarning("Can't convert int config value from string, key: %s, string value: %s\n", key, stringValue);
        return false;
    }

    result = static_cast<int>(longResult);
    return true;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants