Page MenuHomePhabricator

Add a missing null pointer check in CommandObjectThread.cpp.
ClosedPublic

Authored by sas on Mar 23 2015, 10:26 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

sas updated this revision to Diff 22485.Mar 23 2015, 10:26 AM
sas retitled this revision from to Add a missing null pointer check in CommandObjectThread.cpp..
sas updated this object.
sas edited the test plan for this revision. (Show Details)
sas added a reviewer: clayborg.
sas added a subscriber: Unknown Object (MLST).
clayborg accepted this revision.Mar 23 2015, 10:50 AM
clayborg edited edge metadata.
This revision is now accepted and ready to land.Mar 23 2015, 10:50 AM
emaste accepted this revision.Mar 23 2015, 11:01 AM
emaste added a reviewer: emaste.
emaste added a subscriber: emaste.

LGTM, I think I had this same change in a local branch at one point.

This revision was automatically updated to reflect the committed changes.

How do you get a thread that has NO frame 0? That seems weird, we always have a register context to make frame 0...

Anyway, if you're going to do this, add it to the "thread == NULL" checks earlier on in the function so we can get out of the command with an appropriate error message. If we don't have a frame 0 we probably don't have registers so I'm not really sure how we're going to do any kind of stepping...

Jim

sas added a comment.Mar 23 2015, 12:29 PM

This is a patch that I've been keeping in my branches for a while. I
hit a null pointer dereference there. Looks like Ed had the same
issue. I'll move the check to the beginning of the function as you
suggested though.

I encountered an issue like this early on while bringing up a new target (FreeBSD/mips64 I think), while the work was incomplete. Of course once I finished we always have frame 0, which is why I didn't commit it at the time.

But it (or, perhaps just an assert) presumably will be handy when someone else goes down the same path.

Not worth changing the patch but fwiw you could have added eFlagRequiresFrame to the CommandObjectParsed flags for this command to do the same thing. My main reaction echoes Jim's - I don't know what it means to have a thread and no frame 0. But given that two lldb devs have hit it, I guess early in the startup of a new platform you can get hit this.

Yes, I think there are a lot of places around in lldb where we assume that if you have a valid thread you have a valid frame 0. You're not going to get very far if you don't get that right.

Jim