[analyzer/ffi] Crash

[dcharkes]

Log: https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8757721845749192753/+/u/run_test.dart_for_docs_shard_and_subshard_None/test_stdout

dartdoc:stderr: #0      ExpressionExtension.typeOrThrow (package:analyzer/src/dart/ast/extensions.dart:170:7)
dartdoc:stderr: #1      FfiVerifier._validateRefPrefixedIdentifier (package:analyzer/src/generated/ffi_verifier.dart:1628:34)
dartdoc:stderr: #2      FfiVerifier.visitPrefixedIdentifier (package:analyzer/src/generated/ffi_verifier.dart:325:11)
dartdoc:stderr: #3      PrefixedIdentifierImpl.accept (package:analyzer/src/dart/ast/ast.dart:14487:50)
dartdoc:stderr: #4      CommentReferenceImpl.visitChildren (package:analyzer/src/dart/ast/ast.dart:3425:17)
dartdoc:stderr: #5      RecursiveAstVisitor.visitCommentReference (package:analyzer/dart/ast/visitor.dart:936:10)
dartdoc:stderr: #6      CommentReferenceImpl.accept (package:analyzer/src/dart/ast/ast.dart:3421:50)
dartdoc:stderr: #7      NodeListImpl.accept (package:analyzer/src/dart/ast/ast.dart:13022:20)
dartdoc:stderr: #8      CommentImpl.visitChildren (package:analyzer/src/dart/ast/ast.dart:3344:17)
dartdoc:stderr: #9      RecursiveAstVisitor.visitComment (package:analyzer/dart/ast/visitor.dart:930:10)
dartdoc:stderr: #10     CommentImpl.accept (package:analyzer/src/dart/ast/ast.dart:3340:50)
dartdoc:stderr: #11     AnnotatedNodeImpl.visitChildren (package:analyzer/src/dart/ast/ast.dart:188:17)
dartdoc:stderr: #12     ClassDeclarationImpl.visitChildren (package:analyzer/src/dart/ast/ast.dart:2903:11)
dartdoc:stderr: #13     RecursiveAstVisitor.visitClassDeclaration (package:analyzer/dart/ast/visitor.dart:918:10)
dartdoc:stderr: #14     FfiVerifier.visitClassDeclaration (package:analyzer/src/generated/ffi_verifier.dart:165:11)
dartdoc:stderr: #15     ClassDeclarationImpl.accept (package:analyzer/src/dart/ast/ast.dart:2899:50)
dartdoc:stderr: #16     NodeListImpl.accept (package:analyzer/src/dart/ast/ast.dart:13022:20)
dartdoc:stderr: #17     CompilationUnitImpl.visitChildren (package:analyzer/src/dart/ast/ast.dart:3662:21)
dartdoc:stderr: #18     RecursiveAstVisitor.visitCompilationUnit (package:analyzer/dart/ast/visitor.dart:942:10)
dartdoc:stderr: #19     CompilationUnitImpl.accept (package:analyzer/src/dart/ast/ast.dart:3655:50)
dartdoc:stderr: #20     LibraryAnalyzer._computeVerifyErrors (package:analyzer/src/dart/analysis/library_analyzer.dart:420:10)
dartdoc:stderr: #21     LibraryAnalyzer._computeDiagnostics.<anonymous closure> (package:analyzer/src/dart/analysis/library_analyzer.dart:271:7)
dartdoc:stderr: #22     _LinkedHashMapMixin.forEach (dart:collection-patch/compact_hash.dart:633:13)
dartdoc:stderr: #23     LibraryAnalyzer._computeDiagnostics (package:analyzer/src/dart/analysis/library_analyzer.dart:270:11)
dartdoc:stderr: #24     LibraryAnalyzer.analyze (package:analyzer/src/dart/analysis/library_analyzer.dart:101:5)
dartdoc:stderr: #25     AnalysisDriver._analyzeFile.<anonymous closure> (package:analyzer/src/dart/analysis/driver.dart:1370:11)
dartdoc:stderr: <asynchronous suspension>
dartdoc:stderr: #26     PerformanceLog.runAsync (package:analyzer/src/dart/analysis/performance_logger.dart:50:14)
dartdoc:stderr: <asynchronous suspension>
dartdoc:stderr: #27     AnalysisDriver._getResolvedLibrary (package:analyzer/src/dart/analysis/driver.dart:1751:5)
dartdoc:stderr: <asynchronous suspension>
dartdoc:stderr: #28     AnalysisDriver.performWork (package:analyzer/src/dart/analysis/driver.dart:1193:7)
dartdoc:stderr: <asynchronous suspension>
dartdoc:stderr: #29     AnalysisDriverScheduler._run (package:analyzer/src/dart/analysis/driver.dart:2313:7)
dartdoc:stderr: <asynchronous suspension>

