Commit Graph

207 Commits

Author SHA1 Message Date
John Stiles
ff5bb37b72 Replace illegal DSLTypes with Poison types.
The fuzzer has been poking various holes in DSL by intentionally
creating illegal types (e.g. private or not ES2-compatible), then
finding ways to use those types, e.g. constructors or swizzles.

Previously we were mitigating those by calling `reportIllegalTypes` at
the locations where the type was used. Now, we detect the illegal type
usage at the source, and return a poison DSLType. This prevents the
illegal type from leaking out at all, and stops the problem at its
source. It also allows us to remove calls to `reportIllegalTypes`
sprinkled through the code, as those are now redundant.

Change-Id: Id50b50f72849111d80f76e4fdc2cb6094d3009bd
Bug: oss-fuzz:39597
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/455999
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-10-06 00:12:07 +00:00
John Stiles
0dd68c3b29 Add tests for constructors of private types.
This turns out to work fine, but we didn't cover it in any test case.

Change-Id: I98c40dc023bc9f0739beeb6e4163cde087a0be99
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/455499
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-10-04 16:38:19 +00:00
Ethan Nicholas
de42a9d2fc Fixed unsupported type errors in pure DSL
Change-Id: I5a57e2db46734ca08825e6aef7a6363bcaada45a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/453759
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-10-04 14:32:19 +00:00
John Stiles
7e947ab4b6 Reland "Mark GLSL reserved names as reserved in SkSL grammar."
This reverts commit 5f15c695f9.

Reason for revert: landed http://ag/15959743 to fix Android roll

Original change's description:
> Revert "Mark GLSL reserved names as reserved in SkSL grammar."
>
> This reverts commit 57f3fc4cde.
>
> Reason for revert: breaking Android roll
>
> Original change's description:
> > Mark GLSL reserved names as reserved in SkSL grammar.
> >
> > We now reject every reserved name in the ES2 docs as an unexpected
> > token, except for the rule that all names beginning with `gl_` are
> > reserved. (Unfortunately, sksl_frag bends the rules by directly
> > declaring a builtin variable named `gl_SecondaryFragColorEXT`.)
> >
> > Change-Id: I5dcb40b754720ca97fe3d80e2f9072beaa39fcdb
> > Bug: skia:11115
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/454737
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> > Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
>
> Bug: skia:11115
> Change-Id: Ica56f48dc76ef1e52780acaf59b8ad9143637637
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/454860
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>

Bug: skia:11115
Change-Id: I012b8d4e03be7f9c888c26d912552412529b4fb6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/455159
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-10-01 18:31:50 +00:00
Ethan Nicholas
5f15c695f9 Revert "Mark GLSL reserved names as reserved in SkSL grammar."
This reverts commit 57f3fc4cde.

Reason for revert: breaking Android roll

Original change's description:
> Mark GLSL reserved names as reserved in SkSL grammar.
>
> We now reject every reserved name in the ES2 docs as an unexpected
> token, except for the rule that all names beginning with `gl_` are
> reserved. (Unfortunately, sksl_frag bends the rules by directly
> declaring a builtin variable named `gl_SecondaryFragColorEXT`.)
>
> Change-Id: I5dcb40b754720ca97fe3d80e2f9072beaa39fcdb
> Bug: skia:11115
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/454737
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>

Bug: skia:11115
Change-Id: Ica56f48dc76ef1e52780acaf59b8ad9143637637
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/454860
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2021-10-01 14:53:12 +00:00
John Stiles
57f3fc4cde Mark GLSL reserved names as reserved in SkSL grammar.
We now reject every reserved name in the ES2 docs as an unexpected
token, except for the rule that all names beginning with `gl_` are
reserved. (Unfortunately, sksl_frag bends the rules by directly
declaring a builtin variable named `gl_SecondaryFragColorEXT`.)

Change-Id: I5dcb40b754720ca97fe3d80e2f9072beaa39fcdb
Bug: skia:11115
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/454737
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-09-30 19:55:47 +00:00
John Stiles
b05bbd03f9 Fix for fuzzer-discovered error with invalid var-initializers.
If a VarDeclaration line contained multiple variables, and the first
variable had an illegal initializer-expression, the Declare() would
return a Nop. AddVarDeclaration did not expect to see a Nop and would
assert once we tried to process the second var-declaration. Now, we
allow adding var declarations to a Nop.

Bulked up some tests to cover local and global variables (since those
are parsed in separate functions) and to check both the first
initializer as well as follow-on initializers (since those are parsed in
separate parts of the var-decl handler).

Change-Id: I66341191698175b490a659715cb8edaafe2f75ae
Bug: oss-fuzz:39032
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452696
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-09-24 20:15:31 +00:00
John Stiles
1c5eb4b371 Disallow continue inside a switch.
This fails on several platforms in practice, and is of very limited
real-world utility.

Change-Id: Ib476396fc33cb51af6bbcf7fe822d30703ed995d
Bug: skia:12467
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450993
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-09-21 19:26:40 +00:00
Ethan Nicholas
2816dcfc67 Fixed fuzzer-discovered bug with interface blocks
While I was in this code, I realized that the setVariable method of
InterfaceBlock was unused and there was therefore no reason to be
storing a pointer instead of a reference.

Bug: oss-fuzz:39000
Change-Id: If7505ba87f4060370cfd32ca2e30c76648965101
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450446
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-09-21 14:12:47 +00:00
Ethan Nicholas
f62934b85a Fixed DSLParser error uncovered by ossfuzz
Missed one more case of Optional<Wrapper<Expression>>. This should be
the last one.

Bug: oss-fuzz:38944
Change-Id: Ic7f790cd99e2a3ee1c3874cc767a4702265d1723
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450476
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2021-09-20 17:57:14 +00:00
Ethan Nicholas
c973d26854 Fixed DSLParser assertion error uncovered by fuzzer
Bug: oss-fuzz:38108
Change-Id: I0e055d837923f00b982bc395dbf29b6ff59a3b21
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/448896
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-09-20 13:48:41 +00:00
Ethan Nicholas
0dc1e0f797 Fixed DSLParser unary expression error handling
Missed a case when eliminating optional/wrapper in an earlier CL.

Change-Id: If7f80ea6e2172acadf7b0087fe1a05853ccae445
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449838
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-09-17 17:55:13 +00:00
John Stiles
d0665d9a04 Fix fuzzer-discovered null deref with empty interface blocks.
Change-Id: I95bfa33f674bce0098cc6d4469c8c85ee8b051f5
Fixes: oss-fuzz:38875
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449817
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-09-17 14:19:14 +00:00
John Stiles
eb68973c2f Disallow matrix ctors which overflow a column.
The GLSL spec allows matrix constructors containing vectors that would
split between multiple columns of the matrix. However, in practice, this
does not actually work well on a lot of GPUs!

- "cast not allowed", "internal error":
	Tegra 3
	Quadro P400
	GTX 660
	GTX 960
- Compiles, but generates wrong result:
	RadeonR9M470X
	RadeonHD7770

Since this isn't a pattern we expect to see in user code, we now report
it as an error at compile time. mat2(vec4) is treated as an exceptional
case and still allowed.

Change-Id: Id6925984a2d1ec948aec4defcc790a197a96cf86
Bug: skia:12443
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449518
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-09-16 22:14:49 +00:00
Ethan Nicholas
2280058446 Reenable DSLParser
Change-Id: I1819b2c40902611d7e86245bff73ad8c2bd7629c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449060
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-09-16 15:28:19 +00:00
Brian Osman
a81e7e2e71 Report an error if sk_LastFragColor is referenced without fbFetchSupport
Adjusted default caps in skslc to be consistent with runtime behavior,
and added optional settings mode to enable the feature. Tests for both
scenarios. (The error test crashed prior to the fix).

Bug: oss-fuzz:38726
Change-Id: I5270d4837ac982085d7baf5abd4b361f7bfb8562
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449062
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-09-15 15:41:49 +00:00
Ethan Nicholas
fd1332f5a7 Reland "Fixed DSL assertion error on source files containing nulls"
This is a reland of db38ad7b14

Original change's description:
> Fixed DSL assertion error on source files containing nulls
>
> The assertion was there to make sure we weren't running off the end of
> the source, but naturally fails in the presence of legitimate embedded
> nulls.
>
> Change-Id: I3b80499e9b182c9ea046c479f35d7a965d548401
> Bug: oss-fuzz:38107
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/447182
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>

Bug: oss-fuzz:38107
Change-Id: Idb1a6b7c64d2bb954edadae828d6de808158fd3f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/448660
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2021-09-15 15:00:23 +00:00
John Stiles
7591d4b5ef Replace trio of XxxxLiteral types with a single Literal type.
Throughout SkSL we've begun using doubles as a convenient way to store
any SkSL value (int, float, bool) in a single type. This idea has now
been extended to literals. Rather than having three expression kinds for
integers, floats and boolean literals, we can have just one. These can
be accessed in a type-specific way (`floatValue`, `intValue`, and
`boolValue` return the expected type, or assert if it's not the
matching type), or in a type-agnostic way (`value` will return a double
and works on any type of Literal).

This allows us to remove a complex template trick (Literal<T> is gone),
removes two redundant Expression types, and and lets us reduce our code
size in ConstantFolder, FunctionCall, etc.

Most of the conversion process was pretty straightforward:
* `IntLiteral::Make` becomes `Literal::MakeInt`
* `x.is<IntLiteral>()` becomes `x.isIntLiteral()`
* `x.as<IntLiteral>.value()` becomes `x.as<Literal>.intValue()`

Change-Id: Ic328533611e4551669c7fc9d7f9c03e34699f3f6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/447836
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-09-13 17:32:34 +00:00
John Stiles
dc652564a7 Fix for-statement assertion detected by the fuzzer.
The fuzzer found that the `DetectVarDeclarationWithoutScope` check was
placed too late in the function, and could be skipped over by for-loops
containing multiple variables. This was caught in ForStatement::Make,
which mirrors the Convert postconditions with matching assertions.

Change-Id: I6e9d97c7c9ca969aba65e601bbcd9fe676105838
Bug: oss-fuzz:38560
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/448116
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-09-13 13:34:48 +00:00
Michael Ludwig
47d824415e Revert "Fixed DSL assertion error on source files containing nulls"
This reverts commit db38ad7b14.

Reason for revert: breaking g3 roll since it thinks the test case is "binary" not flagged as binary

Original change's description:
> Fixed DSL assertion error on source files containing nulls
>
> The assertion was there to make sure we weren't running off the end of
> the source, but naturally fails in the presence of legitimate embedded
> nulls.
>
> Change-Id: I3b80499e9b182c9ea046c479f35d7a965d548401
> Bug: oss-fuzz:38107
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/447182
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>

Bug: oss-fuzz:38107
Change-Id: I650d12d728b5d932bda79e81205b873d8b44771f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/447936
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
2021-09-11 18:13:13 +00:00
Ethan Nicholas
db38ad7b14 Fixed DSL assertion error on source files containing nulls
The assertion was there to make sure we weren't running off the end of
the source, but naturally fails in the presence of legitimate embedded
nulls.

Change-Id: I3b80499e9b182c9ea046c479f35d7a965d548401
Bug: oss-fuzz:38107
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/447182
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-09-10 18:44:22 +00:00
Ethan Nicholas
c9d65f0b8a Fixed duplicate function definition error discovered by fuzzer
Bug: oss-fuzz:38140
Change-Id: I76a1b3ef8289b3089192d043d173677c00741a54
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445836
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-09-10 17:35:52 +00:00
Ethan Nicholas
bf4a7d5235 Fixed DSLParser crash with invalid swizzle
In response to a non-identifier token after a dot, DSLParser would
attempt to swizzle a zero-length field and fail an assertion.

The same basic code path exists in the old compiler, but the resulting
parse error causes the process to abort before it attempts to process
the zero-length swizzle.

Bug: oss-fuzz:38106
Change-Id: Ifd997ce1d564b5f6ef0a9a785d8d9e254785e600
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446185
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-09-10 14:29:22 +00:00
John Stiles
ff6c5bf5ed Generate more diagnostics from IRGenerator, when given bad code.
Two minor changes:
- Converting a Block with bad statements will now generate a partial
  block instead of nullptr.

The change mirrors how DSL behaves; functions containing invalid
statements will now be created and added to the program. Previously, we
would discard a function definition with any invalid statements inside;
this prevented duplicate-function-definition errors from appearing.

- Converting a return with a bad expression will now generate a
  poisoned return instead of nullptr.

This change improves diagnostics for functions with invalid return
statements. If we eliminate the return statements (by returning null),
we report bad return statements as "function can exit without returning
a value" (which is confusing).

Change-Id: I6d998d5c50585f8d96bb7e3cb7f59b63125d6a62
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446325
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-09-07 20:13:55 +00:00
Brian Osman
0ad2d013b1 SkSL: Turn DSL parser off again
Several fuzzer issues, and one Chromium issue that's blocking the roll.

Bug: chromium:1246795
Bug: skia:12423
Change-Id: I00370b74569b447e543d9a1f22c588eb493063da
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445960
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
2021-09-07 14:21:57 +00:00
Ethan Nicholas
360db877be Fix line numbers of several DSLParser errors
Change-Id: I82e0d29fbcb5be93fe20a1fb442e2a429e6cab8e
Bug: skia:12411
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445644
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-09-03 22:12:22 +00:00
Ethan Nicholas
0ed278b42d Flip the switch to activate DSLParser
Change-Id: Id894eb70273454716eb33c85dff2056333e90cdd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445281
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2021-09-03 19:07:17 +00:00
Ethan Nicholas
b61a243294 Refactored swizzle domain testing
This moves the swizzle domain test into the path used by the DSL so that
the error check benefits both sides. To make this possible we need to
be able to distinguish between equivalent swizzle components like x and
r, so they aren't collapsed down to the same component until the very
end.

Change-Id: I48f2582886391eabd7ce6eae949babdeead6051e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445280
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2021-09-03 14:50:57 +00:00
Brian Osman
b42c3834af Remove support for unsized arrays
These only existed for geometry shader interface blocks.

Change-Id: Ie82252715fe5e6babb85e3b437c6edd811fab955
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442695
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-08-27 22:14:16 +00:00
John Stiles
7bd3f1cc78 Fix fuzzer-discovered assertion with global variables.
Our analysis pass for checking if an expression is a constant-expression
would assert if the expression contained a TypeReference or a
FunctionReference. This could happen if you passed in an expression that
had not yet been type-coerced. This check seemed overly strict, so the
assertion has been removed (although such an expression will be reported
as 'not a constant expression').

This bit us in global-variable declaration, where we checked if a
global variable's initial-value expression was constant before coercing
it to the variable's type. This has also been reordered so the type-
coercion happens first. (Either order is now valid, but the type-
coercion related errors tend to be more detailed.)

Change-Id: I5104cf817767d65fd84421243d9530734ba624a9
Bug: oss-fuzz:37710
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442693
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-08-27 21:38:19 +00:00
Brian Osman
99ddd2a98d Remove (unused) geometry shader support
Bug: skia:8451 skia:10827
Change-Id: I5b38a1d72cd4558f8e2a92aaf9b12f05efce0923
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442683
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-08-27 19:41:10 +00:00
John Stiles
8d13084535 Migrate function-body finalization out of IRGenerator.
This is a first step towards replacing `finalizeFunction` with a
`FunctionDefinition::Convert` method living outside of the IRGenerator.

