In this CL, Debugger::FindCompiledFunctions always just returns
Error::null(), but when eager compilation of functions is implemented
(https://dart-review.googlesource.com/c/sdk/+/338740), it will introduce
the possibility of Debugger::FindCompiledFunctions truly returning
errors. So, this CL adds the error propagation logic in advance to
avoid making the eager function compilation CL too large.
TEST=this is effectively just a refactor, so CI
Change-Id: Ibedbebf19fed306c8b93c98ba71b7cc57b90e174
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/369161
Reviewed-by: Alexander Aprelev <aam@google.com>
It is dead code. Breakpoint requests that only have a line number are just handled by passing column_number=-1 to SetBreakpointAtLineCol.
TEST=CI
Change-Id: I44fb9f805c364edb074bf9cdbd3fcbe3888b3bff
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/369160
Reviewed-by: Alexander Aprelev <aam@google.com>
TEST=verified that
pkg/vm_service/test/breakpoint_resolves_immediately_in_compiled_field_initializer_test.dart
fails without the changes in this CL and passes with them, verified that
none of the existing debugger tests got broken by this CL
Change-Id: I6acb5576a80e5d633b012c866fe90bf13d2c1ba6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/369162
Commit-Queue: Derek Xu <derekx@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
compiled prior to the latest reload
TEST=verified that
pkg/vm_service/test/breakpoint_resolution_after_reloading_test.dart
fails without the changes in this CL and passes with them, verified that
none of the existing debugger tests got broken by this CL
Change-Id: Ie816a3ad65a17ef9f497f209f2203d679728cb75
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366860
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Derek Xu <derekx@google.com>
This CL adds @pragma('vm:awaiter-link') in various places in
Stream implementation to facilitate unwinding and expands
async unwinding logic with more information about
Stream internals.
At the same time be more conservative when checking if an
exception thrown from async method handled: failing to unwind
the stack fully creates situations when we incorrectly report
caught exceptions as uncaught, which frustrates users.
To distinguish stream subscriptions with and without error
handlers we add a state bit. Otherwise, it looks like all
subscriptions have error handlers because if no error
handler is installed we eagerly install error handler forwarding
the error to `Zone.handleUncaughtError`.
Fixes https://github.com/dart-lang/sdk/issues/53334
Fixes https://github.com/dart-lang/sdk/issues/54788
Fixes https://github.com/dart-lang/sdk/issues/47985
TEST=runtime/vm/dart/awaiter_stacks/stream_methods_test.dart,pkg/vm_service/test/pause_on_unhandled_async_exceptions6_test.dart,pkg/vm_service/test/pause_on_unhandled_async_exceptions7_test.dart
CoreLibraryReviewExempt: No behavioral change. Async changes reviewed by lrhn@
Cq-Include-Trybots: luci.dart.try:vm-aot-android-release-arm64c-try,vm-aot-android-release-arm_x64-try,vm-aot-dwarf-linux-product-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-optimization-level-linux-release-x64-try
Change-Id: Ic51f926867092dd0adbe801b753f57c357c7ace2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/322720
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Lasse Nielsen <lrn@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
Previously, when taking a tear-off, a separate Context object was
allocated in order to capture receiver. Now, receiver is stored directly
in the Closure object in the 'context' field. This saves 1 object
allocation per tear-off and makes tear-offs cheaper compared to
explicit closures which can share context with other closures.
Benchmarks in AOT mode:
x64:
TearOff.NotInlined +40%
TearOff.NotInlined.InTry +43%
TearOff.Inlined.InTry +47%
arm64:
TearOff.NotInlined +27-43%
TearOff.NotInlined.InTry +29-43%
TearOff.Inlined.InTry +58-94%
arm64c:
TearOff.NotInlined +71%
TearOff.NotInlined.InTry +72%
TearOff.Inlined.InTry +96%
TEST=ci
Issue: https://github.com/dart-lang/sdk/issues/54808
Change-Id: I3ad95e8a8a4fc23f856bbc0fe238da58a9d25b8d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/350945
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Our async unwinding code used to only respect `catchError`
which occured between async frames and ignored trailing
`catchError` like in the code below
Future(...).catchError((_) { /* handle error */ });
This CL also simplifies how we communicate the presence of the
exception handler to the debugger: the code in the debugger did
not actually care about which frame catches the error (for async
handlers), so we don't need to precisely mark async gaps with
`has_catch_error` flag. Instead we have a single boolean
produced by unwinding which signals whether there we encountered
an asynchronous error handler or not.
Fixes https://github.com/flutter/flutter/issues/141882
TEST=service/pause_on_unhandled_async_exceptions5
Change-Id: Id6f6a97ee5444c197b2c621f68d1e47082fc8997
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/350320
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
This is a reland of commit e727f4ecbd
We allow the arguments array in
EvaluateCompiledExpressionHelper to contain
Object::optimized_out().ptr() when we have determined that the receiver
has been optimized out but isn't used by the compiled expression. So, I
have moved the assertion that checks for optimized out arguments from
EvaluateCompiledExpressionHelper to
Instance::EvaluateCompiledExpression.
TEST=vm-linux-debug-x64 tryjob
Original change's description:
> [VM/Debugger] Ignore optimized out variables when building scope in ActivationFrame::BuildParameters
>
> TEST=pkg/vm_service/test/evaluate_optimized_out_variable_test.dart
>
> Issue: https://github.com/dart-lang/sdk/issues/53996
> Change-Id: I5e6f0b2c02455af73c2108e6996039c95d3f1f31
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/347940
> Reviewed-by: Alexander Markov <alexmarkov@google.com>
> Commit-Queue: Derek Xu <derekx@google.com>
Change-Id: Id6df0b0b3e0d26239068041126b034e9469b87af
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/347945
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Derek Xu <derekx@google.com>
Acquire reload opreation scope when deoptimizing the world to ensure locks can be acquired for compilation.
Set up scope for operations that can be run while the world is deoptimized and stopped to avoid races.
Ensure code stays unoptizimed when single stepping, prevent other isolates to reoptimize it.
TEST=DeoptimizeFramesWhenSettingBreakpoint
BUG=https://github.com/flutter/flutter/issues/140878
Change-Id: Id4c891bd585d42365fd3a60cfb9a4869892c2b03
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345743
Reviewed-by: Martin Kustermann <kustermann@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>
The value of best_token_pos was
exact_token_pos.Pos() - (requested_column - *best_column), but this
value is guaranteed to be the same as best_fit_pos.
TEST=CI
Change-Id: I96f21cc45ebbac94dfe35fa39af922a227e1fde6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/338080
Commit-Queue: Derek Xu <derekx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
RwLock, rather that SafepointRwLock, is okay to use for these two breakpoint_locations and single_stepping_set locks because they are short-lived, should not spawn over the safepoint.
TEST=ci
Change-Id: Iba83291978ba7980d0c6fd0a0a2cf2174c801359
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/323202
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Alexander Aprelev <aam@google.com>
When evaluating an expressino at a breakpoint that's inside a closure,
the closure may refer to anything it closes over. That includes the
`this` from an enclosing method.
So depending on whether a closure's parent chain is an instance method
or a static method, the expression evaluation function is going to be an
instance method or a static method.
=> We walk up the parent chain to determine the correct enclosing class.
=> This avoids making a Type object with a cid from a top-level class (see [0])
Handling this correctly will now try to get the `this` when invoking an
eval function that has an enclosing instance method. Though we may often
have "<optimized out>" the `this` (e.g. due to not capturing it in
closure context chain).
=> We still allow running the expression evaluation function in this
case, but only if the expression being evaluated doesn't access `this`.
A similar issue occurs when trying to use variables in the eval
expression that the closure didn't capture. This results in a confusing
CFE compile-time error. This is a separate issue and tracked in [1].
=> We update the test to distinuish the cases that this CL makes passing
and those that are failing due to [1].
Fixes [0] https://github.com/dart-lang/sdk/issues/53061
See also [1] https://github.com/dart-lang/sdk/issues/53087
TEST=Fixes part of service/evaluate_activation_test failures
Change-Id: I3bb24e7338c7b2f12d5340311d944cb59a455641
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/317540
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
The main contribution of this CL is unification of disparate
handling of various functions like `Future.timeout`,
`Future.wait`, `_SuspendState.createAsyncCallbacks` and
`_SuspendState._createAsyncStarCallback` into a single
`@pragma('vm:awaiter-link')` which allows Dart developers
to specify where awaiter unwinder should look for the next
awaiter.
For example this allows unwinding to succeed for the code like this:
Future<int> outer(Future<int> inner) {
@pragma('vm:awaiter-link')
final completer = Completer<int>();
inner.then((v) => completer.complete(v));
return completer.future;
}
This refactoring also ensures that we preserve information
(including Function & Code objects) required for awaiter
unwinding across all modes (JIT, AOT and AOT with DWARF stack
traces). This guarantees users will get the same information
no matter which mode they are running in. Previously
we have been disabling awaiter_stacks tests in some AOT
modes - which led to regressions in the quality of produced
stacks.
This CL also cleans up relationship between debugger and awaiter
stack returned by StackTrace.current - which makes stack trace
displayed by debugger (used for stepping out and determinining
whether exception is caught or not) and `StackTrace.current`
consistent.
Finally we make one user visible change to the stack trace:
awaiter stack will no always include intermediate listeners
created through `Future.then`. Previously we would sometimes
include these listeners at the tail of the stack trace,
which was inconsistent.
Ultimately this means that code like this:
Future<int> inner() async {
await null; // asynchronous gap
print(StackTrace.current); // (*)
return 0;
}
Future<int> outer() async {
int process(int v) {
return v + 1;
}
return await inner().then(process);
}
void main() async {
await outer();
}
Produces stack trace like this:
inner
<asynchronous suspension>
outer.process
<asynchronous suspension>
outer
<asynchronous suspension>
main
<asynchronous suspension>
And when stepping out of `inner` execution will stop at `outer.process`
first and the next step out will bring execution to `outer` next.
Fixes https://github.com/dart-lang/sdk/issues/52797
Fixes https://github.com/dart-lang/sdk/issues/52203
Issue https://github.com/dart-lang/sdk/issues/47985
TEST=ci
Bug: b/279929839
CoreLibraryReviewExempt: CL just adds @pragma to facilitate unwinding
Cq-Include-Trybots: luci.dart.try:vm-aot-linux-product-x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-dwarf-linux-product-x64-try
Change-Id: If377d5329d6a11c86effb9369dc603a7ae616fe7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/311680
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
The [PatchClass] no longer needs to refer to a original [Class] and a
patched [Class] objects: Since the CFE does handling of patches the VM
only has a single [Class] object, so those two fields are always the
same.
=> Change `PatchClass::{origin_class,patched_class} -> `PatchClass::wrapped_class`
We also remove the `Field::Origin()` / `Function::origin()` methods
as they return the same as the `Owner()` would return.
TEST=ci
Change-Id: Iec0849f6ffc2026760dad89a9bcf07e9469bc8b6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/311840
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
The name is more accurately reflects what this stack trace
contains: despite what causal implies it does *not* actually
reflect the stack which initiated the async operation. Instead
it contains the chain of listeners which will run when
the async operation completes - its awaiters.
TEST=ci
Change-Id: Ie7309c9b1c39246e0fd4c14f7b9c515bcdfbbe10
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/311384
Reviewed-by: Derek Xu <derekx@google.com>
We no longer rewrite suspendable function into closures nested
inside the original function, so the whole synthetic async breakpoint
machinery is not needed.
Refactor `Breakpoint` class to make one-shot and per-closure
separate properties of breakpoint, which allows creating
one-shot-per-closure breakpoints.
TEST=existing service tests
Change-Id: I208afe13f36efa40f5746e44867bd24684cf5f03
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310601
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Derek Xu <derekx@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
This field was supposed[1] to replace Stack.asyncCausalFrames but IDEs
and other service protocol users never adopted it.
The mechanism for collecting awaiterFrames and asyncCausalFrames
was originally considerably different, but we have since unified
both[2] after we switched to lazy async stack traces[3].
Today the only differences between the two are:
- asyncCausalFrames represens async gaps explicitly, while
awaiterFrames does not;
- when asynchronous function is running its synchronous part
before the first suspension awaiterFrames will still represent
the corresponding frame as an asynchronous activation, while
asyncCausalFrames will treat the same frame as regular frame.
Consequently having this field does not add any value.
This CL removes it from the response and updates all tests to
test against asyncCausalFrames.
[1]: https://codereview.chromium.org/2692803006/
[2]: https://dart-review.googlesource.com/c/sdk/+/167561
[3]: https://github.com/dart-lang/sdk/issues/37668
Tested: pkg/vm_service, service, service_2
Bug: https://github.com/dart-lang/sdk/issues/51763
Change-Id: If2b035a8840047423b8ed8ce3b5d81155e9f38d0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/301062
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Our compiler shouldn't depend on current isolate, since it can
run on any isolate within an IG.
Doing this change, reveals two existing dependencies on current
isolate from compiler
- resolving native symbols in unoptimized compilations
- issuing of debug events for breakpoints
For the former we'll re-enter the currently active isolate that
triggered unoptimized compilation.
=> We may want to change that embedder API to not be based on
handles and instead give embedder a simple `const char*`.
For the ladder we'll enter the isolate corresponding to the
breakpoint debug event to be issued. We are at place where
all mutators are stopped, so that does seem okish.
=> Future could remove this by making Object Id Ring per-IG
Issue https://github.com/dart-lang/sdk/issues/48523
TEST=service_2/break_on_function_many_child_isolates_test/dds
Change-Id: Id246db5972ae505e82f637ce04bb2302bed76257
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/278901
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
* Merge ClassTable and SharedClassTable back together;
* Simplify handling of multiple arrays growing in sync;
* Refactor how reload deals with ClassTable.
The last change is the most important because it makes it
much easier to reason about the code. We move away from
copying bits and pieces of the class table and shared
class table into reload contexts.
Having two class table fields in the isolate group makes
it easier to reason about. One field contains program
class table (one modified by kernel loader and accessed
by various program structure cid lookups) and heap
walk class table (used by GC visitors). Normally these
two fields point to the same class table, but during
hot reload we temporary split them apart: original
class table is kept as a heap walk class table, while
program class table is replaced by a clone and updated
by reload.
If reload succeeds we drop original class table and
set program class table as heap walk one.
If reload fails we drop the program class table and
restore original one from heap walk table.
TEST=ci
Cq-Include-Trybots: luci.dart.try:vm-kernel-reload-linux-release-x64-try,vm-kernel-reload-linux-debug-x64-try,vm-kernel-reload-rollback-linux-debug-x64-try,vm-kernel-reload-rollback-linux-release-x64-try,vm-kernel-linux-debug-x64-try,vm-kernel-precomp-tsan-linux-release-x64-try,vm-kernel-tsan-linux-release-x64-try,vm-kernel-precomp-asan-linux-release-x64-try,vm-kernel-asan-linux-release-x64-try
Change-Id: I8b66259fcc474dea7dd2af063e4772df99be06c4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/258361
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
During 'await', when registering 'then' and 'error' callbacks in
a custom Zone, the stack trace should be collected synchronously
as there are no awaiters yet, but the suspended function and its
callers are still on the stack.
This change corrects the new implementation of async to collect
these stack traces synchronously.
Class StackZoneSpecification from package:stack_trace relies on the
stack traces during callback registration in order to provide more
detailed chain of async stack traces via Chain.capture.
Without this change package:stack_trace captured truncated Chains.
Issue: https://github.com/dart-lang/sdk/issues/48378
Issue: https://github.com/flutter/flutter/issues/105340
TEST=vm/dart/causal_stacks/zone_callback_stack_traces_test
Change-Id: Iee348aa5c9abb4126f6c5d8215a6dc2cee57e124
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/247620
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
The new implementation is based on suspend/resume stubs and doesn't
use desugaring of async functions on kernel AST.
Previously, new implementation of async/async* was only supported in
AOT mode. This change adds all necessary bits for the JIT mode:
* Suspending variable-length frames (for unoptimized code).
* Handling of Code and pool pointers in Dart stack frames.
* OSR.
* Deoptimization.
* Hot reload.
* Debugger.
The new implementation is not enabled in JIT mode yet.
Design doc: go/compact-async-await.
TEST=ci
Issue: https://github.com/dart-lang/sdk/issues/48378
Change-Id: I477d6684bdce7cbc1edb179ae2271ff598b7dcc5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/246081
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
This CL
* Makes :async_op only have two parameters: We take advantage
of the fact that errors not only have exception != null but
also stacktrace != null.
* Makes :async_op have no optional parameters. This reduces
code size significantly.
* Removes unused parameter in _awaitHelper calls
* Wrap the then callback instead of the error callback. (needed
to make optional parameters required)
This results in 2% code savings on a big g3 app.
The size of :async_op shrinks on average by 11%
TEST=ci
Change-Id: I38d5fba4ebebc780b48dac5aa6a250d2c7952bfd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/233362
Reviewed-by: Slava Egorov <vegorov@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
In general any async function can return X or Future<X>. Though the
future implementation has to do different things depending on which case
we're in. It does so by using a `<obj> is Future<T>` test. This test is very
expensive if many different classes flow into `<obj>`.
Though most functions do not return `Future` objects from async
functions. We can determine that statically by looking if any returned
expression could be a future. If not, we can bypass the `is Future<T>`
test entirely.
Issue https://github.com/dart-lang/sdk/issues/48226
Issue https://github.com/dart-lang/sdk/issues/48235
TEST=pkg/front_end/testcases/general/async_function_returns_future_or.dart
Change-Id: If655bdbdddc214dd7b12be9905b3c788252547d0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/230662
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
Alexander Markov