Skip to content

Emit platform-native line endings in requesting assembly chain#127945

Open
Copilot wants to merge 2 commits intomainfrom
copilot/update-requesting-assembly-chain
Open

Emit platform-native line endings in requesting assembly chain#127945
Copilot wants to merge 2 commits intomainfrom
copilot/update-requesting-assembly-chain

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 8, 2026

  • Update native chain construction in appdomain.cpp to use \r\n on Windows, \n on Unix
  • Remove ReplaceLineEndings() calls from BadImageFormatException.ToString(), FileLoadException.ToString(), and FileNotFoundException.ToString()
  • Rebuild CLR + libs successfully
  • Run existing MissingTransitiveDependency_ShowsRequestingAssemblyChain test - passes (39/39)
  • Run exception-specific tests (BadImageFormatException, FileLoadException, FileNotFoundException) - all pass
  • Run code review and CodeQL validation - passed
Original prompt

In dotnet/runtime, update the implementation from PR #125795 so the requesting assembly chain uses proper platform line endings when it is constructed in native code, instead of normalizing the chain later in managed ToString() overrides.

Context:

  • PR Include requesting assembly chain in assembly load failure exceptions #125795 added _requestingAssemblyChain to BadImageFormatException, FileLoadException, and FileNotFoundException.
  • The native chain builder currently appends transitive entries with a hard-coded LF separator, e.g. in src/coreclr/vm/appdomain.cpp:
    chain.Append(W("\n --> "));
  • The managed exception ToString() overrides compensate by calling _requestingAssemblyChain.ReplaceLineEndings() before formatting SR.IO_FileLoad_RequestedBy in:
    • src/libraries/System.Private.CoreLib/src/System/BadImageFormatException.cs
    • src/libraries/System.Private.CoreLib/src/System/IO/FileLoadException.cs
    • src/libraries/System.Private.CoreLib/src/System/IO/FileNotFoundException.cs

Requested change:

  1. Change the native chain construction to append the correct platform newline (\n on Unix, \r\n on Windows) before -->, using an existing CoreCLR convention/helper/constant if one exists, or a small local/native helper/conditional if not.
  2. Remove the ReplaceLineEndings() calls from the three managed ToString() overrides and format _requestingAssemblyChain directly.
  3. Keep behavior equivalent: exception strings should still use platform-appropriate line endings and include the same requesting assembly chain text.
  4. Add or update tests if practical to cover/validate the line-ending behavior without making tests culture-sensitive. Avoid asserting localized text unless the test explicitly controls UI culture.

Please create a pull request with the fix.

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

User: why do we need to call ReplaceLineEndings in the ToString overrides?

Assistant: We call ReplaceLineEndings() to normalize the embedded newlines inside _requestingAssemblyChain to the current platform’s newline convention before Exception.ToString() emits them.

The requesting chain is built in native code with hard-coded \n separators:

  • first assembly: LibB, Version=...
  • subsequent assemblies appended like: \n --> LibA, Version=...

But the managed ToString() implementations build the rest of the exception text using Environment.NewLineConst. On Windows that is \r\n, while the native chain would otherwise contain lone \n. So without:

_requestingAssemblyChain.ReplaceLineEndings()

the final exception string would mix line endings:

System.IO.FileNotFoundException: ...
\r\nFile name: ...
\r\nRequested by: LibB...
\n --> LibA...
\n --> Root...
\r\n   at ...

With ReplaceLineEndings(), the chain’s internal separators match the rest of ToString() output.

So it’s not needed for the assembly-chain logic itself; it’s for consistent, platform-appropriate exception formatting and avoiding mixed line endings in logs, tests, and string consumers. On Unix it likely makes no observable difference, but on Windows it prevents LF-only continuation lines inside an otherwise CRLF-formatted exception string.

User: can we not use proper line endings in the native code?

Assistant: Yes, that would probably be better: have the native code emit the platform-appropriate newline in the diagnostic string, then the managed ToString() overrides can just format _requestingAssemblyChain directly.

Right now the native code hard-codes "\n --> ":

chain.Append(W("\n --> "));

That’s why the managed side normalizes with:

_requestingAssemblyChain.ReplaceLineEndings()

A cleaner alternative would be to use a native helper/constant for the runtime’s platform newline, e.g. conceptually:

#ifdef TARGET_UNIX
#define CLR_NEWLINE W("\n")
#else
#define CLR_NEWLINE W("\r\n")
#endif

chain.Append(CLR_NEWLINE W(" --> "));

or whatever existing CoreCLR convention/helper is preferred for platform newlines.

Then the managed side can become:

s += Environment.NewLineConst + SR.Format(SR.IO_FileLoad_RequestedBy, _requestingAssemblyChain);

That avoids doing normalization every time ToString() is called and keeps the line-ending decision at the point where the multi-line string is constructed.

The main thing to watch for is not to use Environment.NewLine from native code directly, since this path is intentionally low-level exception construction and should avoid extra managed calls/allocations. But a native compile-time/platform constant would be fine.

User: make a pull request that does this

Assistant: I’m waiting for your response to the confirmation dialog so I can continue creating the pull request.

This pull request was created from Copilot chat.

Copilot AI self-assigned this May 8, 2026
Copilot AI review requested due to automatic review settings May 8, 2026 04:25
Copilot AI review requested due to automatic review settings May 8, 2026 04:25
Copilot stopped work on behalf of MichalStrehovsky due to an error May 8, 2026 04:34
Copilot AI requested a review from MichalStrehovsky May 8, 2026 04:34
…uction and remove managed ReplaceLineEndings() normalization

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/f53f4e08-76fb-4fb1-bfb2-c79be29bf11e

Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot May 8, 2026 05:04
@github-actions github-actions Bot added the area-ExceptionHandling-coreclr only use for closed issues label May 8, 2026
@MichalStrehovsky MichalStrehovsky marked this pull request as ready for review May 8, 2026 07:01
Copilot AI review requested due to automatic review settings May 8, 2026 07:01
@MichalStrehovsky MichalStrehovsky changed the title [WIP] Update implementation for requesting assembly chain line endings Emit platform-native line endings in requesting assembly chain May 8, 2026
@MichalStrehovsky
Copy link
Copy Markdown
Member

Size statistics

Pull request #127945

Project Size before Size after Difference
TodosApi-linux 24491512 24491512 0
TodosApi-windows 25798656 25797632 -1024
avalonia.app-linux 18904048 18904048 0
avalonia.app-windows 19444736 19443200 -1536
hello-linux 1254480 1250384 -4096
hello-minimal-linux 1106904 1106904 0
hello-minimal-windows 790016 779264 -10752
hello-windows 951296 938496 -12800
kestrel-minimal-linux 5273056 5273056 0
kestrel-minimal-windows 4820992 4819968 -1024
reflection-linux 1841160 1841160 0
reflection-windows 1716736 1703424 -13312
webapiaot-linux 9695728 9695728 0
webapiaot-windows 10231808 10230272 -1536
winrt-component-minimal-windows 738304 727040 -11264

@dotnet-policy-service
Copy link
Copy Markdown
Contributor

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

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 makes the requesting-assembly chain string use platform-native newlines at the point it’s constructed in CoreCLR native code, allowing the managed ToString() overrides to emit _requestingAssemblyChain as-is (without later line-ending normalization).

Changes:

  • Updated AppDomain::GetParentAssemblyChain to append \r\n on Windows and \n on Unix before each " --> " continuation.
  • Removed _requestingAssemblyChain.ReplaceLineEndings() from ToString() in BadImageFormatException, FileLoadException, and FileNotFoundException.

Reviewed changes

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

File Description
src/coreclr/vm/appdomain.cpp Emits platform-native newlines when appending " --> " entries to the requesting-assembly chain.
src/libraries/System.Private.CoreLib/src/System/BadImageFormatException.cs Stops normalizing requesting-chain line endings during ToString().
src/libraries/System.Private.CoreLib/src/System/IO/FileLoadException.cs Stops normalizing requesting-chain line endings during ToString().
src/libraries/System.Private.CoreLib/src/System/IO/FileNotFoundException.cs Stops normalizing requesting-chain line endings during ToString().

Comment thread src/coreclr/vm/appdomain.cpp
@MichalStrehovsky
Copy link
Copy Markdown
Member

/ba-g filed #127992 on the failure, can't seem to be able to rerun build analysis to pick it up

@MichalStrehovsky MichalStrehovsky enabled auto-merge (squash) May 9, 2026 12:38
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