[S] Change in dart/sdk[main]: [vm,dyn_modules] Fix evaluation expression function self-reference.
0 views
Skip to first unread message
Tess Strickland (Gerrit)
Nov 25, 2025, 12:37:58 PM (7 days ago) Nov 25
to Alexander Markov, rev...@dartlang.org, vm-...@dartlang.org
Attention needed from Alexander Markov
Tess Strickland voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Alexander Markov
Submit Requirements:
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tess Strickland (Gerrit)
Nov 26, 2025, 8:14:30 AM (6 days ago) Nov 26
to Commit Queue, Alexander Markov, rev...@dartlang.org, vm-...@dartlang.org
Attention needed from Alexander Markov
Tess Strickland voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Alexander Markov
Submit Requirements:
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alexander Markov (Gerrit)
Dec 1, 2025, 10:30:57 AM (yesterday) Dec 1
to Tess Strickland, Alexander Markov, Commit Queue, rev...@dartlang.org, vm-...@dartlang.org
Attention needed from Tess Strickland
Alexander Markov voted and added 1 comment![]( "Open in Gerrit")
Votes added by Alexander Markov
| Code-Review | +1 |
1 comment
File runtime/vm/bytecode_reader.cc
Line 1091, Patchset 3 (Latest):
} else if (class_name.ptr() == Symbols::DebugClassName().ptr()) {Alexander Markov . unresolved
We can recognize expression evaluation class by checking `library.url() == Symbols::EvalSourceUri().ptr()` without depending on the class name. VM didn't look at the class name before, and CFE can freely change it, so the new check looks inconsistent with kernel loader and more fragile than it needs to be.
Also, by checking the library uri only in case of not found class we make sure to avoid overhead of an extra check for every referenced class.
Related details
Attention is currently required from:
- Tess Strickland
Submit Requirements:
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alexander Markov (Gerrit)
Dec 1, 2025, 11:52:02 AM (23 hours ago) Dec 1
to Tess Strickland, Alexander Markov, Commit Queue, rev...@dartlang.org, vm-...@dartlang.org
Attention needed from Tess Strickland
Alexander Markov added 1 comment![]( "Open in Gerrit")
Commit Message
Related details
Attention is currently required from:
- Tess Strickland
Submit Requirements:
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tess Strickland (Gerrit)
8:10 AM (3 hours ago) 8:10 AM
to Alexander Markov, Commit Queue, rev...@dartlang.org, vm-...@dartlang.org
Attention needed from Alexander Markov
Tess Strickland voted and added 3 comments![]( "Open in Gerrit")
Votes added by Tess Strickland
| Auto-Submit | +1 |
| Commit-Queue | +1 |
3 comments
Patchset-level comments
File-level comment, Patchset 5 (Latest):
Tess Strickland . resolved
Thanks, Alex! Removing the new symbol invalidated the +1, so PTAL again.
Commit Message
Line 33, Patchset 3:
Alexander Markov . resolved
Tess StricklandNit: `Fixes https://github.com/dart-lang/sdk/issues/61948`
Done
File runtime/vm/bytecode_reader.cc
Line 1091, Patchset 3:
} else if (class_name.ptr() == Symbols::DebugClassName().ptr()) {Alexander Markov . resolved
We can recognize expression evaluation class by checking `library.url() == Symbols::EvalSourceUri().ptr()` without depending on the class name. VM didn't look at the class name before, and CFE can freely change it, so the new check looks inconsistent with kernel loader and more fragile than it needs to be.
Also, by checking the library uri only in case of not found class we make sure to avoid overhead of an extra check for every referenced class.
Attention is currently required from:
- Alexander Markov
Submit Requirements:
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alexander Markov (Gerrit)
9:33 AM (2 hours ago) 9:33 AM
to Tess Strickland, Alexander Markov, Commit Queue, rev...@dartlang.org, vm-...@dartlang.org
Attention needed from Tess Strickland
Alexander Markov voted![]( "Open in Gerrit")
Attention is currently required from:
- Tess Strickland
Submit Requirements:
Code-Owners
Code-Review
Commit-Message-Has-TEST
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Commit Queue (Gerrit)
9:33 AM (2 hours ago) 9:33 AM
to Tess Strickland, Alexander Markov, rev...@dartlang.org, vm-...@dartlang.org
Commit Queue submitted the change![]( "Open in Gerrit")
Change information
Commit message:
[vm,dyn_modules] Fix evaluation expression function self-reference.
If the evaluation expression function is for an expression in the scope
of a generic function with a parameter whose type includes one of its
type parameters, then the graph for the parameter type will contain a
self reference to the evaluation expression function.
Normally, this self reference would be resolved via the scoped_function_
field. However, while scoped_function_class_ is set to the class containing the expression evaluation function (as expected),
if ReadObject() encounters that class, it returns the "real" class
the evaluation expression should be evaluated in. This means the
self-reference isn't detected.
Add an additional check for the right name/class for the expression
evaluation function.
TEST=pkg/vm_service/test/evaluate_function_type_parameters_test
Cq-Include-Trybots: luci.dart.try:vm-linux-release-x64-try,vm-dyn-linux-debug-x64-try,vm-dyn-mac-debug-arm64-try
Change-Id: Id04735059d00d964a8f00885857d3255c18ecf99
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/464520
Commit-Queue: Alexander Markov <alexm...@google.com>
Reviewed-by: Alexander Markov <alexm...@google.com>
Auto-Submit: Tess Strickland <sstr...@google.com>
Files:
- M runtime/vm/bytecode_reader.cc
Change size: S
Delta: 1 file changed, 15 insertions(+), 11 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Alexander Markov
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages