-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
.NET 6 - R2R Composite image crashes the app #65879
Comments
@acemod13: Could you share the entire .csproj? Thanks. |
Test.zip |
@FirehawkV21 Are you facing the crash even when you have the --self-contained true? |
How about add the code to your csproj file? <ItemGroup>
<!--
The SDK will precompile the assemblies that are distributed with the application. For self-contained applications, this set of assemblies will include the framework. C++/CLI binaries are not eligible for ReadyToRun compilation.
-->
<PublishReadyToRunExclude Include="DirectWriteForwarder.dll" />
</ItemGroup> |
Sorry, the PublishReadyToRunExclude is not helpful. |
@FirehawkV21 Could you please confirm if the crash is observed even with --self-contained? |
I can confirm that the app still crashes. Even with |
Me too. |
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
Hi @FirehawkV21 does this still repro on .NET 6? |
@mangod9 , It still happens in .NET 6. Tested with SDK 6.0.302. |
@AntonLapounov could you please take a look? |
The issue is caused by incorrect layout of the module's static fields used to initialize
In the generated R2R code for the non-composite mode this list is non-empty — the range
However, in the generated R2R for the composite mode, it becomes empty — the range
@trylek, do you know what part of the compiler may be responsible for that? The crash is caused by |
Thanks Anton for sharing the detailed analysis. The code dealing with static fields is here: Line 53 in e47ffcb
This C# implementation must match the C++ CoreCLR runtime code residing at: runtime/src/coreclr/vm/methodtablebuilder.cpp Line 7733 in e47ffcb
runtime/src/coreclr/vm/methodtablebuilder.cpp Line 7852 in e47ffcb
in combination with runtime/src/coreclr/vm/ceeload.cpp Line 875 in e47ffcb
The process has two phases: first, when a module is loaded, the code in ceeload.cpp does a quick pass over statics and calculates the sizes of the arrays to allocate for this module (DomainLocalModule and ThreadLocalModule); please note that in this phase "other modules" are generally not available so that the method cannot count on exact sizes of all types and such, that's why it does various tricks like boxing structs. In methodtablebuilder the exact static layout for a single class is calculated. |
If I'm right to understand you that the discrepancy is related to the special |
It still happens in .NET 7. Publish Configuration: Details: |
I think I finally understand what's going on; most of it was actually discovered by Anton this summer. The problem is the algorithm for copying RVA fields in Crossgen2. They are basically copied in random order based on when they are encountered in the dependency analysis process, which doesn't work for Managed C++ that used the C++ style of global initializers involving an array of method pointers that are traversed in order by the method Interestingly enough even the Field RVA ECMA metadata table doesn't guarantee the fields to come in RVA order so that, when constructing the At this moment I believe that the correct fix is to introduce a new node type representing the entire file of copied RVA fields for a given module that will be internally used by |
I have realized there were several holes in my above reasoning so I investigated the matter further. The problem is not ordering of the RVA fields as I originally thought, that should be dealt with by means of In light of this fact I believe the problem is basically caused by incomplete design w.r.t. this corner case i.e. it requires additional design work, not a mere hotfix. In fact the only way to hotfix this would be to basically duplicate the RVA field file in the composite image, basically doubling its size in the R2R compilation, and I think that is wasteful and unnecessary. By virtue of the R2R design we always have the primary component assemblies available at runtime even in composite mode so we shouldn't need to copy the RVA fields over to the composite image, we can (and should) access them directly in the primary component assembly metadata blobs. I think this should be dealt with by means of a new RVA field fixup comprising the ECMA module and index of the RVA field in its metadata that would return the address of the field within the component assembly module via indirection into its RVA field metadata table. The JIT interface would then simply state that the address of the RVA field is accessible via this new fixup and it would no longer need to copy RVA fields to the composite image. In non-composite build mode this ends up as a mere optimization where we directly know the RVA of the field based on the (copied) metadata blob. We could theoretically use the same optimization in composite mode but it would be more tricky as it would require the composite compilation to have knowledge of the process of rewriting the individual component assemblies so that it could use the rewritten RVA of the field in question. For now I tend to think that is undesirable as otherwise rewriting of the component assemblies is completely orthogonal to composite compilation and can be potentially parallelized; I guess it is also somewhat questionable why we relocate the RVA fields at all during single-assembly compilation. From a broader perspective I would assume we should strive to mostly keep the original ECMA module intact and that includes the location of RVA fields (that can be potentially hard-coded in some native Managed C++ code although we don't support that scenario just yet). If we fixed single-module compilation to stop tampering with RVA field addresses, the described optimization could be easily implemented in composite mode too. |
We're having this problem too when we try to publish with Single File and R2R enabled together. In our application we have a XAML markup extension that changes the font application wide to Segoe UI (we include Segoe UI's font file inside our application as a resource). On startup our application tries to display its splash screen and the extension gets called to change the font on the splash screen. Before the splash screen can even be displayed, the application now crashes with the stack trace being identical to @EgemenCiftci's report. What is interesting is that the problem doesn't happen with Dot Net 6, but using Dot Net 7 causes the problem to occur. For now we've worked around this by turning off R2R. |
The problem still exist once the target framework is changed from 7.0 to 6.0 then it is working. |
@EgemenCiftci The problem is now resolved with the latest Dotnet 7 update. |
Hmm, it's great to hear if you're unblocked but I'm not aware of any recent changes to Crossgen2 that would contribute to this, I believe that the core underlying problem with RVA fields I described above still exists, I'm now working on fixing it fully in .NET 8. |
Hello @dragnilar! I believe I fixed the underlying issue with the PR that got merged in May i.o.w. it should be included in the latest .NET 8 previews (starting with Preview 5 I believe). It would be great if you or someone on this thread could confirm that the bug is indeed gone in .NET 8 so that we can close this issue. Thanks Tomas |
Closed as presumably fixed, please reopen or create a new issue if you're still hitting this. |
Problem description:
When
<PublishReadyToRunComposite>
is set to true, the WPF app would crash when trying to call something fromSystem.Windows.Media.FontFamily
.Actual behavior:
The app crashes at startup.
Expected behavior:
The app starts normally.
Minimal repro:
<PublishReadyToRunComposite>
and<PublishReadyToRun>
to the csproj file and set it to true.dotnet publish -c Release -r win-x64
(may occur in win-x86, but I haven't tested).The text was updated successfully, but these errors were encountered: