Looks pretty good. A few changes I would like to see:
1 - switch away from using std::unique_ptr<> to use std::shared_ptr<> for the
JITLoader instances:
typedef lldb::JITLoaderSP (*JITLoaderCreateInstance) (Process *process,
bool force);
Newer plugins are adopting this method in case we ever want to inherit from
std::shared_from_this()...
2 - make a JITLoaderList class that encapsulates the storage details of the
JITLoader objects (std::vector<std::unique_ptr<JITLoader>> is what you had, we
should probably switch to std::vector<JITLoaderSP>) and also protects access to
the list with a recursive mutex. LLDB is multi-threaded and we should make sure
we never can hose this collection by accessing it from two threads. The class
should probably have simple functions like:
class JITLoaderList
{
JITLoaderList();
~JITLoaderList();
void Append (const JITLoaderSP &jit_loader_sp);
void Remove (const JITLoaderSP &jit_loader_sp);
size_t GetSize() const;
JITLoaderSP GetLoaderAtIndex (size_t idx);
void DidLaunch();
void DidAttach();
};
3 - switch any functions that were taking/returning
"std::vector<std::unique_ptr<JITLoader>>" over to use the new JITLoaderList
class.
4 - Process had these new members added:
+ JITLoaderList m_jit_aps;
+ bool m_did_load_jit_aps;
You might be able to switch over to:
std::unique_ptr<JITLoaderList> m_jit_loaders_ap;
Then you accessor switches to be:
JITLoaderList &
Process::GetJITLoaders ()
{
if (!m_jit_loaders_ap) {
m_jit_loaders_ap.reset (new JITLoaderList());
JITLoader::LoadPlugins(this, *m_jit_loaders_ap);
}
return *m_jit_loaders_ap;
}
5 - Any code that used to be using the iterators can just be replaced with the
JITLoaderList::DidLaunch() and JITLoaderList::DidAttach() and the iteration can
happen internally in the JITLoaderList class. If you need a generic iterator
for the JITLoader objects inside a JITLoaderList class, you can make a:
void
ForEach (std::function <bool(JITLoader &)> const &callback);
In the JITLoaderList class. This can work well because thee "ForEach" function
can take the mutex to protect the collection's integrity during iteration, and
the "bool" return value from the function can indicate if iteration should
continue or stop...
On Mar 3, 2014, at 2:32 PM, Andrew MacPherson <[email protected]> wrote:
> This patch adds support for loading debug info from JITed code on Linux
> through a new JITLoader plugin. The patch was largely written by Keno Fischer.
>
> The patch can be verified like this:
> • <Take any test.c program with a main()>
> • /build/bin/clang -O0 -g -emit-llvm -S test.ll test.c
> • /build/bin/lldb -- /build/bin/lli --use-mcjit test.ll
> • br set -f test.c -l <some line>
> • r
> • <hit breakpoint>
> • bt (to see stack trace with filenames and lines)
> Let me know if there's anything that should be updated or if there is a
> better way to submit a larger patch like this.
>
> Thanks,
>
> Andrew
>
> <linux-jit.patch>_______________________________________________
> lldb-dev mailing list
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
_______________________________________________
lldb-dev mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
