Fix compiler crashes for invalid IL sequences (#81940)
authorMichal Strehovský <MichalStrehovsky@users.noreply.github.com>
Fri, 10 Feb 2023 08:15:47 +0000 (17:15 +0900)
committerGitHub <noreply@github.com>
Fri, 10 Feb 2023 08:15:47 +0000 (17:15 +0900)
The JIT test tree has many tests that test invalid IL. We were crashing on some of them in optimized builds.

The problem is that optimized builds look at the program twice - first time when building whole program view, and second time for codegen. The whole program view building doesn't look at the IL in detail and doesn't need to detect invalid IL like "filter block that doesn't end with endfilter". At the same time, scanning phase must precompute the whole world, including situations when invalid code is going to be replaced by a call to a throw helper.

We were ending up in situations where the throw helper was not predicted and we didn't even know how the vtable of `InvalidProgramException` should look like (because it wasn't scanned). A simple fix is to always generate the helper. It's a bit of garbage 99% of time, but probably fine.

src/coreclr/tools/aot/ILCompiler/ILCompiler.props
src/coreclr/tools/aot/ILCompiler/Program.cs

index da20948..8d27378 100644 (file)
@@ -88,6 +88,7 @@
   <ItemGroup>
     <Compile Include="..\..\Common\CommandLineHelpers.cs" Link="CommandLineHelpers.cs" />
     <Compile Include="..\..\Common\InstructionSetHelpers.cs" Link="InstructionSetHelpers.cs" />
+    <Compile Include="..\..\Common\TypeSystem\IL\HelperExtensions.cs" Link="HelperExtensions.cs" />
   </ItemGroup>
 
   <ItemGroup>
index b82350c..8385514 100644 (file)
@@ -259,6 +259,28 @@ namespace ILCompiler
                     (ModuleDesc module, IRootingServiceProvider rooter) => rooter.AddReflectionRoot(module.GetGlobalModuleType(), "Command line root")));
             }
 
+            // Unless explicitly opted in at the command line, we enable scanner for retail builds by default.
+            // We also don't do this for multifile because scanner doesn't simulate inlining (this would be
+            // fixable by using a CompilationGroup for the scanner that has a bigger worldview, but
+            // let's cross that bridge when we get there).
+            bool useScanner = Get(_command.UseScanner) ||
+                (_command.OptimizationMode != OptimizationMode.None && !multiFile);
+
+            useScanner &= !Get(_command.NoScanner);
+
+            bool resilient = Get(_command.Resilient);
+            if (resilient && useScanner)
+            {
+                // If we're in resilient mode (invalid IL doesn't crash the compiler) and using scanner,
+                // assume invalid code is present. Scanner may not detect all invalid code that RyuJIT detect.
+                // If they disagree, we won't know how the vtable of InvalidProgramException should look like
+                // and that would be a compiler crash.
+                MethodDesc throwInvalidProgramMethod = typeSystemContext.GetHelperEntryPoint("ThrowHelpers", "ThrowInvalidProgramException");
+                compilationRoots.Add(
+                    new GenericRootProvider<MethodDesc>(throwInvalidProgramMethod,
+                    (MethodDesc method, IRootingServiceProvider rooter) => rooter.AddCompilationRoot(method, "Invalid IL insurance")));
+            }
+
             //
             // Compile
             //
@@ -361,15 +383,6 @@ namespace ILCompiler
             InteropStateManager interopStateManager = new InteropStateManager(typeSystemContext.GeneratedAssembly);
             InteropStubManager interopStubManager = new UsageBasedInteropStubManager(interopStateManager, pinvokePolicy, logger);
 
-            // Unless explicitly opted in at the command line, we enable scanner for retail builds by default.
-            // We also don't do this for multifile because scanner doesn't simulate inlining (this would be
-            // fixable by using a CompilationGroup for the scanner that has a bigger worldview, but
-            // let's cross that bridge when we get there).
-            bool useScanner = Get(_command.UseScanner) ||
-                (_command.OptimizationMode != OptimizationMode.None && !multiFile);
-
-            useScanner &= !Get(_command.NoScanner);
-
             // Enable static data preinitialization in optimized builds.
             bool preinitStatics = Get(_command.PreinitStatics) ||
                 (_command.OptimizationMode != OptimizationMode.None && !multiFile);
@@ -486,9 +499,8 @@ namespace ILCompiler
                 .UseOptimizationMode(_command.OptimizationMode)
                 .UseSecurityMitigationOptions(securityMitigationOptions)
                 .UseDebugInfoProvider(debugInfoProvider)
-                .UseDwarf5(Get(_command.UseDwarf5));
-
-            builder.UseResilience(Get(_command.Resilient));
+                .UseDwarf5(Get(_command.UseDwarf5))
+                .UseResilience(resilient);
 
             ICompilation compilation = builder.ToCompilation();