Context:

• Roll Flutter Engine from f3d48be76630 to 367874519359 (3 revisions) flutter/flutter#142379
• Revert Dart to Version 3.4.0-82.0.dev flutter/engine#50131

[dcharkes]

FfiVerifier._validateRefPrefixedIdentifier (package:analyzer/src/generated/ffi_verifier.dart:1628

https://dart-review.googlesource.com/c/sdk/+/342763 removed that line.

before revert: 'dart_revision': 1d0b890 3.4.0-85.0 last commit 148d5ac
after revert: 'dart_revision': d0b48a0 3.4.0-82.0 last commit b6647fd

b6647fd...148d5ac So it must be the dart-ffi commit.

Since the offending line was removed (fixed) in that PR, we might have version skew and are using an older analyzer to analyze the code which then trips over new code added.

[dcharkes]

dartdoc:stderr: dartdoc 8.0.2 (/b/s/w/ir/x/w/.pub-cache/global_packages/dartdoc/bin/dartdoc.dart-3.4.0-83.0.dev.snapshot) failed: Bad state: No type: StructPointer

So this used 3.4.0-83.0 422d048 which also contains the commit c2e15cf.

I am a bit surprised about 3.4.0-83.0.dev.snapshot, that should not be able to produce that stacktrace because that exact version of the code was replaced in the last commit for 3.4.0-83.0.

[dcharkes]

Executing: (cd "/b/s/w/ir/x/w/flutter/dev/docs" ; /b/s/w/ir/x/w/flutter/bin/dart global run --enable-asserts dartdoc --output /b/s/w/ir/x/w/flutter/dev/docs/doc/flutter --allow-tools --no-validate-links --link-to-source-excludes /b/s/w/ir/x/w/flutter/bin/cache --link-to-source-root /b/s/w/ir/x/w/flutter --link-to-source-uri-template [https://github.com/flutter/flutter/blob/master/%f%#L%l](https://github.com/flutter/flutter/blob/master/%25f%25#L%25l)% --inject-html --use-base-href --header /b/s/w/ir/x/w/flutter/dev/docs/styles.html --header /b/s/w/ir/x/w/flutter/dev/docs/analytics-header.html --header /b/s/w/ir/x/w/flutter/dev/docs/survey.html --header /b/s/w/ir/x/w/flutter/dev/docs/snippets.html --header /b/s/w/ir/x/w/flutter/dev/docs/opensearch.html --footer /b/s/w/ir/x/w/flutter/dev/docs/analytics-footer.html --footer-text /b/s/w/ir/x/w/flutter/dev/docs/footer.html --allow-warnings-in-packages Flutter,platform_integration,flutter_localizations,flutter,flutter_driver,integration_test,flutter_test,flutter_goldens,flutter_web_plugins --exclude-packages analyzer,args,barback,csslib,flutter_goldens,flutter_goldens_client,front_end,fuchsia_remote_debug_protocol,glob,html,http_multi_server,io,isolate,js,kernel,logging,mime,mockito,node_preamble,plugin,shelf,shelf_packages_handler,shelf_static,shelf_web_socket,utf,watcher,yaml --exclude dart:io/network_policy.dart,package:Flutter/temp_doc.dart,package:http/browser_client.dart,package:intl/intl_browser.dart,package:matcher/mirror_matchers.dart,package:quiver/io.dart,package:quiver/mirrors.dart,package:vm_service_client/vm_service_client.dart,package:web_socket_channel/html.dart --favicon /b/s/w/ir/x/w/flutter/dev/docs/favicon.ico --package-order flutter,Dart,platform_integration,flutter_test,flutter_driver --auto-include-dependencies)

@zanderso What version is /b/s/w/ir/x/w/flutter/bin/dart in this command? Apparently it is not the version Dart is being rolled to? I would have expected 3.4.0-85.0.

Also, it's being reported as 3.4.0-83.0 but has a stacktrace that can only occur in 3.4.0-82.0 and earlier.

Could you elaborate on how this is set up?

If we have to support version skew during the rolling, we should probably revert https://dart-review.googlesource.com/c/sdk/+/342763, land the fix in the ffi_verifier.dart first, wait for that to roll and then reland https://dart-review.googlesource.com/c/sdk/+/342763. (I did the fix in ffi_verifier because some example somewhere else in the doc triggers it.)

[zanderso]

I'm skeptical that there's any version skew. The logs come from an engine -> framework roll where the most recent version of Dart was 3.4.0-83.0, so the dartdoc snapshot version in the logs makes sense to me. /b/s/w/ir/x/w/flutter/bin/dart is using the version of Dart being rolled to. The Flutter SDK uses an exact copy of the Dart SDK. 3.4.0-83.0 contained only one commit: https://dart.googlesource.com/sdk.git/+/c2e15cff6d1d63f94772ecf41f32f56656ed49db. Is it possible that the Dart SDK build used a stale version of that generated analyzer file when building dartdoc?

[dcharkes]

Ah right, the stack trace was from the firs roll that failed. Here's one from the last Dart version before the revert:

dartdoc:stderr: dartdoc 8.0.2 (/b/s/w/ir/x/w/.pub-cache/global_packages/dartdoc/bin/dartdoc.dart-3.4.0-85.0.dev.snapshot) failed: Bad state: No type: StructPointer
dartdoc:stderr: #0      ExpressionExtension.typeOrThrow (package:analyzer/src/dart/ast/extensions.dart:170:7)
dartdoc:stderr: #1      FfiVerifier._validateRefPrefixedIdentifier (package:analyzer/src/generated/ffi_verifier.dart:1628:34)
dartdoc:stderr: #2      FfiVerifier.visitPrefixedIdentifier (package:analyzer/src/generated/ffi_verifier.dart:325:11)

Source: https://github.com/flutter/flutter/runs/20954221253

Is it possible that the Dart SDK build used a stale version of that generated analyzer file when building dartdoc?

Dartdoc is DEPSed in in the Dart SDK.

And Dartdoc depends on a published version of the analyzer: https://github.com/dart-lang/dartdoc/blob/main/pubspec.yaml (version skew if the pubspec.yaml is obeyed)

As far as I understand, the dependencies for dartdoc in pubspec.yaml get overridden in the Dart SDK. The fact that it is overridden means that the applied fix in the analyzer is actually picked up by dart doc inside the Dart SDK build. But apparently in the dart inside flutter it is not?

@srawlins What version of the analyzer is supposed to be in a dart doc? The one in pkg/analyzer in the SDK or the one that is pointed to in the pubspec.yaml in the package?

[dcharkes]

Inside the Dart SDK the analyzer used in dart doc is definitely not version skewed.

Reintroducing the problem:

$ git diff
diff --git a/pkg/analyzer/lib/src/generated/ffi_verifier.dart b/pkg/analyzer/lib/src/generated/ffi_verifier.dart
index a392ca74062..64c1c289fff 100644
--- a/pkg/analyzer/lib/src/generated/ffi_verifier.dart
+++ b/pkg/analyzer/lib/src/generated/ffi_verifier.dart
@@ -1651,7 +1651,7 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
   /// Validate the invocation of the extension method
   /// `Pointer<T extends Struct>.ref`.
   void _validateRefPrefixedIdentifier(PrefixedIdentifier node) {
-    var targetType = node.prefix.staticType;
+    var targetType = node.typeOrThrow;
     if (!_isValidFfiNativeType(targetType,
         allowVoid: false, allowEmptyStruct: true)) {
       final AstNode errorNode = node;

$ dart doc --sdk-docs --dry-run --verbose
Using the following options: [--sdk-docs, --output=doc/api, --resources-dir=/Users/dacoharkes/dart-sdk/sdk/xcodebuild/ReleaseARM64/dart-sdk/bin/resources/dartdoc/resources, --no-validate-links, --no-generate-docs, --verbose-warnings, --show-stats]

dartdoc 8.0.4-wip (/Users/dacoharkes/dart-sdk/sdk/dartdev) failed: Bad state: No type: StructPointer.ref
#0      ExpressionExtension.typeOrThrow (package:analyzer/src/dart/ast/extensions.dart:151:7)
#1      FfiVerifier._validateRefPrefixedIdentifier (package:analyzer/src/generated/ffi_verifier.dart:1654:27)
#2      FfiVerifier.visitPrefixedIdentifier (package:analyzer/src/generated/ffi_verifier.dart:334:11)

main branch without edits:

$ tools/build.py -mrelease create_platform_sdk runtime && dart doc --sdk-docs --dry-run --verbose
[...]
[20/20] STAMP obj/create_platform_sdk.stamp
buildtools/ninja/ninja -C xcodebuild/ReleaseARM64 -j1000 -l64 create_platform_sdk runtime done.
The build took 28.998 seconds
Using the following options: [--sdk-docs, --output=doc/api, --resources-dir=/Users/dacoharkes/dart-sdk/sdk/xcodebuild/ReleaseARM64/dart-sdk/bin/resources/dartdoc/resources, --no-validate-links, --no-generate-docs, --verbose-warnings, --show-stats]
Found 0 warnings and 0 errors.

So there must be something different about the dart that's built inside the Flutter build.

[zanderso]

The Dart SDK is not built inside the Flutter build. It is only copied to the Flutter gs bucket.

[dcharkes]

The Dart SDK is not built inside the Flutter build. It is only copied to the Flutter gs bucket.

Doesn't that cause version skew? Or is that exactly the version that is being rolled in? Then what's the DEPS file in flutter engine for that has git hashes?

[dcharkes]

From the stack trace it's clear that we have version skew somewhere, so let's revert to unblock the Dart into Flutter roll.

[jason-simmons]

The script used in this test is running the Dartdoc 8.0.2 package, not the dart doc command from the Dart SDK version associated with the engine:

https://github.com/flutter/flutter/blob/master/dev/bots/docs.sh#L114
https://github.com/flutter/flutter/blob/master/dev/tools/create_api_docs.dart#L557

[dcharkes]

The script used in this test is running the Dartdoc 8.0.2 package, not the dart doc command from the Dart SDK version associated with the engine:

https://github.com/flutter/flutter/blob/master/dev/bots/docs.sh#L114 https://github.com/flutter/flutter/blob/master/dev/tools/create_api_docs.dart#L557

Thanks @jason-simmons! That explains it.

Should we consider running dart doc instead? Or is this by design? (E.g. flutter_tools uses published deps via pubspec instead of DEPS like flutter/engine and Dart.)

[zanderso]

cc @gspencergoog IIRC is familiar with the dartdoc setup?

[gspencergoog]

The version of dartdoc is pinned for the same reason that all the other dependencies are pinned: so that we can roll it back to an earlier version to unblock the Flutter build if something incompatible happens with the dartdoc package.

We could just use the dart doc command if we want to bind the Dart SDK version and the dartdoc version pinning together. I don't see that as too big of an issue, although it means that instead of just rolling back dartdoc, we'd have to roll back the Dart SDK version.

I'm not really sure how the dart doc command and the dartdoc package differ. I assume that it's just using a version of the package pinned by the Dart SDK build, but works the same otherwise.

[dcharkes]

The version of dartdoc is pinned for the same reason that all the other dependencies are pinned: so that we can roll it back to an earlier version to unblock the Flutter build if something incompatible happens with the dartdoc package.

That makes sense.

This also means that package:analyzer is version skewed if used indirectly via package:dartdoc. Which means that CLs like mine break the flutter roll.

It also means that dart doc in Flutter's SDK is a different version than the one being tested on the Flutter CI.

We could just use the dart doc command if we want to bind the Dart SDK version and the dartdoc version pinning together. I don't see that as too big of an issue, although it means that instead of just rolling back dartdoc, we'd have to roll back the Dart SDK version.

Yeah, so this is a trade-off, either

• Flutter can roll back dartdoc individually, or
• Flutter could support atomic updates to the Dart SDK (including analyzer and dartdoc), and the dart doc from the Dart that's rolled into Flutter is identical to the one being tested on the CI.

I'm not sure which one we'd be more comfortable giving up.

If we want to stick with the status quo, we should address at least two things:

1. We definitely have a communication problem between the Dart SDK and usages of the Dart SDK. The Dart SDK with package:analyzer and package:dartdoc does not communicate anywhere (through CI) that version skew should be supported. So if flutter wants to use a different version of dartdoc and analyzer in some parts, we should test this somewhere. @whesse Could we add this to the Dart 3H bot? (And then we should also run the Flutter 3H bot by default on any changes to pkg/analyzer in the Dart SDK, because people (me) won't add it manually.)
2. Branch alignment on Dart+Flutter releases need to manually check that the published version of package:analyzer and package:dartdoc that are pinned in Flutter are identical to the ones in the Dart SDK. cc @whesse.

If we want to change the status quo so that package:analyzer and package:dartdoc roll into Flutter (which they do partially already through dart itself), we should address:

I'm not really sure how the dart doc command and the dartdoc package differ. I assume that it's just using a version of the package pinned by the Dart SDK build, but works the same otherwise.

1. $ dart doc --help != $ dart pub global run dartdoc --help So currently it's not the same. I do think that @jonasfj and @mit-mit are on a quest to make sure that we get better alignment between packages and dart commands.

As for the current roll breakage: I've created a revert and will reland the analyzer fixes first.