[PATCH] For SBDebugger::Create assertion failure

Subprojects LLDB
1

Hi folks,

Whilst starting to play with the lldb C++ API, I've noticed that this
simple little program causes an assertion failure (and hence a core
dump).

#include <cstdio>
#include "lldb/API/LLDB.h"

int main()
{
    fprintf(stderr, "lldb-app\n");
// lldb::SBDebugger::Initialize();
    lldb::SBDebugger dbgr = lldb::SBDebugger::Create(true);
    fprintf(stderr, "SBDebugger::Create!\n");
    return 0;
}

As guessed, by uncommenting out the Initialize call, the assertion is
satisfied by Create and friends, and the code runs to completion.

To me, this is bad on 2 counts:

1. If Create depends on Initialize then Initialize should be called
internally.
2. From a public API perspective, even if Create and Initialize need to
be called separately, a crash (failed assert) seems a little harsh.
Shouldn't an error return (or exception throw) be used to communicate
the user's mistake in this case?

I did some digging into SBDebugger::Initialize and it seems safe for
this to be called internally by Create.

So I'm proposing that I fix this issue with following patch:

Index: source/API/SBDebugger.cpp

2

The assertion failure in detail is this:

(gdb) bt
#0 0x0000003675e35c39 in raise () from /lib64/libc.so.6
#1 0x0000003675e37348 in abort () from /lib64/libc.so.6
#2 0x0000003675e2eb96 in __assert_fail_base () from /lib64/libc.so.6
#3 0x0000003675e2ec42 in __assert_fail () from /lib64/libc.so.6
#4 0x00007fd956ee1e29 in lldb_private::Debugger::Debugger (this=0xd478f0, log_callback=0x0, baton=0x0)
at /home/mg11/src/main/devtools/main/llvm/tools/lldb/source/Core/Debugger.cpp:649
#5 0x00007fd956ee13a3 in lldb_private::Debugger::CreateInstance (log_callback=0x0, baton=0x0)
at /home/mg11/src/main/devtools/main/llvm/tools/lldb/source/Core/Debugger.cpp:530
#6 0x00007fd9572456de in lldb::SBDebugger::Create (source_init_files=0x1, callback=0x0, baton=0x0)
at /home/mg11/src/main/devtools/main/llvm/tools/lldb/source/API/SBDebugger.cpp:172
#7 0x00007fd95724560d in lldb::SBDebugger::Create (source_init_files=0x1)
at /home/mg11/src/main/devtools/main/llvm/tools/lldb/source/API/SBDebugger.cpp:153
#8 0x00000000004008d8 in main () at main.cpp:10
(gdb) fr 4
#4 0x00007fd956ee1e29 in lldb_private::Debugger::Debugger (this=0xd478f0, log_callback=0x0, baton=0x0)
at /home/mg11/src/main/devtools/main/llvm/tools/lldb/source/Core/Debugger.cpp:649
649 assert (default_platform_sp.get());
(gdb) list
644 if (log_callback)
645 m_log_callback_stream_sp.reset (new StreamCallback (log_callback, baton));
646 m_command_interpreter_ap->Initialize ();
647 // Always add our default platform to the platform list
648 PlatformSP default_platform_sp (Platform::GetHostPlatform());
649 assert (default_platform_sp.get());
650 m_platform_list.Append (default_platform_sp, true);
651
652 m_collection_sp->Initialize (g_properties);
653 m_collection_sp->AppendProperty (ConstString(“target”),
(gdb)

3

It is quite common for shared libraries to have initialize and terminate calls. We have this in LLDB.

I would rather not have to look through all API calls that might require LLDB to be initialized and have to possibly manually call SBDebugger::Initialize() in there. The initialization does quite a bit and the assertion quickly should tell you that you must call SBDebugger::Initialize() first. If it isn't clear from the assertion, we can change the assertion to be more descriptive. But again, many other classes in the lldb::SB* API have functions that might be able to be used without having to start with a debugger, so I would rather not have to go through all of the API and add the "init the debugger if not already initialized".

This also allows you to use LLDB as a shared library and do:

SBDebugger::Initialize()
// Do something that takes up a bunch of memory
SBDebugger::Terminate()

This allows all resources to be freed up after the Terminate() call.

So I vote to leave things as is.

Greg

4

It is quite common for shared libraries to have initialize and terminate calls. We have this in LLDB.

Agreed. Lots of libraries have to initialise resource, then release the
resource upon terminate.

But why have an Initialise method when you _already_ have a Create
method? Likewise a Terminate method when you _already_ have a Destroy
method.

Surely when a "thing" is created that is also when it is initialised?

I would rather not have to look through all API calls that might require LLDB to be initialized and have to possibly manually call SBDebugger::Initialize() in there. The initialization does quite a bit and the assertion quickly should tell you that you must call SBDebugger::Initialize() first. If it isn't clear from the assertion, we can change the assertion to be more descriptive. But again, many other classes in the lldb::SB* API have functions that might be able to be used without having to start with a debugger, so I would rather not have to go through all of the API and add the "init the debugger if not already initialized".

This also allows you to use LLDB as a shared library and do:

SBDebugger::Initialize()
// Do something that takes up a bunch of memory
SBDebugger::Terminate()

Agreed. But surely in between the invocations of Initialise and
Terminate, you have in some sense "created" an instance of an
SBDebugger?

So I vote to leave things as is.

To avoid rocking the boat, I reluctantly concede.

Matt

Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, www.youtube.com/user/CSRplc, Facebook, www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at www.aptx.com.

5

It is quite common for shared libraries to have initialize and terminate calls. We have this in LLDB.

Agreed. Lots of libraries have to initialise resource, then release the
resource upon terminate.

But why have an Initialise method when you _already_ have a Create
method? Likewise a Terminate method when you _already_ have a Destroy
method.

Surely when a "thing" is created that is also when it is initialised?

You are assuming there is no global context within the debugger like all registered plug-ins, lists, settings, global module list... These all work without requiring a debugger object. We could switch things so that all state is contained in the lldb_private::Debugger object, but then all places making static function calls would now need to have a debugger object. So think of SBDebugger::Initialize() more of a LLDB::Initialize() and SBDebugger::Terminate() as LLDB::Terminate(). We placed it into the SBDebugger class just for convenience since this is the most senior object in the hierarchy, but I can see how this is confusing.

I would rather not have to look through all API calls that might require LLDB to be initialized and have to possibly manually call SBDebugger::Initialize() in there. The initialization does quite a bit and the assertion quickly should tell you that you must call SBDebugger::Initialize() first. If it isn't clear from the assertion, we can change the assertion to be more descriptive. But again, many other classes in the lldb::SB* API have functions that might be able to be used without having to start with a debugger, so I would rather not have to go through all of the API and add the "init the debugger if not already initialized".

This also allows you to use LLDB as a shared library and do:

SBDebugger::Initialize()
// Do something that takes up a bunch of memory
SBDebugger::Terminate()

Agreed. But surely in between the invocations of Initialise and
Terminate, you have in some sense "created" an instance of an
SBDebugger?

No that isn't required. There are other API calls you might be able to use (not many). Repeating what I stated above: SBDebugger::Initialize()/Terminate() are more really like LLDB::Initialize() and LLDB::Terminate(), so these calls are really things that populate all shared library globals.

So I vote to leave things as is.

To avoid rocking the boat, I reluctantly concede.

Since we already have this in the SBDebugger public API I would rather not change it, but it would be probably clearer if we had:

namespace lldb {
    void Initialize(); // Must be called prior to calling any other lldb::SB API calls
    void Terminate(); // No lldb::SB API calls should be called after calling this and before calling lldb::Initialize() again
}

Does that help explain what SBDebugger::Initialize() and SBDebugger::Terminate() actually are despite the wrong class scoping?

Greg

6

Want to say I agree with Greg on this one, but did want to answer one question you had.

7

Thanks Greg,

Yes, my confusion stemmed from the scoping of the functions.

It would have been better, as you'd stated, if we'd have got:

namespace lldb {
  void Initialize();
  void Terminate();
}

But I appreciate that the C++ API is hard to change. Sorry about
previous confusion.

There's a couple of additions I would like to make to the API soon,
which will benefit the kalimba non-8-bit byte stuff. I'll build test
cases first and add a review before doing anything drastic first.

thanks
Matt

8

Thanks Zach,

Yes, as I wrote in my reply to Greg, the issue is the scoping, since
SBDebugger::Initialise/Terminate operate on the API as a whole, not just
an SBDebugger instance.

Matt

9

Yes, I don't think we can remove SBDebugger::Initialize/Terminate, but we could document them:

(lldb) script
Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.

help(lldb.SBDebugger.Initialize)

Help on function Initialize in module lldb:

Initialize()
    Initialize()

We could even add lldb::Initialize & Terminate and say in the SBDebugger::Initialize & Terminate that we'll get rid of them at some point, so use the lldb:: ones instead.

Jim

10

Sounds good. Just know it is always possible to add API without any issue. Do let us know what those APIs are and we can make sure they will work going forward.

11

Well, yes, a clean API design would (eventually) move the
Initialise/Terminate functions out of the scope the SBDebugger entity.

Matt