[Tarantool-patches] [PATCH 1/1] build: turn off LTO for exports.c

[Tarantool-patches] [PATCH 1/1] build: turn off LTO for exports.c

[Vladislav Shpilevoy]

There were lots of errors of kind:

/builds/M4RrgQZ3/0/tarantool/tarantool/src/exports.h:395:1: error: variable ‘uuid_nil’ redeclared as function
  EXPORT(uuid_nil)
  ^
 /builds/M4RrgQZ3/0/tarantool/tarantool/src/lib/uuid/tt_uuid.c:39:22: note: previously declared here
  const struct tt_uuid uuid_nil;

when LTO was enabled. That happened because exports.c file, to
take symbol addresses, declared lots of functions and variables
from all the code base as

    extern void <symbol>(void);

This is crazy, but it worked for normal builds. Because symbol is
symbol. The compilers couldn't find conflicting declarations,
because they never met in one compilation unit.

However the lie was revealed by linker with LTO enabled. It could
see, that actual symbol definitions didn't match their exports in
exports.c. It could live with mismatching function-function or
variable-variable cases, but couldn't withstand function-variable
mismatches. When a symbol was declared as a variable in one place
and as a function in another.

This was the case for variables:
- uuid_nil
- tarantool_lua_ibuf
- log_pid
- log_format
- crc32_calc
- _say
- log_level

The errors were false positive, because the symbols were never
used for anything except taking their addresses. To calm the
linker down exports.c now does not participate in LTO.

Closes #5001
---
Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-5001-lto-exports-fnolto
Issue: https://github.com/tarantool/tarantool/issues/5001

 src/CMakeLists.txt | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 7a718fde9..c2fb7841c 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -246,6 +246,19 @@ if(BUILD_STATIC)
     endif()
 endif()
 
+if (ENABLE_LTO)
+    # Exports compilation unit is entirely a hack. It references
+    # symbols among static libraries and object files declaring
+    # them all as functions. To avoid header dependencies. This is
+    # not detected by the compilers, since they never see
+    # conflicting definitions in one compilation unit. But this
+    # makes LTO mad, because the linker sees all the definitions,
+    # and is especially angry when a variable is declared as a
+    # function. To get rid of these false positive errors the
+    # exports file is not link-time optimized.
+    set_source_files_properties(exports.c PROPERTIES COMPILE_FLAGS -fno-lto)
+endif()
+
 add_executable(
     tarantool main.cc exports.c
     ${LIBUTIL_FREEBSD_SRC}/flopen.c
-- 
2.21.1 (Apple Git-122.3)

Re: [Tarantool-patches] [PATCH 1/1] build: turn off LTO for exports.c

[Timur Safin]

Yup, let's hack a problem with using hack! 
(Though using entirely hacking approaches in this particular case
is most consistent)

LGTM

Timur

: -----Original Message-----
: From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
: Sent: Wednesday, May 20, 2020 2:31 AM
: To: tarantool-patches@dev.tarantool.org; tsafin@tarantool.org;
: avtikhon@tarantool.org
: Subject: [PATCH 1/1] build: turn off LTO for exports.c
: 
: There were lots of errors of kind:
: 
: /builds/M4RrgQZ3/0/tarantool/tarantool/src/exports.h:395:1: error:
: variable ‘uuid_nil’ redeclared as function
:   EXPORT(uuid_nil)
:   ^
:  /builds/M4RrgQZ3/0/tarantool/tarantool/src/lib/uuid/tt_uuid.c:39:22:
: note: previously declared here
:   const struct tt_uuid uuid_nil;
: 
: when LTO was enabled. That happened because exports.c file, to
: take symbol addresses, declared lots of functions and variables
: from all the code base as
: 
:     extern void <symbol>(void);
: 
: This is crazy, but it worked for normal builds. Because symbol is
: symbol. The compilers couldn't find conflicting declarations,
: because they never met in one compilation unit.
: 
: However the lie was revealed by linker with LTO enabled. It could
: see, that actual symbol definitions didn't match their exports in
: exports.c. It could live with mismatching function-function or
: variable-variable cases, but couldn't withstand function-variable
: mismatches. When a symbol was declared as a variable in one place
: and as a function in another.
: 
: This was the case for variables:
: - uuid_nil
: - tarantool_lua_ibuf
: - log_pid
: - log_format
: - crc32_calc
: - _say
: - log_level
: 
: The errors were false positive, because the symbols were never
: used for anything except taking their addresses. To calm the
: linker down exports.c now does not participate in LTO.
: 
: Closes #5001
: ---
: Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-5001-lto-
: exports-fnolto
: Issue: https://github.com/tarantool/tarantool/issues/5001
: 
:  src/CMakeLists.txt | 13 +++++++++++++
:  1 file changed, 13 insertions(+)
: 
: diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
: index 7a718fde9..c2fb7841c 100644
: --- a/src/CMakeLists.txt
: +++ b/src/CMakeLists.txt
: @@ -246,6 +246,19 @@ if(BUILD_STATIC)
:      endif()
:  endif()
: 
: +if (ENABLE_LTO)
: +    # Exports compilation unit is entirely a hack. It references
: +    # symbols among static libraries and object files declaring
: +    # them all as functions. To avoid header dependencies. This is
: +    # not detected by the compilers, since they never see
: +    # conflicting definitions in one compilation unit. But this
: +    # makes LTO mad, because the linker sees all the definitions,
: +    # and is especially angry when a variable is declared as a
: +    # function. To get rid of these false positive errors the
: +    # exports file is not link-time optimized.
: +    set_source_files_properties(exports.c PROPERTIES COMPILE_FLAGS -fno-
: lto)
: +endif()
: +
:  add_executable(
:      tarantool main.cc exports.c
:      ${LIBUTIL_FREEBSD_SRC}/flopen.c
: --
: 2.21.1 (Apple Git-122.3)

Re: [Tarantool-patches] [PATCH 1/1] build: turn off LTO for exports.c

[Alexander V. Tikhonov]

Hi Vlad, thanks a lot for the patch. I've checked it on all test
jobs at avtikhon/gh-5001-lto-exports-fnolto-full-ci branch.
Results became better:
  https://gitlab.com/tarantool/tarantool/pipelines/149423666
LTO build passed:
  https://gitlab.com/tarantool/tarantool/-/jobs/566687392
Test box/push.test.lua failed as expected:
  https://gitlab.com/tarantool/tarantool/-/jobs/566687395
The new issue will be created on it as we discussed it verbally.
Patch LGTM.

On Wed, May 20, 2020 at 01:31:27AM +0200, Vladislav Shpilevoy wrote:
> There were lots of errors of kind:
> 
> /builds/M4RrgQZ3/0/tarantool/tarantool/src/exports.h:395:1: error: variable ‘uuid_nil’ redeclared as function
>   EXPORT(uuid_nil)
>   ^
>  /builds/M4RrgQZ3/0/tarantool/tarantool/src/lib/uuid/tt_uuid.c:39:22: note: previously declared here
>   const struct tt_uuid uuid_nil;
> 
> when LTO was enabled. That happened because exports.c file, to
> take symbol addresses, declared lots of functions and variables
> from all the code base as
> 
>     extern void <symbol>(void);
> 
> This is crazy, but it worked for normal builds. Because symbol is
> symbol. The compilers couldn't find conflicting declarations,
> because they never met in one compilation unit.
> 
> However the lie was revealed by linker with LTO enabled. It could
> see, that actual symbol definitions didn't match their exports in
> exports.c. It could live with mismatching function-function or
> variable-variable cases, but couldn't withstand function-variable
> mismatches. When a symbol was declared as a variable in one place
> and as a function in another.
> 
> This was the case for variables:
> - uuid_nil
> - tarantool_lua_ibuf
> - log_pid
> - log_format
> - crc32_calc
> - _say
> - log_level
> 
> The errors were false positive, because the symbols were never
> used for anything except taking their addresses. To calm the
> linker down exports.c now does not participate in LTO.
> 
> Closes #5001
> ---
> Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-5001-lto-exports-fnolto
> Issue: https://github.com/tarantool/tarantool/issues/5001
> 
>  src/CMakeLists.txt | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
> index 7a718fde9..c2fb7841c 100644
> --- a/src/CMakeLists.txt
> +++ b/src/CMakeLists.txt
> @@ -246,6 +246,19 @@ if(BUILD_STATIC)
>      endif()
>  endif()
>  
> +if (ENABLE_LTO)
> +    # Exports compilation unit is entirely a hack. It references
> +    # symbols among static libraries and object files declaring
> +    # them all as functions. To avoid header dependencies. This is
> +    # not detected by the compilers, since they never see
> +    # conflicting definitions in one compilation unit. But this
> +    # makes LTO mad, because the linker sees all the definitions,
> +    # and is especially angry when a variable is declared as a
> +    # function. To get rid of these false positive errors the
> +    # exports file is not link-time optimized.
> +    set_source_files_properties(exports.c PROPERTIES COMPILE_FLAGS -fno-lto)
> +endif()
> +
>  add_executable(
>      tarantool main.cc exports.c
>      ${LIBUTIL_FREEBSD_SRC}/flopen.c
> -- 
> 2.21.1 (Apple Git-122.3)
> 

Re: [Tarantool-patches] [PATCH 1/1] build: turn off LTO for exports.c

[Vladislav Shpilevoy]

Pushed to master.