The issue is that the RhpRethrow stubs do not initialize the values of m_kind for the ExInfo objects they allocate on stack. Depending on the kind of garbage that gets assigned to m_kind, the stack iterator either takes the code path of reporting the gcroots of the RhpRethrow callsite, or takes the code path with the RemapHardwareFaultToGCSafePoint (if m_kind ends up getting a HW exception flag).
The piece of code that initializes the m_kind field of the ExInfo object on rethrows is in ExceptionHandling.cs, and there’s a window of opportunity where GC collection can happen before m_kind gets initialized correctly.
The bug needs the following conditions to occur:
1) Garbage value in ExInfo.m_kind before initialization causes the stack iterator to take one code path, then after initialization take the other code path.
2) GC collection happens in the middle, before m_kind gets properly initialized
3) Gc roots reported in each of the 2 different code paths are different
4) The GC collection in step #2 causes a relocation of a reference object of interest.
The fix is to just initialize the field to something deterministic that makes sense (zero in that case). This would cause the stack iterator to use the gcroots reporting of the RhpRethrow callsite, until the field is initialized to a more meaningful value (ex: in case of a rethrow of a HW exception) where we would apply the correct algorithm to determine a more correct gcroots reporting point to use based on where execution is headed (ex: use the gcroots reporting point of a finally block if the rethrow is for a HW exception, and in a catch block)
[tfs-changeset: 1709684]
Use LLVM arguments and returns where possible (no GC references) instead of passing them on the shadow stack. The argument optimization saves 3% optimized (5% compressed). The return change adds about 0.5% to the uncompressed file, but saves about 6% compressed. They both save size in debug. This also greatly simplifies debugging and reading code. Also includes a fix to the class constructor runner where it was calling cctors with the wrong signature and some test fixes.
* Improve StreamWriter format perf
Override the format overloads for TextWriter and skip the extra string allocation for the formatted string by using StringBuilder directly, copying straight to the output buffer.
Improves performance roughtly 10% and cuts allocations by 10x and up (formatting a string into a string goes to *zero* allocations).
* Fix copy/paste slipup- test added to CoreFX PR
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Restore CoreCLR and CoreRun
* To test the ready-to-run binaries produced by ILCompiler, restore Microsoft.NetCore.App, which contains the CoreCLR runtime plus the set of framework assemblies defined by netcoreapp2.1 which should be enough to run most tests.
* Also restore CoreRun.exe, the host used to execute test binaries.
ReadyToRun ETW Test Harness
* Add a test harness to validate ready-to-run binaries properly execute the ready-to-run native code instead of silently falling back to Jit.
* To validate this, The Microsoft.Diagnostics.Tracing (EventTrace) package is used to enable CLR ETW events during test execution. The ModuleLoad event is intercepted to find the module IDs of assemblies relevant to test (recall, all CLR ETW events are machine-wide across all CoreCLR / CLR runtimes currently running). The MethodLoadVerbose events are intercepted and their module ID is matched with those filtered in the ModuleLoad event. This allows us to focus solely on the test binaries and not the dependent framework.
* An optional whitelist file can be provided, whose format is a text file with one method name per line. The output of test failures can be pasted into a whitelist file to create a baseline.
Ready-to-run test system integration
* Add a new code generator type to runtest.cmd supporting readytorun mode.
* Ready-to-run tests need to know the location of the CoreCLR runtime (they're executed with CoreRun) as well as the ETW test harness test project.
* Add a ready-to-run specific set of tests (from Tomas' private bringup branch) which will only run in ready-to-run mode. This is controlled by the presence of a file named "readytorun" in the test source folder. This works the same way as the web assembly tests, where we opt in until the compiler support is mature enough to run all tests in this mode.
* The ready-to-run tests will run as part of the larger suite when runtest.cmd is invoked. To run just ready-to-run tests, use runtest.cmd /mode readytorun.
* `S.R.CompilerServices.Unsafe` is often useful in repro code
* We treat warnings as errors in all of CoreRT, but warnings about unused fields or unused assignments often happen and are not helpful in repro project.
Roslyn decided to do a breaking change to the ECMA-335 file format by generating an illegal custom modifier for the `unmanaged` constraint. The purpose of the modifier is to poison such types for old versions of the C# compiler (the modifier otherwise isn't necessary for the feature to work). This also poisons a lot of the .NET ecosystem too (breaking everything ranging from Mono to the C++/CLI compiler).
We need to update our stack to support this and unblock our customers.
[tfs-changeset: 1709243]
* Handle multidimensional array instantiation (#5421)
newobj instruction on multidimensional array is now processed with
ArrayHelpers.NewObjArray. Enabled instantiating multidimensional arrays.
* Add testing for multidimensional array instantiation.
Only nullity and lengths are checked due to get & set not working on
multidimensional arrays.
* Partial delegate support
I have refactored imports, runtime functions and method entrypoints
for lazy entry registration in accordance with Michal's suggestion.
The change also implements the new delegate ctor helper siganture
encoding. For now I have ignored the runtime lookup part with
a TODO comment.
Thanks
Tomas
* Minor cleanup - moving method marking logic to MethodWithGCInfo
* Addressing PR feedback
1) Per Michal's suggestion, I have created a new property
OffsetOfDelegateFirstTarget with separate definitions in CII.RyuJit
vs. CII.ReadyToRun. Please note this is somewhat different than
TargetABI because TargetABI is a plain constant whereas the offset
is a function of the target pointer size.
2) I have implemented the IEquatable interface on all three helper
structs used for dictionary indexation in R2RCGNF.
3) Per Simon's suggestion I have moved the non-reloc dependency
in DelegateCtorSignature to the new override
ComputeNonRelocationBasedDependencies. I have used the same technique
in MethodWithGCInfo where it substantially simplifed the code.
Thanks
Tomas
* Add EventWrittenEventArgs public properties OSThreadId and TimeStamp and plumb through OSThreadId.
* Plumb ActivityId and RelatedActivityId to EventListener.
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
The API review board didn't approve this to be a public API due to lack of usage data for now, but did provide guidelines to make this approvable in the future.
This is doing two things:
* Rename the attribute
* By default, removed methods are going to throw, with an opt out (that might not be officially available). This is pretty easy to implement for Project N, because DR is intertwined with S.P.CoreLib and we can just define the exception there. It will likely block the adoption of this by IL Linker (or any other tool that is not hardwirded to work against a specific runtime).
[tfs-changeset: 1709140]
* Added version increment for TrimExcess and EnsureCapacity
* Added old unit test to exclusion list
* Excluded missing unit tests
Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
RD.XML root provider was adding ModuleMetadataNodes into the dependency graph even though we were not doing metadata analysis (analysis already happened during the scanning phase). This was crashing the code I added yesterday with an invalid cast because metadata nodes don't expect to be in the graph if we're not analyzing (nobody is going to look at them). The crash only happens one has RD.XML so we didn't catch it in the CI.
* Complete set of my remaining changes resolved against latest
This is the remainder of my R2R changes rebased against the
CorInfoImpl refactorings. I'll try to further rebase the change
against my last pending PR (preparatory R2R changes) once I get
the approval to merge it in.
Thanks
Tomas
* Remove the "GetOrCreate" prefixes per Michal's feedback
* New ReadyToRunCompilationModuleGroup
* Move ReadyToRunMultiFileCompilationModuleGroup to ILCompiler.ReadyToRun project
* Make readytorun compilation module group independent from multi-file
* Fix merge errors
* Remove changes to MultiFileCompilationModuleGroup that were causing errors
* Move ReadyToRunSingleAssemblyCompilationModuleGroup out of multifile block
* Rebasing to latest master R2R branch + rebase fixes
* Addressed Michal's PR feedback, reversal of unwanted files
Now we have a R2R-specific field layout algorithm, we can revert
the changes to the general versions (CompilerMFLA and CompilerTSC).
I have addressed specific Michal's feedback by removing a superfluous
'using' statement.
I have reverted the use of repro/Program.cs as our temporary testbed.
Thanks
Tomas
* Revert all changes to MultiFileCompilationModuleGroup
* Initial support for multi-module compilation
This change basically introduces a new helper struct ModuleToken
encapsulating the combination of an EcmaModule context module
and a token relative to that module. This combo allows for full
token resolution in all cases. Most of the change basically
mechanically replaces mdToken with the new ModuleToken type.
In R2RCGCompilation, we construct CorInfoImpl instances lazily
per EcmaModule; this allows for back-propagation of module info
to R2RCGNF.
* Additional cleanup of my previous multi-module change
I found one more place that needed cleanup to support multi-module
cleanly - construction of SignatureContext. This needs to be done
in ModuleToken rather than in R2RCGNF.
Thanks
Tomas
* Mechanical cleanup - removal of unnecessary 'using' clauses
Thanks to the ModuleToken encapsulation we can remove the
Internal.JitInterface inclusion from many files. I've gone over
all the files using the VS "remove unnecessary usings" refactoring
and in many cases I managed to substantially reduce the using
set.
Thanks
Tomas
* Modify DelayLoadHelperThunk to derive from AssemblyStubNode
At this point the refactoring is somewhat hybrid as the
X64.InstructionEncoder doesn't support all the instructions
and addressing modes I need. Let me know if you want me
to add the currently hardcoded instructions as helpers to
the encoder.
Thanks
Tomas
* Additional cleanup of the DelayLoadHelperThunk assembly stub
Per Michal's feedback I added the PUSH instructions to X64Emitter
and I modified EmitCode to use EmitJMP. I have also split the
implementations per architecture.
Thanks
Tomas
This cleans up the codebase by removing a bunch of workarounds. This was pretty easy after 045a28051c gave us the option to emit different data structures for places that don't support relative pointers and be able to switch dynamically at runtime.
When the option is set, metadata generation at the type level becomes all or nothing - either the entire type gets metadata, or none of it gets it. This makes a certain class of missing metadata issues easier to troubleshoot.
Jan Kotas
Anipik
Fadi Hanna
Morgan Brown
Jeremy Kuhne
Simon Nattress
Simon Nattress
Tanner Gooding
Michal Strehovský
Michal Strehovsky
Brian Robbins
Blealtan
Tomáš Rylek
Dávid Kaya
Ahson Khan