[vm/service]: VM plumbing for Throw.forErrorHandling

Use the new Throw.forErrorHandling AST flag to ignore synthetic error
handling statements in coverage. When the flag is found on a throw node,
all the TokenPositions on the child nodes are set to kNoSource.

Questions for reviewers:
- Are there any VM use cases that need the real TokenPosition here?
- Is there a better way of encoding this flag in the TokenPosition?
- Should we add a new sentinel TokenPosition instead?

Bug: https://github.com/dart-lang/sdk/issues/54005
Fixes: https://github.com/dart-lang/sdk/issues/53519
Fixes: https://github.com/dart-lang/sdk/issues/54941
Change-Id: Ic44fe2fa0359188b890d5ed762e3ff8c593c850d
TEST=SourceReport_Regress53519_Destructuring
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/353920
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Liam Appelbe <liama@google.com>

runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc

@@ -1338,6 +1338,14 @@ void StreamingFlowGraphBuilder::block_expression_depth_dec() {
   --flow_graph_builder_->block_expression_depth_;
 }
 
+void StreamingFlowGraphBuilder::synthetic_error_handler_depth_inc() {
+  ++synthetic_error_handler_depth_;
+}
+
+void StreamingFlowGraphBuilder::synthetic_error_handler_depth_dec() {
+  --synthetic_error_handler_depth_;
+}
+
 intptr_t StreamingFlowGraphBuilder::CurrentTryIndex() {
   return flow_graph_builder_->CurrentTryIndex();
 }
@@ -1432,6 +1440,14 @@ intptr_t StreamingFlowGraphBuilder::PeekArgumentsCount() {
   return PeekUInt();
 }
 
+TokenPosition StreamingFlowGraphBuilder::ReadPosition() {
+  TokenPosition position = KernelReaderHelper::ReadPosition();
+  if (synthetic_error_handler_depth_ > 0 && position.IsReal()) {
+    position = TokenPosition::Synthetic(position.Pos());
+  }
+  return position;
+}
+
 LocalVariable* StreamingFlowGraphBuilder::LookupVariable(
     intptr_t kernel_offset) {
   return flow_graph_builder_->LookupVariable(kernel_offset);
@@ -4026,7 +4042,11 @@ Fragment StreamingFlowGraphBuilder::BuildThrow(TokenPosition* p) {
 
   Fragment instructions;
 
-  ReadByte();  // read flags.
+  const uint8_t flags = ReadByte();
+  const bool is_synthetic_error_handler = (flags & kThrowForErrorHandling) != 0;
+  if (is_synthetic_error_handler) {
+    synthetic_error_handler_depth_inc();
+  }
 
   instructions += BuildExpression();  // read expression.
 
@@ -4036,6 +4056,10 @@ Fragment StreamingFlowGraphBuilder::BuildThrow(TokenPosition* p) {
   instructions += ThrowException(position);
   ASSERT(instructions.is_closed());
 
+  if (is_synthetic_error_handler) {
+    synthetic_error_handler_depth_dec();
+  }
+
   return instructions;
 }
 

runtime/vm/compiler/frontend/kernel_binary_flowgraph.h

@@ -113,6 +113,8 @@ class StreamingFlowGraphBuilder : public KernelReaderHelper {
   intptr_t block_expression_depth();
   void block_expression_depth_inc();
   void block_expression_depth_dec();
+  void synthetic_error_handler_depth_inc();
+  void synthetic_error_handler_depth_dec();
   intptr_t CurrentTryIndex();
   intptr_t AllocateTryIndex();
   LocalVariable* CurrentException();
@@ -135,6 +137,8 @@ class StreamingFlowGraphBuilder : public KernelReaderHelper {
   const TypeArguments& PeekArgumentsInstantiatedType(const Class& klass);
   intptr_t PeekArgumentsCount();
 
+  TokenPosition ReadPosition();
+
   // See BaseFlowGraphBuilder::MakeTemporary.
   LocalVariable* MakeTemporary(const char* suffix = nullptr);
   Fragment DropTemporary(LocalVariable** variable);
@@ -460,6 +464,7 @@ class StreamingFlowGraphBuilder : public KernelReaderHelper {
   CallSiteAttributesMetadataHelper call_site_attributes_metadata_helper_;
   Object& closure_owner_;
   intptr_t num_ast_nodes_ = 0;
+  intptr_t synthetic_error_handler_depth_ = 0;
 
   friend class KernelLoader;
 

runtime/vm/source_report_test.cc

@@ -1292,6 +1292,54 @@ main() {
       buffer);
 }
 
+ISOLATE_UNIT_TEST_CASE(SourceReport_Regress53519_Destructuring) {
+  // WARNING: This MUST be big enough for the serialized JSON string.
+  const int kBufferSize = 1024;
+  char buffer[kBufferSize];
+  const char* kScript = R"(
+main() {
+  destructure({
+    'hello': 'world',
+    'count': [1, 2, 3],
+  });
+}
+
+String destructure(Map<String, dynamic> map) {
+  final {'hello': world, 'count': count} = map;
+  return 'Hello $world, count: $count';
+}
+)";
+
+  Library& lib = Library::Handle();
+  lib ^= ExecuteScript(kScript);
+  ASSERT(!lib.IsNull());
+  const Script& script =
+      Script::Handle(lib.LookupScript(String::Handle(String::New("test-lib"))));
+
+  SourceReport report(SourceReport::kCoverage);
+  JSONStream js;
+  report.PrintJSON(&js, script);
+  const char* json_str = js.ToCString();
+  ASSERT(strlen(json_str) < kBufferSize);
+  ElideJSONSubstring("classes", json_str, buffer);
+  ElideJSONSubstring("libraries", buffer, buffer);
+  EXPECT_STREQ(
+      "{\"type\":\"SourceReport\",\"ranges\":["
+
+      // main
+      "{\"scriptIndex\":0,\"startPos\":1,\"endPos\":78,\"compiled\":true,"
+      "\"coverage\":{\"hits\":[1,12,24,61],\"misses\":[]}},"
+
+      // destructure
+      "{\"scriptIndex\":0,\"startPos\":81,\"endPos\":216,\"compiled\":true,"
+      "\"coverage\":{\"hits\":[81,144,160,214],\"misses\":[]}}],"
+
+      // Only one script in the script table.
+      "\"scripts\":[{\"type\":\"@Script\",\"fixedId\":true,\"id\":\"\","
+      "\"uri\":\"file:\\/\\/\\/test-lib\",\"_kind\":\"kernel\"}]}",
+      buffer);
+}
+
 ISOLATE_UNIT_TEST_CASE(SourceReport_BranchCoverage_if) {
   // WARNING: This MUST be big enough for the serialized JSON string.
   const int kBufferSize = 1024;

tests/standalone/regress_53519_test.dart

@@ -0,0 +1,27 @@
+// Copyright (c) 2024, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// Test that throws during destructuring have the correct stack trace.
+// Regression test for https://github.com/dart-lang/sdk/issues/53519
+
+import 'package:expect/expect.dart';
+
+String destructure(Map<String, dynamic> map) {
+  final {'hello': world, 'count': count} = map;
+  return 'Hello $world, count: $count';
+}
+
+main() {
+  try {
+    destructure({
+      'hello': 'world',
+      // No count entry, so the destructuring fails.
+    });
+  } catch (e, s) {
+    print(s);
+    // Expect that the stack trace contains an entry for the destructure
+    // function at line 11.
+    Expect.isTrue(s.toString().contains(RegExp(r'destructure \(.*:11:3\)')));
+  }
+}