This is an archive of the discontinued LLVM Phabricator instance.

Fix segfault notification in lldb-mi
ClosedPublic

Authored by ki.stfu on Feb 9 2015, 6:23 AM.

Details

Summary

This patch adds system exception handling in lldb-mi + tests.

All tests pass on OS X.

Diff Detail

Event Timeline

ki.stfu updated this revision to Diff 19577.Feb 9 2015, 6:23 AM
ki.stfu retitled this revision from to Fix segfault notification in lldb-mi.
ki.stfu updated this object.
ki.stfu edited the test plan for this revision. (Show Details)
ki.stfu added reviewers: abidh, clayborg, zturner.
ki.stfu added subscribers: abidh, clayborg, zturner, Unknown Object (MLST).
abidh edited edge metadata.Feb 9 2015, 10:51 AM

Is this a known stop reason "exception-received" for MI. I had a quick look but could not find it. Code looks ok to me. But I have to yet run and make sure that it passes on Linux. As I noted yesterday in few other tests, output can be a little bit different which can cause the test to fail.

ki.stfu planned changes to this revision.EditedFeb 9 2015, 10:57 AM

I need to do some changes in tests.

clayborg requested changes to this revision.Feb 9 2015, 11:11 AM
clayborg edited edge metadata.

Make a test case that access violates and remove code from the MI driver in main.c

test/tools/lldb-mi/main.c
15–32

This should go into a test case binary, probably doesn't belong in the actual MI driver.

ki.stfu updated this revision to Diff 19646.Feb 10 2015, 12:16 AM
ki.stfu edited edge metadata.

Fix tests

In D7500#120713, @abidh wrote:

Is this a known stop reason "exception-received" for MI. I had a quick look but could not find it.

I think we can set the reason to "exception-raised", if it's better.

Make a test case that access violates and remove code from the MI driver in main.c

This case checks lldb-mi functionality. Why would you want to remove it from here?

test/tools/lldb-mi/main.c
15–32

Why? In this case I'm checking that lldb-mi prints the *stopped, so this test relates to MI driver.

emaste added a subscriber: emaste.Feb 10 2015, 6:26 AM
emaste added inline comments.
test/tools/lldb-mi/main.c
15–32

I'm guessing it's just a case if mistaken identity. When I first glanced at this in email I spotted only "tools/lldb-mi/main.c" and thought it was the lldb-mi driver itself, not a test for lldb-mi. Perhaps also true for @clayborg.

clayborg accepted this revision.Feb 10 2015, 9:40 AM
clayborg edited edge metadata.

Never mind, as Ed said, I thought this was in the MI driver binary. It actually is in a test case binary...

This revision is now accepted and ready to land.Feb 10 2015, 9:40 AM
ki.stfu closed this revision.Feb 10 2015, 9:00 PM