[Lldb-commits] [PATCH] D44321: Support demangling for D symbols via dlopen
johanengelen added inline comments.
Comment at: source/Plugins/Language/D/DLanguage.cpp:1
+//===-- DLanguage.cpp
+//
fix header, and also need to clang-format the file
Comment at: source/Plugins/Language/D/DLanguage.cpp:20
+
+char* lldbd_demangle(size_t length, const char* mangled);
+void d_initialize();
add comment about the purpose of these fwd decls.
Comment at: source/Plugins/Language/D/DLanguage.cpp:24
+// TODO:MOVE
+struct SharedLib{
+ void* handle;
Did you look into using llvm/Support/DynamicLibrary.h ?
That would make the code crossplatform (notably: Windows).
Comment at: source/Plugins/Language/D/DLanguage.cpp:35
+
+ // Return true of `dlopen` succeeded
+ bool open(const char* libraryFile, int flag)
typo: "if"
Comment at: source/Plugins/Language/D/DLanguage.cpp:108
+
+auto fun0=lib2->getFun("d_initialize");
+(*fun0)();
Would it help to initialize druntime using a static module constructor in the
lldbdplugin dll?
(then you can also do de-init using a static module destructor)
Comment at: source/Plugins/Language/D/DLanguage.h:1
+//===-- DLanguage.h --*- C++ -*-===//
+//
fix header line
Repository:
rL LLVM
https://reviews.llvm.org/D44321
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44321: Support demangling for D symbols via dlopen
johanengelen added inline comments.
Comment at: source/Plugins/Language/D/DLanguage.cpp:108
+
+auto fun0=lib2->getFun("d_initialize");
+(*fun0)();
timotheecour wrote:
> johanengelen wrote:
> > Would it help to initialize druntime using a static module constructor in
> > the lldbdplugin dll?
> > (then you can also do de-init using a static module destructor)
> I don't really like static module constructor because it adds cyclic
> dependencies, see for vibe.d moving away from it:
> https://forum.dlang.org/post/qtabwblpaqwpteyst...@forum.dlang.org
> explicit calling `d_initialize` is simple enough.
>
>
>
Module ctors don't add cyclic dependencies by themselves. There is also no
danger of that here.
How do you de-initialize druntime? (without de-init, there is a big mem leak)
https://reviews.llvm.org/D44321
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44321: Support demangling for D symbols via dlopen
johanengelen added a comment.
In https://reviews.llvm.org/D44321#1034141, @timotheecour wrote:
> > It's a little more complicated for D because it's an out-of-tree compiler
> > so it poses interesting challenges.
>
> the demangling itself is thoroughly tested in
> https://github.com/dlang/druntime/blob/master/src/core/demangle.d
It's not the correct demangling of strings that needs to be tested. What needs
testing:
- recognising D language and D mangled name
- loading of dynamic lib (plus testing that when loading fails lldb doesn't
crash/exit; note: errmsg can be improved to e.g. "Failed to load D language
plugin...")
- test that indeed the demangling function of the dyn lib is called
To test this, I think you can make a mock dynamic lib that implements the
required interface, but in C++. The mock demangler could be something like this
or even simpler:
extern "C" char* lldbd_demangle(size_t length, const char* mangled) {
if (mangled == "_D3fooFZv") // pseudo code
return "void foo()";
else
return mangled;
}
Then build that library in the test, and test loading and use on a fabricated
piece of code (probably easiest to use C code with this in it: `extern "C"
_D3fooFZv() { trap(); }`.
https://reviews.llvm.org/D44321
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44321: Support demangling for D symbols via dlopen
johanengelen added a comment.
In https://reviews.llvm.org/D44321#1034168, @johanengelen wrote:
> extern "C" char* lldbd_demangle(size_t length, const char* mangled) {
> if (mangled == "_D3fooFZv") // pseudo code
> return "void foo()";
> else
> return mangled;
> }
>
Actually, this is not the correct interface is it? The returned pointer should
point to newly allocated memory using `malloc`, right?
Good to document that somewhere.
https://reviews.llvm.org/D44321
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44321: Support demangling for D symbols via dlopen
johanengelen added a comment. In https://reviews.llvm.org/D44321#1038160, @timotheecour wrote: > > How do you de-initialize druntime? (without de-init, there is a big mem > > leak) > > There is no memory leak because `d_initialize` once (using c++11 static > initialization pattern) and is intended to last for duration of application; > so druntime will be initialized only once, upon 1st use. When druntime is initialized, a number of resources are allocated (e.g. memory and mutex). Yes you initialize druntime once, I can see that. You don't deinitialize druntime at all: that's the resource leak. https://reviews.llvm.org/D44321 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D44321: Support demangling for D symbols via dlopen
johanengelen added a comment. In https://reviews.llvm.org/D44321#1038241, @timotheecour wrote: > > When druntime is initialized, a number of resources are allocated (e.g. > > memory and mutex). Yes you initialize druntime once, I can see that. You > > don't deinitialize druntime at all: that's the resource leak. > > Where would you want me to deinit? inside ` DLanguage::Terminate` ? I don't know enough of LLDB to know where to put the deinitialization. Sorry, can't help you there, except for making it easy and doing the deinitialization using a dtor in the library... https://reviews.llvm.org/D44321 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
