[lldb-dev] [PATCH] Linux JIT support

[Greg Clayton]

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 <andrew.macp at gmail.com> 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
> lldb-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev