Page MenuHomePhabricator

[lldb] Fix a potential bug that may cause assert failure in CommandObject::CheckRequirements
ClosedPublic

Authored by MrHate on Fri, May 22, 9:49 AM.

Details

Summary

CommandObject::CheckRequirements requires cleaning up m_exe_ctx between commands. Function HandleOptionCompletion returns without cleaning up m_exe_ctx could cause assert failure in later CheckRequirements.

Diff Detail

Event Timeline

MrHate created this revision.Fri, May 22, 9:49 AM
JDevlieghere added inline comments.Fri, May 22, 2:37 PM
lldb/source/Interpreter/CommandObject.cpp
281

Should we reset the context here as well? Maybe it's wroth wrapping this in a simple RAII class that unsets the context on destruction.

MrHate updated this revision to Diff 265834.Fri, May 22, 8:44 PM
MrHate edited the summary of this revision. (Show Details)

Thanks very much Jonas for pointing out my carelessness! And with your RAII idea, I think we can use the smart pointer to simulate the defer thing.

teemperor requested changes to this revision.Wed, May 27, 3:50 AM

LLVM actually has it's own cod for doing this (IIUC, it has the benefit of being easier to migrate to an upcoming standard library utility class with the same purpose).

Beside that this LGTM.

lldb/source/Interpreter/CommandObject.cpp
273

The LLVM way of doing this is auto reset_ctx = llvm::make_scope_exit([this]() { Cleanup(); }); (from the #include "llvm/ADT/ScopeExit.h" header).

This revision now requires changes to proceed.Wed, May 27, 3:50 AM
MrHate updated this revision to Diff 266488.Wed, May 27, 4:35 AM
teemperor accepted this revision.Wed, May 27, 4:50 AM

LGTM, thanks!

This revision is now accepted and ready to land.Wed, May 27, 4:50 AM
This revision was automatically updated to reflect the committed changes.