Skip to content

Commit

Permalink
Updated library version selection to avoid ODR violations.
Browse files Browse the repository at this point in the history
The previous strategy of force-inlining methods that are dependent
on the library version is not effective with MSVC in debug mode,
when linking the static library. The compiler does not inline
methods despite the markup, and during linking the static library
with the user's module the linker may or may not pick up user's
definitions of these methods.

When building the library, do not define path methods that depend
on the library version. Instead, explicitly call v4 internal methods
throughout the implementation. This way the compiled library does not
contain v4 versions of these methods, and therefore does not conflict
with user's definitions of these methods.

Fixes #279.
  • Loading branch information
Lastique committed Feb 7, 2023
1 parent 5bebf78 commit 7509619
Show file tree
Hide file tree
Showing 11 changed files with 1,008 additions and 783 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ jobs:
compiler: clang++-14
cxxstd: "03-gnu,11-gnu,14-gnu,17-gnu,20-gnu,2b-gnu"
cxxflags: -stdlib=libc++
linkflags: -stdlib=libc++
linkflags: "-stdlib=libc++ -lubsan"
ubsan: 1
build_variant: debug
os: ubuntu-22.04
Expand Down
25 changes: 25 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,31 @@ endif()
add_library(boost_filesystem ${BOOST_FILESYSTEM_SOURCES})
add_library(Boost::filesystem ALIAS boost_filesystem)

get_target_property(BOOST_FILESYSTEM_TARGET_TYPE boost_filesystem TYPE)
if(BOOST_FILESYSTEM_TARGET_TYPE STREQUAL "SHARED_LIBRARY")
set(CMAKE_REQUIRED_LIBRARIES "-Wl,--no-undefined")
check_cxx_source_compiles("#include <${CMAKE_CURRENT_SOURCE_DIR}/config/has_linkflag_no_undefined.cpp>" BOOST_FILESYSTEM_HAS_LINKFLAG_NO_UNDEFINED)
unset(CMAKE_REQUIRED_LIBRARIES)
if(NOT BOOST_FILESYSTEM_HAS_LINKFLAG_NO_UNDEFINED)
set(CMAKE_REQUIRED_LIBRARIES "-Wl,-undefined,error")
check_cxx_source_compiles("#include <${CMAKE_CURRENT_SOURCE_DIR}/config/has_linkflag_no_undefined.cpp>" BOOST_FILESYSTEM_HAS_LINKFLAG_UNDEFINED_ERROR)
unset(CMAKE_REQUIRED_LIBRARIES)
endif()
if(BOOST_FILESYSTEM_HAS_LINKFLAG_NO_UNDEFINED)
if(NOT CMAKE_VERSION VERSION_LESS 3.13)
target_link_options(boost_filesystem PRIVATE "-Wl,--no-undefined")
else()
target_link_libraries(boost_filesystem PRIVATE "-Wl,--no-undefined")
endif()
elseif(BOOST_FILESYSTEM_HAS_LINKFLAG_UNDEFINED_ERROR)
if(NOT CMAKE_VERSION VERSION_LESS 3.13)
target_link_options(boost_filesystem PRIVATE "-Wl,-undefined,error")
else()
target_link_libraries(boost_filesystem PRIVATE "-Wl,-undefined,error")
endif()
endif()
endif()

target_include_directories(boost_filesystem PUBLIC include)
target_include_directories(boost_filesystem PRIVATE src)

Expand Down
24 changes: 24 additions & 0 deletions build/Jamfile.v2
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,27 @@ rule check-cxx20-atomic-ref ( properties * )
return $(result) ;
}

# The rule checks if the linker supports requiring no unresolved symbols
rule check-linkflag-no-undefined ( properties * )
{
local result ;

if <link>shared in $(properties)
{
if [ configure.builds ../config//has_linkflag_no_undefined : $(properties) : "has -Wl,--no-undefined" ]
{
result = <linkflags>"-Wl,--no-undefined" ;
}
else if [ configure.builds ../config//has_linkflag_undefined_error : $(properties) : "has -Wl,-undefined,error" ]
{
result = <linkflags>"-Wl,-undefined,error" ;
}
}

#ECHO Result: $(result) ;
return $(result) ;
}

project boost/filesystem
: requirements
<host-os>hpux,<toolset>gcc:<define>_INCLUDE_STDC__SOURCE_199901
Expand All @@ -117,6 +138,8 @@ project boost/filesystem
<conditional>@check-statx
<conditional>@select-windows-crypto-api
<conditional>@check-cxx20-atomic-ref
# Make sure no undefined references are left from the library
<conditional>@check-linkflag-no-undefined
<target-os>windows:<define>_SCL_SECURE_NO_WARNINGS
<target-os>windows:<define>_SCL_SECURE_NO_DEPRECATE
<target-os>windows:<define>_CRT_SECURE_NO_WARNINGS
Expand All @@ -131,6 +154,7 @@ project boost/filesystem
# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105329
# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105651
<toolset>gcc-12:<cxxflags>"-Wno-restrict"

: source-location ../src
: usage-requirements # pass these requirement to dependents (i.e. users)
<link>shared:<define>BOOST_FILESYSTEM_DYN_LINK=1
Expand Down
5 changes: 5 additions & 0 deletions config/Jamfile.v2
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,8 @@ exe has_bcrypt : has_bcrypt.cpp : <include>../src <library>bcrypt ;
explicit has_bcrypt ;
obj is_windows_ce : is_windows_ce.cpp ;
explicit is_windows_ce ;

lib has_linkflag_no_undefined : has_linkflag_no_undefined.cpp : <link>shared <linkflags>"-Wl,--no-undefined" ;
explicit has_linkflag_no_undefined ;
lib has_linkflag_undefined_error : has_linkflag_no_undefined.cpp : <link>shared <linkflags>"-Wl,-undefined,error" ;
explicit has_linkflag_undefined_error ;
18 changes: 18 additions & 0 deletions config/has_linkflag_no_undefined.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright 2023 Andrey Semashev

// Distributed under the Boost Software License, Version 1.0.
// See http://www.boost.org/LICENSE_1_0.txt

// See library home page at http://www.boost.org/libs/filesystem

#if defined(_MSC_VER)
// MSVC's link.exe does not support -Wl,... flags, but doesn't fail the linking.
// The linker may be used by different compilers, not only MSVC.
// Luckily, those compilers all pretend to be MSVC.
#error "MSVC and compatible compilers don't support -Wl,... flags"
#endif

int main()
{
return 0;
}
1 change: 1 addition & 0 deletions doc/release_history.html
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ <h2>1.82.0</h2>
<li><b>v4:</b> <code>path::remove_filename</code> now presesrves the trailing directory separator. (<a href="https://github.com/boostorg/filesystem/issues/271">#271</a>)</li>
<li>Added <code>path::remove_filename_and_trailing_separators</li>, which removes the filename and directory separators preceding it from the path. This behavior is similar to <code>path::remove_filename</code> in Filesystem <b>v3</b>, but is also usable in <b>v4</b>.
<li>Added <code>path::replace_filename</code>, which replaces filename in a path.</li>
<li>Updated implementation of the library version selection to avoid ODR violations. (<a href="https://github.com/boostorg/filesystem/issues/279">#279</a>)</li>
</ul>

<h2>1.81.0</h2>
Expand Down
Loading

0 comments on commit 7509619

Please sign in to comment.