Commit Graph

110 Commits

Author SHA1 Message Date
Cary Clark
5a2057aee9 fix fuzzer
abort if incoming data is out of range

TBR=reed@google.com
BUG=676866

Change-Id: I7d4850611654a399e32ea2012b23ca369dc53e70
Reviewed-on: https://skia-review.googlesource.com/6525
Reviewed-by: Cary Clark <caryclark@google.com>
Commit-Queue: Cary Clark <caryclark@google.com>
2017-01-03 16:27:36 +00:00
Cary Clark
7eb01e00b1 simplify bug
The path contains a cubic with a very tight curve.
Split the cubic into pieces so that the individual
curves are better behaved.

Use both inflections and max curvature to 
potentially split cubics. Since this may require
a bit of work, preflight to ignore cubics that
monotonically change in x and y.

Only one of the three tests referred to by the bug
below repro'd. Use path.dumpHex() instead of 
path.dump() to capture the crashing data.

TBR=reed@google.com
BUG=skia:6041

Change-Id: I29a264f87242cacc7c421e7685b90aca81621c74
Reviewed-on: https://skia-review.googlesource.com/5702
Reviewed-by: Cary Clark <caryclark@google.com>
Commit-Queue: Cary Clark <caryclark@google.com>
2016-12-08 20:29:37 +00:00
Cary Clark
918fb1fe6f fuzz hang fix and pathops client debugging
The ASAN fuzzer on chrome caught a hanging state.

To capture the data, allow the pathops client debugging
to run in a release build.

TBR=reed@google.com
BUG=665295

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4834

Change-Id: I6b2c2baabd63994f63aa730d2ee7828986b5ab89
Reviewed-on: https://skia-review.googlesource.com/4834
Commit-Queue: Cary Clark <caryclark@google.com>
Reviewed-by: Cary Clark <caryclark@google.com>
2016-11-15 19:03:36 +00:00
caryclark
96dc1c9efa fix more chrome asan fuzzer failures
Small change to gracefully quit
when fuzzer values cause pathops
to fail.

TBR=reed@google.com
BUG=657411, 657559
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2426393004

Review-Url: https://chromiumcodereview.appspot.com/2426393004
2016-10-20 11:34:10 -07:00
caryclark
a35ab3e6e0 fix fuzzers
Many old pathops-related fuzz failures have built up while
the codebase was under a state a flux. Now that the code
is stable, address these failures.

Most of the CL plumbs the debug global state to downstream
routines so that, if the data is not trusted (ala fuzzed)
the function can safely exit without asserting.

TBR=reed@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2426173002

Review-Url: https://chromiumcodereview.appspot.com/2426173002
2016-10-20 08:32:18 -07:00
caryclark
e6522ea38f fix some fuzz
TBR=reed@google.com
BUG=654939, 655829, 656149
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2425733002

Review-Url: https://codereview.chromium.org/2425733002
2016-10-17 07:54:33 -07:00
caryclark
0871401538 fix another fuzz
Add one more early exit.

TBR=reed@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2401673004

Review-Url: https://codereview.chromium.org/2401673004
2016-10-07 12:57:47 -07:00
caryclark
221a4bb55b fix fuzz busters
When fuzz is hit
just quit

TBR=reed@google.com
BUG=skia:5837, skia:5838, skia:5839
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2404483002

Review-Url: https://codereview.chromium.org/2404483002
2016-10-07 11:15:16 -07:00
Cary Clark
40f23780e7 fuzzer fix
Error out when fuzzing
conditions are met.

Also, make degenerate
input line ends consistent.

TBR=reed@google.com
BUG=653452, 626164, skia:5829

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=3048

Change-Id: I23a01adde9dec07b54d66ab2418b3ea0b96e4456
Reviewed-on: https://skia-review.googlesource.com/3048
Commit-Queue: Cary Clark <caryclark@google.com>
Reviewed-by: Cary Clark <caryclark@google.com>
2016-10-06 16:22:51 +00:00
caryclark
0c52b1740e validate using nx to match bounds
Check the path against the bounds
using Nx instead of straight scalars,

