Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stripping all symbols except `export`ed functions #2431

Closed
p0nce opened this issue Nov 30, 2017 · 26 comments

Comments

@p0nce
Copy link
Contributor

commented Nov 30, 2017

For binary size reason, we use list of symbols to export in final builds.

ldc2 -exported_symbols_list list-of-symbols.lst -dead_strip <rest-of-cmdline>

These two flags together allow to reduce object code dramatically since all symbols are public by default on POSIX else (about 3x reduction IIRC).

Now that LDC supports the export attribute, would it be possible to avoid to write such a list of symbols, and remove the need for -exported_symbols_list? The problem is that it's error prone and the information is already there through export.

(edit: I'm not sure if export does anything on POSIX though)

@patrickkh7788

This comment has been minimized.

Copy link

commented Dec 1, 2017

+1

@dnadlinger

This comment has been minimized.

Copy link
Member

commented Dec 1, 2017

This should definitely be supported, but as command option that can be explicitly enabled. Hiding everything non-exported by default would be very surprising given the current state of affairs on all the compilers.

Implementation-wise, this should be fairly straightforward. For all-at-once compilation, we can also internalise them during compile time, avoiding the need to codegen unused functions, enabling non-ABI-preserving optimisations, and so on. The trick is just to do this in a fashion that doesn't violate user expectations, since export is so under-specified and -used.

@p0nce

This comment has been minimized.

Copy link
Contributor Author

commented Dec 1, 2017

This should definitely be supported, but as command option that can be explicitly enabled. Hiding everything non-exported by default would be very surprising given the current state of affairs on all the compilers.

I agree with you.

@p0nce

This comment has been minimized.

Copy link
Contributor Author

commented Dec 1, 2017

For example -export-only-symbols-marked-export would be a very explicit name.
(more realistically, -exported_symbols_export? this way the switch begins the same as the switch it replaces)

@patrickkh7788

This comment has been minimized.

Copy link

commented Jun 21, 2018

this is great, can this be used to generate static lib ?

@p0nce

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2018

What would be the price tag of such a feature?

@kinke

This comment has been minimized.

Copy link
Member

commented Jul 8, 2018

Well this might really be an easy one. Assuming function/global declarations don't need to be touched (=> default/public visibility for all) and adapting the visibilities of definitions only is enough, this hack (without cmdline option) might get you started:

diff --git a/gen/declarations.cpp b/gen/declarations.cpp
index 1ca7af49..8c138225 100644
--- a/gen/declarations.cpp
+++ b/gen/declarations.cpp
@@ -293,6 +293,9 @@ public:
         if (gvar->hasDLLImportStorageClass()) {
           gvar->setDLLStorageClass(LLGlobalValue::DLLExportStorageClass);
         }
+        if (!global.params.isWindows && !decl->isExport()) {
+          gvar->setVisibility(LLGlobalValue::HiddenVisibility);
+        }

         // Also set up the debug info.
         irs->DBuilder.EmitGlobalVariable(gvar, decl);
diff --git a/gen/functions.cpp b/gen/functions.cpp
index f9c34f21..0622c3ac 100644
--- a/gen/functions.cpp
+++ b/gen/functions.cpp
@@ -564,6 +564,9 @@ void DtoDeclareFunction(FuncDeclaration *fdecl) {
     func->setDLLStorageClass(fdecl->isImportedSymbol()
                                  ? LLGlobalValue::DLLImportStorageClass
                                  : LLGlobalValue::DLLExportStorageClass);
+  } else if (!global.params.isWindows && !fdecl->isImportedSymbol() &&
+             !fdecl->isExport()) {
+    func->setVisibility(LLGlobalValue::HiddenVisibility);
   }

   IF_LOG Logger::cout() << "func = " << *func << std::endl;

If the declarations need to match too, that would probably be a serious problem when linking against shared druntime/Phobos, which would need a ton of export in order to import/declare their symbols with default/public visibility.

@p0nce

This comment has been minimized.

Copy link
Contributor Author

commented Jul 12, 2018

Sorry I just can't take more on my plate and get into a new codebase, but can contribute money. If there is no price because no one is interested enough, then so be it let's not do this. It's not critical by any means.

(EDIT : deleted lame blabbering).

@patrickkh7788

This comment has been minimized.

Copy link

commented Aug 8, 2018

@kinke

Can we has a options to enable export only symbol, and build druntime/Phobos without that options enabled ?

This is very useful for modified druntime or betterC project. to hidden the internal symbol and only show the export one.

The un want long template symbol will increase the binary size and expose code details.

For WASM project -L--strip-all is not strip all symbol name, and the template made the WASM size increase huge.

If WASM project need use Druntime or Phobos module, still no need to export the symbol into javascript( need link Druntime inside the WASM binary file).

@patrickkh7788

This comment has been minimized.

Copy link

commented Aug 13, 2018

@kinke update ?

@p0nce

This comment has been minimized.

Copy link
Contributor Author

commented Aug 13, 2018

@patrickkh7788 I don't think there is any more to add, if someone want to step-up and fix it the LDC devs already gave us the start of a solution, and a rationale for what would be accepted.

@kinke

This comment has been minimized.

Copy link
Member

commented Oct 26, 2018

The Linux/ld equivalent for the Mac -exported_symbols_list workaround appears to be --version-file, see https://forum.dlang.org/post/yvmnkvzgoxhcfavjayky@forum.dlang.org.

@p0nce

This comment has been minimized.

Copy link
Contributor Author

commented Oct 27, 2018

Still interested by funding this!

@MrSmith33

This comment has been minimized.

Copy link
Contributor

commented Nov 1, 2018

Does this look like a correct result?

// hello.d
extern(C) export int test__exportedFunDef() { return 42; }
extern(C) int test__nonExportedFunDef() { return 101; }

extern(C) export int test__exportedFunDecl();
extern(C) int test__nonExportedFunDecl();

extern(C) export int test__exportedVarDef;
extern(C) int test__nonExportedVarDef;

extern(C) extern export int test__exportedVarDecl;
extern(C) extern int test__nonExportedVarDecl;
>ldc2 hello.d -betterC -shared && nm libhello.so | grep 'test__'
00000000000006c0 T test__exportedFunDef
0000000000000000 B test__exportedVarDef
00000000000006d0 T test__nonExportedFunDef
0000000000000004 B test__nonExportedVarDef

>ldc2 hello.d -betterC -shared -export-only-symbols-marked-export && nm libhello.so | grep 'test__'
0000000000000650 T test__exportedFunDef
0000000000000000 B test__exportedVarDef
@kinke

This comment has been minimized.

Copy link
Member

commented Nov 1, 2018

The only real question is

If the declarations need to match too, that would probably be a serious problem when linking against shared druntime/Phobos, which would need a ton of export in order to import/declare their symbols with default/public visibility.

@MrSmith33

This comment has been minimized.

Copy link
Contributor

commented Nov 1, 2018

The only real question is

How do I test this?
Would it be possible to only compile user shared library with new flag, but compile druntime/phobos without it?

@kinke

This comment has been minimized.

Copy link
Member

commented Nov 1, 2018

Well, that's exactly why I didn't want to pursue this myself, as I thought someone else interested in this (apparently enough people) should be able to figure out the answer to that question and contribute a bit to D development. If the answer is 'yeah, declarations don't need to match', then the implementation is trivial and all I really need to finish the sketch above.

Anyway,

Would it be possible to only compile user shared library with new flag, but compile druntime/phobos without it?

Yep, that's exactly it. You should be able to compile & link a binary with the new cmdline option with shared druntime/Phobos (the normal one, compiled without the new flag obviously, as it wouldn't export anything otherwise), accessing some non-templated symbol from the default libs to make sure that imported symbol (not explicitly marked as export) can still be accessed.

@kinke

This comment has been minimized.

Copy link
Member

commented Nov 1, 2018

Sorry, that's actually just the high-level goal and not sufficient to answer the question.

To test whether declaration visibility doesn't have to match the definition visibility, you'd need 2 modules and compile them separately (2 LDC invocations):

A.d, to be compiled with the new flag:

export void exportedFoo() {}
void normalFoo() {} // hidden visibility

B.d, to be compiled without it (=> defaulting to public visibility, for normalFoo too):

import A;
void main()
{
    exportedFoo();
    normalFoo();
}

If linking succeeds, we have the answer. Verifying that it also works with LTO would be nice.

@MrSmith33

This comment has been minimized.

Copy link
Contributor

commented Nov 1, 2018

Doing

$ ldc2 a.d -c -export-only-symbols-marked-export
$ ldc2 b.d -c
$ ldc2 a.o b.o

produces working a program.

If that's correct way to link on linux, because using ld gives:

$ ld a.o b.o
ld: warning: cannot find entry symbol _start; defaulting to 0000000000400120
a.o: In function `ldc.register_dso':
a.d:(.text.ldc.register_dso+0x3a): undefined reference to `_d_dso_registry'
b.o: In function `ldc.register_dso':
b.d:(.text.ldc.register_dso+0x3a): undefined reference to `_d_dso_registry'
b.o: In function `main':
b.d:(.text.main[main]+0x21): undefined reference to `_d_run_main'
@MrSmith33

This comment has been minimized.

Copy link
Contributor

commented Nov 1, 2018

$ ldc2 a.d -c -export-only-symbols-marked-export -flto=thin
$ ldc2 b.d -c -flto=thin
$ ldc2 a.o b.o -flto=thin -flto-binary=/usr/lib/llvm-6.0/lib/LLVMgold.so

Seems to produce working executable too

@kinke

This comment has been minimized.

Copy link
Member

commented Nov 1, 2018

Nice, thx! [Don't worry about the ld error, you just forgot to add the druntime lib.]

Care to open a PR? As you seem to have added the cmdline option, not sure if you had to add more stuff...

@MrSmith33

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

Will open a PR!
What would be a good alternative to -export-only-symbols-marked-export?

@MrSmith33

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

Current help message is Export only symbols marked as 'export' on linux and OSX

@MrSmith33

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2018

Will go with -export-marked-symbols

MrSmith33 added a commit to MrSmith33/ldc that referenced this issue Nov 2, 2018
…ort' on linux and OSX. Resolves ldc-developers#2431.
@patrickkh7788

This comment has been minimized.

Copy link

commented Nov 3, 2018

This will work on all non-windows system like Freebsd , WebASM ?

@MrSmith33

This comment has been minimized.

Copy link
Contributor

commented Nov 3, 2018

@patrickkh7788 yes

@kinke kinke closed this in 7bcd6d3 Nov 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.