Previously this code would assert that we had no early returns from a
vertex-program main() method; this has been turned into an error.
(The original assertion was also tied to fRTFlip, because the *problem*
with early-returns in main is tied to the lack of RTFlip fixups, but
we fundamentally don't allow early returns, so it makes more sense to
just universally disallow it.)

Change-Id: Iba0742f7ef3cbc83995ea130fec1eb1ef2556c44
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442691
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-08-27 18:51:52 +00:00
John Stiles
842ee5bd8d Fix assertion when calling a built-in function at global scope.
The fuzzer invented a much more elaborate example, but I was able to
winnow it down to a simple otherwise-normal test case. This also fixes
a latent DSL bug; DSL functions were not updating the list of referenced
intrinsics, so the compiler might emit finished programs that called
built-in functions that didn't exist in the code.

Change-Id: I095bb566b9db9f87cbe9460732c300b7973eb112
Bug: oss-fuzz:37659
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442325
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-08-27 17:05:21 +00:00
John Stiles
537c8a7ad5 Fix assertion when returning intermediate types.
The fuzzer managed to trigger an assertion by returning an invalid type
from a void function. We were neglecting to clear out the expression
when reporting it as invalid, leaving it for `checkValid` to find later.

Change-Id: Icc152c867a3316fe994967e192601fb4d10da98f
Bug: oss-fuzz:37704
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442678
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-08-27 16:27:15 +00:00
John Stiles
f95048cc4d Report intermediate expressions as errors in For next-exprs.
We weren't coercing the expression because we don't care about its type,
but that allowed intermediate-expressions to pass through without
reporting an error. Now we coerce the expression to its present type,
which will always fail if the type is disallowed and succeed otherwise.

Change-Id: Ic0de0d17f0f5d56360575efe992ce4d74dec2a5a
Bug: oss-fuzz:37620
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441876
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-08-25 17:26:21 +00:00
Ethan Nicholas
553239bc12 Added reportPendingErrors to SkSL code generation
This fixes an assertion failure uncovered by the fuzzer.

