Page MenuHomePhabricator

Reduce the number of components initialized by lldb-server to reduce static binary size.
ClosedPublic

Authored by flackr on Feb 25 2015, 6:53 AM.

Details

Summary

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.

Diff Detail

Repository
rL LLVM

Event Timeline

flackr updated this revision to Diff 20672.Feb 25 2015, 6:53 AM
flackr retitled this revision from to Reduce the number of components initialized by lldb-server to reduce static binary size..
flackr updated this object.
flackr edited the test plan for this revision. (Show Details)
flackr added a reviewer: tberghammer.
flackr set the repository for this revision to rL LLVM.
flackr added a subscriber: Unknown Object (MLST).
emaste added a subscriber: emaste.Feb 25 2015, 6:55 AM
tberghammer requested changes to this revision.Feb 25 2015, 7:38 AM
tberghammer edited edge metadata.

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?

This revision now requires changes to proceed.Feb 25 2015, 7:38 AM
flackr updated this revision to Diff 20694.Feb 25 2015, 12:04 PM
flackr edited edge metadata.

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.

flackr added inline comments.Feb 25 2015, 12:07 PM
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.

flackr added inline comments.Feb 25 2015, 1:16 PM
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.

flackr updated this revision to Diff 20704.Feb 25 2015, 2:22 PM
flackr edited edge metadata.

Add python back to the components initialized / terminated for LLGS - found that "lldb-server platform" needs python. No noticeable affect to binary size.

tberghammer added a comment.EditedFeb 26 2015, 3:13 AM

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.

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).

Confirmed, no regressions on Linux or Mac full test suite.

clayborg requested changes to this revision.Feb 26 2015, 9:54 AM
clayborg edited edge metadata.

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
228

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.

241–244

Remove the #if defined (APPLE) and always compile these for cross debugging.

244

Remove the #if defined (APPLE) and always compile these for cross debugging.

291–292

Remove the #if defined (APPLE) and always compile these for cross debugging.

309

Remove the #if defined (APPLE) and always compile these for cross debugging.

352–355

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.

This revision now requires changes to proceed.Feb 26 2015, 9:54 AM
flackr updated this revision to Diff 20793.Feb 26 2015, 1:38 PM
flackr edited edge metadata.

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
241–244

ProcessMachCore and ProcessKDP depend on DynamicLoaderDarwinKernel which depends on PlatformDarwinKernel which includes Mac-specific sources (Host/macosx/cfcpp/CFCBundle.h)

291–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.

tberghammer requested changes to this revision.Feb 27 2015, 2:26 AM
tberghammer edited edge metadata.

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()'
This revision now requires changes to proceed.Feb 27 2015, 2:26 AM
flackr updated this revision to Diff 20855.Feb 27 2015, 8:33 AM
flackr edited edge metadata.

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.

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()'

Done, thanks for catching that and reminding me.

clayborg accepted this revision.Feb 27 2015, 9:41 AM
clayborg edited edge metadata.

Ok, thanks for trying. We shouldn't require the inclusion of MacOSX plug-ins for this patch.

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?

zturner added inline comments.
CMakeLists.txt
360

Can you revert all changes to this file?

flackr updated this revision to Diff 20863.Feb 27 2015, 10:23 AM
flackr edited edge metadata.

Removed no-op changes to CMakeLists.txt

flackr added inline comments.Feb 27 2015, 10:24 AM
CMakeLists.txt
360

Of course, done. I missed that after testing whether cmake would just find xml2 from GnuWin32 on Windows.

tberghammer accepted this revision.Feb 28 2015, 6:13 AM
tberghammer edited edge metadata.
This revision is now accepted and ready to land.Feb 28 2015, 6:13 AM
This revision was automatically updated to reflect the committed changes.
emaste added a comment.Mar 3 2015, 4:03 PM

This broke the FreeBSD configure+make build

emaste added a comment.Mar 3 2015, 4:17 PM

Hopefully fixed with r231180

vharron added a subscriber: vharron.Mar 3 2015, 8:49 PM

Sorry Ed! Thanks for the fix.

emaste added a comment.Mar 3 2015, 9:13 PM

Sorry Ed! Thanks for the fix.

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