Add all deserialization fatal output to the pretty stack trace #37743

bnbarham · 2021-06-02T06:21:56Z

Rather than outputting diagnostics and to stderr, output all the extra
information added when deserialization fatally fails to the pretty stack
trace instead. Since the pretty stack trace is added to crash logs, this
should avoid the dance of requesting the compiler output

Moves the previous "**** DESERIALIZATION FAILURE ..." output to the
last pretty stack trace line
Removes the module and compiler version notes added to the fatal
diagnostic
Adds a new effective compiler version line for all frontend failure.
Somewhat duplicates the line from the driver, but adds in the
effective version
Adds a new line for the full misc version of the module that failed.
May double up with previous "While reading from ..." lines that are
added in various deserialization methods, but better to have it
twice than not at all

bnbarham · 2021-06-02T06:22:08Z

@swift-ci please test

swift-ci · 2021-06-02T07:48:43Z

Build failed
Swift Test Linux Platform
Git Sha - 126e03facf3f99360b3c51b4ec140743c763f576

bnbarham · 2021-06-02T08:09:32Z

@swift-ci please test

swift-ci · 2021-06-02T08:20:35Z

Build failed
Swift Test Linux Platform
Git Sha - ae066483a5e08a003bed299675cf081091f70ed9

swift-ci · 2021-06-02T09:56:52Z

Build failed
Swift Test OS X Platform
Git Sha - ae066483a5e08a003bed299675cf081091f70ed9

xymus · 2021-06-02T17:36:15Z

lib/Serialization/ModuleFileSharedCore.cpp

+  os << "module '" << Name << "' with full misc version '" << MiscVersion
+      << "'";
+  if (Bits.IsAllowModuleWithCompilerErrorsEnabled)
+    os << " (built while allowing compiler errors)";


Could we use the flag instead here built with -experimental-allow-module-with-compiler-errors? It would be more precise and we could add more relevant flags to it in the future.

Sure, sounds good 👍

xymus · 2021-06-02T17:37:10Z

test/Frontend/crash-in-user-code.swift

@@ -12,6 +12,7 @@
 // CHECK: Stack dump:
 // CHECK-NEXT: Program arguments:
 // CHECK-NEXT: Swift version
+// CHECK-NEXT: Compiling with Swift version


This is part of the existing output right?

No, which also answers your question below - the driver current outputs "Swift version ..." but does so before parsing any flags. I added a second version output after flags are passed so that we can pass in the effective version as well, which is the "Compiling with..." line (though note some tests are failing because apparently this can actually be Apple Swift sometimes).

I could remove the first version, but crashes during argument parsing would be missing the version then. Thoughts?

It looks we only want the effective language version out of that line and from what I see swift::getSwiftFullVersion directly prints the value of LangOpts.EffectiveLanguageVersion. How about we bring that logic up and print the language version on its own line only if it's different than getCurrentLanguageVersion?

xymus · 2021-06-02T17:38:46Z

lib/Serialization/ModuleFileSharedCore.cpp

-void ModuleFileSharedCore::fatal(llvm::Error error) {
-  logAllUnhandledErrors(std::move(error), llvm::errs(),
-                        "\n*** DESERIALIZATION FAILURE (please include this "
-                        "section in any bug report) ***\n");


It's a good change but I feel nostalgic with the idea of losing the part the nobody read (please include this section in any bug report)!

I think the main issue is that most of the time the compiler output wouldn't even be seen - ie. there'd be a crash and it gets reported based on the crash log and not the output.

xymus · 2021-06-02T17:40:12Z

lib/FrontendTool/FrontendTool.cpp

+    os << "Compiling with " <<
+        version::getSwiftFullVersion(LangOpts.EffectiveLanguageVersion);
+    if (LangOpts.AllowModuleWithCompilerErrors)
+      os << " while allowing modules with compiler errors enabled";


Here too we could list the flag instead.

xymus · 2021-06-02T17:51:49Z

lib/Serialization/Deserialization.cpp

-        }
-        getContext().Diags.diagnose(
-            SourceLoc(), diag::serialization_compatibility_version_mismatch,
-            effectiveVersionBuffer, Core->Name, compatVersionBuffer);


Do we still have the information about the Swift language compatibility version of the module somewhere in the output? This note has been useful to me at least once to quickly explain a deserialization failure due to changes in ClangImporter behaviors between two Swift versions.

See above re. the effective version, as an example output:

Stack dump: 0. Program arguments: ...... 1. Swift version 5.5-dev (LLVM <hash>, Swift <hash>) 2. Compiling with Swift version 5.5-dev effective-4.2 (LLVM <hash>, Swift <hash>) 3. While evaluating request TypeCheckSourceFileRequest(source_file "<stdin>") 4. While type-checking statement at [<stdin>:1:13 - line:1:21] RangeText="_ = Sub." 5. While type-checking expression at [<stdin>:1:13 - line:1:21] RangeText="_ = Sub." 6. While evaluating request QualifiedLookupRequest(0x7fa27c81ce00 TopLevelCodeDecl line=1, {Lib.(file).Sub}, 'disappearingMethod', { NL_ProtocolMembers, NL_RemoveNonVisible, NL_RemoveOverridden, NL_OnlyTypes }) 7. While evaluating request DirectLookupRequest(directly looking up 'disappearingMethod' on Lib.(file).Sub with options { }) 8. While loading members for 'Sub' (in module 'Lib') 9. *** DESERIALIZATION FAILURE *** module 'Lib' with full misc version '5.5(4.1.50)/Swift version 5.5-dev (LLVM <hash>, Swift <hash>)' could not find 'disappearingMethod()' in parent class

So here we can see the version being compiled is effective-4.2 (2.) and the module was compiled with (4.1.50) (9.)

xymus

Thank you for this!

