Tarantool development patches archive
 help / color / mirror / Atom feed
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

  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