From: "Timur Safin" <tsafin@tarantool.org> To: 'Vladislav Shpilevoy' <v.shpilevoy@tarantool.org>, tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH v2 2/2] cmake: remove dynamic-list linker option Date: Sun, 19 Apr 2020 01:07:58 +0300 [thread overview] Message-ID: <001301d615cd$d21c5740$765505c0$@tarantool.org> (raw) In-Reply-To: <d9e22fbc-3611-8ddf-377d-5ec202dc4fe6@tarantool.org> : From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> : Subject: Re: [PATCH v2 2/2] cmake: remove dynamic-list linker option : : Thanks for the review and the patch! : : ... : The problem here is that the function's result is never used. I am : afraid some smart ass compiler may notice that, since exports.c and : main.cc are built together, and may optimize this call out. Yup, in theory, LTO should be able to proceed cross-function, intra-module optimizations and figure out that this chains of references is unnecessary. But I still hope that not all external symbols we injected would be deleted with LTO. We will need to recheck with LTO enabled. : : Also return of a value assumes a caller needs to do something with it. : I didn't like that in the Nikolay's solution. I wanted to make a function, : which does all the work by itself. Returning anything as value was a simple trick to make compiler think that unused array is used elsewhere. LTO should reveal the truth, unfortunately. : : However I like that all exports are in a separate file, not surrounded : by hacks of exports.c. So I applied your solution with some code : style problems fixed. Thanks for the help. : : The patch diff is close to 100%, so below is the whole new patch, : without incremental diff. : : ==================== : : cmake: remove dynamic-list linker option : : dynamic-list (exported_symbols_list on Mac) was used to forbid : export of all symbols of the tarantool executable except a given : list. Motivation of that was to avoid hacking the linker with : false usage of symbols needed to be exported. As a consequence, : symbols not listed in these options became invisible. : : Before these options, when a symbol was defined, but not used in : the final executable, the linker could throw it away, even though : many symbols were used by Lua FFI, or should be visible for user's : dynamic modules. Where the linker, obviously, can't see if they : are needed. : : To make the linker believe the symbols are actually needed there : was a hack with getting pointers at these functions and doing : something with them. : : For example, assume we have 'test()' function in 'box' static : library: : : int : test(void); : : It is not used anywhere in the final executable. So to trick the : linker there is a function 'export_syms()' declared, which takes a : pointer at 'test()' and seemingly does something with it (or : actually does - it does not matter): : : void : export_syms() : { : void *syms[] = {test}; : if (time(NULL) == 0) { : syms[0](); : syms[1](); : ... : } : } : : Some users want to use not documented but visible symbols, so the : patch removes the dynamic-list option, and returns the linker : hack back. But with 0 dependencies in the export file. : : Closes #2971 : Despite all the future LTO problems we may see, right now it's LGTM Regards, Timur
next prev parent reply other threads:[~2020-04-18 22:08 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-04-17 23:48 [Tarantool-patches] [PATCH v2 0/2] Unhide symbols Vladislav Shpilevoy 2020-04-17 23:48 ` [Tarantool-patches] [PATCH v2 1/2] cmake: remove double usage of some source files Vladislav Shpilevoy 2020-04-18 9:22 ` Timur Safin 2020-04-18 16:51 ` Vladislav Shpilevoy 2020-04-17 23:48 ` [Tarantool-patches] [PATCH v2 2/2] cmake: remove dynamic-list linker option Vladislav Shpilevoy 2020-04-18 12:39 ` Timur Safin 2020-04-18 16:51 ` Vladislav Shpilevoy 2020-04-18 22:07 ` Timur Safin [this message] 2020-04-22 22:08 ` [Tarantool-patches] [PATCH v2 0/2] Unhide symbols Vladislav Shpilevoy 2020-04-22 23:27 ` Timur Safin 2020-04-24 7:47 ` Kirill Yukhin 2020-04-27 10:44 ` Timur Safin 2020-04-27 21:38 ` Vladislav Shpilevoy 2020-04-27 22:58 ` Vladislav Shpilevoy 2020-05-18 21:32 ` Vladislav Shpilevoy 2020-05-19 9:02 ` Kirill Yukhin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='001301d615cd$d21c5740$765505c0$@tarantool.org' \ --to=tsafin@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v2 2/2] cmake: remove dynamic-list linker option' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox