This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Pass] Register a signal handler when generating crash reproducers.
ClosedPublic

Authored by rriddle on Apr 16 2020, 11:13 AM.

Details

Summary

The current implementation uses CrashRecoveryContext, but this only supports recovering in a certain number of cases. This revision adds a signal handler to support even more situations.

This revision was able to properly generate a reproducer for a segfault in the Inliner, that the current recovery couldn't.

Depends On D78314

Diff Detail

Event Timeline

rriddle created this revision.Apr 16 2020, 11:13 AM

Can you clarify a bit in the description what kind of situation this allows to handle that weren't possible before?

Also it isn't clear to me why do we need a stack of RecoveryReproducerContext? (maybe you can describe this part in the code comments)

rriddle updated this revision to Diff 260715.Apr 28 2020, 11:45 AM

Resolve comments

Can you clarify a bit in the description what kind of situation this allows to handle that weren't possible before?

Added an example. This allowed for me to generate a reproducer during an Inliner segfault that I couldn't otherwise.

Also it isn't clear to me why do we need a stack of RecoveryReproducerContext? (maybe you can describe this part in the code comments)

There could be multiple pass managers running simultaneously, with different MLIRContexts. Unless we guarantee that there is only ever one context at the same time(something I would be against), we need a stack to handle this case. There is also another situation right now, caused by the lack of dynamic pass pipelines ;), where a pass manager could get run within another. Though regardless of when we fix the second problem, the first one will still exist.

rriddle edited the summary of this revision. (Show Details)Apr 28 2020, 11:48 AM

There could be multiple pass managers running simultaneously, with different MLIRContexts.

Make sense, but then I have another question (see inline)

mlir/lib/Pass/Pass.cpp
617

So this seems to enforce that we always delete the stack of RecoveryReproducerContext in the order in which they were created, how does it happen? If multiple threads from the application are running different PassManagers for example?

rriddle updated this revision to Diff 260834.Apr 28 2020, 9:35 PM

Resolve comments

rriddle marked 2 inline comments as done.Apr 28 2020, 9:35 PM
rriddle added inline comments.
mlir/lib/Pass/Pass.cpp
617

Ooops, fixed.

mehdi_amini accepted this revision.Apr 28 2020, 10:24 PM
This revision is now accepted and ready to land.Apr 28 2020, 10:24 PM
rriddle updated this revision to Diff 261058.Apr 29 2020, 3:08 PM
rriddle marked an inline comment as done.

Add missing signals include

This revision was automatically updated to reflect the committed changes.