Stop the inliner after it has inlined 2500 statements in a program.

johnstiles-google · Skia Commit-Bot · commit 031a76756e24 · 2020-11-13T23:02:11.000Z
This prevents OOMing when given a pathological input, but is large enough that almost all inputs should continue to compile as-is. Change-Id: If5c46711b886ee08495bfd09af537e9dc7ea5649 Bug: skia:10945, oss-fuzz:27442 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334838 Commit-Queue: John Stiles <johnstiles@google.com> Reviewed-by: Brian Osman <brianosman@google.com> Auto-Submit: John Stiles <johnstiles@google.com>
diff --git a/gn/sksl_tests.gni b/gn/sksl_tests.gni
@@ -257,7 +257,6 @@ sksl_shared_tests = [
 sksl_inliner_tests = [
   "$_tests/sksl/inliner/DoWhileBodyMustBeInlinedIntoAScope.sksl",
   "$_tests/sksl/inliner/DoWhileTestCannotBeInlined.sksl",
-  "$_tests/sksl/inliner/ExponentialGrowth.sksl",
   "$_tests/sksl/inliner/ForBodyMustBeInlinedIntoAScope.sksl",
   "$_tests/sksl/inliner/ForInitializerExpressionsCanBeInlined.sksl",
   "$_tests/sksl/inliner/ForWithReturnInsideCannotBeInlined.sksl",
@@ -323,6 +322,7 @@ sksl_blend_tests = [
 
 sksl_settings_tests = [
   "$_tests/sksl/glsl/TypePrecision.sksl",
+  "$_tests/sksl/inliner/ExponentialGrowth.sksl",
   "$_tests/sksl/inliner/InlinerCanBeDisabled.sksl",
   "$_tests/sksl/inliner/InlinerWrapsEarlyReturnsWithDoWhileBlock.sksl",
   "$_tests/sksl/shared/CrossIntrinsic.sksl",
diff --git a/src/sksl/SkSLInliner.cpp b/src/sksl/SkSLInliner.cpp
@@ -56,6 +56,8 @@
 namespace SkSL {
 namespace {
 
+static constexpr int kInlinedStatementLimit = 2500;
+
 static bool contains_returns_above_limit(const FunctionDefinition& funcDef, int limit) {
     class CountReturnsWithLimit : public ProgramVisitor {
     public:
@@ -310,6 +312,7 @@ void Inliner::reset(const Context* context, ModifiersPool* modifiers,
     fSettings = settings;
     fCaps = caps;
     fInlineVarCounter = 0;
+    fInlinedStatementCounter = 0;
 }
 
 String Inliner::uniqueNameForInlineVar(const String& baseName, SymbolTable* symbolTable) {
@@ -465,6 +468,9 @@ std::unique_ptr<Statement> Inliner::inlineStatement(int offset,
         }
         return nullptr;
     };
+
+    ++fInlinedStatementCounter;
+
     switch (statement.kind()) {
         case Statement::Kind::kBlock: {
             const Block& b = statement.as<Block>();
@@ -760,6 +766,11 @@ bool Inliner::isSafeToInline(const FunctionDefinition* functionDef) {
         return false;
     }
 
+    // Enforce a limit on inlining to avoid pathological cases. (inliner/ExponentialGrowth.sksl)
+    if (fInlinedStatementCounter >= kInlinedStatementLimit) {
+        return false;
+    }
+
     if (functionDef == nullptr) {
         // Can't inline something if we don't actually have its definition.
         return false;
@@ -1148,6 +1159,11 @@ bool Inliner::analyze(Program& program) {
         return false;
     }
 
+    // Enforce a limit on inlining to avoid pathological cases. (inliner/ExponentialGrowth.sksl)
+    if (fInlinedStatementCounter >= kInlinedStatementLimit) {
+        return false;
+    }
+
     ProgramUsage* usage = program.fUsage.get();
     InlineCandidateList candidateList;
     this->buildCandidateList(program, &candidateList);
@@ -1200,6 +1216,11 @@ bool Inliner::analyze(Program& program) {
         *candidate.fCandidateExpr = std::move(inlinedCall.fReplacementExpr);
         madeChanges = true;
 
+        // Stop inlining if we've reached our hard cap on new statements.
+        if (fInlinedStatementCounter >= kInlinedStatementLimit) {
+            break;
+        }
+
         // Note that nothing was destroyed except for the FunctionCall. All other nodes should
         // remain valid.
     }
diff --git a/src/sksl/SkSLInliner.h b/src/sksl/SkSLInliner.h
@@ -95,6 +95,7 @@ class Inliner {
     const Program::Settings* fSettings = nullptr;
     const ShaderCapsClass* fCaps = nullptr;
     int fInlineVarCounter = 0;
+    int fInlinedStatementCounter = 0;
 };
 
 }  // namespace SkSL
diff --git a/src/sksl/SkSLMain.cpp b/src/sksl/SkSLMain.cpp
@@ -18,6 +18,7 @@
 #include "src/sksl/ir/SkSLUnresolvedFunction.h"
 
 #include <fstream>
+#include <limits.h>
 #include <stdarg.h>
 #include <stdio.h>
 
@@ -183,6 +184,9 @@ static bool detect_shader_settings(const SkSL::String& text,
                 if (settingsText.consumeSuffix(" NoInline")) {
                     settings->fInlineThreshold = 0;
                 }
+                if (settingsText.consumeSuffix(" InlineThresholdMax")) {
+                    settings->fInlineThreshold = INT_MAX;
+                }
                 if (settingsText.consumeSuffix(" Sharpen")) {
                     settings->fSharpenTextures = true;
                 }
diff --git a/tests/sksl/inliner/ExponentialGrowth.sksl b/tests/sksl/inliner/ExponentialGrowth.sksl
@@ -1,3 +1,5 @@
+/*#pragma settings InlineThresholdMax*/
+
 void fn1()  { sk_FragColor.x = 0; }
 void fn2()  { fn1(); fn1(); fn1(); }
 void fn3()  { fn2(); fn2(); fn2(); }
diff --git a/tests/sksl/inliner/golden/ExponentialGrowth.glsl b/tests/sksl/inliner/golden/ExponentialGrowth.glsl
diff --git a/tests/sksl/inliner/golden/ExponentialGrowthStandaloneSettings.glsl b/tests/sksl/inliner/golden/ExponentialGrowthStandaloneSettings.glsl

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+/#pragma settings InlineThresholdMax/`
	`2`	`+`
`1`	`3`	`void fn1() { sk_FragColor.x = 0; }`
`2`	`4`	`void fn2() { fn1(); fn1(); fn1(); }`
`3`	`5`	`void fn3() { fn2(); fn2(); fn2(); }`