Commit Graph

47 Commits

Author SHA1 Message Date
John Stiles
49b409e680 Simplify for init-stmts at the IR level.
Most backends don't like init-stmts with multiple VarDeclarations inside
them, so we no longer emit IR that does this. This lets us avoid doing
backend-level fixups.

Change-Id: Ide839de18953a73e0f9c7a690df59a7bc3523f89
Bug: skia:11860
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398221
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-04-20 14:38:36 +00:00
Brian Osman
8f1dff6a9e Remove layout(marker) from runtime effect SkSL
This is another strange, experimental feature that clutters the
implementation and isn't used by anyone (to my knowledge).

Change-Id: I538b7eca0cd28aab32f4739b23459731ade9105e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398226
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-04-19 18:48:45 +00:00
Brian Osman
e49703faf2 Remove custom SkVertices data and runtime effect varying support
This was an experimental feature. It worked (but only the GPU backend).
It was never adopted or used by anyone, to my knowledge. It's a large
amount of code, and a strange corner of SkSL for users to stumble into.

Bug: skia:10680
Change-Id: I0dda0364bce7dbffa58c32de4c7801ec2a6bc42e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398222
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-04-19 17:49:25 +00:00
John Stiles
8e2a84be3d Fix for fuzzer-discovered error in SPIR-V compilation.
Unused InterfaceBlocks were not added to the ProgramUsage map. The
ProgramUsageVisitor now makes sure to account for them during its
initial scan.

Change-Id: If3afac8e954c5b685ddc6b63b0f771d8c0b8f207
Bug: oss-fuzz:33405
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398016
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-04-19 15:55:04 +00:00
John Stiles
d02615698e Fix assertion when performing type-coercion with enums.
In http://review.skia.org/393397, I replaced a Constructor::Convert call
with a call directly to ConstructorCompoundCast::Make.

This worked fine if the input expression was actually a compound, but
if it was not, the code would assert/crash. The fuzzer detected this
error right away. (Enums are not considered to be a scalar, a vector or
a matrix in SkSL.)

Change-Id: Ie0df4c5771ff4f4d8f5251d4703e9c3516b6baad
Bug: oss-fuzz:33113
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395720
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-04-12 16:40:31 +00:00
John Stiles
97186dc957 Fix various fuzzer-generated tests.
Two of the tests are now (correctly) detected as invalid code, but were
previously checked into /shared/ because they compiled when the fuzzer
first reported them (that is, after the crash was fixed). These have
been moved into the /errors/ test folder.

One of the tests was only generating an error because its main function
was named `a` instead of `main`, so I renamed it to `main`.

Change-Id: I1a2346fb16e304b0c66ff377a3f9bf7e7ee89ba9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/394899
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-04-12 13:44:59 +00:00
John Stiles
781496ffe5 Add unit test for array of zero or negative size.
Change-Id: Icc710e414388e4026a5e9819a53b8dac8ee0a2d1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/394896
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-04-09 14:38:47 +00:00
John Stiles
d47330feed Add support for struct constructors to SkSL.
This is allowed in OpenGL ES2, and its absence in SkSL has been a pain
point for new users adopting Runtime Effects.

Change-Id: Id2ed78261a2cd2b14b49ad22cb74cdc9e0905f8a
Bug: skia:11368
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/393418
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-04-09 13:54:16 +00:00
John Stiles
0a12b85f16 Add regression test for oss-fuzz finding.
The actual fix happened at prior CL http://review.skia.org/392197, which
reworked how vector-cast constructors function.

Change-Id: Ifb71ec913b349e65d38458dc615441e7a73efddc
Bug: oss-fuzz:32851
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/392841
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-04-05 19:39:16 +00:00
John Stiles
fabed8bb79 Fix fuzzer-discovered error with variable declarations.
As soon as a single VarDeclaration is successfully created, its Variable
is added to the current symbol table. However, if a variable-declaration
line declared several variables in a row, we would stop if ANY of the
declarations contained an error and discard the entire statement, but
would continue processing the rest of the program. This left us in a
position where some Variables existed in the SymbolTable with valid,
reachable names, but their corresponding VarDeclaration statement had
been thrown away as erroneous. Since Variables point back to
VarDeclarations for their initialValues, this gave us a stale pointer.
Any future reference to that variable name which could trigger an
access to its initialValue would read from this dead pointer.

This CL fixes the conversion of VarDeclarations so that we no longer
throw away any VarDeclarations associated with a successfully-parsed
Variable.

Change-Id: If8ec3c160933e48a0e1f36414234b3a849d8978c
Bug: oss-fuzz:32587
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/389636
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-03-29 14:35:09 +00:00
John Stiles
1b843bdcb7 Fix up outdated tests.
Removed error tests which no longer test anything:
- UseWithoutInitialize...: we no longer track variable use-before-init.
- InlineDivideByZero: we no longer constant-fold the result of an
  inlined function call into its parent statement.
- Unreachable: we no longer report unreachable statements.

And fixed some minor test-case issues:
- StaticSwitchConditionalBreak: we still check this, but the function
  was being dead-stripped before this check could run. Renamed to main.
- OssFuzzXxxxx: these cases no longer report errors, but they are still
  valuable as regression tests; moved to `shared/`.

Change-Id: Iade3cff821dc998cacfd02f62d3ac4625e48904c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/387820
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-03-22 21:11:47 +00:00
John Stiles
e67bd1371e Migrate VarDeclaration conversion out of IRGenerator.
Change-Id: If5fb4f99d327bb429f60e8d6c526720dd02b0928
Bug: skia:11342
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386800
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-03-22 14:25:26 +00:00
John Stiles
2810beed23 Improve do-while test in CanExitWithoutReturningValue.
This put the coverage for do-while loops on par with for loops.

Change-Id: I53e0d733edd02a6a139792a8d74c68116453e5ff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385500
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-17 14:50:56 +00:00
Brian Osman
fc4b9919d3 Force global initializers to be constant expressions
Prevents us from accepting code that can't be correctly transformed to
GLSL, like:

  uniform float x;
  float y = x;

(Previously, writing code like that in a runtime effect would
effectively produce the exact same code all the way through to GLSL, and
the driver would fail to compile it).

Bug: skia:11336
Change-Id: Iaa797587c4a4a7289ed59ce2736cf0bf0fc5bca3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384698
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-03-15 16:54:05 +00:00
John Stiles
97d8d7fce0 Remove obsolete TODO.
The referenced bug is closed, and the compiler does correctly emit an
error here.

Change-Id: I824f7819a1e077163f528ac503f3743aab5e58a1
Bug: skia:11322
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384697
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-03-15 14:43:25 +00:00
John Stiles
f3a28db703 Eliminate control-flow analysis.
We no longer derive a performance benefit from this pass in practice,
and it is a very expensive compilation step. It is also prone to fuzz-
related errors.

Doc: http://go/optimization-in-sksl

Change-Id: Ief08ffac659a8fe7fe92c92b9a5da14c9f713bc2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/381261
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-11 13:24:54 +00:00
John Stiles
0dd1a77e12 Add noinline keyword to SkSL.
As you might expect, a function tagged with `noinline` will never be
considered as a candidate for inlining.

Change-Id: Ia098f8974e6de251d78bb2a76cd71db8a86bc19c
Bug: skia:11362
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/382337
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-03-10 15:39:48 +00:00
John Stiles
51d33982ad Add Convert/Make factory functions to IndexExpression.
Change-Id: I7a7874e58bf53978afce8a41b26092406b6490ed
Bug: skia:11342
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/380360
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-08 15:55:59 +00:00
John Stiles
b3dcbb12ef Detect functions that fail to return a value, without using CFG.
This check now runs at function finalization time, before constant
propagation has occurred; this affected the "DeadIfStatement" test.

Our detection isn't smart enough to realize that a loop will run zero
times, so it treats `for` and `while` loops as always running at least
once. This isn't strictly correct, but it actually mirrors how the CFG
implementation works anyway. The only downside is that we would not flag
code like `for (i=0; i<0; ++i) { return x; }` as an error.

Change-Id: I5e43a6ee3a3993045559f0fb0646d36112543a94
Bug: skia:11377
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379056
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-03-04 22:47:05 +00:00
John Stiles
2b6ec98a82 Disallow unscoped for blocks which declare a variable.
This should be legal, and we support this, but some versions of Android
do not: http://screen/3bkQewHF3xUMn5v There's no point in allowing
these shaders to exist; they can't compile on real-world clients, and
these vardecls are borderline meaningless (as the variables being
declared aren't reachable by any other statements).

Change-Id: Ie1351933c90caee9124eeab8983364ec030b2653
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379584
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-04 17:04:40 +00:00
John Stiles
2dda50dd50 Add return-value check to the function finalizer.
Rather than have the inliner own this responsibility, the function
finalizer now detects if a function is supposed to return a value but
never actually does. This will allow us to detect this error case even
if the inliner is disabled. The inliner should no longer encounter
functions that claim to return a value but don't, so it will now assert
if one is encountered. (The inliner still has the logic to handle this
case gracefully, just in case.)

The check is currently very simple and doesn't analyze the structure of
the function, so it won't report cases where some paths return a value
and others don't, e.g. this will pass the test:

    int func() { if (something()) return 123; }

(This is good enough to resolve the inliner issue, though, as it only
occurred in functions with no value-returns at all.)

Change-Id: I21f13daffe66c8f2e72932b320ee268ba9207bfa
Bug: oss-fuzz:31469, oss-fuzz:31525, skia:11377
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/377196
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-03 16:29:22 +00:00
Brian Osman
dc4f8cd7ea Add early_fragment_tests layout qualifier to SkSL
Bug: skia:11356
Change-Id: I16322e6396dc7e7c8c50ba1d39e07311cf3bd346
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376116
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-03-02 16:45:03 +00:00
John Stiles
0c2d14a1b9 Fix fuzzer-discovered error with inlining.
In http://review.skia.org/375776, an optimization was added to the
Inliner, causing it to skip generation of unnecessary temporary
variables. The fuzzer immediately discovered a flaw in this logic: the
"unnecessary" variable was actually used in the rare case that a
function failed to actually return a value. The inliner didn't detect
this case. Of course, this isn't a valid program either, so now we
report the error and cleanly fail.

Change-Id: I1f201cfd33f45cace3be93765a4e214e43a46e69
Bug: oss-fuzz:31469, oss-fuzz:31525
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/377101
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-03-01 20:06:50 +00:00
Brian Osman
a654faaf7f Add permittedLayoutFlags to checkModifiers
For now, just use this to prevent *any* layout qualifiers from appearing
on functions, or their parameters.

Bug: skia:11301
Change-Id: I05d8118c7121048c6ef49695a54e3714a8f8687e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376796
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-02-26 18:38:31 +00:00
Brian Osman
7b361499c9 Align SkSL const rules more closely with GLSL
This adds Analysis::IsConstantExpression, to determine if an expression
is a constant-expression. It now expands to cover 'const' local and
global variables, because we also enforce that the initializer on those
variables is - in turn - a constant expression.

This fixes 10837 - previously you could initialize a const variable with
a non-constant expression, and we'd emit GLSL that contained that same
pattern, which would fail to compile at the driver level. That should
not be possible any longer.

Bug: skia:10679
Bug: skia:10837
Change-Id: I517820ef4da57fff45768c0b04c55aebc18d3272
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375856
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-02-26 17:44:11 +00:00
John Stiles
bb8542f086 Fix fuzzer-discovered error with ~ prefix on literals.
The expression `~123` was making a PrefixExpression of type $intLiteral.
It should be converted to type `int` when the ~ prefix is applied.

This change also changes the output from oss-fuzz:27614. Both programs
are essentially nonsense expressions with no real behavior, so this is
fine.

Change-Id: I586be149ce95136fabee72fdd3473814d54948cf
Bug: oss-fuzz:31410
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376620
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-02-26 17:30:26 +00:00
Brian Osman
a77ed8b382 Add flags for all layout qualifiers, check for duplicates
Now, even if a qualifier has a default value, we will know that it
appeared in the text. We can use that to check for redundant qualifiers
(as is being done here), and in the IR generator to prevent any use of
certain qualifiers, depending on context. (eg, runtime effects, wrong
shader stage, on a parameter declaration, etc.)

Bug: skia:11301
Change-Id: I2cd6ad35c2b4c4d6f87ade97e80aea84dc16ee4b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/374616
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-02-24 16:22:26 +00:00
Brian Osman
4a015c5092 Require initializers on const variable declarations
Bug: skia:10837
Change-Id: I33da2eb1e723ed04ab62d65c21e54306dd362bed
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372677
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-02-23 20:55:01 +00:00
John Stiles
2b3d1f2086 Remove constant propagation from static-switch test.
Constant propagation might be going away, but static-switches are likely
here to stay. Avoid conflating the two in this test.

Change-Id: If4b6c99c85f124d3bbc20da858693f09f5e4fd59
Bug: skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/374117
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-02-23 15:39:05 +00:00
John Stiles
04ca41acf3 Fix switch optimization pass.
The optimizer now properly recognizes all types of exits from a switch
statement. Break, continue and return are all potential exits and need
to be considered when determining the exit path from the switch.

Previously, dead code elimination was hiding the effects of this bug
from us, but it meant that an optimized switch had the potential to
generate lots of worthless IR nodes which then needed to be detected and
eliminated by the CFG. In particular, this affected the enum form of
blend, causing a catastrophic amount of extra work to be done.

Change-Id: If857e38cadfc016884624ea4db25a273ad3dce5b
Bug: skia:11352
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372958
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-02-23 15:38:24 +00:00
Brian Osman
ffee4766fa SkSL: Allow 'const' on function parameters
Change-Id: I672345116e3b5538c0f7e8c5f2f74aa56bb81e6d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372676
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-02-22 21:40:26 +00:00
John Stiles
463fad21ba Add test for duplicate default switch-case.
Surprisingly, this error is actually caught by our parser, which
interprets the default label in a unique way. From the parser comments:

  "Requiring default: to be last (in defiance of C and GLSL) was a
   deliberate decision. Other parts of the compiler may rely upon this
   assumption."

The comment is true--we don't check for duplicate default switch-case
labels anywhere else in the code, just here in the parser.

We rely on this, so we should have a test for it.

Change-Id: I6df5c565aca4d4b8565b96638dce9504efc39ccc
Bug: skia:11340
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372617
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-02-19 20:31:50 +00:00
Brian Osman
58ee89879f Remove "buffer" modifier from SkSL
Change-Id: I4df18946cdb3d9f1f7833461f913f2df94696821
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372197
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-02-18 21:48:05 +00:00
Brian Osman
9ca38ff4b4 Remove GLSL memory qualifiers from SkSL
These were all unused, and only implemented on one backend.

Change-Id: Ibd2fcef1a971e6c1bd9da0784c5d852a60708484
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372117
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-02-18 19:57:05 +00:00
Brian Osman
9cb3f98191 Remove PLS support from SkSL
Change-Id: I885149c73be63c223ac88a697ffe046a7f8384d0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372116
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-02-18 19:16:45 +00:00
Brian Osman
2fe67f9680 Disallow initializers on uniform variables
Bug: skia:11335
Change-Id: I88c952cbfe2d2c5920e17675da1674928f37b982
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371480
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-02-18 01:01:38 +00:00
John Stiles
973009b04b Clean up type coercion code and IRGenerator::checkValid().
When coercing a type, we would previously call checkValid() so we could
detect function-references and type-references, so we could get a nicer
error message.

It turns out that we can just do the "is this a type-reference/
function-reference?" check directly inside coerce() and get the same
improved error messages. Since we should be coercing all our values to
the right type, and type/function-references aren't coercible to
anything, this should catch them all. I don't expect any of these
to survive all the way to the end of IR generation.

(In case one of these types does slip through, I've left the error case
in checkValid, but I've also put in an assertion. If the fuzzer can
make that assertion fire, we are probably missing a call to coerce()
somewhere.)

This cleanup is meant to help migrate coerce() out of IRGenerator.

Change-Id: I031809adf439b1766048768b782c57e7f2494006
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371479
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-02-17 22:39:48 +00:00
John Stiles
80b02af6ba Fix assertion when arrays are double-declared.
Multi-dimensional arrays aren't legal in GLSL/SkSL, so this should be
caught and flagged as an error. The parser now verifies that a
variable's type isn't an array-type before accepting a `[` token to
open an array on the variable name.

This CL also refactors the IR generator's `convertArraySize` method to
make sure that various checks are made for all callers. Originally this
restructuring was used to verify array multi-dimensionality, but that
didn't detect errors inside struct declarations (which get no error
checking inside the IR generator) so the IR generator updates no longer
need to check the array dimensions.

