This should always be true but sometimes is not, during platform bring
up. As recommended by Jim Ingham, an assertion should be enough here to
help.
This addresses post commit comments in http://reviews.llvm.org/D8554.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Good change as long as it accomplishes the same thing as your null check against GetStackFrameAtIndex(0). eFlagRequiresFrame is slightly different, checking that the ExecutionContext got a frame but I suppose the ECX got it from the same place -- GetStackFrameAtIndex(0) so it should be fine.
I wouldn't raise such a pedantic point except that lldb is clearly in an odd state when this code path is erroring out; normal assumptions may not be safe.
Agreed. The only small benefit of this patch is to help a bit during target bring up.
eFlagRequiresFrame is fine as long as you are looking for the selected target, process, thread and frame.
The only thing that bothers me a bit about this patch is that somebody might think we need to go clean up all the places where a thread doesn't have a frame 0, and worse, actually try to do something reasonable in that case. That would be wasted effort. To that end, an assert here might in fact be a better solution. In any case, it might be good to put a comment in Thread.h making it clear that GetFrameAtIndex(0) is always expected to return a valid frame.
Jim
That would be fine with me, and would have helped in the specific case I encountered during bring-up.