* [Tarantool-patches] [PATCH] cmake: cleanup src/CMakeLists.txt @ 2020-06-11 0:25 HustonMmmavr 2020-06-14 21:34 ` Alexander Turenko 2020-06-15 21:20 ` Vladislav Shpilevoy 0 siblings, 2 replies; 9+ messages in thread From: HustonMmmavr @ 2020-06-11 0:25 UTC (permalink / raw) To: tarantool-patches, yaroslav.dynnikov, avtikhon, alexander.turenko Removed definition and initialization of EXPORT_LIST variable at file src/CMakeLists.txt. After patch 03790ac551 this variable is unused (no reference to this variable after its initialization can be found in whole project) and it is only misleading. Closes #5066 --- I've builded tarantool before applying this changes and after. Then I've checked difference in tarantool binary file symbols with nm and diff commands and there was no difference. Issue: https://github.com/tarantool/tarantool/issues/5066 Branch: https://github.com/tarantool/tarantool/tree/HustonMmmavr/gh-5066-delete-unusued-export-list @ChangeLog - Cleanup src/CMakeLists.txt (gh-5066) src/CMakeLists.txt | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 7099e9bef..d86e6bfb1 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -213,33 +213,7 @@ add_subdirectory(box) set(TARANTOOL_C_FLAGS ${CMAKE_C_FLAGS} PARENT_SCOPE) set(TARANTOOL_CXX_FLAGS ${CMAKE_CXX_FLAGS} PARENT_SCOPE) -set(EXPORT_LIST) if(BUILD_STATIC) - # for each static library we should find a corresponding shared library to - # parse and reexport library api functions - foreach(libstatic - ${READLINE_LIBRARIES} - ${CURL_LIBRARIES} - ${OPENSSL_LIBRARIES} - ${ICU_LIBRARIES}) - if (${libstatic} MATCHES "lib[^/]+.a") - string(REGEX MATCH "lib[^/]+.a" libname ${libstatic}) - string(REGEX REPLACE "\\.a$" "" libname ${libname}) - string(REGEX REPLACE "^lib" "" libname ${libname}) - find_library(SYMBOLS_LIB NAMES ${libname}) - # add found library to export list - list(APPEND EXPORT_LIST ${SYMBOLS_LIB}) - # set variable to allow rescan (CMake depended) - set(SYMBOLS_LIB "SYMBOLS_LIB-NOTFOUND") - elseif (${libstatic} STREQUAL bundled-libcurl OR - ${libstatic} STREQUAL bundled-ares) - message("We don't need to export symbols from statically linked ${libstatic}, skipped") - else() - message(WARNING "${libstatic} should be a static") - endif() - endforeach(libstatic) - string(REPLACE ";" " " EXPORT_LIST "${EXPORT_LIST}") - if (HAVE_OPENMP) # Link libgomp explicitly to make it static. Avoid linking # against DSO version of libgomp, which implied by -fopenmp -- 2.26.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Tarantool-patches] [PATCH] cmake: cleanup src/CMakeLists.txt 2020-06-11 0:25 [Tarantool-patches] [PATCH] cmake: cleanup src/CMakeLists.txt HustonMmmavr @ 2020-06-14 21:34 ` Alexander Turenko 2020-06-15 17:27 ` Mavr Huston 2020-06-15 21:20 ` Vladislav Shpilevoy 1 sibling, 1 reply; 9+ messages in thread From: Alexander Turenko @ 2020-06-14 21:34 UTC (permalink / raw) To: HustonMmmavr; +Cc: tarantool-patches, Vladislav Shpilevoy, yaroslav.dynnikov LGTM. Vlad, can you look at the patch? WBR, Alexander Turenko. On Thu, Jun 11, 2020 at 03:25:10AM +0300, HustonMmmavr wrote: > Removed definition and initialization of EXPORT_LIST variable at file > src/CMakeLists.txt. After patch 03790ac551 this variable is unused > (no reference to this variable after its initialization can be found > in whole project) and it is only misleading. > > Closes #5066 I would also mark it as follow up for #2971. > --- > I've builded tarantool before applying this changes and after. > Then I've checked difference in tarantool binary file symbols with > nm and diff commands and there was no difference. > > Issue: https://github.com/tarantool/tarantool/issues/5066 > Branch: https://github.com/tarantool/tarantool/tree/HustonMmmavr/gh-5066-delete-unusued-export-list > > @ChangeLog > - Cleanup src/CMakeLists.txt (gh-5066) This change is not visible for a user, so I would not add a changelog entry for it. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Tarantool-patches] [PATCH] cmake: cleanup src/CMakeLists.txt 2020-06-14 21:34 ` Alexander Turenko @ 2020-06-15 17:27 ` Mavr Huston 0 siblings, 0 replies; 9+ messages in thread From: Mavr Huston @ 2020-06-15 17:27 UTC (permalink / raw) To: Alexander Turenko Cc: tarantool-patches, Vladislav Shpilevoy, yaroslav.dynnikov [-- Attachment #1: Type: text/plain, Size: 1198 bytes --] Thanks! I've marked it as follow up for #2971 at commit message. пн, 15 июн. 2020 г. в 00:35, Alexander Turenko < alexander.turenko@tarantool.org>: > LGTM. > > Vlad, can you look at the patch? > > WBR, Alexander Turenko. > > On Thu, Jun 11, 2020 at 03:25:10AM +0300, HustonMmmavr wrote: > > Removed definition and initialization of EXPORT_LIST variable at file > > src/CMakeLists.txt. After patch 03790ac551 this variable is unused > > (no reference to this variable after its initialization can be found > > in whole project) and it is only misleading. > > > > Closes #5066 > > I would also mark it as follow up for #2971. > > > --- > > I've builded tarantool before applying this changes and after. > > Then I've checked difference in tarantool binary file symbols with > > nm and diff commands and there was no difference. > > > > Issue: https://github.com/tarantool/tarantool/issues/5066 > > Branch: > https://github.com/tarantool/tarantool/tree/HustonMmmavr/gh-5066-delete-unusued-export-list > > > > @ChangeLog > > - Cleanup src/CMakeLists.txt (gh-5066) > > This change is not visible for a user, so I would not add a changelog > entry for it. > [-- Attachment #2: Type: text/html, Size: 1848 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Tarantool-patches] [PATCH] cmake: cleanup src/CMakeLists.txt 2020-06-11 0:25 [Tarantool-patches] [PATCH] cmake: cleanup src/CMakeLists.txt HustonMmmavr 2020-06-14 21:34 ` Alexander Turenko @ 2020-06-15 21:20 ` Vladislav Shpilevoy 2020-06-17 15:29 ` Mavr Huston 1 sibling, 1 reply; 9+ messages in thread From: Vladislav Shpilevoy @ 2020-06-15 21:20 UTC (permalink / raw) To: HustonMmmavr, tarantool-patches, yaroslav.dynnikov, avtikhon, alexander.turenko Hi! On 11/06/2020 02:25, HustonMmmavr wrote: > Removed definition and initialization of EXPORT_LIST variable at file > src/CMakeLists.txt. After patch 03790ac551 this variable is unused > (no reference to this variable after its initialization can be found > in whole project) and it is only misleading. Actually I am not sure it is not needed. Seems like purpose of this exporter was the same as for static libraries in a non-static build - not to allow to remove any public symbols of these libraries. Because linker can eliminate some parts of static libraries, if sees they are not used in the final executable. So probably EXPORT_LIST for static build should be exported just like exports.h, and it was missed in #2971. > Closes #5066 > --- > I've builded tarantool before applying this changes and after. > Then I've checked difference in tarantool binary file symbols with > nm and diff commands and there was no difference. Have you tried static build or normal build? Did you see the content of EXPORT_LIST, what is there? Isn't this related to https://github.com/tarantool/tarantool/issues/4559? > Issue: https://github.com/tarantool/tarantool/issues/5066 > Branch: https://github.com/tarantool/tarantool/tree/HustonMmmavr/gh-5066-delete-unusued-export-list ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Tarantool-patches] [PATCH] cmake: cleanup src/CMakeLists.txt 2020-06-15 21:20 ` Vladislav Shpilevoy @ 2020-06-17 15:29 ` Mavr Huston 2020-06-17 23:09 ` Vladislav Shpilevoy 0 siblings, 1 reply; 9+ messages in thread From: Mavr Huston @ 2020-06-17 15:29 UTC (permalink / raw) To: Vladislav Shpilevoy Cc: Alexander Turenko, tarantool-patches, yaroslav.dynnikov [-- Attachment #1: Type: text/plain, Size: 2469 bytes --] EXPORT_LIST contains following libraries in case of static build (with normal build it's empty): /usr/lib/x86_64-linux-gnu/libreadline.so /usr/lib/x86_64-linux-gnu/libcurses.so /usr/lib/x86_64-linux-gnu/libform.so /usr/lib/x86_64-linux-gnu/libtinfo.so /usr/lib/x86_64-linux-gnu/libz.so /opt/local/lib/libssl.so /opt/local/lib/libcrypto.so /usr/lib/x86_64-linux-gnu/libz.so /opt/local/lib/libicui18n.so /opt/local/lib/libicuuc.so /opt/local/lib/libicudata.so It doesn’t contains libcurl because it’s bundled statically. So it isn’t related to https://github.com/tarantool/tarantool/issues/4559 but this problem may be solved with next patch: https://github.com/tarantool/tarantool/tree/rosik/refactor-static-build. At this patch added flag --disble-symbos-hiding ( https://github.com/tarantool/tarantool/blob/rosik/refactor-static-build/cmake/BuildLibCURL.cmake#L93 ) at building libcurl and after that most of libcurl symbols are visible from tarantool binary вт, 16 июн. 2020 г. в 00:20, Vladislav Shpilevoy <v.shpilevoy@tarantool.org >: > Hi! > > On 11/06/2020 02:25, HustonMmmavr wrote: > > Removed definition and initialization of EXPORT_LIST variable at file > > src/CMakeLists.txt. After patch 03790ac551 this variable is unused > > (no reference to this variable after its initialization can be found > > in whole project) and it is only misleading. > > Actually I am not sure it is not needed. Seems like purpose of this > exporter was the same as for static libraries in a non-static build - > not to allow to remove any public symbols of these libraries. > > Because linker can eliminate some parts of static libraries, if sees > they are not used in the final executable. > > So probably EXPORT_LIST for static build should be exported just like > exports.h, and it was missed in #2971. > > > Closes #5066 > > --- > > I've builded tarantool before applying this changes and after. > > Then I've checked difference in tarantool binary file symbols with > > nm and diff commands and there was no difference. > > Have you tried static build or normal build? Did you see the content > of EXPORT_LIST, what is there? Isn't this related to > https://github.com/tarantool/tarantool/issues/4559? > > > Issue: https://github.com/tarantool/tarantool/issues/5066 > > Branch: > https://github.com/tarantool/tarantool/tree/HustonMmmavr/gh-5066-delete-unusued-export-list > [-- Attachment #2: Type: text/html, Size: 6579 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Tarantool-patches] [PATCH] cmake: cleanup src/CMakeLists.txt 2020-06-17 15:29 ` Mavr Huston @ 2020-06-17 23:09 ` Vladislav Shpilevoy 2020-06-19 13:02 ` Yaroslav Dynnikov 0 siblings, 1 reply; 9+ messages in thread From: Vladislav Shpilevoy @ 2020-06-17 23:09 UTC (permalink / raw) To: Mavr Huston; +Cc: Alexander Turenko, tarantool-patches, yaroslav.dynnikov On 17/06/2020 17:29, Mavr Huston wrote: > > EXPORT_LIST contains following libraries in case of static build (with normal build it's empty): > > /usr/lib/x86_64-linux-gnu/libreadline.so > > /usr/lib/x86_64-linux-gnu/libcurses.so > > /usr/lib/x86_64-linux-gnu/libform.so > > /usr/lib/x86_64-linux-gnu/libtinfo.so > > /usr/lib/x86_64-linux-gnu/libz.so > > /opt/local/lib/libssl.so > > /opt/local/lib/libcrypto.so > > /usr/lib/x86_64-linux-gnu/libz.so > > /opt/local/lib/libicui18n.so > > /opt/local/lib/libicuuc.so > > /opt/local/lib/libicudata.so > > > It doesn’t contains libcurl because it’s bundled statically. So it isn’t related to https://github.com/tarantool/tarantool/issues/4559but this problem may be solved with next patch: https://github.com/tarantool/tarantool/tree/rosik/refactor-static-build. At this patch added flag --disble-symbos-hiding (https://github.com/tarantool/tarantool/blob/rosik/refactor-static-build/cmake/BuildLibCURL.cmake#L93) at building libcurl and after that most of libcurl symbols are visible from tarantool binary The problem is not in the hiding. It is about removal. Not used symbols from static libs may be removed from the final executable. Hide or not hide rules are applied to what is left. That is the single reason why we had exports file before 2971 and have exports.h and exports.c now. You can try it by yourself - just add an unused function to lib/bit to bit.h and bit.c. And don't use it anywhere. You may even add 'export' to it, or change visibility rules using __attribute__ - it does not matter. If the function is not used and is not added to exports.h, you won't see it in the executable. (At least it was so last time I tried, works not with all libs, but with lib/bit it worked). Seems EXPORT_LIST was used to extract all symbols from the static libs and force their exposure + forbid their removal. Here the symbols were retrieved: https://github.com/tarantool/tarantool/commit/03790ac5510648d1d9648bb2281857a7992d0593#diff-6b9c867c54f1a1b792de45d5262f1dcfL20-L25 Here the libs were passed to mkexports: https://github.com/tarantool/tarantool/commit/03790ac5510648d1d9648bb2281857a7992d0593#diff-95e351a3805a1dafa85bf20b81d086e6L253-L260 We probably should resurrect that part. Rename the current exports.h to exports.h.in and generate exports.h during cmake. Like it was done for exports before 2971. To forbid symbols removal. Not to unhide them. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Tarantool-patches] [PATCH] cmake: cleanup src/CMakeLists.txt 2020-06-17 23:09 ` Vladislav Shpilevoy @ 2020-06-19 13:02 ` Yaroslav Dynnikov 2020-06-19 23:39 ` Vladislav Shpilevoy 0 siblings, 1 reply; 9+ messages in thread From: Yaroslav Dynnikov @ 2020-06-19 13:02 UTC (permalink / raw) To: Vladislav Shpilevoy; +Cc: tarantool-patches, Alexander Turenko [-- Attachment #1: Type: text/plain, Size: 5724 bytes --] I've researched the linking process and here is what I've found. First of all, let's talk about symbols removal. Here is an interesting note on how it works: https://stackoverflow.com/questions/55130965/when-and-why-would-the-c-linker-exclude-unused-symbols One of the basic important concepts is an ELF section. It's a chunk of data (assembly code in our case) that linker operates on. A section may contain code of one or more functions (depending on build arguments), but it can't be split during linking. Usually, when we build an object file with `gcc libx.c -c -o libx.o`, all functions from libx.c go into the single ".text" section. This single object file is usually archived with others into single libx.a, which is later processed during main executable linking. By default (but I can't provide the proof), when we call `gcc -lx` linker operates on object files from the archive - if at least one symbol is used, whole .o (not .a) is included in the resulting binary. There are also two linker options that influence this behavior. At first, there is `-Wl,--whole-archive`, which makes it to include whole `.a` instead of `.o` granularity. Secondly, there is `-Wl,--gc-sections` which could remove unused sections, but in basic example it's useless since all symbols from .o belong to the same .text section. To make `--gc-sections` have an effect one should compile object files with `-ffunction-sections` flag. It'll generate a separate section for every function so the linker could gc unused ones. See: ```console$ cat libx.c #include <stdio.h> void fA() { printf("fA is here\n"); } void fB() { printf("fB is here\n"); } $ gcc libx.c -c -o libx.o -ffunction-sections $ readelf -S libx.o | grep .text [ 1] .text PROGBITS 0000000000000000 00000040 [ 5] .text.fA PROGBITS 0000000000000000 00000056 [ 6] .rela.text.fA RELA 0000000000000000 000002d8 [ 7] .text.fB PROGBITS 0000000000000000 0000006d [ 8] .rela.text.fB RELA 0000000000000000 00000308 ``` Now let's move to the `libbit` which Vlad mentioned. I've investigated how compiler options influence the resulting binary. Unused functions from bit.c are really remover, but only with Release flags, and here is why: There are only 2 functions implemented in bit.c, and both are unused. All the others are inlines in bit.h and externs from luajit. When tarantool is built in debug mode, the inlining is off, so other modules truly link to the bit.o and all symbols remain including unused functions. But if we specify -O2 flag, inlining takes place, and all the symbols from bit.o becomes unused, so the linker drops the whole object file. Finally, speaking about this patch, my proposal is to merge this PR as is. And since we know how to manage linking, other problems can be solved separately (if they ever occur). Best regards Yaroslav Dynnikov On Thu, 18 Jun 2020 at 02:09, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote: > On 17/06/2020 17:29, Mavr Huston wrote: > > > > EXPORT_LIST contains following libraries in case of static build (with > normal build it's empty): > > > > /usr/lib/x86_64-linux-gnu/libreadline.so > > > > /usr/lib/x86_64-linux-gnu/libcurses.so > > > > /usr/lib/x86_64-linux-gnu/libform.so > > > > /usr/lib/x86_64-linux-gnu/libtinfo.so > > > > /usr/lib/x86_64-linux-gnu/libz.so > > > > /opt/local/lib/libssl.so > > > > /opt/local/lib/libcrypto.so > > > > /usr/lib/x86_64-linux-gnu/libz.so > > > > /opt/local/lib/libicui18n.so > > > > /opt/local/lib/libicuuc.so > > > > /opt/local/lib/libicudata.so > > > > > > It doesn’t contains libcurl because it’s bundled statically. So it isn’t > related to https://github.com/tarantool/tarantool/issues/4559but this > problem may be solved with next patch: > https://github.com/tarantool/tarantool/tree/rosik/refactor-static-build. > At this patch added flag --disble-symbos-hiding ( > https://github.com/tarantool/tarantool/blob/rosik/refactor-static-build/cmake/BuildLibCURL.cmake#L93) > at building libcurl and after that most of libcurl symbols are visible from > tarantool binary > > The problem is not in the hiding. It is about removal. Not used symbols > from > static libs may be removed from the final executable. Hide or not hide > rules > are applied to what is left. That is the single reason why we had exports > file > before 2971 and have exports.h and exports.c now. You can try it by > yourself - > just add an unused function to lib/bit to bit.h and bit.c. And don't use it > anywhere. You may even add 'export' to it, or change visibility rules using > __attribute__ - it does not matter. If the function is not used and is not > added to exports.h, you won't see it in the executable. (At least it was so > last time I tried, works not with all libs, but with lib/bit it worked). > > Seems EXPORT_LIST was used to extract all symbols from the static libs and > force their exposure + forbid their removal. Here the symbols were > retrieved: > > https://github.com/tarantool/tarantool/commit/03790ac5510648d1d9648bb2281857a7992d0593#diff-6b9c867c54f1a1b792de45d5262f1dcfL20-L25 > > Here the libs were passed to mkexports: > > https://github.com/tarantool/tarantool/commit/03790ac5510648d1d9648bb2281857a7992d0593#diff-95e351a3805a1dafa85bf20b81d086e6L253-L260 > > We probably should resurrect that part. Rename the current exports.h to > exports.h.in > and generate exports.h during cmake. Like it was done for exports before > 2971. To > forbid symbols removal. Not to unhide them. > [-- Attachment #2: Type: text/html, Size: 7348 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Tarantool-patches] [PATCH] cmake: cleanup src/CMakeLists.txt 2020-06-19 13:02 ` Yaroslav Dynnikov @ 2020-06-19 23:39 ` Vladislav Shpilevoy 2020-06-23 20:31 ` Yaroslav Dynnikov 0 siblings, 1 reply; 9+ messages in thread From: Vladislav Shpilevoy @ 2020-06-19 23:39 UTC (permalink / raw) To: Yaroslav Dynnikov; +Cc: tarantool-patches, Alexander Turenko Hi! Thanks for the investigation! On 19/06/2020 15:02, Yaroslav Dynnikov wrote: > I've researched the linking process and here is what I've found. > > First of all, let's talk about symbols removal. Here is an interesting note on > how it works: https://stackoverflow.com/questions/55130965/when-and-why-would-the-c-linker-exclude-unused-symbols > One of the basic important concepts is an ELF section. It's a chunk of data > (assembly code in our case) that linker operates on. A section may contain code > of one or more functions (depending on build arguments), but it can't be split > during linking. > > Usually, when we build an object file with `gcc libx.c -c -o libx.o`, all > functions from libx.c go into the single ".text" section. This single object > file is usually archived with others into single libx.a, which is later > processed during main executable linking. > > By default (but I can't provide the proof), when we call `gcc -lx` linker > operates on object files from the archive - if at least one symbol is used, > whole .o (not .a) is included in the resulting binary. There are also two linker > options that influence this behavior. At first, there is `-Wl,--whole-archive`, > which makes it to include whole `.a` instead of `.o` granularity. Secondly, there is > `-Wl,--gc-sections` which could remove unused sections, but in basic example > it's useless since all symbols from .o belong to the same .text section. To make > `--gc-sections` have an effect one should compile object files with > `-ffunction-sections` flag. It'll generate a separate section for every function > so the linker could gc unused ones. > > See: > ```console$ cat libx.c > #include <stdio.h> > > void fA() { > printf("fA is here\n"); > } > void fB() { > printf("fB is here\n"); > } > $ gcc libx.c -c -o libx.o -ffunction-sections > $ readelf -S libx.o | grep .text > [ 1] .text PROGBITS 0000000000000000 00000040 > [ 5] .text.fA PROGBITS 0000000000000000 00000056 > [ 6] .rela.text.fA RELA 0000000000000000 000002d8 > [ 7] .text.fB PROGBITS 0000000000000000 0000006d > [ 8] .rela.text.fB RELA 0000000000000000 00000308 > ``` > > Now let's move to the `libbit` which Vlad mentioned. > I've investigated how compiler options influence the resulting binary. Unused > functions from bit.c are really remover, but only with Release flags, and here > is why: > > There are only 2 functions implemented in bit.c, and both are unused. All the > others are inlines in bit.h and externs from luajit. When tarantool is built in > debug mode, the inlining is off, so other modules truly link to the bit.o and > all symbols remain including unused functions. But if we specify -O2 flag, > inlining takes place, and all the symbols from bit.o becomes unused, so the > linker drops the whole object file. So do you mean, that if a library consists of more than 1 C file (and builds more than one .o file), and functions from some of them are not used, these .o files and their code will be removed? If it is true, it does not look like green light for the patch and requires more experiments. For example, try to add a new library with several C files, use only some of them in the Tarantool executable (just add to exports.h), and see if code from unused .o files is removed even when they are built into .a file. However, as I said in private - I am ok with pushing this patch now. I just don't see why is it necessary. Why is it so important to push it? Push just for push? Just to close an issue? It does not improve anything, EXPORT_LIST still may appear to be useful along with some other things I removed in 2971, when I didn't think of the static build. > Finally, speaking about this patch, my proposal is to merge this PR as is. > And since we know how to manage linking, other problems can be solved separately > (if they ever occur). > > Best regards > Yaroslav Dynnikov ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Tarantool-patches] [PATCH] cmake: cleanup src/CMakeLists.txt 2020-06-19 23:39 ` Vladislav Shpilevoy @ 2020-06-23 20:31 ` Yaroslav Dynnikov 0 siblings, 0 replies; 9+ messages in thread From: Yaroslav Dynnikov @ 2020-06-23 20:31 UTC (permalink / raw) To: Vladislav Shpilevoy; +Cc: tarantool-patches, Alexander Turenko [-- Attachment #1: Type: text/plain, Size: 5293 bytes --] Hi, Vlad. Sorry for the late answer. I was going to recheck ideas you've proposed, but didn't have time for that. So my answer is based on my knowledge only. On Sat, 20 Jun 2020, 02:39 Vladislav Shpilevoy, <v.shpilevoy@tarantool.org> wrote: > Hi! Thanks for the investigation! > > On 19/06/2020 15:02, Yaroslav Dynnikov wrote: > > I've researched the linking process and here is what I've found. > > > > First of all, let's talk about symbols removal. Here is an interesting > note on > > how it works: https://stackoverflow.com/questions/55130965/when-drop > unuseddrop unusedand-why-would-the-c-linker-exclude-unused-symbols > <https://stackoverflow.com/questions/55130965/when-and-why-would-the-c-linker-exclude-unused-symbols> > > One of the basic important concepts is an ELF section. It's a chunk of > data > > (assembly code in our case) that linker operates on. A section may > contain code > > of one or more functions (depending on build arguments), but it can't be > split > > during linking. > > > > Usually, when we build an object file with `gcc libx.c -c -o libx.o`, all > > functions from libx.c go into the single ".text" section. This single > object > > file is usually archived with others into single libx.a, which is later > > processed during main executable linking. > > > > By default (but I can't provide the proof), when we call `gcc -lx` linker > > operates on object files from the archive - if at least one symbol is > used, > > whole .o (not .a) is included in the resulting binary. There are also > two linker > > options that influence this behavior. At first, there is > `-Wl,--whole-archive`, > > which makes it to include whole `.a` instead of `.o` granularity. > Secondly, there is > > `-Wl,--gc-sections` which could remove unused sections, but in basic > example > > it's useless since all symbols from .o belong to the same .text section. > To make > > `--gc-sections` have an effect one should compile object files with > > `-ffunction-sections` flag. It'll generate a separate section for every > function > > so the linker could gc unused ones. > > > > See: > > ```console$ cat libx.c > > #include <stdio.h> > > > > void fA() { > > printf("fA is here\n"); > > } > > void fB() { > > printf("fB is here\n"); > > } > > $ gcc libx.c -c -o libx.o -ffunction-sections > > $ readelf -S libx.o | grep .text > > [ 1] .text PROGBITS 0000000000000000 00000040 > > [ 5] .text.fA PROGBITS 0000000000000000 00000056 > > [ 6] .rela.text.fA RELA 0000000000000000 000002d8 > > [ 7] .text.fB PROGBITS 0000000000000000 0000006d > > [ 8] .rela.text.fB RELA 0000000000000000 00000308 > > ``` > > > > Now let's move to the `libbit` which Vlad mentioned. > > I've investigated how compiler options influence the resulting binary. > Unused > > functions from bit.c are really remover, but only with Release flags, > and here > > is why: > > > > There are only 2 functions implemented in bit.c, and both are unused. > All the > > others are inlines in bit.h and externs from luajit. When tarantool is > built in > > debug mode, the inlining is off, so other modules truly link to the > bit.o and > > all symbols remain including unused functions. But if we specify -O2 > flag, > > inlining takes place, and all the symbols from bit.o becomes unused, so > the > > linker drops the whole object file. > > So do you mean, that if a library consists of more than 1 C file (and > builds > more than one .o file), and functions from some of them are not used, these > .o files and their code will be removed? > Exactly. Moreover, it depends on the gcc command syntax. Suppose we have libx.a with two object files: fA.o and fB.o. Function from fA is used, and fB isn't. Then `gcc main.c libx.a` will produce binary with both fA and fB. While `gcc main.c -lx` will include fA only, and unused fB will be dropped. It's also mentioned in the SO question <https://stackoverflow.com/questions/55130965/when-and-why-would-the-c-linker-exclude-unused-symbols> I linked in the previous message. > If it is true, it does not look like green light for the patch and requires > more experiments. For example, try to add a new library with several C > files, > use only some of them in the Tarantool executable (just add to exports.h), > and see if code from unused .o files is removed even when they are built > into > .a file. > I guess the solution will be adding --whole-archive flag. But i'm not sure yet. > > However, as I said in private - I am ok with pushing this patch now. I just > don't see why is it necessary. Why is it so important to push it? Push just > for push? Just to close an issue? It does not improve anything, EXPORT_LIST > still may appear to be useful along with some other things I removed in > 2971, > when I didn't think of the static build. > You're right, it's not that important. I've already told Alex Barulev that we'll postpone this cleanup until we finish with static build refactoring. > > > Finally, speaking about this patch, my proposal is to merge this PR as > is. > > And since we know how to manage linking, other problems can be solved > separately > > (if they ever occur). > > > > Best regards > > Yaroslav Dynnikov > [-- Attachment #2: Type: text/html, Size: 6889 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-06-23 20:32 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-11 0:25 [Tarantool-patches] [PATCH] cmake: cleanup src/CMakeLists.txt HustonMmmavr 2020-06-14 21:34 ` Alexander Turenko 2020-06-15 17:27 ` Mavr Huston 2020-06-15 21:20 ` Vladislav Shpilevoy 2020-06-17 15:29 ` Mavr Huston 2020-06-17 23:09 ` Vladislav Shpilevoy 2020-06-19 13:02 ` Yaroslav Dynnikov 2020-06-19 23:39 ` Vladislav Shpilevoy 2020-06-23 20:31 ` Yaroslav Dynnikov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox