Sema: Fix a fuzzer crash #85016
Sema: Fix a fuzzer crash #85016
Conversation
slavapestov
force-pushed
the
prepared-fuzz
branch
3 times, most recently
from
cb58dd8 to
eaa38f3
Compare
|
@swift-ci Please smoke test |
lib/Sema/ConstraintSystem.cpp
Outdated
| for (auto &fix : contextualFixes) | ||
| genericParamInferredTypes.insert(fix.first->getFixedType(resultTypeVar)); | ||
| for (auto &fix : contextualFixes) { | ||
| if (fix.first->hasFixedType(resultTypeVar)) |
There was a problem hiding this comment.
Free type variables are allowed only with FreeTypeVariableBinding::Allow set, I think a better fix here might be to actually get an overload from every solution instead and extra their result type because if hasFixedType is ever produces true it could mean that overloads were chosen in sub-scope when solutions diverged.
There was a problem hiding this comment.
Looks like this is indeed the case:
Solution 1: locator@0xbb4859000 [OverloadedDeclRef@<stdin>:4:12] with Swift.(file).?? as ??: (consuming $T8?, @autoclosure () throws -> $T8) throws -> $T8
Solution 2: locator@0xbb4859000 [OverloadedDeclRef@<stdin>:4:12] with Swift.(file).?? as ??: (consuming $T17?, @autoclosure () throws -> $T17) throws -> $T17
There was a problem hiding this comment.
Ok! I’ll hold off on landing this until I figure out the right fix.
There was a problem hiding this comment.
Slightly higher up is where we get an overload from the first fix, I think all you need to do here is just that for each of the solutions involved.
There was a problem hiding this comment.
extra their result type
This should say "extract" right?
This test case crashes when prepared overloads are disabled, but passes when enabled. To avoid messing up tests if we have to turn the flag on and off, fix the crash.
slavapestov
force-pushed
the
prepared-fuzz
branch
from
eaa38f3 to
69c8d15
Compare
|
@swift-ci Please smoke test |
|
@xedin Here is a new version that I think does what you asked, PTAL. I don't see any differences in diagnostic output and the fuzzer crash passes now just like the previous version. If you can think of any test cases to add, that would great, otherwise I'll go ahead and land this fix. |
| auto argParamMatch = argMatching->second.parameterBindings[i]; | ||
|
|
||
| // FIXME: We're just looking at the first solution's overload here, | ||
| // is that correct? |
There was a problem hiding this comment.
This is fine for now, the whole method expects the first overload with an issue to be generic which is not really correct but we can refactor that separately.
There was a problem hiding this comment.
Eh, I now understand what is going on here - the expectation is that the overload is the same across solutions and only generic parameter bindings are different…
2 participants
This test case crashes when prepared overloads are disabled, but passes when enabled. To avoid messing up tests if we have to turn the flag on and off, fix the crash.