Fix a crash in StackPromotion; case not handled in EscapeAnalysis. #28917
Conversation
|
@swift-ci test |
|
@swift-ci test source compatibility |
|
@slavapestov this fixes our i386 stdlib/asserts bot |
|
Build failed |
|
The first fix I pushed simply checked The rebase should fix it. |
|
@swift-ci test |
|
Build failed |
|
Build failed |
StackPromotion queries EscapeAnalysis. The escape information for the result of an array semantic was missing because it was an ill-formed call. This fix introduces a new check to determine whether a call is well formed so it can be handled consistently everywhere. Fixes <rdar://problem/58113508> Interpreter/SDK/objc_fast_enumeration.swift failed on iphonesimulator-i386 The bug was introduced here: commit 8b926af Author: Andrew Trick <atrick@apple.com> Date: Mon Dec 16 16:04:09 2019 -0800 EscapeAnalysis: Add PointerKind and interior/reference flags The bug can occur when a semantic "array.ininitialized" call (Array.adoptStorage) at the SIL level has direct "release_value" instructions that don't use the tuple_extract values corresponding to the call's returned value. This is part of the tragedy of not supporting multiple call results. When users of the call's result can use either the entire tuple (which is a meaningless value), or the individual tuple extracts, then there's no way to handle calls uniformly. The change that broke this was to remove the special handling of TupleExtract instructions. That special handling was identical to the way that any normal pointer projection is handled. So, as a simplification, the new code just expects that case to be handled naturally as a pointer projection. This case was already guarded by a check to determine whether the TupleExtract was actually the result of an array.uninitialized call, in which case the normal handling does not apply. However, because of the weirdness mentioned above, the handling of "array.ininitialized" may silently bail out. When that happens, the TupleExtract gets neither the special handling, nor the normal handling. Note that the old special handling of TupleExtract was bizarre and relied on corresponding weirdness in the code that handles "array.uninitialized". The new code simply creates a separate graph node for each result of the call, and creates the edges that exactly correspond to load and stores on those results. So, rather than reinstating the previous code, which I can't quite reason about, this fix creates a single check to determine whether a TupleExtract is treated as an "array.uninitialized" result or not, and use that check in both places.
|
@swift-ci test |
|
Build failed |
|
Build failed |
StackPromotion queries EscapeAnalysis. The escape information for the
result of an array semantic was missing because it was an ill-formed
call. This fix introduces a new check to determine whether a call is
well formed so it can be handled consistently everywhere.
Fixes rdar://problem/58113508
Interpreter/SDK/objc_fast_enumeration.swift failed on
iphonesimulator-i386
The bug was introduced here:
commit 8b926af
Author: Andrew Trick atrick@apple.com
Date: Mon Dec 16 16:04:09 2019 -0800
The bug can occur when a semantic "array.ininitialized"
call (Array.adoptStorage) at the SIL level has direct "release_value"
instructions that don't use the tuple_extract values corresponding to
the call's returned value. This is part of the tragedy of not
supporting multiple call results. When users of the call's result can
use either the entire tuple (which is a meaningless value), or the
individual tuple extracts, then there's no way to handle calls
uniformly.
The change that broke this was to remove the special handling of
TupleExtract instructions. That special handling was identical to the
way that any normal pointer projection is handled. So, as a
simplification, the new code just expects that case to be handled
naturally as a pointer projection. This case was already guarded by a
check to determine whether the TupleExtract was actually the result of
an array.uninitialized call, in which case the normal handling does
not apply. However, because of the weirdness mentioned above, the
handling of "array.ininitialized" may silently bail out. When that
happens, the TupleExtract gets neither the special handling, nor the
normal handling.
Note that the old special handling of TupleExtract was bizarre and
relied on corresponding weirdness in the code that handles
"array.uninitialized". The new code simply creates a separate graph
node for each result of the call, and creates the edges that exactly
correspond to load and stores on those results. So, rather than
reinstating the previous code, which I can't quite reason about, this
fix creates a single check to determine whether a TupleExtract is
treated as an "array.uninitialized" result or not, and use that check
in both places.