Bug: oss-fuzz:37469
Change-Id: I626c003cfa8a0bc65851899df3a7695dbe29200b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441311
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-08-23 20:54:40 +00:00
John Stiles
3f37322d71 Fix const function-parameter assertion discovered by fuzzer.
During constant-folding, we baked in an assertion stating that any
const-typed variable reference ought to have an initial value, because
you can't declare a const variable without assigning a value. However,
function parameters are an exception to this rule! They are variable
references and are allowed to be const, but will not have an initial
value. (In this case, `const` just means you can't alter the value.)

In this case, all we needed to do was remove the assertion; we already
treated this case defensively and with the appropriate care.

Change-Id: I61242c6d08c59886c6992898f195771e6334f2b4
Bug: oss-fuzz:37465
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441239
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-08-23 19:28:29 +00:00
John Stiles
7253a201a3 Improve error-checking during array-size conversion.
The fuzzer noticed insufficient guards in IndexExpression::Convert when
converting an array size from an IntLiteral to a SKSL_INT. We had code
in IRGenerator which did this properly, so I moved our array-size
conversion logic into SkSLType and had IndexExpression share it.

Also, a variety of tests around similar error conditions were added.

Change-Id: I51529dea25f9029f81ae236511610069d66be29f
Bug: oss-fuzz:37462
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441236
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-08-23 14:35:00 +00:00
John Stiles
8f633ef2b5 Fix assertion discovered by fuzzer.
We now stop processing a var-declaration if its array-size expression is
invalid. Previously, we'd pass a null array-size expression into
convertVar, which would assert (but would fail cleanly afterwards).

Change-Id: I976f3326e32afbc7045a86d73c0dcb28f418a6f4
Bug: oss-fuzz:37457
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441079
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-08-23 14:23:46 +00:00
John Stiles
efde90d973 Add $es3 modifier to SkSL.
This modifier is currently allowed on built-in functions only.

The presence of this modifier will be used to indicate intrinsics which
are ES3-specific (and therefore, not allowed in user code under typical
circumstances).

Change-Id: Ice6be8d9d1b2bf0c8f07f2a89f335bb2f90f6681
Bug: skia:12202
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439057
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-08-13 14:07:40 +00:00
John Stiles
ded41aafb2 Fix fuzzer-discovered error with SPIR-V interface arrays.
OSSFuzz discovered a minor variation of oss-fuzz:36770 which tickled a
different bug in SPIR-V RTFlip handling; we did not properly handle the
case where the InterfaceBlock is an array. SPIR-V does not support this
at all, but the IRGenerator allows it, and we don't detect it an an
error until later in the compilation process.

Change-Id: I80bd67a13dad878717dc122462132a2ed675532d
Bug: oss-fuzz:36850
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437018
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-08-05 21:44:16 +00:00
John Stiles
b806da4501 Honor lowp/mediump/highp precision qualifiers in IRGenerator.
This CL does not update the DSLParser to honor these precision
qualifiers; that will be done in a followup.

Change-Id: Ib629bc99c0e6c7afb550a381d4e3b6ccc26aa64e
Bug: skia:12248
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436337
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-08-04 21:29:10 +00:00
John Stiles
7729e0a97e Add hints to slicing-constructor error messages.
This is intended to make it easier for users porting GLSL code with
slicing constructors; the error message should make it clear that a
swizzle can be used here instead.

Change-Id: Ib03276ee910dfffd6146717dbd347b058f9940d1
Bug: skia:12193
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/427200
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-07-13 14:12:23 +00:00
John Stiles
ce7cdcd7b4 Add test for SkSL vector-slice constructors.
SkSL does not support shrinking a vector via casting. Use a swizzle
instead.

Change-Id: Ieba78a05dad9c55f44c765924e28f0c7e1667a67
Bug: skia:12193
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/427198
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-07-12 20:33:44 +00:00
Ethan Nicholas
59ff4e2b78 Moved swizzle error checking into Swizzle::Convert
Change-Id: Id2676ffaba1dafc4a0485d99e1c5b87b990e0861
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/426976
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-07-12 18:47:40 +00:00
John Stiles
49c417621b Shore up matrix/vector conversion tests.
The tests now check bool4-mat2 conversions, which fortunately do work,
and the vector-to-matrix tests include int and bool conversions as well.

Change-Id: I971271838a93081b9258deb7c1d13b7732fb2440
Bug: skia:12067
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/426757
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-07-12 15:02:51 +00:00
John Stiles
f9ad6ec852 Add support for mat2-to-ivec4 conversions in SkSL.
The fuzzer quickly discovered that the newly introduced mat2-to-vec4
conversion code did not account for integer vectors. We now handle
`ivec4(mat2)` casts properly. This required some non-trivial
restructuring of the logic, but in the vast majority of cases, the types
will match and the end result will be identical.

Change-Id: If07c2fe4b4345bd767384b1802374910f65cd3f0
Bug: oss-fuzz:35998
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/426756
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-07-12 14:09:18 +00:00
John Stiles
9191e813a0 Enforce consistency in swizzle domains.
GLSL disallows mixing swizzle domains within a single swizzle:
http://screen/93eHNQDbx35hMdk
SkSL now disallows it as well.

Change-Id: Ied2e11ee04285b143a864e28cac30335f01aad0e
Bug: skia:10621
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/426458
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-07-09 20:30:22 +00:00
John Stiles
6de2e1db03 Add support for matrix-to-vector conversions in SkSL.
GLSL supports casting vec4 into mat2 and vice versa, so SkSL should have
equivalent support. This CL allows the Compound constructor to take a
matrix as input, and fixes up backends to do the right thing when a
matrix shows up in the compound-constructor path.

Change-Id: I13289ad0a27ba59bddc3706093820594efebc693
Bug: skia:12067
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/426003
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-07-09 17:55:22 +00:00
Brian Osman
c9145f3402 Remove enum support from SkSL
Bug: skia:11296
Change-Id: I7d41614957d6fa535faadebbeca890b54b6977ac
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/425996
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-07-09 14:03:15 +00:00