Bug: skia:11322
Change-Id: Id33f4bdfb544019ddf995a8196c3c09cfe5a4525
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/369916
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-02-12 23:49:04 +00:00
John Stiles
8dabeac58c Improve error reporting for invalid var-decls.
We now interpret any statement of the form `Type identifier...` as a
var-declaration and report errors as such. Previously, if a var-decl
statement generated an error during parse, we'd report errors as if it
were an expression-statement, which meant that slightly-invalid code
could return out-of-context, misleading errors.

Bug: skia:11287
Change-Id: I2c6cf2984760eb34593c80cb30f8c4e007d42027
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370036
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-02-12 22:26:36 +00:00
Brian Osman
6c3b23fcf3 SkSL: Ensure struct declarations contain at least one member
Bug: skia:11314
Change-Id: I66476543462ae378a5bfb6cbd902dfa2f5fc45f5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/369917
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-02-12 20:57:56 +00:00
John Stiles
bb1505f2ab Move static-switch and static-if tests out of scanCFG.
We don't need to do these tests every time we run CFG optimization; we
can do them once at the end of optimization, as a separate step.

Change-Id: If0e72fbacb938b62387fd2ffdbf34d1153bf3bd4
Bug: skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/369481
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-02-12 15:32:32 +00:00
Ethan Nicholas
c0f9815609 Added detection for non-constant-folded divide by zero errors in
SkSL.