R=mtklein@google.com
BUG=skia:5541
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2388903006

Review-Url: https://codereview.chromium.org/2388903006
2016-10-05 12:16:52 -07:00
Cary Clark
b9ae53727b fix fuzz
This fix is slightly interesting; if the final
close of the contour degenerates into a zero-length
line, remove the previous line from the generated
contour.

TBR=reed@google.com
BUG=skia:5822

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=3004

Change-Id: Id668d13ccf6aad9bc81d78588fc77437527a0b7b
Reviewed-on: https://skia-review.googlesource.com/3004
Reviewed-by: Cary Clark <caryclark@google.com>
2016-10-05 14:41:37 +00:00
Cary Clark
3fdf52cf38 fix fuzz with null check
TBR=reed@google.com
BUG=skia:5819

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2925

Change-Id: I1957d3f8f5c78bed41bb7fe413ae870df24f7c50
Reviewed-on: https://skia-review.googlesource.com/2925
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Cary Clark <caryclark@google.com>
2016-10-04 18:40:13 +00:00
caryclark
e3a4e993ef fix cubic linear test
Check to see if the line between end points is
degenerate before measuring control points.

Also, add test case for a bug to see if it
shows up on any platform.

TBR=reed@google.com
BUG=skia:5169, skia:5240
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2375053002

Review-Url: https://codereview.chromium.org/2375053002
2016-09-28 09:22:17 -07:00
caryclark
414c4295f9 allow conic chop to fail
Fuzzy values may cause the conic chop to fail.

Check to see if the values are all finite, and
require the caller to do the same.

R=reed@google.com
BUG=650178
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2368993002

Review-Url: https://codereview.chromium.org/2368993002
2016-09-26 11:03:54 -07:00
caryclark
cc09372730 fix msan bug in pathops
Msan and Valgrind found an uninitialized memory mistake in
pathops. This also fixes similar bugs where not all parts
of the geometry were covered in the loop iteration.

R=borenet@google.com
NOTREECHECKS=true
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2366893003

Review-Url: https://codereview.chromium.org/2366893003
2016-09-23 09:32:27 -07:00
caryclark
27c015dfcf split tight quads and conics
Tight quads and conics may nearly fold over on themselves, confusing
coincidence against other curves. Split them at their max curvature
early on to avoid complicating later logic.

TBR=reed@google.com
BUG=skia:5131
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2357353002

Review-Url: https://codereview.chromium.org/2357353002
2016-09-23 05:47:20 -07:00
caryclark
1c10607244 don't close a contour with a line if nothing's there
R=kjlubick@google.com
BUG=skia:5789
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2360083006

Review-Url: https://codereview.chromium.org/2360083006
2016-09-22 12:52:21 -07:00
caryclark
e7bb5b2266 fix next kevin fuzz
Fix one more fuzzer crash.

R=kjlubick@google.com
BUG=skia:5775
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2357373002

Review-Url: https://codereview.chromium.org/2357373002
2016-09-22 05:20:07 -07:00
caryclark
45f04b8ea8 fix skia pathops fuzzers
Add isolated tests.

R=kjlubick@google.com
BUG=skia:5775
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2358043002

Review-Url: https://codereview.chromium.org/2358043002
2016-09-21 08:46:56 -07:00
caryclark
bbfe92bc1d fix fuzzer bugs
Add a couple more cases where Op() fails and
returns false when the out of range input
values make the internal numeric unstable.

TBR=reed@google.com
BUG=647834, 648068
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2348263002

Review-Url: https://codereview.chromium.org/2348263002
2016-09-19 06:00:35 -07:00
caryclark
c9b90d15df fix fuzzer
Earlier fix aborted coincident loop on wrong condition.
This aborts only when it reaches the end, and then
propagates the error out correctly.

TBR=reed@google.com
BUG=644680, 644684
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2327053002

Review-Url: https://codereview.chromium.org/2327053002
2016-09-09 07:41:36 -07:00
caryclark
b393a49e5f fix pathops fuzzers
Extreme values trigger asserts that in range
values allow.

