The fuzzer discovered that it could overflow the program-size value.
Rewrote the logic to use SkSafeMath everywhere, and to early-exit as
soon as a statement manages to exceed the program size.
Change-Id: I01511b2201173c95ebc1ac602901410ac9d74d73
Bug: oss-fuzz:38697
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449098
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The fuzzer has discovered a bug in our program size-checking logic; for
loops that immediately contain another for loop (with no block) were not
counting the inner loop's iterations. This allowed it to exceed our
maximum program-size threshold (and time out during SkVM compilation).
This test demonstrates the issue. A followup will fix it.
Change-Id: I3b7d4c8a4f0ed04cf0aba3f1a32fdad7d6d784e7
Bug: oss-fuzz:37837
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449096
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>
Bug: skia:12302
Change-Id: Ifc107ca2cf13c1daa59521b93fe4ad1d3c215258
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/447297
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
DSL was improperly allowing interface blocks in runtime shaders, which
caused PipelineStageGenerator to get upset.
Bug: oss-fuzz:38131
Change-Id: I593e68f2cab3db9151d606e65e2826ffa9c494e2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446324
Commit-Queue: Ravi Mistry <rmistry@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
To do this with a clean conscience, I needed to convert the unroll-
counting logic from a linear time algorithm to constant-time. Getting
all the edge cases correct requires a lot of care, and there are now
plenty of unit tests.
Change-Id: I620909d069ac425b7310e345bf80ec844fe035f8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445643
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
An ES2-compatible for loop supports six separate rel-ops:
< <= > >= != ==
Each rel-op, in addition to its expected usage, is also able to
represent a loop which never terminates, as well as a loop which
terminates instantly. Since SkVM unrolls these loops, we should make
sure we do it properly. We now have unit tests for all of these cases.
Change-Id: Icae04d48bc158bf8c0c98db97f76756a1a29110c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445756
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Bug: skia:12302
Change-Id: I4ff394f1f9d93d2def19a9f9d49cb208651aff10
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445639
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Also update RELEASE_NOTES to describe new syntax.
Change-Id: I2666551b98f80b61ae3a48c92a9e306cdc7242b0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444735
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Our program-size analysis pass needs to recurse into called functions;
depending on the exact order of functions in the program, this recursion
can hypothetically be as deep as the deepest function-call chain. Set an
upper bound on recursion here, so we don't overflow the stack while
trying to check the program size. In practice, 50 frames is far deeper
than a regular shader should ever go.
Change-Id: I733ee48dad6f8053facdfd9f6d8a2b9b2a4af188
Bug: skia:12396
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445279
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The fuzzer is currently learning to make unboundedly-large programs by
nesting medium-size loops repeatedly. SkVM doesn't have a mechanism to
limit the ensuing explosion of code and ends up making unreasonably deep
stacks and/or unreasonably large programs.
SkSL now enforces an upper bound of approximately 100,000 IR nodes on a
fully-flattened, fully-inlined strict-ES2 program. The limit is picked
out of thin air, but this should be enough to prevent SkVM from going
haywire while still being large enough to handle any reasonable program.
We can definitely tune this value if we find that it is too large
(admitting dangerous code) or too small (rejecting good code).
Change-Id: I11735636175721fbc79460b4e194d8e4b42dc47d
Bug: skia:12396, oss-fuzz:37827, oss-fuzz:37837
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444358
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:12302
Change-Id: I8cf958acf9214d0de903a4097647afd74f2a659e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441541
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
After further discussion, using intrinsics with signatures similar to
sample keeps us looking like GLSL. However, using "sample" is still
misleading, so this adds explicit "shade", "filter", and "blend"
intrinsics. After migrating clients, the "sample" versions will be
removed.
Bug: skia:12302
Change-Id: Ia03e4b3794fc1fc5ae3c3099a7a350343ec7702e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441457
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Much easier to maintain, especially with an upcoming change to the
sampling syntax.
Change-Id: I378811b7be0afcce5b7e68a942e7b46d96568155
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441518
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Change-Id: I97986c45599274441660d1e947592768960c6d27
Bug: skia:12348
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439949
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Surprisingly, we didn't actually have a preexisting test covering this.
Error reporting is lackluster in this CL but will be improved in the
followup.
Change-Id: I0b1cdb5a82f066af6b9d3fd9c39748080c2e18c0
Bug: skia:12348
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439996
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We can now add functions to sksl_public.sksl with an $es3 prefix. These
will be allowed in a Runtime Effect when strict-ES2 mode is disabled.
Note that the CPU backend still doesn't have support for these calls,
and will fail ungracefully (assertion, nonsense result) if these
intrinsics are used.
The testing here is limited, due to an unrelated bug in SPIR-V
(skia:12340)
Change-Id: I9c911bc2b77f5051e80844607e7fd08ad386ee56
Bug: skia:12202, skia:12340
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439058
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Most of the code generated by the fuzzer is nonsense, but there is a
method to its madness. The crash is only triggered under specific
conditions:
- The runtime effect has enough helper functions to mostly fill up the
call graph hash-map. It won't rehash until it gets close to capacity.
- There must be several calls to built-in functions, in order to add
elements to the call graph to force a rehash.
The fuzzer-generated code manages to satisfy both these requirements.
Change-Id: I9a1d7535557fedd4e9bfece3930ac86ede291ffe
Bug: oss-fuzz:36655
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437118
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@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 relaxes our rules to allow calls to declared (but not yet defined)
functions. With that rule change, we have to specifically detect static
recursion and produce an error.
Bug: skia:12137
Change-Id: I39cc281fcd73fb30014bc7b43043552623727e03
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/431537
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:12137
Change-Id: I609dd2578bf39a30e036ea85281886f8c4554579
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/431038
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I4280b5710dd8749ba766ba74d7a8886bc4e024bb
Bug: oss-fuzz:35124
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/417200
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>
Runtime Blend effects always take two input colors--source and
destination--instead of one. This CL adds a new ProgramKind for blend
effects, a new program module (empty for now), and adds a test to
confirm that the signature for blend functions is checked. Currently
these are only accessible via skslc; there's no Runtime Effect API to
create one and the dest color isn't hooked up to anything.
Change-Id: I5272a811d2d76b878cfdf3429efa78c9c8b3fd97
Bug: skia:12080
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/416798
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
In particular, this optimizes abs() and sign() when all inputs are known
at compile time. This resolves a TODO on a test case in
`IllegalIndexing.rts`.
Change-Id: Ica310522a85b42dc7ae255bd25004a6629d04176
Bug: skia:10835
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/405676
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
All internal usage has migrated to MakeFor..., this removes the old
program kind, and updates some tests.
Bug: skia:11813
Change-Id: I56733b071270e1ae3fab5d851e23acf6c02e3361
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402536
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
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>
These enforce stricter rules about the signature of main, and each one
uses a separate pre-include module. That prevents color filters from
being able to reference sk_FragCoord (or coords passed to main) at all.
It also limits the versions of sample() that are exposed.
In the new world, an effect created for a specific stage of the Skia
pipeline can only be used to create instances of that stage (SkShader or
SkColorFilter). For now, SkRuntimeEffect::Make uses kRuntimeEffect,
which continues to be more lenient and allow creation of either shaders
or color filters from a single effect. After we migrate all clients, we
can deprecate and then delete that mode.
Bug: skia:11813
Change-Id: I0afd79a72beeec84da42c86146e8fcd8d0e4c09f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395716
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:11374
Change-Id: I63d605eabbe514a0469d00d8a671969874f3edd4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/393081
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Also adds tests of non-uniform shader declarations. These are currently
allowed, but will be detected as an error in the next CL.
Bug: skia:11374
Change-Id: I3fee0a0c97ae590f7bc6952cb367f7e94436b891
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/393080
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Allowed today, will soon be an error.
Bug: skia:11813
Change-Id: I5c13de7657fa85f13fa6d80e1d890225d8a3e868
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/392439
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
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>
None of these are legal in GLSL ES 1.0. Added a new test that previously
compiled without error. Started out with just assignment and equality,
then realized that sequence and ternary should be blocked, too.
Bug: skia:11323
Change-Id: I02691f819565afabeadbb12cab6c07acf40093f7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370880
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This is enforced by ANGLE in Strict ES2 mode; we need to enforce it as
well.
Change-Id: I6e2f547ad8e0ce817742cf84659764cf6bce38b9
Bug: skia:11270
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366339
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 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>