It would previously catch 1 / 0, but fail to detect x / 0.

Bug: skia:11051
Change-Id: I3adb5942cce03a7ad40a13a8ca5d5a7f2029d6ad
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366720
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
2021-02-06 03:13:16 +00:00
John Stiles
248f57b39b Reject array constructors with the wrong number of elements.
We now catch this error at IR generation time; previously we'd send it
to the driver (where it would fail to compile).

Change-Id: I45890214ffa164be1c0f359320f942bc4dc479ca
Bug: skia:11265
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/365697
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-02-03 22:33:02 +00:00
Brian Osman
00fea5b41f Remove ASTNode::TypeData
Pre-cleanup as I start looking at how structs are parsed and handled in
the IR.

Bug: skia:11228
Change-Id: I6334d1073211cbbdf69ddffa8df420c45fd59fcc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/361059
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-01-28 16:56:59 +00:00
John Stiles
e3a91cf31c Fix fuzzer-discovered error with variable scoping.
In GLSL and SkSL, control statements don't require explicit braces
around single-statement children. That is, the `match = true` child
statement here doesn't need to be braced.

    if (condition) match = true;

Because there are no braces, we never create a Block or a dedicated
SymbolTable here.  This is normally not a problem, but the fuzzer
discovered that it can dump things into the symbol table inside a child
statement:

    if (condition) int newSymbol;

This becomes problematic because the symbol name now outlives its block.
This means `newSymbol` can be referred to later, which should be illegal
(and can cause the optimizer to blow up since the structure is bogus).

There doesn't seem to be any reason to allow this code to compile; the
user can add an explicit scope here to make it reasonable, and it's
(almost) meaningless to declare a symbol that's instantly going to fall
out of scope. This code is now rejected with an error message.

Change-Id: I44778e5b59652d345b10eecd4c88efbf7d86a5e0
Bug: oss-fuzz:29849
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358960
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-26 17:08:59 +00:00
John Stiles
76389b7d24 Fix fuzzer-discovered errors in SkSL parser.
The previous change caused varDeclarations() to sometimes return an
expression-statement. This only made sense in the context of being
called from Parser::statement(). Other places which called
varDeclarations() expect vardecls and nothing else.

Change-Id: I562657cadfa20dcd77b527f2dc43dca0c6bf389f
Bug: oss-fuzz:29845
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/358528
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-01-26 00:47:13 +00:00
John Stiles
bfc9be0f77 Migrate SkSL test inputs to the resources/ directory.
This will allow us to load these inputs for unit testing in `dm`.

Change-Id: Id256ba7c30d3ec94b98048e47af44cf9efe580d5
Bug: skia:11009
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/357282
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-01-22 18:57:29 +00:00