Skip to content

[Driver] SR-3175: Include the terminating signal number in the driver output #6787

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.

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

Conversation

matthewcarroll
Copy link
Contributor

@matthewcarroll matthewcarroll commented Jan 13, 2017

  • Add the signal number of the terminated task to the output of the driver on platforms for which the signal number is available. The new key in the parseable driver output is "signal".
  • Add a test to verify that the signal number is emitted.
  • Add documentation for the new "signal" key emitted in the parseable driver output.

https://bugs.swift.org/browse/SR-3175

In this commit, example output of swiftc -Xfrontend -debug-crash-immediately is:

{
"kind": "signalled",
"name": "compile",
"pid": 32822,
"output": "0 swift ...

"error-message": "Illegal instruction: 4",
"signal": 4
<unknown>:0: error: unable to execute command: Illegal instruction: 4
<unknown>:0: error: compile command failed due to signal 4 (use -v to see invocation)
}

* Add the signal number of the terminated task to the output of the driver on platforms for which the signal number is available. The new key in the parseable driver output is "signal".
* Add a test to verify that the signal number is emitted.
* Add documentation for the new "signal" key emitted in the parseable driver output.

https://bugs.swift.org/browse/SR-3175
Diags.diagnose(SourceLoc(), diag::error_command_signalled,
SignalledCmd->getSource().getClassName(), Signal.getValue());
}
else {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: else in line with } please

public:
SignalledMessage(const Job &Cmd, ProcessId Pid, StringRef Output,
StringRef ErrorMsg) : TaskOutputMessage("signalled", Cmd,
StringRef ErrorMsg, Optional<int> Signal) : TaskOutputMessage("signalled", Cmd,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: with the additional parameter, the alignment of the two lines that follow are totally out of whack.

@CodaFi
Copy link
Contributor

CodaFi commented Jan 14, 2017

@swift-ci please smoke test.

@CodaFi
Copy link
Contributor

CodaFi commented Jan 14, 2017

Whoa, an NPE took down the build! // @shahmishal

@CodaFi
Copy link
Contributor

CodaFi commented Jan 14, 2017

@swift-ci please smoke test OS X platform.

@matthewcarroll
Copy link
Contributor Author

@CodaFi,

I merged the branch hughbe:taskqueue-fixes from #6812

@CodaFi
Copy link
Contributor

CodaFi commented Jan 14, 2017

Can you knock off the merge commit either by folding it into the previous one or by cherry-picking @hughbe's branch please?

@matthewcarroll matthewcarroll force-pushed the SR-3175-Include-signal-number-in-parseable-output-message branch from 81bf314 to 09abbfe Compare January 15, 2017 01:53
@shahmishal
Copy link
Member

@swift-ci please smoke test.

1 similar comment
@hughbe
Copy link
Contributor

hughbe commented Jan 16, 2017

@swift-ci please smoke test.

@hughbe
Copy link
Contributor

hughbe commented Jan 18, 2017

Is this PR ready to go? Merging will unblock the Windows (clang and MSVC) build

@matthewcarroll
Copy link
Contributor Author

@hughbe Yes it's ready.

@CodaFi What do you think?

@CodaFi
Copy link
Contributor

CodaFi commented Jan 18, 2017

⛵️

@CodaFi CodaFi merged commit 3eba4fd into swiftlang:master Jan 18, 2017
@matthewcarroll matthewcarroll changed the title SR-3175: Include the terminating signal number in the driver output [Driver] SR-3175: Include the terminating signal number in the driver output Jan 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants