We now detect attribute, varying, precision and invariant as reserved.
Change-Id: I8c90655a70b1bad31bf6143c3fdcb2ce582320b1
Bug: skia:12484
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/459479
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
`samplerCube` is a type which we don't support at all. It has been added
to the reserved-word list.
`textureCube` was in our list of built-in types, but was not actually
used in any way; it wasn't actually added to the root or private symbol
tables, and was totally unreferenced by the code. It's been deleted.
Change-Id: I4f79ce5d40ac6ebdb2a7067fa60cc79e316b01b6
Bug: skia:12484
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/459123
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This reverts commit eb68973c2f.
Reason for revert: ES2 conformance test checks this
Original change's description:
> 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>
Bug: skia:12443
Change-Id: I5a32744c88b9b830ad657488824c8c7dd0b0a652
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458056
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
GLSL treats builtin types and user-defined types differently; `int` and
`float` are keywords and cannot be used to name variables. However, it's
fine for a user type like `struct xyz` to be hidden by a variable
`int xyz` or even `xyz xyz` (i.e., a variable of type `struct xyz` named
`xyz`).
We now honor that distinction and include tests for it. This will fix
several ES2 conformance tests (local_struct_variable_hides_struct_type,
local_int_variable_hides_struct_type, etc.).
Change-Id: I7a45c70707087f9f355ce5b06b032fed16683f3e
Bug: skia:12527
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458721
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This fixes GLSL ES2 conformance test `array`.
Change-Id: I6ebee9253e1e8c394d9ddb6899e3a0940b7a38ef
Bug: skia:12495
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458718
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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: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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
