The DSLParser struct nesting check didn't work, and on top of that
should have been part of the core DSL itself as opposed to being
relegated to the parser.
Change-Id: I680d0dc703309136970353204add14a6c9e03aa8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445113
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This allows us to remove the static-recursion analysis pass entirely,
while still providing the same results.
Change-Id: If1564cd4df55be86ca4e0bf53ecc094ba76007df
Bug: skia:12396
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445296
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This picks up the change to use Bot-Commit+1 for RecreateSKPs
Bug: skia:12124
Change-Id: I4a22536e216a05fef15686503dbbfbb02b5e4808
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445109
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Ravi Mistry <rmistry@google.com>
The previous version of the code would actually end up skipping over
recursive code silently. (In fact, CheckProgramUnrolledSize is always
called immediately after Analysis::DetectStaticRecursion regardless of
whether or not any cycles were found, and the "cycle detected" assert
was not firing.) The code now actually detects cycles correctly; this
can be verified by uncommenting the call to error().
In a followup CL, I will improve the error reporting so that the cycle
is tracked and can be reported. Right now, we can detect a cycle but
don't keep a copy of the stack.
Change-Id: I9eb7b6333522d334657af4aa8d09fd6ad371ad69
Bug: skia:12396
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445112
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Also fixes some minor warnings.
Bug: skia:12086
Change-Id: Ia476a7a196b490022978761e92f935a6d668136a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441797
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
This work is being done by the skiastatus Gerrit plugin.
Bug: skia:12403
Change-Id: I42b81dcc9e2b4d5c99f9756eabed186f489a9adb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445105
Commit-Queue: Ravi Mistry <rmistry@google.com>
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Private / unsupported types are now restricted to module code.
Change-Id: I98cb2e0822560a274758d99ecf0ca09883d0c3a1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445097
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
A client of Flutter is seeing a crash when trying to release the
MTLTexture in GrMtlAttachment. The range of CLs involved includes the
one that disables retained references in the command buffer, so this
is a speculative change to see if that is the culprit.
Change-Id: Idd6e6b264086d671be52f456ef56f1fc5bafade8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444498
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
We were reporting immediate errors in this function at the correct
location, but by not setting the offset of the statement itself, later
errors discovered during analysis were being reported with no line
number.
Change-Id: I7f4c5a0db774bbcf3901b38e05a43cebbda0d1ed
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445102
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
RedeclareStructTypeWithName was reporting an error on the wrong line.
Change-Id: Ibe954ca5013dbedca8cf015660cf3d28fa97987d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445101
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Once we have created a DSLVar, we need to return it to avoid internal
errors.
Change-Id: I28dacf49e2f5219660ddbc6e21100e3fb0675742
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445099
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@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>
We were previously relying on the RTAdjust var's (essentially
meaningless in that case) value being non-null even when RTAdjust was
an interface block field instead of a var. This breaks runtime effects
built by the DSLParser, which does not set RTAdjust unless there's
actually an RTAdjust var.
Change-Id: I0938e1562ebf997af7b1f4dd9beb60fcd7909a20
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445096
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This allows processing to continue in the face of invalid array sizes,
matching the error reporting behavior of SkSLParser.
Change-Id: Ifc0572aecf829563e4a77f01962a6e05aa9e98e8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444761
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Change-Id: I06ba9dd9ed8af8555233ddfa10d3e0ec6babc2ea
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444759
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
There was no good reason for this behavior in the first place, and this
change makes the error reporting behavior better match SkSLParser.
Change-Id: I7b69e7bcc64173c0ac6523e075c1f24e2be00ed0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444758
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Most of the logic in IRGenerator::finish has moved to
Compiler::finalize. The @if/@switch pass has been combined with the pass
that verifies no dangling FunctionReference/TypeReference expressions,
saving one walk through the IR tree. Most program-finalization logic now
exists in Compiler and Analysis.
This change reorders our error generation logic slightly, and manages to
squeeze a few extra (valid) errors out of one of our fuzzer-generated
tests, but is not really intended to affect results in any significant
way.
Change-Id: I461de7c31f3980dedf74424e7826c032b1f40fd2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444757
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>
As these checks were in the AST processing side of things, they were not
catching errors in DSL code. SkSLAnalysis is also a better home for this
function than SkSLIRGenerator.
Change-Id: I8149825047570300cc09425deba1339ac0edb7ab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444656
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This doesn't rely on anything Compiler-specific, and can just be a plain
Analysis pass.
Change-Id: I8564ae2a750c6daa6c449e6fa56355cc047f7010
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444496
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:11837
Change-Id: Ia79f76c18587741000367edba303c5f7c0c1087d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444178
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This matches the behavior of SkSLParser and fixes some spurious
followup errors.
Change-Id: Id1eae353eb6536a6385d04d0426e0de736db6b57
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444360
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This adds line numbers to a bunch of errors which were previously
missing them.
Change-Id: Id77c08d168b0a2d8a0131a53aa004bf37b8bec02
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444176
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Chromium has been updated, this is no longer needed
Change-Id: I0bcf65c79d454c50796e04e33c213de2295c0e6a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441877
Auto-Submit: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This fixes spurious errors that occur subsequent to an error converting
and array size.
Change-Id: I23fda9ef744f9144726d4fd28584676b33a4c7bb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444177
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This eliminates an extra "expected int literal" error when the
array size is an invalid expression.
Change-Id: Iaf5d15316df3ec5200d51d73c14d7e428ce17be9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/443236
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Change-Id: I0fdaf04525f02e8827839a0f43bb9181309cbb56
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444137
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
The fuzzer discovered that, when we attempt to verify that an array
doesn't contain any literal values that are out-of-range for its base
type, we pay a linear-time cost based on the size of the array. This
happens even when the array value isn't known at compile time; we still
iterate over its slot count and diligently discover that every single
constant-subexpression slot in the expression is "null".
We now have a helper function on Expression,
`allowsConstantSubexpressions`, which only returns true for expression
kinds that can contain constant subexpressions. We use this helper to
skip over this linear-per-subexpression check when the expression
cannot possibly contain a constant subexpression. In particular,
`AnyConstructor::compareConstant` and `Type::checkForOutOfRangeLiteral`
will now early-out for expressions that can't possibly contain a
constant subexpression.
Change-Id: Ia34e422afa67b478a8616acb0a0e9cd211b29698
Bug: oss-fuzz:37900
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444136
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>
This adds position tracking to individual fields and an error for
illegal field modifiers.
Change-Id: Ie121699bde94e831f33a0bc021349a1cd757d08b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/443888
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
A few nice properties fall out of this:
- IntrinsicSets become an implementation detail that most code never has
to think about.
- The dehydrated data is marginally smaller because it no longer needs
to store the IntrinsicSet array; it's re-derived when the
FunctionDefinition is created via Convert.
finalizeFunction is largely unnecessary now, but it still had one
lingering use; it appends the sk_Position fixup to the end of main()
when compiling a vertex program. Added appendRTAdjustFixupToVertexMain
to IRGenerator to handle this case. This could be done in Convert as
well if the RTAdjust fields weren't buried inside of the IRGenerator.
Change-Id: I7451ea9c64112a376ad36902d36c29a9cf147504
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442817
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
I want to free up GrD3DTypesPriv to actually be private types that
include real d3d objects.
Change-Id: Id38d6baae4fa68c19301b27d4f9d51eb1d9c5db0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/443676
Commit-Queue: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Auto-Submit: Greg Daniel <egdaniel@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
With this change, it will either match or improve on the reports we
currently get from SkSLParser.
Change-Id: I9b3c0f0c2225bf47fec141a1c01c94d9c2ab6a6b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/443056
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