bnbarham · 2021-06-03T03:40:54Z

To give an example of the full stderr that's now output:

<unknown>:0: error: fatal error encountered while reading from module 'Lib'; please submit a bug report (https://swift.org/contributing/#reporting-bugs) and include the project
Please submit a bug report (https://swift.org/contributing/#reporting-bugs) and include the project and the crash backtrace.
Stack dump:
0.	Program arguments: ...
1.	Swift version 5.5-dev (LLVM <hash>, Swift <hash>)
2.	Compiling with Swift version 5.5-dev effective-4.2 (LLVM <hash>, Swift <hash>) and -experimental-allow-module-with-compiler-errors
3.	While evaluating request TypeCheckSourceFileRequest(source_file "<stdin>")
4.	While type-checking statement at [<stdin>:1:13 - line:1:21] RangeText="_ = Sub."
5.	While type-checking expression at [<stdin>:1:13 - line:1:21] RangeText="_ = Sub."
6.	While evaluating request QualifiedLookupRequest(0x7fc86e810e00 TopLevelCodeDecl line=1, {Lib.(file).Sub}, 'disappearingMethod', { NL_ProtocolMembers, NL_RemoveNonVisible, NL_RemoveOverridden, NL_OnlyTypes })
7.	While evaluating request DirectLookupRequest(directly looking up 'disappearingMethod' on Lib.(file).Sub with options {  })
8.	While loading members for 'Sub' (in module 'Lib')
9.	*** DESERIALIZATION FAILURE ***
module 'Lib' with full misc version '5.5(4.1.50)/Swift version 5.5-dev (LLVM <hash>, Swift <hash>)' (built with -experimental-allow-module-with-compiler-errors)
could not find 'disappearingMethod()' in parent class

As I mentioned in one of the comments, I'm not a huge fan of doubling up the compiler version, @xymus do you think it's worth keeping the one before arguments are parsed? Also, assuming we have program arguments there's also the double up with the -experimental-allow-module-with-compiler-errors there and the one in Compiling with ... and -experimental-allow-module-with-compiler-errors.

bnbarham · 2021-06-03T03:44:36Z

@swift-ci please test

Rather than outputting diagnostics and to stderr, output all the extra information added when deserialization fatally fails to the pretty stack trace instead. Since the pretty stack trace is added to crash logs, this should avoid the dance of requesting the compiler output - Moves the previous "**** DESERIALIZATION FAILURE ..." output to the last pretty stack trace line - Removes the module and compiler version notes added to the fatal diagnostic - Adds a new effective compiler version line for all frontend failure. Somewhat duplicates the line from the driver, but adds in the effective version - Adds a new line for the full misc version of the module that failed. May double up with previous "While reading from ..." lines that are added in various deserialization methods, but better to have it twice than not at all

bnbarham · 2021-06-03T22:34:04Z

Okay, changed to just give the effective version and removed the superfluous "-experimental-allow-module-with-errors":

<unknown>:0: error: fatal error encountered while reading from module 'Lib'; ...
Stack dump:
0.	Program arguments: ...
1.	Swift version 5.5-dev (LLVM <hash>, Swift <hash>)
2.	Compiling with effective version 4.2
...
8.	While loading members for 'Sub' (in module 'Lib')
9.	*** DESERIALIZATION FAILURE ***
module 'Lib' with full misc version '5.5(4.1.50)/Swift version 5.5-dev (LLVM <hash>, Swift <hash>)' (built with -experimental-allow-module-with-compiler-errors)
could not find 'disappearingMethod()' in parent class

@swift-ci please test

swift-ci · 2021-06-04T02:12:34Z

Build failed
Swift Test OS X Platform
Git Sha - 599ba8b

bnbarham · 2021-06-04T02:14:53Z

@swift-ci please test macOS platform

bnbarham requested a review from xymus June 2, 2021 06:21

bnbarham force-pushed the prefer-pretty-stack-trace branch from 126e03f to ae06648 Compare June 2, 2021 08:07

xymus reviewed Jun 2, 2021

View reviewed changes

xymus approved these changes Jun 2, 2021

View reviewed changes

xedin approved these changes Jun 2, 2021

View reviewed changes

bnbarham force-pushed the prefer-pretty-stack-trace branch from ae06648 to 452c3e4 Compare June 3, 2021 03:44

bnbarham force-pushed the prefer-pretty-stack-trace branch from 452c3e4 to 599ba8b Compare June 3, 2021 22:32

bnbarham merged commit a68ce7b into swiftlang:main Jun 4, 2021

bnbarham deleted the prefer-pretty-stack-trace branch June 4, 2021 22:36

bnbarham mentioned this pull request Jun 4, 2021

[5.5] Add all deserialization fatal output to the pretty stack trace #37796

Merged

Add all deserialization fatal output to the pretty stack trace #37743

Add all deserialization fatal output to the pretty stack trace #37743

Uh oh!

Conversation

bnbarham commented Jun 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bnbarham commented Jun 2, 2021

Uh oh!

swift-ci commented Jun 2, 2021

Uh oh!

bnbarham commented Jun 2, 2021

Uh oh!

swift-ci commented Jun 2, 2021

Uh oh!

swift-ci commented Jun 2, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bnbarham Jun 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xymus left a comment

Choose a reason for hiding this comment

Uh oh!

bnbarham commented Jun 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bnbarham commented Jun 3, 2021

Uh oh!

bnbarham commented Jun 3, 2021

Uh oh!

swift-ci commented Jun 4, 2021

Uh oh!

bnbarham commented Jun 4, 2021

Uh oh!

Uh oh!

bnbarham commented Jun 2, 2021 •

edited

Loading

bnbarham Jun 2, 2021 •

edited

Loading

bnbarham commented Jun 3, 2021 •

edited

Loading