Disable asserts and other tests to prevent
extreme values from crashing.

TBR=reed@google.com
BUG=644684. 644680, 644640
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2316173002

Review-Url: https://codereview.chromium.org/2316173002
2016-09-07 08:21:09 -07:00
caryclark
ef7cee4bbc provide safe exit for runaway intersections
Curve intersections with extreme numbers may
cause the intersection template code to loop
forever. Detect this by looking for
marking more spans gone than exist, and
return without any intersections found.

TBR=reed@google.com
BUG=643855
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2310113002

Review-Url: https://codereview.chromium.org/2310113002
2016-09-06 09:05:55 -07:00
caryclark
30b9fdd6a1 pathops coincident work
This is working towards fixing all bugs around simplifying the tiger.

This installment simplifies the point-t intersection list as it is built rather than doing the analysis once the intersections are complete. This avoids getting the list in an inconsistent state and makes coincident checks faster and more stable.

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2237223002

TBR=reed@google.com
BUG=skia:5131

Review-Url: https://codereview.chromium.org/2237223002
2016-08-31 14:36:30 -07:00
caryclark
79418093c1 if the winding of the top can't be computed, give up
TBR=reed@google.com
BUG=641478
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2283993002

Review-Url: https://codereview.chromium.org/2283993002
2016-08-26 14:24:24 -07:00
caryclark
1682612871 mark fuzz test flaky since it may fail on some platforms
NOTRY=true
TBR=bungeman@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2277893004

Review-Url: https://codereview.chromium.org/2277893004
2016-08-24 13:00:42 -07:00
caryclark
ef4f32ac85 remove point aliases
This removes the notion of keeping track of every different t value
that resolves to the same or a similar point. Other fixes make
this concept unnecessary, and removing it simplifies the code.

This removes an allocation, and speeds up paths with many
overlapping curves.

As a bonus, four fuzzer tests that failed before now succeed.

TBR=reed@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2275703003

Review-Url: https://codereview.chromium.org/2275703003
2016-08-24 09:24:18 -07:00
caryclark
bb51f4a3a7 fix pathops fuzz
Extreme cubics may split so that one half is a point.
Discard this rather than generating a degenerate line.

TBR=reed@google.com
BUG=640025
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2276503002

Review-Url: https://codereview.chromium.org/2276503002
2016-08-23 07:38:49 -07:00
caryclark
429428660b fix fuzzes
TBR=reed@google.com
BUG=639157, 638783
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255243003

Review-Url: https://codereview.chromium.org/2255243003
2016-08-19 07:01:33 -07:00
caryclark
a1b42d91a5 fix fuzz bug
TBR=reed@google.com
BUG=637968, 638002
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2250573003

Review-Url: https://codereview.chromium.org/2250573003
2016-08-16 10:25:29 -07:00
caryclark
c6d855f7f3 start working on tiger again
The tiger tests are a suite under development
that exercise many coincident edge cases.

This fixes the case when a duplicate point is not
ignored when the coincident span references the
primary point.

TBR=reed@google.com
BUG=skia:5131
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2239803002

Review-Url: https://codereview.chromium.org/2239803002
2016-08-11 11:59:48 -07:00
caryclark
d5b9173fe8 add flaky test option
One fuzzer generated pathops test
fails everywhere except for one
builder. Add a flaky state to the
pathops test framework to handle
this until I can investigate
further.

TBR=reed@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2221153005

Review-Url: https://codereview.chromium.org/2221153005
2016-08-09 05:04:29 -07:00
caryclark
643ede6921 template intersection fuzz fixes
Plumb in the ability to ignore asserts for out of range input
deeper into the template intersection code.

Exit gracefully when error conditions are found.

TBR=reed@google.com
BUG=632607,632628,633063,633065,634251,633608
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2224823004

Review-Url: https://codereview.chromium.org/2224823004
2016-08-08 14:27:45 -07:00
caryclark
fc560e09b3 fuzz wednesday
Add debug 'skip assert' access to SkCoincidentSpans.
That permits suppressing asserts to make it easier
to detect when this fuzz data generates unparsable
data.

