Separate out the necessary component initialization for lldb-server such that the linker can greatly reduce the binary size. With this patch the size of lldb-server on my 64 bit linux release build drops from 46MB to 26MB. I've run tests and manually tested remote debugging but let me know if there's anything else I can do to verify I haven't left anything uninitialized which is used by lldb-server.
Details
- Reviewers
clayborg tberghammer - Commits
- rGab3269d275e7: Reduce the number of components initialized by lldb-server to reduce static…
rLLDB230963: Reduce the number of components initialized by lldb-server to reduce static…
rL230963: Reduce the number of components initialized by lldb-server to reduce static…
Diff Detail
- Repository
- rL LLVM
Event Timeline
Please do the same separation for the Terminate methods also. In most of the case (probably all) it is ensured that calling Terminate without Initialize won't cause any problem but I think it is a very bad practice to rely on this behavior and I expect some further reduction in size from that change.
tools/lldb-server/lldb-gdbserver.cpp | ||
---|---|---|
510–511 | Are you sure you don't have to initialize the debugger? |
Separated termination to allow only calling Terminate on components initialized by InitializeForLLGS. No noticeable increased binary size reduction but this is definitely more correct behavior moving forward.
tools/lldb-server/lldb-gdbserver.cpp | ||
---|---|---|
510–511 | It does not seem to be necessary, but I could update Debugger::initialize to take which lldb initialization we require if we want to initialize in case it's relied on to coordinate when to call lldb_private::Terminate if you think we should coordinate this through Debugger. |
tools/lldb-server/lldb-gdbserver.cpp | ||
---|---|---|
510–511 | Sorry, parameters wouldn't work because it's not known at link time what values the function will be called with, but Debugger could not implicitly call lldb_private::Initialize though then the reference counting it does should probably also be moved to the initialize method. |
Add python back to the components initialized / terminated for LLGS - found that "lldb-server platform" needs python. No noticeable affect to binary size.
It looks good to me but please run the full test suit before and after this change and make sure there is no regression (both on Linux and Mac if possible).
Added Greg as a reviewer, please wait for a response from him.
This would also be a good time to try and always initialize all plugins. There are many apple plug-ins which should be completely safe to compile into LLDB for cross debugging to MacOSX and iOS. See inlined comments above.
source/lldb.cpp | ||
---|---|---|
227 | Note that these #defines are needed because ProcessLinux only runs natively on linux, unlike the #if defined (APPLE) ones that are removing code for no real reason since apple always runs through GDB remote and all other plug-ins are abstracted from the host system and should be able to be cross compiled. | |
240–243 | Remove the #if defined (APPLE) and always compile these for cross debugging. | |
243 | Remove the #if defined (APPLE) and always compile these for cross debugging. | |
292 | Remove the #if defined (APPLE) and always compile these for cross debugging. | |
305 | Remove the #if defined (APPLE) and always compile these for cross debugging. | |
343–346 | Remove the #if defined (APPLE) and the #endif. These plug-ins should be always compiled into LLDB binaries so that cross debugging can be done. There should be no code that relies headers from a native system in any of these. |
Add most apple plugins to all OS compilations and lldb initialization except those which require Mac specific includes.
Hey, I tried to move everything over and then moved plugins back to guarded by #if defined(APPLE) on a case by case basis as I found out why they would not compile and included the reasons in responses to your original comments. Please take a look, thanks.
source/lldb.cpp | ||
---|---|---|
240–243 | ProcessMachCore and ProcessKDP depend on DynamicLoaderDarwinKernel which depends on PlatformDarwinKernel which includes Mac-specific sources (Host/macosx/cfcpp/CFCBundle.h) | |
292 | ObjectFileMacho: includes RegisterContextDarwin_x86_64 and extends RegisterContextDarwin_x86_64 which includes <mach/*> DynamicLoaderDarwinKernel, as mentioned earlier includes mac only resources. SymbolVendorMacOSX requires libxml2, while I was able to update the Linux build to find this I had some trouble getting cmake to find it in Windows. If we want to add this I think (I may be mistaken) we need to include the path to GnuWin32 (which includes libxml2) as a cmake config parameter so that it can locate libxml2. |
Please fix the configure+make based compilation as it needs the same changes you made in cmake (I am not sure if xcode needs this changes too or not).
/mnt/ssd/ll/git/build/configure/Release+Asserts/lib/liblldbInitAndLog.a(lldb.o): In function `lldb_private::InitializeForLLGS()': /mnt/ssd/ll/git/llvm/tools/lldb/source/lldb.cpp:(.text._ZN12lldb_private17InitializeForLLGSEv+0x11a): undefined reference to `ObjectContainerUniversalMachO::Initialize()' /mnt/ssd/ll/git/llvm/tools/lldb/source/lldb.cpp:(.text._ZN12lldb_private17InitializeForLLGSEv+0x12e): undefined reference to `SystemRuntimeMacOSX::Initialize()' /mnt/ssd/ll/git/build/configure/Release+Asserts/lib/liblldbInitAndLog.a(lldb.o): In function `lldb_private::TerminateLLGS()': /mnt/ssd/ll/git/llvm/tools/lldb/source/lldb.cpp:(.text._ZN12lldb_private13TerminateLLGSEv+0x88): undefined reference to `ObjectContainerUniversalMachO::Terminate()' /mnt/ssd/ll/git/llvm/tools/lldb/source/lldb.cpp:(.text._ZN12lldb_private13TerminateLLGSEv+0x9c): undefined reference to `SystemRuntimeMacOSX::Terminate()'
Fix configure + Makefile build. xcode should not need any changes given it's just enabling things which were previously mac only, but I will double check a build using xcode.
Ok, thanks for trying. We shouldn't require the inclusion of MacOSX plug-ins for this patch.
Thanks. Just to confirm, should I commit the patch including what I could (diff 5)? Or defer on including them and commit diff 3?
CMakeLists.txt | ||
---|---|---|
360 ↗ | (On Diff #20855) | Can you revert all changes to this file? |
CMakeLists.txt | ||
---|---|---|
360 ↗ | (On Diff #20855) | Of course, done. I missed that after testing whether cmake would just find xml2 from GnuWin32 on Windows. |
No worries, it's good to get these cleanups in.
BTW there are a set of straightforward instructions for getting a FreeBSD VM set up for testing on our wiki: https://wiki.freebsd.org/lldb
Note that these #defines are needed because ProcessLinux only runs natively on linux, unlike the #if defined (APPLE) ones that are removing code for no real reason since apple always runs through GDB remote and all other plug-ins are abstracted from the host system and should be able to be cross compiled.