fmtlib / fmt

A modern formatting library
https://fmt.dev
Other
20.74k stars 2.49k forks source link

is_printable is not compiled into the static lib #2786

Closed risa2000 closed 2 years ago

risa2000 commented 2 years ago

When building the static libs (on Windows), I noticed that function

bool __cdecl fmt::v8::detail::is_printable(unsigned int)

is exported from the fmt.lib but the implementation is not included.

This is a snippet for the fmt.lib symbol dump.

99D 00000000 SECT44B notype ()    External    | ?size@?$basic_string_view@_W@v8@fmt@@QEBA_KXZ (public: unsigned __int64 __cdecl fmt::v8::basic_string_view<wchar_t>::size(void)const )
99E 00000000 UNDEF  notype ()    External     | ?is_printable@detail@v8@fmt@@YA_NI@Z (bool __cdecl fmt::v8::detail::is_printable(unsigned int))
99F 00000000 SECT415 notype ()    External    | ?needs_escape@detail@v8@fmt@@YA_NI@Z (bool __cdecl fmt::v8::detail::needs_escape(unsigned int))

It guess the problem is that the implementation is in format-inl.h which is not compiled into the lib.

vitaut commented 2 years ago

Do you get an error or is it more of a theoretical question?

risa2000 commented 2 years ago

I am building my static lib, which uses fmt lib and with it it pulls in also the exported symbol is_printable. Then when linking my app against my static lib and fmt.lib, the build fails because the inline version is not compiled in the app, and the non-inline version is missing from fmt.lib.

vitaut commented 2 years ago

is_printable is defined here: https://github.com/fmtlib/fmt/blob/0742606f199d9839123d36c5cb65c548f5f46f6e/include/fmt/format-inl.h#L2531

Note that it is not inline (except for the header-only mode).

risa2000 commented 2 years ago

I might have misread format.h into thinking that format-inl.h is not included into the build when building against the static lib, but the fact is that is_printable is not compiled into the final fmt.lib (as shown in my snippet I posted above), and is nowhere else to find either.

So I might turn the question around. Is it supposed to be in fmt.lib? If not, then why is fmt.lib exporting the symbol for it? If yes, then there is clearly a problem with library build.

vitaut commented 2 years ago

I'm not sure why is_printable is not a part of the static library in your setup. I tried to repro it locally but to no avail:

$ nm -a libfmtd.a
...
0000000000002070 T fmt::v8::detail::is_printable(unsigned int)
mwinterb commented 2 years ago

format.cc includes format-inl.h: https://github.com/fmtlib/fmt/blob/c28500556a027fc3b84dc06e0021c719480fd1f3/src/format.cc#L8

vitaut commented 2 years ago

Do you use the default CMake config provided by {fmt}? Also what version of do you use and does the issue happen on current master?

risa2000 commented 2 years ago

I am using Visual Studio 2019 native x64 toolchain environment to configure and build fmt this way:

D:\Work_OSS\fmt\build>cmake -G Ninja  -DCMAKE_BUILD_TYPE=Release -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_MSVC_RUNTIME_LIBRARY="MultiThreaded$<$<CONFIG:Debug>:Debug>DLL" ..
-- CMake version: 3.20.21032501-MSVC_2
-- The CXX compiler identification is MSVC 19.29.30140.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/VC/Tools/MSVC/14.29.30133/bin/Hostx64/x64/cl.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Module support is disabled.
-- Version: 8.1.2
-- Build type: Release
-- CXX_STANDARD: 11
-- Performing Test has_std_11_flag
-- Performing Test has_std_11_flag - Failed
-- Performing Test has_std_0x_flag
-- Performing Test has_std_0x_flag - Failed
-- Performing Test SUPPORTS_USER_DEFINED_LITERALS
-- Performing Test SUPPORTS_USER_DEFINED_LITERALS - Success
-- Performing Test FMT_HAS_VARIANT
-- Performing Test FMT_HAS_VARIANT - Success
-- Required features: cxx_variadic_templates
-- Target 'doc' disabled (requires doxygen)
-- Looking for C++ include pthread.h
-- Looking for C++ include pthread.h - not found
-- Found Threads: TRUE
-- Performing Test HAVE_FNO_DELETE_NULL_POINTER_CHECKS
-- Performing Test HAVE_FNO_DELETE_NULL_POINTER_CHECKS - Failed
-- FMT_PEDANTIC: OFF
-- Configuring done
-- Generating done
-- Build files have been written to: D:/Work_OSS/fmt/build

D:\Work_OSS\fmt\build>ninja
[65/65] Linking CXX executable bin\printf-test.exe

D:\Work_OSS\fmt\build>dumpbin /symbols fmt.lib > fmt.lib.txt

CMAKE_MSVC_RUNTIME_LIBRARY is CMake way to instruct the toolchain to link against DLL runtime.

is_printable is not in fmt.lib

99D 00000000 SECT44B notype ()    External    | ?size@?$basic_string_view@_W@v8@fmt@@QEBA_KXZ (public: unsigned __int64 __cdecl fmt::v8::basic_string_view<wchar_t>::size(void)const )
99E 00000000 UNDEF  notype ()    External     | ?is_printable@detail@v8@fmt@@YA_NI@Z (bool __cdecl fmt::v8::detail::is_printable(unsigned int))
99F 00000000 SECT415 notype ()    External    | ?needs_escape@detail@v8@fmt@@YA_NI@Z (bool __cdecl fmt::v8::detail::needs_escape(unsigned int))

Just to add, I am building the current master. If I revert back to the commit before is_printable was introduced the library works fine (but I guess it should be expected).

vitaut commented 2 years ago

Could you post the full dumpbin output?

risa2000 commented 2 years ago

Here is the dumpbin output.

fmt.lib.txt

vitaut commented 2 years ago

According to the full dumpbin output is_printable is defined:

DAA 00000000 SECT57E notype ()    External    | ?is_printable@detail@v8@fmt@@YA_NI@Z (bool __cdecl fmt::v8::detail::is_printable(unsigned int))

I think the UNDEF entry is OK because it should be resolved to the one above by a linker.

risa2000 commented 2 years ago

Hmm, I did not realize that the symbol could be exported more than once. I have to have some other problem then, because the linker fails, complaining about not finding this particular function. I am not at the machine which exhibits this behavior right now, but I will see if I could come up with some reproducible scenario.

risa2000 commented 2 years ago

I did a quick test with the simple code which builds fine. So the problem must be in my convoluted case when linking my library which uses fmtlib.

fmt_test.cpp:

#include <fmt/format.h>

namespace fmt::v8::detail {
bool is_printable(unsigned int);
}

int main(int argc, char* argv[])
{
    auto test = fmt::v8::detail::is_printable(42);
    return test ? 0 : 1;
}

CMakeLists.txt:

cmake_minimum_required (VERSION 3.15)
project (fmt_test)
set (CMAKE_CXX_STANDARD 17)
set (CMAKE_CXX_STANDARD_REQUIRED ON)
find_package(fmt 8.0.0 REQUIRED HINTS D:/Work_OSS/fmt/out/install/x64-DLL-Release)

add_executable (fmt_test fmt_test.cpp )
target_link_libraries (fmt_test PRIVATE fmt::fmt)
vitaut commented 2 years ago

Closing for now then but feel free to reopen with a repro.

scramsby commented 1 year ago

I just hit this same issue when working through an upgrade and I think I tracked it down to an issue where we were building C++ static libs with two different C++ build systems that have slightly different toolchains or configs which caused different C++ name mangling of is_printable(). Our temporary hack workaround was to add extern "C" { ... } around the declaration and definition of the is_printable() function until we can fix the toolchain mismatch.