When library_filters were given, we used to prefill the script_table_,
then just assume that any scripts not in the script_table_ must have
been filtered out. We wrote it this way to avoid checking the filters
in every GetScriptIndex call. But in some cases (eg mixins),
lib.LoadedScripts() can miss some scripts, so they'd be incorrectly
omitted from the table.
The new implementation lazy loads the scripts, the same way it works
when there are no library_filters. Skipped scripts are still placed in
the script_table_, but given an index of -1, and omitted from
script_table_entries_.
Bug: https://github.com/dart-lang/sdk/issues/49887
Change-Id: Ide938ddfa9a3750c72c615e296b1a23875e46ab8
TEST=CI (also manually tested that the bug is fixed, but it's a really fiddly setup and I'm not sure how to put it in a unit test. See bug for details)
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/260076
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Liam Appelbe <liama@google.com>
The source report RPC needs to be able to distinguish branch coverage vs
normal coverage token positions. So encode normal positions as 2 * pos
and branch positions as 2 * pos + 1.
TEST=CI
Change-Id: I247796d1eb4c97947f9b38aa56bc318ecea09425
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/224320
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Liam Appelbe <liama@google.com>
Redirecting factories are never hit in source coverage report because
front-end replaces calls to redirecting factories with calls to their
targets. This change excludes redirecting factories from source
coverage reports.
TEST=vm/cc/SourceReport_Regress95008_RedirectingFactory
Fixes https://github.com/flutter/flutter/issues/95008
Change-Id: I0f6af291f7ee0c042521c92063092f990426b995
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/223600
Reviewed-by: Liam Appelbe <liama@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Sets and gets to late variables count as uncovered even when they are
covered, because they have internal branches that throw exceptions.
These exceptions should not be hit in well formed code, so they
shouldn't count towards coverage totals.
These exceptions are all thrown using static functions on the internal
LateError class, so we can just ignore any calls that target a function
on this class.
Bug: https://github.com/dart-lang/coverage/issues/341
Fixes: https://github.com/dart-lang/coverage/issues/341
Change-Id: I1dfa5d64b7a2030121d84d7aad7c7a1649b900a8
TEST=Added a unit test
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/221740
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Liam Appelbe <liama@google.com>
More precisely, ignore private constructors with no parameters, on
abstract classes with no subclasses, no non-static members, and no
non-static functions (other than the constructor).
This is an idiom used to make static utility classes, and these
constructors are not intended to be used. So counting them as coverage
misses incorrectly lowers the coverage percentage.
Fixes: https://github.com/dart-lang/sdk/issues/47021
TEST=Added unit test
Change-Id: Ib94dd939c4637d5c04fe45009fc92be435e164da
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/220225
Commit-Queue: Liam Appelbe <liama@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Every known user of the coverage report just wants the line numbers. At
the moment they have to do a second RPC to get the Script object, so
they can translate the token positions into line numbers.
Slower test times with coverage are usually caused by the extra time it
takes to run the RPCs. So reporting the line number directly will halve
the time it takes to get coverage, for most users.
Bug: https://github.com/flutter/flutter/issues/86722
Change-Id: I7b8d436669713ebc7b7096790a02593b9cb94dda
TEST=CI
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/211081
Commit-Queue: Liam Appelbe <liama@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Currently we have things called XPtr which are not what you get from ptr().
Old world:
handle->raw() returns RawObject* (tagged)
raw_obj->ptr() returns RawObject* (untagged)
After 6fe15f6df9:
handle->raw() returns ObjectPtr
obj_ptr->ptr() returns ObjectLayout*
New world:
handle->ptr() returns ObjectPtr
obj_ptr->untag() returns UntaggedObject*
TEST=ci
Change-Id: I6c7f34014cf20737607caaf84979838300d12df2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/149367
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
As part of making the compiler and other subsystems independent
of `Isolate` we have to move various state from `Isolate` to
`IsolateGroup` (or to another place).
The access of `ObjectStore::closure_functions()` was spread out over our
codebase, including from `Isolate`. This CL moves it to a centralized
place - ClosureFunctionsCache.
Issue https://github.com/dart-lang/sdk/issues/36097
TEST=Pure refactoring - relying on existing test coverage.
Change-Id: I3b282623f07614cdc2c2a863bd5b9d6c7748d36f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/177130
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
As part of making the compiler and other subsystems independent
of `Isolate` we have to move various state from `Isolate` to
`IsolateGroup`.
The class_table and object_store were already moved to `IsolateGroup`.
This CL only replaces usages of `Isolate::{object_store,class_table}`
with the equivalent in `IsolateGroup`.
Issue https://github.com/dart-lang/sdk/issues/36097
TEST=Pure refactoring - relying on existing test coverage.
Change-Id: I34a0682d715b054d6c5faff077a513980f59a348
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/177126
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Instead, split each old use into the following cases:
* If the TokenPosition value is expected to be a real token position,
then use TokenPosition::Pos().
* If the TokenPosition is being serialized in some way, then use
TokenPosition::Serialize() and change the place where the
TokenPosition is recreated to use TokenPosition::Deserialize().
* If the value of the TokenPosition is being printed for debugging
purposes, then just use TokenPosition::ToCString() instead.
That is, we try to pin down when token positions are expected to
be real vs. when other types of token positions can be found.
Another source of possible error when using token positions is to
convert between synthetic and real token positions. In the past,
synthetic token positions may have been based off real token positions,
but that is no longer the case. Thus, all methods that allow
that conversion have been removed, and instead there is a new static
method for constructing synthetic tokens from valid nonces.
This CL also makes it so that Pos() and relational operators on token
positions are only defined on real token positions, to avoid any
assumptions about what the value encoded in synthetic positions mean. To
help with cases where non-real token positions may occur, four helper
methods are added:
* TokenPosition::Min(a, b): A static method that returns the smallest
real token position provided. If neither `a` or `b` are real,
returns `a`.
* TokenPosition::Max(a, b): A static method that returns the largest
real token position provided. If neither `a` or `b` are real,
returns `a`.
* TokenPosition::IsWithin(start, end): Determines whether `this` falls
between `start` and `end` (inclusive). If `this` is non-real, then it
must be either `start` or `end` if synthetic, otherwise false.
Otherwise, we mimic the old style of range checking, which means that
non-real starts and ends are treated as less than every real token.
* TokenPosition::CompareForSorting(other): Unlike the relational
operators, provides a comparison between any types of token positions
for purposes such as sorting. Currently only used in the profiler.
It also changes TokenPosition::ToCString() to tag synthetic token
positions, so they can be distinguished from real ones at a glance.
TEST=Existing test suite on trybots, especially the observatory tests
which make heavy use of the debugger and the unit tests for the
profiler/source report modules.
Bug: https://github.com/dart-lang/sdk/issues/44436
Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-linux-debug-x64-try,vm-kernel-linux-release-x64-try,vm-kernel-nnbd-linux-release-x64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-nnbd-linux-release-x64-try,vm-kernel-linux-product-x64-try,vm-kernel-precomp-linux-product-x64-try
Change-Id: Ic06aa0bc7a1f0fbac7257ed22ca5e7e0ccd7f3f2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/174924
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
This CL makes use of the now included constant constructor coverage
in the dill file.
It works like this:
* When the CFE evaluates constants, every constant constructor
invocation evaluated saves the reference to the constructor in the
`Source` (from the Components uri to source table) for the callers
Library.
* This data is loaded into the VM in a "raw" format.
* When a request for coverage comes in, the VM - on top of the normal
coverage processing - goes through all scripts to find constant
constructor coverage for the requested script and offset. Note that
all scripts must be checked because library A can have evaluated a
constructor from library B - so even if only coverage for library B
was requested, library A has to be checked.
For all constructors found the start and end position is reported as
covered. Note that this does not mark any initializes and there are
(at least currently) no good way of marking which initializes were
evaluated (because it has to be stable across edits even when the
`advanced invalidation feature` is enabled).
* Note that the reason for the coverage to work on references - as
hinted above - is because we want it to be stable across hot reloads
even if/when advanced invalidation is enabled. This means, that
library A cannot record "positional coverage" for library B because
library B might get (for instance) new comments that will make any old
offsets invalid. By using references we always lookup in the current
world and use the correct offsets.
https://github.com/dart-lang/sdk/issues/38934
TEST=Existing test suite, new tests for the new coverage added.
Change-Id: I29531247a4b91a99d9a459cfdefbb9798e9c948f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/175246
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
This reverts commit 5370c56c80.
Reason for revert: Failures on
dartk-android-product-arm
dartk-android-product-arm
dartkp-linux-release-arm-qemu
like this:
sizeof(ScriptLayout) got 64, Script_InstanceSize expected 56
../../runtime/vm/dart.cc: 160: error: CheckOffsets failed. Try updating offsets by running ./tools/run_offsets_extractor.sh
sizeof(ScriptLayout) got 64, AOT_Script_InstanceSize expected 56
../../runtime/vm/dart.cc: 160: error: CheckOffsets failed. Try updating offsets by running ./tools/run_offsets_extractor.sh
Original change's description:
> [VM] Read and report constant constructor coverage from dill
>
> This CL makes use of the now included constant constructor coverage
> in the dill file.
>
> It works like this:
> * When the CFE evaluates constants, every constant constructor
> invocation evaluated saves the reference to the constructor in the
> `Source` (from the Components uri to source table) for the callers
> Library.
> * This data is loaded into the VM in a "raw" format.
> * When a request for coverage comes in, the VM - on top of the normal
> coverage processing - goes through all scripts to find constant
> constructor coverage for the requested script and offset. Note that
> all scripts must be checked because library A can have evaluated a
> constructor from library B - so even if only coverage for library B
> was requested, library A has to be checked.
> For all constructors found the start and end position is reported as
> covered. Note that this does not mark any initializes and there are
> (at least currently) no good way of marking which initializes were
> evaluated (because it has to be stable across edits even when the
> `advanced invalidation feature` is enabled).
> * Note that the reason for the coverage to work on references - as
> hinted above - is because we want it to be stable across hot reloads
> even if/when advanced invalidation is enabled. This means, that
> library A cannot record "positional coverage" for library B because
> library B might get (for instance) new comments that will make any old
> offsets invalid. By using references we always lookup in the current
> world and use the correct offsets.
>
> https://github.com/dart-lang/sdk/issues/38934
>
> TEST=Existing test suite, new tests for the new coverage added.
>
> Change-Id: I925963d1a9b9907efe621c72deb7348fa3be5ae8
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/171949
> Commit-Queue: Jens Johansen <jensj@google.com>
> Reviewed-by: Ben Konyi <bkonyi@google.com>
> Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
TBR=vegorov@google.com,bkonyi@google.com,jensj@google.com
Change-Id: I5187e6749d59ded250ec0933a94db0536485b70a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/174470
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
This CL makes use of the now included constant constructor coverage
in the dill file.
It works like this:
* When the CFE evaluates constants, every constant constructor
invocation evaluated saves the reference to the constructor in the
`Source` (from the Components uri to source table) for the callers
Library.
* This data is loaded into the VM in a "raw" format.
* When a request for coverage comes in, the VM - on top of the normal
coverage processing - goes through all scripts to find constant
constructor coverage for the requested script and offset. Note that
all scripts must be checked because library A can have evaluated a
constructor from library B - so even if only coverage for library B
was requested, library A has to be checked.
For all constructors found the start and end position is reported as
covered. Note that this does not mark any initializes and there are
(at least currently) no good way of marking which initializes were
evaluated (because it has to be stable across edits even when the
`advanced invalidation feature` is enabled).
* Note that the reason for the coverage to work on references - as
hinted above - is because we want it to be stable across hot reloads
even if/when advanced invalidation is enabled. This means, that
library A cannot record "positional coverage" for library B because
library B might get (for instance) new comments that will make any old
offsets invalid. By using references we always lookup in the current
world and use the correct offsets.
https://github.com/dart-lang/sdk/issues/38934
TEST=Existing test suite, new tests for the new coverage added.
Change-Id: I925963d1a9b9907efe621c72deb7348fa3be5ae8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/171949
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Since the transition from Source based to Kernel based frontend to the
Dart VM, we have not supported callable redirecting factories. This has
caused all removed code in this CL to be unused and untested!
The support for reifing redirection related information would mainly be
used by embedder / dart:mirrors. Right now its not used by anyone and
the kernel support was never implemented.
I suggest we remove the unused and untested code until a decision has
been made that we actually want to support it - in which case it can be
properly implemented, possibly by re-using some of the deleted code in
this CL.
TEST=CL removes unused code, no test result changes.
Change-Id: I7ed45c85b4efcc1e81ce44cbe08bb555e52101b9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/166853
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Refactor Kernel::CollectTokenPositionsFor, but do not try to factorize code for
fields and functions, as too many handles would need to be passed around.
Bytecode reading creates duplicate script objects requiring scripts to be
matched by their url rather than by their raw address when collecting token
positions in bytecode. However, url comparison is not yet used, because
collecting token positions in the duplicated script 'dart:core/map.dart'
(in default mode) causes crashes in kernel reading. Will revisit.
Fix service test valid_source locations_test.dart and make sure fields
are properly reloaded, except fields injected by fasta.
Remove unused argument 'record' of KernelReaderHelper::ReadPosition().
Handle bytecode stub frame in IsAsyncMachinery() tester in debugger.
Change-Id: Ifbddcaec00e0696f7de13c5cf1e74380b31d2419
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/112402
Commit-Queue: Régis Crelier <regis@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Fix these issues:
Function breakpoint could wrongly be set before the first DebugCheck opcode.
Loaded bytecode closures were not found by FindCompiledFunctions.
Improve verbose debug output by providing address and offset of breakpoint.
Change-Id: Icfeb1204348af313de21cab358603c11e89d2503
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/111309
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Régis Crelier <regis@google.com>
While stepping, do not stop more than once in the same token range.
Adjusted tests expecting multiple stops at same location.
Fixed emitted source position of DebugCheck opcode in async op.
Ignore all possible breakpoint positions until the first DebugCheck opcode of
the function, so as to not stop in the prologue where arguments are not yet
accessible.
Fixed look up of context level in debugger.
Use receiver to look up async op closure in frame instead of named captured
variable.
Made StoreContextVar a 'debug checked' opcode.
Implemented rewind of interpreted frames.
Marked get_source_report_test as (temporarily) failing in status file.
Removed fixed and passing regression test from status file.
Change-Id: I9396d3cf4b6accc85753bb6d227b7c068acc8df7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/109558
Commit-Queue: Régis Crelier <regis@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
The CheckStack bytecode does not denote a debug point anymore after the
introduction of the DebugCheck bytecode.
Fix caller/callee determination in mixed mode.
Factorize some code.
Change-Id: Ide1e0bbad022a83e6113243dc996396f9f5d2f3c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/108565
Commit-Queue: Régis Crelier <regis@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Ryan Macnak