Skip to content

[5.4] fix silent crash when output from compiler is too large #3482

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 14, 2021
Merged

[5.4] fix silent crash when output from compiler is too large #3482

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions Sources/Build/BuildDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,18 +553,22 @@ public final class BuildDelegate: BuildSystemDelegate, SwiftCompilerOutputParser
}

if let output = message.standardOutput {
// Scoop out any errors from the output, so they can later be passed to the advice provider in case of failure.
let regex = try! RegEx(pattern: #".*(error:[^\n]*)\n.*"#, options: .dotMatchesLineSeparators)
for match in regex.matchGroups(in: output) {
self.errorMessagesByTarget[parser.targetName] = (self.errorMessagesByTarget[parser.targetName] ?? []) + [match[0]]
}

// first we want to print the output so users have it handy
if !self.isVerbose {
self.progressAnimation.clear()
}

self.outputStream <<< output
self.outputStream.flush()

// next we want to try and scoop out any errors from the output (if reasonable size, otherwise this will be very slow),
// so they can later be passed to the advice provider in case of failure.
if output.utf8.count < 1024 * 10 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the 8 MB limit calculated in some way, or is it arbitrarily chosen? I don't have an opinion either way. I'm just curious.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

base don experiments with SIL output in crashes. @abertelrud is working on a separate PR to fix this parsing logic and this will be removed then. this PR is focused on "damage control" given that this caused issues in 5.4

let regex = try! RegEx(pattern: #".*(error:[^\n]*)\n.*"#, options: .dotMatchesLineSeparators)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this parsing of error messages be replaced by using TSCUtility.SerializedDiagnostics, which I think is more performant and less reliant on the error messages' structure/format staying unchanged?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry I just saw that this PR is a cherry-pick of #3478, and my comment would've been more appropriate there, or in the form of a new PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abertelrud is working on a separate PR to fix this parsing logic, this PR is focused on "damage control" given that this caused issues in 5.4

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using TSCUtility.SerializedDiagnostics is a great idea, and we should definitely adopt that across SwiftPM. That will be a larger effort than fixing this, also, linker diagnostics are unfortunately not yet part of the serialized diagnostics, so it wouldn't help this particular case. They are very source-code focused and currently (AFAIK) only emitted by Clang and Swiftc. But it's definitely where we should move to in SwiftPM, I believe, with tool-specific adapters that favor serialized diagnostics where supported, and that fall back on parsing stderr where we have to.

for match in regex.matchGroups(in: output) {
self.errorMessagesByTarget[parser.targetName] = (self.errorMessagesByTarget[parser.targetName] ?? []) + [match[0]]
}
}
}
}
}
Expand Down