TBR=reed@google.com
BUG=631564, 631992
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2186973002

Review-Url: https://codereview.chromium.org/2186973002
2016-07-27 08:46:10 -07:00
caryclark
e25a4f6cbe fix fuzz bugs
Add ability for intersection template to detect that
the test contains bounded numbers so that extra
asserts can trigger.

Add some exit points for out of range numbers
in those templates.

TBR=reed@google.com
BUG=631383,631374,631360
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2185703002

Review-Url: https://codereview.chromium.org/2185703002
2016-07-26 09:26:29 -07:00
caryclark
8bc90e2db6 fix fuzz
Extreme inputs trigger asserts intended for in range data.
Return an error without asserting unless the test signals
otherwise.

TBR=reed@google.com
BUG=630736
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2178923002

Review-Url: https://codereview.chromium.org/2178923002
2016-07-25 06:05:08 -07:00
caryclark
8a8accbcd1 limit number of searched roots
Extreme numbers can generate more than
three found cubic roots when the roots
are found through a binary search.

Fail in this case.

TBR=reed@google.com
BUG=630649
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2176733002

Review-Url: https://codereview.chromium.org/2176733002
2016-07-22 10:56:26 -07:00
caryclark
cdeff81bdb conic fuzz fix
If no closest section is found in conic intersection
(which can happen if the numbers are out of range)
abort the intersection.

Also suppress assert fired in this case so it only
checks intersections with in-range values.

TBR=reed@google.com
BUG=630378
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2166813006

Review-Url: https://codereview.chromium.org/2166813006
2016-07-22 03:34:19 -07:00
caryclark
a339bb0d95 let line/cubic intersect at four spots
With this, let's update the fuzzer.

R=kjlubick@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2169983002

Review-Url: https://codereview.chromium.org/2169983002
2016-07-21 12:28:04 -07:00
caryclark
81681949d3 allow curves on side of lines to be very small
Increase the precision of the 'all on one side' pathops
angle test to reduce the number of unorderable segments
found by the cubics_d3 test.

This allows pathsops_unittest release build with -v -V -x
to run without error.

TBR=reed@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2168163002

Review-Url: https://codereview.chromium.org/2168163002
2016-07-21 10:44:07 -07:00
caryclark
1597628fa3 fix fuzzer bug
Fix another fuzzer bug.

Some PathOps asserts only make sense if the incoming data is
well-behaved. Well-behaved tests set debugging state to
trigger these additional asserts.

Formalize this by creating macros similar to SkASSERT that
check to see if the assert should be skipped.

TBR=reed@google.com
BUG=629962
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2169863002

Review-Url: https://codereview.chromium.org/2169863002
2016-07-21 05:48:44 -07:00
caryclark
1493b9772d fix fuzzer
Previous spans always have a valid next pointer. The final span does not.
Change the test for a valid link to take into consideration whether
the links are chased forwards or backwards.

TBR=reed@google.com
BUG=629454
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2166543002

Review-Url: https://codereview.chromium.org/2166543002
2016-07-19 11:29:15 -07:00
caryclark
034f243c6d disable test that fails to compute bounds on nexus 5
TBR=jcgregario@google.com
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2160713003

Review-Url: https://codereview.chromium.org/2160713003
2016-07-18 12:13:23 -07:00
caryclark
55888e4417 pathops coincidence and security rewrite
Most changes stem from working on an examples bracketed
by #if DEBUG_UNDER_DEVELOPMENT  // tiger
These exposed many problems with coincident curves,
as well as errors throughout the code.

Fixing these errors also fixed a number of fuzzer-inspired
bug reports.

* Line/Curve Intersections
Check to see if the end of the line nearly intersects
the curve. This was a FIXME in the old code.

* Performance
Use a central chunk allocator.
Plumb the allocator into the global variable state
so that it can be shared. (Note that 'SkGlobalState'
is allocated on the stack and is visible to children
functions but not other threads.)

* Refactor
Let SkOpAngle grow up from a structure to a class.
Let SkCoincidentSpans grow up from a structure to a class.
Rename enum Alias to AliasMatch.

