[Lldb-commits] [lldb] r355466 - Replace debug-only assert with a plain old assert.

Fri Jan 31 16:53:52 PST 2020

That shouldn't be ASAN specific, that should happen on and LLDB_CONFIGURATION_DEBUG build.  If it only happens on ASAN builds, something else must be going wrong.

BTW, those asserts only make sense if you can't re-enter the command interpreter while a command is active.  In the case of a command that runs the target synchronously (like "step") with stop hooks or breakpoint commands, the "step" command won't have completed when you're running the commands in the stop hook or the breakpoint callback.  So that assumption is clearly not correct.

I don't think those asserts serve any useful purpose beyond reminding us that some day we really need to take the time to tease apart the part of CommandObject that forms the template for the command, and the part that represents this particular execution of that command.  

The CommandObject is a singleton, so it makes no sense for it to hold the data for any given execution.  We skate by on this for the most part because commands tend to do:

1) Store away the execution context for this command invocation
2) Parse Command Input into the option ivars in the CommandObject
3) Use those ivars to figure out what to do
4) Do It
5) Report a result that doesn't rely on the input data 

Since you don't generally get a chance to run another command till stage 4, it doesn't matter that that operation might overwrite the context or option ivars in the current CommandObject.  

This is clearly the wrong design, but unfortunately it has never caused any problems serious enough to motivate fixing it.

Jim

> On Jan 31, 2020, at 4:04 PM, Adrian Prantl <aprantl at apple.com> wrote:
> 
> This could be the known bug where a CommandObject is scheduled while the same CommandObject is already deeper on the command stack. I tried digging up the bugreport for that, but I couldn't find it.
> 
> -- adrian
> 
>> On Jan 31, 2020, at 3:59 PM, Vedant Kumar <vedant_kumar at apple.com> wrote:
>> 
>> 
>> 
>>> On Mar 5, 2019, at 5:07 PM, Adrian Prantl via lldb-commits <lldb-commits at lists.llvm.org> wrote:
>>> 
>>> Author: adrian
>>> Date: Tue Mar  5 17:07:45 2019
>>> New Revision: 355466
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=355466&view=rev
>>> Log:
>>> Replace debug-only assert with a plain old assert.
>>> 
>>> Modified:
>>>   lldb/trunk/source/Interpreter/CommandObject.cpp
>>> 
>>> Modified: lldb/trunk/source/Interpreter/CommandObject.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/CommandObject.cpp?rev=355466&r1=355465&r2=355466&view=diff
>>> ==============================================================================
>>> --- lldb/trunk/source/Interpreter/CommandObject.cpp (original)
>>> +++ lldb/trunk/source/Interpreter/CommandObject.cpp Tue Mar  5 17:07:45 2019
>>> @@ -136,17 +136,15 @@ bool CommandObject::ParseOptions(Args &a
>>> }
>>> 
>>> bool CommandObject::CheckRequirements(CommandReturnObject &result) {
>>> -#ifdef LLDB_CONFIGURATION_DEBUG
>>>  // Nothing should be stored in m_exe_ctx between running commands as
>>>  // m_exe_ctx has shared pointers to the target, process, thread and frame and
>>>  // we don't want any CommandObject instances to keep any of these objects
>>>  // around longer than for a single command. Every command should call
>>> -  // CommandObject::Cleanup() after it has completed
>>> -  assert(m_exe_ctx.GetTargetPtr() == NULL);
>>> -  assert(m_exe_ctx.GetProcessPtr() == NULL);
>>> -  assert(m_exe_ctx.GetThreadPtr() == NULL);
>>> -  assert(m_exe_ctx.GetFramePtr() == NULL);
>>> -#endif
>>> +  // CommandObject::Cleanup() after it has completed.
>>> +  assert(!m_exe_ctx.GetTargetPtr());
>> 
>> Hi all,
>> 
>> I'm actually hitting this assert now when I try debugging clang with a sanitized lldb. Any advice appreciated!
>> 
>> Stop hook #1 added.
>> (lldb) s
>> * thread #1, queue = 'com.apple.main-thread', stop reason = step in
>>  * frame #0: 0x00000001044d2268 clang-10`clang::noteBottomOfStack() at Stack.cpp:42:8
>>    frame #1: 0x0000000100002295 clang-10`main(argc_=64, argv_=0x00007ffeefbff160) at driver.cpp:342:3
>>    frame #2: 0x00007fff722ba3d5 libdyld.dylib`start + 1
>> 
>> Assertion failed: (!m_exe_ctx.GetTargetPtr()), function CheckRequirements, file /Users/vsk/src/llvm-project-master/lldb/source/Interpreter/CommandObject.cpp, line 153.
>> Stack dump:
>> 0.      Program arguments: /Users/vsk/src/builds/llvm-project-master-SAN/bin/lldb -o b main -o r -o target stop-hook add -o bt -o v -o s -o s -- /Users/vsk/src/builds/llvm-project-master-D/bin/clang-10 -cc1 -triple x86_64-apple-macosx10.14.0 -Wdeprecated-objc-isa-usage -Werror=deprecated-objc-isa-usage -emit-obj -disable-free -main-file-name sqlite3.c -mrelocation-model pic -pic-level 2 -mthread-model posix -mframe-pointer=all -fno-rounding-math -masm-verbose -munwind-tables -target-sdk-version=10.14 -target-cpu penryn -dwarf-column-info -debug-info-kind=standalone -dwarf-version=4 -debugger-tuning=lldb -target-linker-version 556 -resource-dir /Users/vsk/src/builds/llvm-project-master-RA/lib/clang/11.0.0 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk -I/usr/local/include -internal-isystem /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/local/include -internal-isystem /Users/vsk/src/builds/llvm-project-master-RA/lib/clang/11.0.0/include -internal-externc-isystem /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include -O2 -fdebug-compilation-dir /Users/vsk/src/builds/llvm-project-master-RA -ferror-limit 19 -fmessage-length 178 -stack-protector 1 -fblocks -fencode-extended-block-signature -fregister-global-dtors-with-atexit -fgnuc-version=4.2.1 -fobjc-runtime=macosx-10.14.0 -fmax-type-align=16 -fdiagnostics-show-option -fcolor-diagnostics -vectorize-loops -vectorize-slp -o /dev/null -x c /Users/vsk/Downloads/sqlite-amalgamation-3300100/sqlite3.c
>> zsh: abort      ~/src/builds/llvm-project-master-SAN/bin/lldb -o "b main" -o "r" -o  -o "s" -
>> 
>> vedant
>> 
>> 
>>> +  assert(!m_exe_ctx.GetProcessPtr());
>>> +  assert(!m_exe_ctx.GetThreadPtr());
>>> +  assert(!m_exe_ctx.GetFramePtr());
>>> 
>>>  // Lock down the interpreter's execution context prior to running the command
>>>  // so we guarantee the selected target, process, thread and frame can't go
>>> 
>>> 
>>> _______________________________________________
>>> lldb-commits mailing list
>>> lldb-commits at lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> 