* Coincidence Rewrite
Add more debugging to coincidence detection.
Parallel debugging routines have read-only logic to report
the current coincidence state so that steps through the
logic can expose whether things got better or worse.

More functions can error-out and cause the pathops
engine to non-destructively exit.

* Accuracy
Remove code that adjusted point locations. Instead,
offset the curve part so that sorted curves all use
the same origin.
Reduce the size (and influence) of magic numbers.

* Testing
The debug suite with verify and the full release suite
./out/Debug/pathops_unittest -v -V
./out/Release/pathops_unittest -v -V -x
expose one error. That error is captured as cubics_d3.
This error exists in the checked in code as well.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128633003

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128633003

Review-Url: https://codereview.chromium.org/2128633003
2016-07-18 10:01:36 -07:00
caryclark
aa7ceb62b5 fix another pathops fuzz bug
Tunnel out the error case when the numerics
create uncalculable angles.

R=mmoroz@chromium.org
TBR=reed@google.com
BUG=624351
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2103863005

Review-Url: https://codereview.chromium.org/2103863005
2016-06-29 10:46:08 -07:00
caryclark
343382e3ac fix fuzz test that exceeds numeric limit
The extreme values here exceed an internal test that expects
computed numbers to be less than MAX_FLT. Use MAX_DBL instead.

R=mmoroz@chromium.org
TBR=reed@google.com
BUG=624346
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2103903004

Review-Url: https://codereview.chromium.org/2103903004
2016-06-29 08:18:38 -07:00
caryclark
3f0753d3ec fix fuzz bugs
Detect more places where the pathops numerics cause numbers
to become nearly identical and subsequently fail. These tests
have extreme inputs and cannot succeed.

Also remove the expectSuccess parameter from PathOpsDebug
and check instead in the test framework.

R=mbarbella@chromium.org
TBR=reed@google.com
BUG=623072,623022
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2103513002

Review-Url: https://codereview.chromium.org/2103513002
2016-06-28 09:23:57 -07:00
caryclark
dae6b97705 fix pathops fuzz bugs
Fail out in a couple of new places when the input data is very
large and exceeds the limits of the pathops machinery.

Most of the change here plumbs in a way to exclude an assert in
one of these exceptional cases. The current SkAddIntersection
implementation and the inner functions it calls has no way to
report an error to the root caller for an early exit, so rather
than add that in, exclude the assert when the test that would
trigger it runs (allowing the test to otherwise ensure that it
properly fails).

TBR=reed@google.com
BUG=617586,617635
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2046713003

Review-Url: https://codereview.chromium.org/2046713003
2016-06-08 04:28:19 -07:00
caryclark
2bec26a716 fix security bug
This fix is a tradeoff. It changes intersection to
treat a case where one coincident run is intersected at one point
and the other edge is not as continuing to be a span.

The old code tried to treat this as a single point.
The old code is probably right, but this change alone
made the data structures inconsistent. Later, extending
the coincident runs would fail by incorrectly discarding
the single point intersection.

As a result, this fixes the security test and one other, but
makes a different test fail. Isolating the failure uncovered
a reduced case that fails with and without the change, so
there are more serious problems here. Those problems are
addressed in a separate CL.

Many of the test edits below remove ill-thought out debugging
messaging that fire off global state, which isn't usable
in a multi-threaded test environment.

In the end, with this fix, all existing tests (modulo one
new failure and one new non-failure) pass in debug and
in the extended release test suites.

TBR=reed@google.com
BUG=614248
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2018513003

Review-Url: https://codereview.chromium.org/2018513003
2016-05-26 09:01:47 -07:00
caryclark
5c5cfe24ef give up if huge paths have unresolvable coincidence
This fuzzy test has enormous curves with coincidence runs that break numerics.
If the computed intersections identify that the span of coincidence has been deleted,
give up and return that the path op failed.

TBR=reed@google.com
BUG=597926
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1854333002

Review URL: https://codereview.chromium.org/1854333002
2016-04-05 07:28:48 -07:00