diff --git a/.github/scripts/tbb-downstream-check.R b/.github/scripts/tbb-downstream-check.R index 214d6df2..5df99e22 100644 --- a/.github/scripts/tbb-downstream-check.R +++ b/.github/scripts/tbb-downstream-check.R @@ -97,77 +97,141 @@ if (status != 0L) dllName <- paste0("check", .Platform$dynlib.ext) -# Report which module supplied the TBB symbols, and fail if that is more than -# one. On Windows the surface can legitimately come from either RcppParallel.dll -# (which links TBB statically) or the 'tbb.dll' stub that RcppParallelLibs() -# offers after '-lRcppParallel' -- in practice it is the stub, since -# RcppParallel.dll turns out not to re-export the runtime at all, which is why -# '-ltbb' is load-bearing rather than a fallback. What must not happen is a -# library split across both: they are separate copies of the oneTBB runtime, so -# an observer registered with one would never fire for arenas owned by the -# other, and unlike the link error this replaced, that failure is silent. -if (.Platform$OS.type == "windows") { - - objdump <- Sys.which("objdump") - output <- if (nzchar(objdump)) - suppressWarnings(system2(objdump, c("-p", shQuote(dllName)), stdout = TRUE, stderr = TRUE)) +# On Windows, check where the TBB symbols came from. RcppParallel builds oneTBB +# as a shared library and links against it, exactly as on other platforms, so +# there should be a single TBB runtime in the process: this library's TBB +# symbols must all come from 'tbb.dll', and RcppParallel.dll must import from it +# too rather than carrying a copy of its own. Two runtimes would be a silent +# failure -- an observer registered with one would never fire for arenas owned +# by the other -- so it is worth asserting rather than assuming. + +# objdump has to match the target architecture, and the first one on the PATH +# may well not: the aarch64 runner leads with an x86_64 objdump from C:/mingw64, +# which exits non-zero having read nothing. Ask R which compiler it builds with +# -- that is the one that produced the PE, so its objdump can always read it -- +# and only then fall back to whatever the PATH offers +objdumpCandidates <- function() { + + cc <- suppressWarnings( + system2( + file.path(R.home("bin"), "R"), + c("CMD", "config", "CC"), + stdout = TRUE, + stderr = FALSE + ) + ) + + # CC may carry arguments (e.g. 'gcc -std=gnu2x'), and may name the compiler + # rather than spell out its path, in which case the PATH has to resolve it + cc <- strsplit(trimws(paste(cc, collapse = " ")), "[[:space:]]+")[[1L]][[1L]] + cc <- if (file.exists(cc)) cc else Sys.which(cc) + + compilers <- c(cc, Sys.which(c("gcc", "clang", "cc"))) + beside <- file.path(dirname(compilers[nzchar(compilers)]), "objdump.exe") + + candidates <- c(beside, Sys.which("objdump")) + candidates <- candidates[nzchar(candidates) & file.exists(candidates)] + + # '/' and '\\' spellings of one path are the same objdump; don't try twice + unique(normalizePath(candidates, winslash = "/", mustWork = FALSE)) - status <- attr(output, "status") +} - # keep only the import tables. the export table follows them, and lists this - # library's own inlined TBB instantiations -- left in, it would be absorbed - # into the last 'DLL Name:' block and credit that library with TBB symbols - # it never imported - exports <- grep("export table|The Export Tables", output) - if (length(exports)) - output <- output[seq_len(exports[[1L]] - 1L)] +tbbImports <- function(dll) { # each imported library opens a 'DLL Name:' block listing the symbols taken - # from it, and runs until the next such block - starts <- grep("DLL Name:", output, fixed = TRUE) - readable <- length(starts) > 0L && !(is.numeric(status) && status != 0L) + # from it; finding at least one is how we know objdump really read the file, + # rather than failing in a way that would look like 'imports nothing' + starts <- integer() + output <- character() + + for (objdump in objdumpCandidates()) { + + output <- suppressWarnings( + system2(objdump, c("-p", shQuote(dll)), stdout = TRUE, stderr = TRUE) + ) + status <- attr(output, "status") + + # keep only the import tables. the export table follows them, and lists + # the library's own inlined TBB instantiations -- left in, it would be + # absorbed into the last 'DLL Name:' block and credit that library with + # TBB symbols it never imported + exports <- grep("export table|The Export Tables", output) + if (length(exports)) + output <- output[seq_len(exports[[1L]] - 1L)] + + starts <- grep("DLL Name:", output, fixed = TRUE) + if (length(starts) && !(is.numeric(status) && status != 0L)) + break + + fmt <- "** '%s' could not read the import table of '%s'" + writeLines(sprintf(fmt, objdump, basename(dll))) + writeLines(output) - if (!nzchar(objdump)) { + starts <- integer() - writeLines("** objdump not found; skipping the import table check") + } + + if (!length(starts)) { + fmt <- "** no usable objdump for '%s'; skipping the import table check" + writeLines(sprintf(fmt, basename(dll))) + return(NULL) + } - } else if (!readable) { + imported <- sub(".*DLL Name:[[:space:]]*", "", output[starts]) + ends <- c(starts[-1L], length(output) + 1L) - # don't let an objdump that couldn't read the file pass as 'no imports': - # the runner's objdump may be built for another target (an x86_64 one - # cannot read an aarch64 PE, and exits non-zero having printed nothing) - fmt <- "** '%s' could not read the import table of '%s'; skipping the check" - writeLines(sprintf(fmt, objdump, dllName)) - writeLines(output) + # attribute the TBB symbols to the module each was taken from + providers <- character() + for (i in seq_along(starts)) { + block <- output[seq(starts[[i]], ends[[i]] - 1L)] + if (any(grepl("_ZN3tbb", block, fixed = TRUE))) + providers <- c(providers, imported[[i]]) + } + + fmt <- "%s imports: %s [tbb symbols from: %s]" + writeLines(sprintf( + fmt, basename(dll), + paste(imported, collapse = ", "), + if (length(providers)) paste(providers, collapse = ", ") else "none" + )) + + tolower(providers) + +} - } else { +if (.Platform$OS.type == "windows") { - imported <- sub(".*DLL Name:[[:space:]]*", "", output[starts]) - ends <- c(starts[-1L], length(output) + 1L) + providers <- tbbImports(dllName) - # attribute the TBB symbols to the module each was taken from - providers <- character() - for (i in seq_along(starts)) { - block <- output[seq(starts[[i]], ends[[i]] - 1L)] - if (any(grepl("_ZN3tbb", block, fixed = TRUE))) - providers <- c(providers, imported[[i]]) - } + if (!is.null(providers)) { - writeLines(sprintf("imports: %s", paste(imported, collapse = ", "))) - writeLines(sprintf("tbb symbols from: %s", if (length(providers)) - paste(providers, collapse = ", ") else "(none; resolved statically)")) + if (length(providers) != 1L) + stop("expected this library's tbb symbols to come from exactly one ", + "module, but found ", length(providers), " (", + paste(providers, collapse = ", "), "); more than one means more ", + "than one copy of the oneTBB runtime") - if (length(providers) > 1L) - stop("the downstream library takes TBB symbols from more than one ", - "module (", paste(providers, collapse = ", "), "); those are ", - "separate copies of the oneTBB runtime, so its observers and its ", - "arenas would belong to different schedulers") + if (!identical(providers, "tbb.dll")) + stop("this library's tbb symbols came from '", providers, + "' rather than the shared 'tbb.dll' runtime") } + # and RcppParallel itself must be a client of that same runtime + rcppParallelDll <- RcppParallel:::archSystemFile("libs", "RcppParallel.dll") + if (!file.exists(rcppParallelDll)) + stop("could not locate RcppParallel.dll within the installed package") + + providers <- tbbImports(rcppParallelDll) + if (!is.null(providers) && !identical(providers, "tbb.dll")) + stop("RcppParallel.dll does not take its tbb symbols from 'tbb.dll' ", + "(got: ", paste(providers, collapse = ", "), + "); it appears to carry its own copy of the runtime") + } -# loading also proves any load-time dependency on the tbb stub resolves +# loading also proves the load-time dependency on the tbb runtime resolves dll <- dyn.load(dllName) on.exit(dyn.unload(dll[["path"]]), add = TRUE) diff --git a/NEWS.md b/NEWS.md index f75c24a8..99d10cf8 100644 --- a/NEWS.md +++ b/NEWS.md @@ -8,23 +8,37 @@ loaded into the process, and failed with "Library not loaded: @rpath/libtbb.dylib" otherwise. (#209) -* Fixed `RcppParallel::tbbLibraryPath()` returning `NULL` on Windows. It looked - for the static archives `libtbb12.a` / `libtbb.a`, which are never installed - with the package: TBB is linked statically into `RcppParallel.dll` there, and - the only TBB library installed alongside it is the `tbb.dll` stub, which is - what it now finds. Previously it succeeded only when the package happened to - be running on the machine that built it against an Rtools TBB, and even then - answered with a path into Rtools -- a build dependency -- rather than with - anything the installed package uses. Note that `tbbLibraryPath("tbbmalloc")` - returns `NULL` on Windows, as no separate tbbmalloc library is installed - there. (#270) - -* `RcppParallel::tbbLibraryPath()` called with no arguments, and the internal - `tbbRoot()`, no longer report the Rtools directory recorded when the package - was configured. Unlike the named-library form, these return their answer - without checking that it exists, so for a pre-built binary (e.g. the CRAN - build) they named a directory on the machine that built the package. On - Windows both now report the package's own library directory. (#270) +* On Windows, RcppParallel now builds the bundled oneTBB as a shared library + and links against it, shipping `tbb.dll` and `tbbmalloc.dll` alongside the + package -- the same arrangement already used on every other platform. + Previously it linked the static TBB provided by Rtools directly into + `RcppParallel.dll`, which meant the TBB version (and ABI) depended on the + user's toolchain, and left downstream packages with no TBB library to link + against. Building it ourselves gives every platform the same oneTBB and makes + the ABI a property of RcppParallel. `TBB_LIB` / `TBB_INC` are still honoured + for anyone supplying their own build. This requires cmake, which Rtools has + shipped since Rtools42; toolchains without one continue to use the tinythread + fallback. + +* As a consequence, `RcppParallel::RcppParallelLibs()` now emits `-ltbb` and + `-ltbbmalloc` on Windows, in addition to `-lRcppParallel` (which remains + necessary there for the entry points RcppParallel compiles itself, such as + `isProcessForkedChild`). Packages that previously resolved TBB symbols out of + `RcppParallel.dll`, or the tbbmalloc API via `-lRcppParallel` alone, should + rebuild against these flags. + +* The `tbb.dll` compatibility stub is gone. It existed to publish the + pre-oneTBB `task_scheduler_observer` entry point on top of a statically + linked runtime, but because it linked the TBB archives itself it amounted to + a second, independent copy of the oneTBB scheduler living in the same + process. That entry point is now exported by the real `tbb.dll`, as it + already was by the shared libraries on other platforms, so binaries built + against RcppParallel 5.1.11 and earlier continue to resolve it. + +* Fixed `RcppParallel::tbbLibraryPath()` returning `NULL` on Windows, and + `tbbRoot()` reporting a directory that need not exist on the machine running + the package -- for a pre-built binary, the Rtools tree of the machine that + built it. Both now describe the installation actually in use. (#270) * Fixed an issue where compiling code including `tbb/parallel_for_each.h` could fail with toolchains that accept `-std=c++20` but provide a diff --git a/R/tbb.R b/R/tbb.R index 6a1b97f9..7a773684 100644 --- a/R/tbb.R +++ b/R/tbb.R @@ -22,13 +22,9 @@ tbbLibraryPath <- function(name = NULL) { return(tbbRoot) # form library names - # - # on Windows the library we install is the 'tbb.dll' stub -- TBB itself is - # inside RcppParallel.dll -- so look for that first. the static archives are - # still tried afterwards, for a TBB_LIB pointed at an Rtools tree at runtime tbbLibNames <- list( "Darwin" = paste0("lib", name, ".dylib"), - "Windows" = c(paste0(name, ".dll"), paste0("lib", name, c("12", ""), ".a")), + "Windows" = paste0(name, ".dll"), "SunOS" = paste0("lib", name, ".so"), "Linux" = paste0("lib", name, c(".so.2", ".so")) ) @@ -89,35 +85,35 @@ tbbCxxFlags <- function() { # Return the linker flags required for TBB on this platform tbbLdFlags <- function() { - # on Windows, we statically link to oneTBB + # on Windows every symbol must be resolved at link time -- there is no + # equivalent of lazy binding or '-undefined dynamic_lookup' -- so a + # downstream package needs both the TBB libraries it calls into and + # RcppParallel itself, for the entry points we compile (e.g. + # isProcessForkedChild). elsewhere those resolve from the process at load + # time and only TBB needs naming if (is_windows()) { - libPath <- archSystemFile("libs") - - ldFlags <- sprintf("-L%s -lRcppParallel", asBuildPath(libPath)) - - # also offer the stub library, which exports the TBB runtime wholesale. - # this is not just a safety net: R CMD SHLIB links RcppParallel.dll - # against an export list generated from our own objects, so the TBB - # entry points pulled in from the static library are not re-exported, - # and a downstream package linking '-lRcppParallel' alone resolves none - # of them (which is what broke rstan). the stub is what makes those - # symbols reachable at all. - # - # it still comes last, so that anything RcppParallel.dll does export - # wins, and so no import of the stub is recorded unless something needs - # it. that matters because the stub carries its own copy of the oneTBB - # runtime: a package split across both would register observers with one - # scheduler while its tasks ran in the other. - # .github/scripts/tbb-downstream-check.R asserts that does not happen - tbbPath <- archSystemFile("lib") - if (file.exists(file.path(tbbPath, "tbb.dll"))) - ldFlags <- paste(ldFlags, sprintf("-L%s -ltbb", asBuildPath(tbbPath))) + libsPath <- archSystemFile("libs") + ldFlags <- sprintf("-L%s -lRcppParallel", asBuildPath(libsPath)) + + # only name the TBB libraries when there are TBB libraries to name: a + # build that fell back to tinythread ships none, and asking the linker + # for them would fail the downstream build outright + if (TBB_ENABLED && !is.null(tbbLibraryPath("tbb"))) { + fmt <- "%s -L%s -l%s -l%s" + ldFlags <- sprintf( + fmt, + ldFlags, + asBuildPath(tbbLibraryPath()), + TBB_NAME, + TBB_MALLOC_NAME + ) + } return(ldFlags) } - + # shortcut if TBB_LIB defined tbbLib <- Sys.getenv("TBB_LINK_LIB", Sys.getenv("TBB_LIB", unset = TBB_LIB)) if (nzchar(tbbLib)) { @@ -150,15 +146,6 @@ tbbLdFlags <- function() { tbbRoot <- function() { - # on Windows, always answer with our own library directory. TBB is linked - # statically into RcppParallel.dll there, and the only TBB library we - # install is the tbb.dll stub, so that directory is the whole story. TBB_LIB - # is worse than useless here: it records the Rtools tree of whichever - # machine ran configure, which for a pre-built binary is a path that need - # not exist on the user's machine at all (#270) - if (is_windows()) - return(archSystemFile("lib")) - if (nzchar(TBB_LIB)) return(TBB_LIB) diff --git a/R/zzz.R b/R/zzz.R index 98f941e7..c7d15bfc 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -12,29 +12,6 @@ loadTbbLibrary <- function(name) { - # On Windows, TBB is statically linked into RcppParallel.dll, but we - # still ship a stub tbb.dll for compatibility with packages linking via - # '-ltbb' (e.g. through StanHeaders). Such packages record a load-time - # dependency on 'tbb.dll', which the Windows loader can only resolve if - # we've already loaded it -- the library directory itself is not on the - # DLL search path. - if (is_windows()) { - - # NOTE: resolve against the package's own library directory, where - # install.libs.R places the stub. this is what tbbRoot() reports on - # Windows too; spell it out here so that loading during .onLoad does - # not depend on the configured TBB_LIB in any way - path <- archSystemFile("lib", paste0(name, ".dll")) - if (!file.exists(path)) { - verboseMessage("tbb library '%s' not found in package 'lib' folder; skipping", name) - return(NULL) - } - - verboseMessage("loading tbb library '%s'", path) - return(dyn.load(path, local = FALSE, now = TRUE)) - - } - path <- tbbLibraryPath(name) if (is.null(path)) { verboseMessage("tbb library '%s' could not be resolved; skipping", name) @@ -52,26 +29,21 @@ loadTbbLibrary <- function(name) { } .onLoad <- function(libname, pkgname) { - - # on Windows, load RcppParallel first - if (.Platform$OS.type == "windows") { - .dllInfo <<- library.dynam("RcppParallel", pkgname, libname) - } - - # load tbb, tbbmalloc + + # load tbb, tbbmalloc first: RcppParallel links against them, and on Windows + # in particular the loader can only satisfy that dependency once they are in + # the process -- the package's library directory is not on the DLL search + # path, so it cannot find them by itself .tbbDllInfo <<- loadTbbLibrary("tbb") .tbbMallocDllInfo <<- loadTbbLibrary("tbbmalloc") - + # load tbbmalloc_proxy, but only if requested useTbbMallocProxy <- Sys.getenv("RCPP_PARALLEL_USE_TBBMALLOC_PROXY", unset = "FALSE") if (useTbbMallocProxy %in% c("TRUE", "True", "true", "1")) .tbbMallocProxyDllInfo <<- loadTbbLibrary("tbbmalloc_proxy") - - # load RcppParallel library if available - if (.Platform$OS.type != "windows") { - .dllInfo <<- library.dynam("RcppParallel", pkgname, libname, local = FALSE) - } - + + .dllInfo <<- library.dynam("RcppParallel", pkgname, libname, local = FALSE) + } .onUnload <- function(libpath) { diff --git a/patches/legacy_tbb_abi.diff b/patches/legacy_tbb_abi.diff index 082ceba1..c1083ec2 100644 --- a/patches/legacy_tbb_abi.diff +++ b/patches/legacy_tbb_abi.diff @@ -1,6 +1,6 @@ --- a/src/tbb/include/oneapi/tbb/task_scheduler_observer.h +++ b/src/tbb/include/oneapi/tbb/task_scheduler_observer.h -@@ -17,10 +17,13 @@ +@@ -17,9 +17,12 @@ #ifndef __TBB_task_scheduler_observer_H #define __TBB_task_scheduler_observer_H @@ -10,23 +10,18 @@ + #include "task_arena.h" -#include - + + namespace tbb { namespace detail { - -@@ -112,5 +115,128 @@ - } +@@ -113,4 +116,128 @@ inline namespace v1 { } // namespace tbb -+ -+// Provided for backwards compatibility. This is a local addition, not part of -+// upstream oneTBB, so the tbb.dll stub built by src/install.libs.R must be -+// able to tell whether the TBB headers in use supply these declarations (the -+// bundled copy does) or whether it has to declare them itself (as with the -+// pristine headers shipped by Rtools). -+#define __TBB_LEGACY_TASK_SCHEDULER_OBSERVER_PROVIDED 1 -+ + ++// Provided for backwards compatibility: binaries built against RcppParallel ++// 5.1.11 and earlier resolve their imports against these declarations. This is ++// a local addition, not part of upstream oneTBB; the out-of-line definition ++// lives in src/tbb/observer_proxy.cpp. +namespace tbb { +namespace interface6 { +class task_scheduler_observer; @@ -52,7 +47,11 @@ + has the task scheduler initialized or is attached to an arena. + + Repeated calls with the same state are no-ops. **/ -+ void __TBB_EXPORTED_METHOD observe( bool state=true ); ++ // TBB_EXPORT is a local addition: on mingw there is no .def file, so the ++ // library's exports come from '__declspec(dllexport)' directives alone, ++ // and without this the definition in observer_proxy.cpp would be compiled ++ // but not exported. It expands to nothing when consuming the headers. ++ TBB_EXPORT void __TBB_EXPORTED_METHOD observe( bool state=true ); + + //! Returns true if observation is enabled, false otherwise. + bool is_observing() const {return my_proxy!=NULL;} @@ -141,35 +140,9 @@ + +} //namespace interface6 + -+} // namespace tbb - - #endif /* __TBB_task_scheduler_observer_H */ ---- a/src/tbb/src/tbb/observer_proxy.cpp -+++ b/src/tbb/src/tbb/observer_proxy.cpp -@@ -317,3 +317,23 @@ - } // namespace r1 - } // namespace detail - } // namespace tbb -+ -+// Not on Windows: there the old ABI is published by the tbb.dll stub built -+// from src/tbb-compat/tbb-compat.cpp, which has to define this itself in -+// order for R CMD SHLIB to pick it up when generating the export list. -+// Defining it here as well makes the stub fail to link with "multiple -+// definition of tbb::internal::task_scheduler_observer_v3::observe". -+#ifndef _WIN32 -+ -+namespace tbb { -+namespace internal { -+ -+void __TBB_EXPORTED_FUNC task_scheduler_observer_v3::observe( bool enable ) { -+ auto* tso = (tbb::detail::d1::task_scheduler_observer*) (this); -+ tbb::detail::r1::observe(*tso, enable); -+} -+ -+} // namespace internal +} // namespace tbb + -+#endif /* _WIN32 */ + #endif /* __TBB_task_scheduler_observer_H */ --- a/src/tbb/src/tbb/def/lin32-tbb.def +++ b/src/tbb/src/tbb/def/lin32-tbb.def @@ -17,6 +17,26 @@ @@ -257,3 +230,23 @@ # Assertions (assert.cpp) __ZN3tbb6detail2r117assertion_failureEPKciS3_S3_ +--- a/src/tbb/src/tbb/observer_proxy.cpp ++++ b/src/tbb/src/tbb/observer_proxy.cpp +@@ -317,3 +317,17 @@ void __TBB_EXPORTED_FUNC observe(d1::task_scheduler_observer &tso, bool enable) + } // namespace r1 + } // namespace detail + } // namespace tbb ++ ++// The pre-oneTBB ABI, provided so that binaries built against RcppParallel ++// 5.1.11 and earlier still resolve their imports. Declared (and, on mingw, ++// exported) in oneapi/tbb/task_scheduler_observer.h. ++namespace tbb { ++namespace internal { ++ ++void __TBB_EXPORTED_METHOD task_scheduler_observer_v3::observe( bool enable ) { ++ auto* tso = (tbb::detail::d1::task_scheduler_observer*) (this); ++ tbb::detail::r1::observe(*tso, enable); ++} ++ ++} // namespace internal ++} // namespace tbb diff --git a/patches/mingw_clang_cmake.diff b/patches/mingw_clang_cmake.diff new file mode 100644 index 00000000..df983c66 --- /dev/null +++ b/patches/mingw_clang_cmake.diff @@ -0,0 +1,32 @@ +--- a/src/tbb/cmake/compilers/Clang.cmake ++++ b/src/tbb/cmake/compilers/Clang.cmake +@@ -65,13 +65,27 @@ + # Clang flags to prevent compiler from optimizing out security checks + set(TBB_COMMON_COMPILE_FLAGS ${TBB_COMMON_COMPILE_FLAGS} -Wformat -Wformat-security -Werror=format-security -fPIC $<$>:-fstack-protector-strong>) + +-# -z switch is not supported on MacOS +-if (NOT APPLE) ++# -z switch is not supported on MacOS or on Windows. The Windows guard is a ++# local addition, mirroring the one GNU.cmake already carries: Rtools for ++# aarch64 Windows drives clang with lld, which rejects the option outright ++# ("lld: error: unknown argument: -z"). Only reachable now that we build TBB ++# as a shared library there, since a static build has no link step. ++if (NOT APPLE AND NOT WIN32) + set(TBB_LIB_LINK_FLAGS ${TBB_LIB_LINK_FLAGS} -Wl,-z,relro,-z,now) + endif() + + set(TBB_COMMON_LINK_LIBS ${CMAKE_DL_LIBS}) + ++# Local addition: on mingw the helpers for -fstack-protector-strong and ++# _FORTIFY_SOURCE (__stack_chk_fail, __stack_chk_guard, __strncpy_chk, ...) ++# live in libssp rather than libgcc, and older toolchains (Rtools42 / gcc 10) ++# do not pull it in implicitly. It has to go here, with the link libraries, ++# rather than in the linker flags: those are emitted before the objects, and ++# GNU ld only resolves symbols already undefined when it reaches a library. ++if (MINGW) ++ set(TBB_COMMON_LINK_LIBS ${TBB_COMMON_LINK_LIBS} ssp) ++endif() ++ + if (NOT CMAKE_CXX_FLAGS MATCHES "_FORTIFY_SOURCE") + set(TBB_COMMON_COMPILE_FLAGS ${TBB_COMMON_COMPILE_FLAGS} $<$>:-D_FORTIFY_SOURCE=2>) + endif () diff --git a/patches/mingw_gnu_cmake.diff b/patches/mingw_gnu_cmake.diff index cd2ff607..13e13f93 100644 --- a/patches/mingw_gnu_cmake.diff +++ b/patches/mingw_gnu_cmake.diff @@ -1,8 +1,6 @@ -diff --git a/src/tbb/cmake/compilers/GNU.cmake b/src/tbb/cmake/compilers/GNU.cmake -index cf6d8bdb..8f9f0b59 100644 --- a/src/tbb/cmake/compilers/GNU.cmake +++ b/src/tbb/cmake/compilers/GNU.cmake -@@ -47,19 +47,44 @@ endif() +@@ -47,19 +47,44 @@ # >=2.31.1). Capturing the output in CMake can be done like below. The version # information is written to either stdout or stderr. To not make any # assumptions, both are captured. @@ -48,12 +46,29 @@ index cf6d8bdb..8f9f0b59 100644 message(TRACE "Extracted GNU assembler version: major=${_tbb_gnu_asm_major_version} minor=${_tbb_gnu_asm_minor_version}") math(EXPR _tbb_gnu_asm_version_number "${_tbb_gnu_asm_major_version} * 1000 + ${_tbb_gnu_asm_minor_version}") -@@ -81,7 +106,14 @@ endif() +@@ -73,6 +98,16 @@ + + set(TBB_COMMON_LINK_LIBS ${CMAKE_DL_LIBS}) + ++# Local addition: on mingw the helpers for -fstack-protector-strong and ++# _FORTIFY_SOURCE (__stack_chk_fail, __stack_chk_guard, __strncpy_chk, ...) ++# live in libssp rather than libgcc, and older toolchains (Rtools42 / gcc 10) ++# do not pull it in implicitly. It has to go here, with the link libraries, ++# rather than in the linker flags: those are emitted before the objects, and ++# GNU ld only resolves symbols already undefined when it reaches a library. ++if (MINGW) ++ set(TBB_COMMON_LINK_LIBS ${TBB_COMMON_LINK_LIBS} ssp) ++endif() ++ + # Ignore -Werror set through add_compile_options() or added to CMAKE_CXX_FLAGS if TBB_STRICT is disabled. + if (NOT TBB_STRICT AND COMMAND tbb_remove_compile_flag) + tbb_remove_compile_flag(-Werror) +@@ -81,8 +116,15 @@ if (NOT ${CMAKE_CXX_COMPILER_ID} STREQUAL Intel) # gcc 6.0 and later have -flifetime-dse option that controls elimination of stores done outside the object lifetime set(TBB_DSE_FLAG $<$>:-flifetime-dse=1>) - set(TBB_COMMON_COMPILE_FLAGS ${TBB_COMMON_COMPILE_FLAGS} $<$>:-fstack-clash-protection>) -+ + + # -fstack-clash-protection is broken on mingw's SEH targets: compiling + # task_dispatcher.h makes GCC 10 abort with "internal compiler error: in + # seh_emit_stackalloc, at config/i386/winnt.c". It is a hardening flag @@ -61,6 +76,7 @@ index cf6d8bdb..8f9f0b59 100644 + if (NOT MINGW) + set(TBB_COMMON_COMPILE_FLAGS ${TBB_COMMON_COMPILE_FLAGS} $<$>:-fstack-clash-protection>) + endif() - ++ # Suppress GCC 12.x-13.x warning here that to_wait_node(n)->my_is_in_list might have size 0 set(TBB_COMMON_LINK_FLAGS ${TBB_COMMON_LINK_FLAGS} $<$>,$>:-Wno-stringop-overflow>) + endif() diff --git a/patches/unversioned_libraries.diff b/patches/unversioned_libraries.diff index 3e16be0f..f50c82a6 100644 --- a/patches/unversioned_libraries.diff +++ b/patches/unversioned_libraries.diff @@ -1,6 +1,20 @@ --- a/src/tbb/src/tbb/CMakeLists.txt +++ b/src/tbb/src/tbb/CMakeLists.txt -@@ -90,8 +90,6 @@ +@@ -50,7 +50,12 @@ add_library(TBB::tbb ALIAS tbb) + + if (WIN32) + target_sources(tbb PRIVATE tbb.rc) +- set_target_properties(tbb PROPERTIES OUTPUT_NAME "tbb${TBB_BINARY_VERSION}") ++ # Name the runtime 'tbb.dll', not mingw's default 'libtbb.dll': binaries ++ # built against RcppParallel 5.1.11 and earlier record a load-time ++ # dependency on that name. The import library keeps its 'lib' prefix, so ++ # '-ltbb' still resolves it. (The binary version suffix upstream applies ++ # here is dropped for the same reason.) ++ set_target_properties(tbb PROPERTIES PREFIX "") + endif() + + # TODO: Add statistics.cpp +@@ -90,8 +95,6 @@ target_compile_options(tbb # Avoid use of target_link_libraries here as it changes /DEF option to \DEF on Windows. set_target_properties(tbb PROPERTIES DEFINE_SYMBOL "" @@ -11,7 +25,7 @@ tbb_handle_ipo(tbb) --- a/src/tbb/src/tbbbind/CMakeLists.txt +++ b/src/tbb/src/tbbbind/CMakeLists.txt -@@ -53,8 +53,6 @@ +@@ -53,8 +53,6 @@ function(tbbbind_build TBBBIND_NAME REQUIRED_HWLOC_TARGET) # Avoid use of target_link_libraries here as it changes /DEF option to \DEF on Windows. set_target_properties(${TBBBIND_NAME} PROPERTIES DEFINE_SYMBOL "" @@ -22,7 +36,7 @@ tbb_handle_ipo(${TBBBIND_NAME}) --- a/src/tbb/src/tbbmalloc/CMakeLists.txt +++ b/src/tbb/src/tbbmalloc/CMakeLists.txt -@@ -70,8 +70,6 @@ +@@ -70,11 +70,14 @@ enable_language(C) # Avoid use of target_link_libraries here as it changes /DEF option to \DEF on Windows. set_target_properties(tbbmalloc PROPERTIES DEFINE_SYMBOL "" @@ -31,9 +45,17 @@ LINKER_LANGUAGE C ) ++# see src/tbb/CMakeLists.txt: name the runtime 'tbbmalloc.dll' on mingw ++if (WIN32) ++ set_target_properties(tbbmalloc PROPERTIES PREFIX "") ++endif() ++ + tbb_handle_ipo(tbbmalloc) + + if (TBB_DEF_FILE_PREFIX) # If there's no prefix, assume we're using export directives --- a/src/tbb/src/tbbmalloc_proxy/CMakeLists.txt +++ b/src/tbb/src/tbbmalloc_proxy/CMakeLists.txt -@@ -54,10 +54,6 @@ +@@ -54,10 +54,6 @@ target_compile_options(tbbmalloc_proxy ${TBB_COMMON_COMPILE_FLAGS} ) @@ -44,3 +66,15 @@ if (UNIX AND NOT APPLE) # Avoid use of target_link_libraries here as it changes /DEF option to \DEF on Windows. set_target_properties(tbbmalloc_proxy PROPERTIES +@@ -66,6 +62,11 @@ if (UNIX AND NOT APPLE) + DEFINE_SYMBOL "") + endif() + ++# see src/tbb/CMakeLists.txt: name the runtime 'tbbmalloc_proxy.dll' on mingw ++if (WIN32) ++ set_target_properties(tbbmalloc_proxy PROPERTIES PREFIX "") ++endif() ++ + # Prefer using target_link_options instead of target_link_libraries to specify link options because + # target_link_libraries may incorrectly handle some options (on Windows, for example). + if (COMMAND target_link_options) diff --git a/src/install.libs.R b/src/install.libs.R index 6381b470..1e56e968 100644 --- a/src/install.libs.R +++ b/src/install.libs.R @@ -39,35 +39,22 @@ # using bundled TBB tbbSrc <- "tbb/build/lib_release" + tbbLibs <- list.files( + path = tbbSrc, + pattern = shlibPattern, + full.names = TRUE + ) - # on Windows the bundled TBB was built as a static library, and is - # already linked into (and re-exported from) RcppParallel.dll; say so - # explicitly, rather than reporting that no runtime libraries were - # found -- there are none to find, and that reads like a failure - if (.Platform$OS.type == "windows") { - - writeLines("** tbb is statically linked into RcppParallel.dll; no runtime libraries to install") - - } else { - - tbbLibs <- list.files( - path = tbbSrc, - pattern = shlibPattern, - full.names = TRUE - ) - - logTbbLibraries(tbbLibs, tbbSrc) - for (tbbLib in tbbLibs) { - system2("cp", c("-P", shQuote(tbbLib), shQuote(tbbDest))) - } - - # restore the versioned library layout shipped by RcppParallel 5.1.11 - # and earlier (a real 'libtbb.so.2' plus a 'libtbb.so' symlink) - if (Sys.info()[["sysname"]] == "Linux") - versionBundledTbbLibraries(tbbDest) - + logTbbLibraries(tbbLibs, tbbSrc) + for (tbbLib in tbbLibs) { + system2("cp", c("-P", shQuote(tbbLib), shQuote(tbbDest))) } + # restore the versioned library layout shipped by RcppParallel 5.1.11 + # and earlier (a real 'libtbb.so.2' plus a 'libtbb.so' symlink) + if (Sys.info()[["sysname"]] == "Linux") + versionBundledTbbLibraries(tbbDest) + } else { # using system tbb @@ -93,88 +80,6 @@ } - # on Windows, we create a stub library that links to us so that - # older binaries (like rstan) can still load. this is only relevant - # when TBB is available: the stub cannot link without it, and old - # TBB-using binaries could not run against a TBB-less RcppParallel - # regardless - if (.Platform$OS.type == "windows" && TBB_ENABLED) { - tbbDll <- file.path(tbbDest, "tbb.dll") - if (file.exists(tbbDll)) { - writeLines("** tbb.dll already exists; skipping tbb.dll stub") - } else { - buildTbbStub(tbbDest) - } - } - -} - -# Build the tbb.dll stub that downstream packages link ('-ltbb' via e.g. -# StanHeaders) and that older binaries resolve their imports against. -buildTbbStub <- function(tbbDest) { - - # remove artifacts from prior builds, which may have been produced - # by a different toolchain or against a different TBB; 'make' would - # otherwise consider them up-to-date and re-ship them as-is - unlink(c("tbb-compat/tbb.dll", Sys.glob("tbb-compat/*.o"))) - - tbbInc <- Sys.getenv("TBB_INC") - if (!nzchar(tbbInc)) - tbbInc <- TBB_INC - - # when TBB was built from the bundled sources there is no TBB_INC to - # consult; its headers were copied into the package's include directory - # during the build, so dispatch on those instead - if (!nzchar(tbbInc)) - tbbInc <- "../inst/include" - - if (file.exists(file.path(tbbInc, "oneapi"))) { - - # with oneTBB, the stub provides the old TBB ABI's - # task_scheduler_observer entry point on top of the new runtime - writeLines("** creating tbb.dll stub (wrapping the oneTBB runtime)") - status <- system("R CMD SHLIB -o tbb-compat/tbb.dll tbb-compat/tbb-compat.cpp") - if (status != 0) - stop("error building tbb.dll stub") - - } else { - - # with older versions of TBB (e.g. Rtools42), tbb-compat.cpp cannot - # build -- there is no oneTBB runtime to wrap -- but the static - # library already provides the old ABI, so re-export it wholesale - writeLines("** creating tbb.dll stub (re-exporting the static tbb library)") - - tbbLib <- Sys.getenv("TBB_LIB") - if (!nzchar(tbbLib)) - tbbLib <- TBB_LIB - - cxx <- system("R CMD config CXX", intern = TRUE) - archive <- file.path(tbbLib, sprintf("lib%s.a", TBB_NAME)) - command <- paste( - cxx, - "-shared -static-libgcc", - "-o tbb-compat/tbb.dll", - "-Wl,--whole-archive", shQuote(archive), "-Wl,--no-whole-archive", - "-lssp" - ) - - writeLines(command) - status <- system(command) - if (status != 0) - stop("error building tbb.dll stub") - - } - - # 'R CMD SHLIB' has been seen to report success on Windows even when the - # link failed, which would otherwise leave us shipping a package with no - # stub at all -- and packages linking '-ltbb' only discover that when they - # fail to load. Check for the artifact rather than trusting the exit code. - if (!file.exists("tbb-compat/tbb.dll")) - stop("tbb.dll stub was not produced") - - if (!file.copy("tbb-compat/tbb.dll", file.path(tbbDest, "tbb.dll"))) - stop("couldn't copy tbb.dll stub to '", tbbDest, "'") - } # Give the bundled TBB libraries the versioned '.so.' names, plus an @@ -401,15 +306,15 @@ useBundledTbb <- function() { ) } - # on Windows, build TBB as a static library: it then gets linked into - # (and re-exported from) RcppParallel.dll, giving the same layout we - # produce with an Rtools-provided oneTBB. the generator has to be named - # explicitly, as cmake otherwise prefers a Visual Studio generator when - # one happens to be installed + # on Windows the generator has to be named explicitly, as cmake otherwise + # prefers a Visual Studio generator when one happens to be installed. TBB is + # built shared here, as on every other platform: upstream does not support + # static builds ("highly discouraged", per its own configure-time warning), + # and a shared library is what lets RcppParallel and downstream packages + # share one runtime rather than each linking a private copy if (.Platform$OS.type == "windows") { cmakeFlags <- c( "-G", "MSYS Makefiles", - "-DBUILD_SHARED_LIBS=0", cmakeFlags ) } @@ -443,10 +348,14 @@ useBundledTbb <- function() { } setwd(owd) - # on Windows (as on wasm) TBB is built as a static library, so collect - # the archives rather than the runtime libraries - shlibPattern <- if (.Platform$OS.type == "windows" || R.version$os == "emscripten") { + # on wasm TBB is built as a static library, so collect the archives; on + # Windows collect the DLLs together with their import libraries, so that + # linking RcppParallel against them below can go through 'libtbb.dll.a' + # rather than relying on the linker accepting the DLL directly + shlibPattern <- if (R.version$os == "emscripten") { "^libtbb.*\\.a$" + } else if (.Platform$OS.type == "windows") { + "^(lib)?tbb.*\\.dll(\\.a)?$" } else if (Sys.info()[["sysname"]] == "Darwin") { "^libtbb.*\\.dylib$" } else { diff --git a/src/tbb-compat/tbb-compat.cpp b/src/tbb-compat/tbb-compat.cpp deleted file mode 100644 index b5799101..00000000 --- a/src/tbb-compat/tbb-compat.cpp +++ /dev/null @@ -1,152 +0,0 @@ - -#include - -#include "../tbb/include/oneapi/tbb/detail/_namespace_injection.h" -#include "../tbb/include/oneapi/tbb/task_arena.h" - -#include "../tbb/src/tbb/observer_proxy.h" -#include "../tbb/src/tbb/main.h" -#include "../tbb/src/tbb/thread_data.h" - -#ifdef _WIN32 -# define DLL_EXPORT __declspec(dllexport) -#else -# define DLL_EXPORT -#endif - -// The old ABI's observer classes, as they stood before oneTBB. The bundled -// copy of oneTBB carries these too (see task_scheduler_observer.h), so only -// declare them when compiling against headers that don't -- i.e. the pristine -// ones Rtools ships. Either way, the out-of-line observe() below is what this -// stub exists to export. -#ifndef __TBB_LEGACY_TASK_SCHEDULER_OBSERVER_PROVIDED - -namespace tbb { - -namespace interface6 { -class task_scheduler_observer; -} - -namespace internal { - -class task_scheduler_observer_v3 { - friend class tbb::detail::r1::observer_proxy; - friend class tbb::detail::r1::observer_list; - friend class interface6::task_scheduler_observer; - - //! Pointer to the proxy holding this observer. - /** Observers are proxied by the scheduler to maintain persistent lists of them. **/ - tbb::detail::r1::observer_proxy* my_proxy; - - //! Counter preventing the observer from being destroyed while in use by the scheduler. - /** Valid only when observation is on. **/ - std::atomic my_busy_count; - -public: - //! Enable or disable observation - /** For local observers the method can be used only when the current thread - has the task scheduler initialized or is attached to an arena. - Repeated calls with the same state are no-ops. **/ - void DLL_EXPORT __TBB_EXPORTED_METHOD observe( bool state=true ); - - //! Returns true if observation is enabled, false otherwise. - bool is_observing() const {return my_proxy!=NULL;} - - //! Construct observer with observation disabled. - task_scheduler_observer_v3() : my_proxy(NULL) { my_busy_count.store(0); } - - //! Entry notification - /** Invoked from inside observe(true) call and whenever a worker enters the arena - this observer is associated with. If a thread is already in the arena when - the observer is activated, the entry notification is called before it - executes the first stolen task. - Obsolete semantics. For global observers it is called by a thread before - the first steal since observation became enabled. **/ - virtual void on_scheduler_entry( bool /*is_worker*/ ) {} - - //! Exit notification - /** Invoked from inside observe(false) call and whenever a worker leaves the - arena this observer is associated with. - Obsolete semantics. For global observers it is called by a thread before - the first steal since observation became enabled. **/ - virtual void on_scheduler_exit( bool /*is_worker*/ ) {} - - //! Destructor automatically switches observation off if it is enabled. - virtual ~task_scheduler_observer_v3() { if(my_proxy) observe(false);} -}; - -} // namespace internal - -namespace interface6 { - -class task_scheduler_observer : public internal::task_scheduler_observer_v3 { - friend class internal::task_scheduler_observer_v3; - friend class tbb::detail::r1::observer_proxy; - friend class tbb::detail::r1::observer_list; - - /** Negative numbers with the largest absolute value to minimize probability - of coincidence in case of a bug in busy count usage. **/ - // TODO: take more high bits for version number - static const intptr_t v6_trait = (intptr_t)((~(uintptr_t)0 >> 1) + 1); - - //! contains task_arena pointer or tag indicating local or global semantics of the observer - intptr_t my_context_tag; - enum { global_tag = 0, implicit_tag = 1 }; - -public: - //! Construct local or global observer in inactive state (observation disabled). - /** For a local observer entry/exit notifications are invoked whenever a worker - thread joins/leaves the arena of the observer's owner thread. If a thread is - already in the arena when the observer is activated, the entry notification is - called before it executes the first stolen task. **/ - /** TODO: Obsolete. - Global observer semantics is obsolete as it violates master thread isolation - guarantees and is not composable. Thus the current default behavior of the - constructor is obsolete too and will be changed in one of the future versions - of the library. **/ - explicit task_scheduler_observer( bool local = false ) { - my_context_tag = local? implicit_tag : global_tag; - } - - //! Construct local observer for a given arena in inactive state (observation disabled). - /** entry/exit notifications are invoked whenever a thread joins/leaves arena. - If a thread is already in the arena when the observer is activated, the entry notification - is called before it executes the first stolen task. **/ - explicit task_scheduler_observer( task_arena & a) { - my_context_tag = (intptr_t)&a; - } - - /** Destructor protects instance of the observer from concurrent notification. - It is recommended to disable observation before destructor of a derived class starts, - otherwise it can lead to concurrent notification callback on partly destroyed object **/ - virtual ~task_scheduler_observer() { if(my_proxy) observe(false); } - - //! Enable or disable observation - /** Warning: concurrent invocations of this method are not safe. - Repeated calls with the same state are no-ops. **/ - void observe( bool state=true ) { - if( state && !my_proxy ) { - __TBB_ASSERT( !my_busy_count, "Inconsistent state of task_scheduler_observer instance"); - my_busy_count.store(v6_trait); - } - internal::task_scheduler_observer_v3::observe(state); - } -}; - -} // namespace interface6 - -} // namespace tbb - -#endif /* __TBB_LEGACY_TASK_SCHEDULER_OBSERVER_PROVIDED */ - -namespace tbb { -namespace internal { - -DLL_EXPORT -void __TBB_EXPORTED_METHOD task_scheduler_observer_v3::observe( bool enable ) { - auto* tso = (tbb::detail::d1::task_scheduler_observer*) (this); - tbb::detail::r1::observe(*tso, enable); -} - -} // namespace internal -} // namespace tbb diff --git a/src/tbb/cmake/compilers/Clang.cmake b/src/tbb/cmake/compilers/Clang.cmake index dcd66634..bc39e8f1 100644 --- a/src/tbb/cmake/compilers/Clang.cmake +++ b/src/tbb/cmake/compilers/Clang.cmake @@ -65,13 +65,27 @@ endif() # Clang flags to prevent compiler from optimizing out security checks set(TBB_COMMON_COMPILE_FLAGS ${TBB_COMMON_COMPILE_FLAGS} -Wformat -Wformat-security -Werror=format-security -fPIC $<$>:-fstack-protector-strong>) -# -z switch is not supported on MacOS -if (NOT APPLE) +# -z switch is not supported on MacOS or on Windows. The Windows guard is a +# local addition, mirroring the one GNU.cmake already carries: Rtools for +# aarch64 Windows drives clang with lld, which rejects the option outright +# ("lld: error: unknown argument: -z"). Only reachable now that we build TBB +# as a shared library there, since a static build has no link step. +if (NOT APPLE AND NOT WIN32) set(TBB_LIB_LINK_FLAGS ${TBB_LIB_LINK_FLAGS} -Wl,-z,relro,-z,now) endif() set(TBB_COMMON_LINK_LIBS ${CMAKE_DL_LIBS}) +# Local addition: on mingw the helpers for -fstack-protector-strong and +# _FORTIFY_SOURCE (__stack_chk_fail, __stack_chk_guard, __strncpy_chk, ...) +# live in libssp rather than libgcc, and older toolchains (Rtools42 / gcc 10) +# do not pull it in implicitly. It has to go here, with the link libraries, +# rather than in the linker flags: those are emitted before the objects, and +# GNU ld only resolves symbols already undefined when it reaches a library. +if (MINGW) + set(TBB_COMMON_LINK_LIBS ${TBB_COMMON_LINK_LIBS} ssp) +endif() + if (NOT CMAKE_CXX_FLAGS MATCHES "_FORTIFY_SOURCE") set(TBB_COMMON_COMPILE_FLAGS ${TBB_COMMON_COMPILE_FLAGS} $<$>:-D_FORTIFY_SOURCE=2>) endif () diff --git a/src/tbb/cmake/compilers/GNU.cmake b/src/tbb/cmake/compilers/GNU.cmake index 8f9f0b59..4c1fbf22 100644 --- a/src/tbb/cmake/compilers/GNU.cmake +++ b/src/tbb/cmake/compilers/GNU.cmake @@ -98,6 +98,16 @@ endif() set(TBB_COMMON_LINK_LIBS ${CMAKE_DL_LIBS}) +# Local addition: on mingw the helpers for -fstack-protector-strong and +# _FORTIFY_SOURCE (__stack_chk_fail, __stack_chk_guard, __strncpy_chk, ...) +# live in libssp rather than libgcc, and older toolchains (Rtools42 / gcc 10) +# do not pull it in implicitly. It has to go here, with the link libraries, +# rather than in the linker flags: those are emitted before the objects, and +# GNU ld only resolves symbols already undefined when it reaches a library. +if (MINGW) + set(TBB_COMMON_LINK_LIBS ${TBB_COMMON_LINK_LIBS} ssp) +endif() + # Ignore -Werror set through add_compile_options() or added to CMAKE_CXX_FLAGS if TBB_STRICT is disabled. if (NOT TBB_STRICT AND COMMAND tbb_remove_compile_flag) tbb_remove_compile_flag(-Werror) diff --git a/src/tbb/include/oneapi/tbb/task_scheduler_observer.h b/src/tbb/include/oneapi/tbb/task_scheduler_observer.h index 86e141a7..db2726a9 100644 --- a/src/tbb/include/oneapi/tbb/task_scheduler_observer.h +++ b/src/tbb/include/oneapi/tbb/task_scheduler_observer.h @@ -116,13 +116,10 @@ inline namespace v1 { } // namespace tbb -// Provided for backwards compatibility. This is a local addition, not part of -// upstream oneTBB, so the tbb.dll stub built by src/install.libs.R must be -// able to tell whether the TBB headers in use supply these declarations (the -// bundled copy does) or whether it has to declare them itself (as with the -// pristine headers shipped by Rtools). -#define __TBB_LEGACY_TASK_SCHEDULER_OBSERVER_PROVIDED 1 - +// Provided for backwards compatibility: binaries built against RcppParallel +// 5.1.11 and earlier resolve their imports against these declarations. This is +// a local addition, not part of upstream oneTBB; the out-of-line definition +// lives in src/tbb/observer_proxy.cpp. namespace tbb { namespace interface6 { class task_scheduler_observer; @@ -148,7 +145,11 @@ class task_scheduler_observer_v3 { has the task scheduler initialized or is attached to an arena. Repeated calls with the same state are no-ops. **/ - void __TBB_EXPORTED_METHOD observe( bool state=true ); + // TBB_EXPORT is a local addition: on mingw there is no .def file, so the + // library's exports come from '__declspec(dllexport)' directives alone, + // and without this the definition in observer_proxy.cpp would be compiled + // but not exported. It expands to nothing when consuming the headers. + TBB_EXPORT void __TBB_EXPORTED_METHOD observe( bool state=true ); //! Returns true if observation is enabled, false otherwise. bool is_observing() const {return my_proxy!=NULL;} diff --git a/src/tbb/src/tbb/CMakeLists.txt b/src/tbb/src/tbb/CMakeLists.txt index 63fd4c73..bf4621aa 100644 --- a/src/tbb/src/tbb/CMakeLists.txt +++ b/src/tbb/src/tbb/CMakeLists.txt @@ -50,7 +50,12 @@ add_library(TBB::tbb ALIAS tbb) if (WIN32) target_sources(tbb PRIVATE tbb.rc) - set_target_properties(tbb PROPERTIES OUTPUT_NAME "tbb${TBB_BINARY_VERSION}") + # Name the runtime 'tbb.dll', not mingw's default 'libtbb.dll': binaries + # built against RcppParallel 5.1.11 and earlier record a load-time + # dependency on that name. The import library keeps its 'lib' prefix, so + # '-ltbb' still resolves it. (The binary version suffix upstream applies + # here is dropped for the same reason.) + set_target_properties(tbb PROPERTIES PREFIX "") endif() # TODO: Add statistics.cpp diff --git a/src/tbb/src/tbb/observer_proxy.cpp b/src/tbb/src/tbb/observer_proxy.cpp index 07254196..3e7104e0 100644 --- a/src/tbb/src/tbb/observer_proxy.cpp +++ b/src/tbb/src/tbb/observer_proxy.cpp @@ -318,22 +318,16 @@ void __TBB_EXPORTED_FUNC observe(d1::task_scheduler_observer &tso, bool enable) } // namespace detail } // namespace tbb -// Not on Windows: there the old ABI is published by the tbb.dll stub built -// from src/tbb-compat/tbb-compat.cpp, which has to define this itself in -// order for R CMD SHLIB to pick it up when generating the export list. -// Defining it here as well makes the stub fail to link with "multiple -// definition of tbb::internal::task_scheduler_observer_v3::observe". -#ifndef _WIN32 - +// The pre-oneTBB ABI, provided so that binaries built against RcppParallel +// 5.1.11 and earlier still resolve their imports. Declared (and, on mingw, +// exported) in oneapi/tbb/task_scheduler_observer.h. namespace tbb { namespace internal { -void __TBB_EXPORTED_FUNC task_scheduler_observer_v3::observe( bool enable ) { +void __TBB_EXPORTED_METHOD task_scheduler_observer_v3::observe( bool enable ) { auto* tso = (tbb::detail::d1::task_scheduler_observer*) (this); tbb::detail::r1::observe(*tso, enable); } } // namespace internal } // namespace tbb - -#endif /* _WIN32 */ diff --git a/src/tbb/src/tbbmalloc/CMakeLists.txt b/src/tbb/src/tbbmalloc/CMakeLists.txt index 26c1891d..a65045b2 100644 --- a/src/tbb/src/tbbmalloc/CMakeLists.txt +++ b/src/tbb/src/tbbmalloc/CMakeLists.txt @@ -73,6 +73,11 @@ set_target_properties(tbbmalloc PROPERTIES LINKER_LANGUAGE C ) +# see src/tbb/CMakeLists.txt: name the runtime 'tbbmalloc.dll' on mingw +if (WIN32) + set_target_properties(tbbmalloc PROPERTIES PREFIX "") +endif() + tbb_handle_ipo(tbbmalloc) if (TBB_DEF_FILE_PREFIX) # If there's no prefix, assume we're using export directives diff --git a/src/tbb/src/tbbmalloc_proxy/CMakeLists.txt b/src/tbb/src/tbbmalloc_proxy/CMakeLists.txt index 609e4f37..574ab32c 100644 --- a/src/tbb/src/tbbmalloc_proxy/CMakeLists.txt +++ b/src/tbb/src/tbbmalloc_proxy/CMakeLists.txt @@ -62,6 +62,11 @@ if (UNIX AND NOT APPLE) DEFINE_SYMBOL "") endif() +# see src/tbb/CMakeLists.txt: name the runtime 'tbbmalloc_proxy.dll' on mingw +if (WIN32) + set_target_properties(tbbmalloc_proxy PROPERTIES PREFIX "") +endif() + # Prefer using target_link_options instead of target_link_libraries to specify link options because # target_link_libraries may incorrectly handle some options (on Windows, for example). if (COMMAND target_link_options) diff --git a/tests/test-tbb-library-path.R b/tests/test-tbb-library-path.R index ec1a8156..17504c1c 100644 --- a/tests/test-tbb-library-path.R +++ b/tests/test-tbb-library-path.R @@ -22,10 +22,10 @@ assert(length(root) == 1L) if (nzchar(root)) assert(dir.exists(root)) -# on Windows the answer is always the package's own library directory, since -# TBB is statically linked into RcppParallel.dll and the tbb.dll stub is the -# only TBB library installed. in particular it must not depend on TBB_LIB. -if (is_windows()) { +# with the bundled TBB the answer is the package's own library directory, on +# every platform. in particular it must not depend on the TBB_LIB environment +# variable, which tbbRoot() does not consult at all. +if (!nzchar(TBB_LIB)) { assert(identical(root, archSystemFile("lib"))) @@ -56,12 +56,11 @@ for (name in c("tbb", "tbbmalloc", "tbbmalloc_proxy")) { } # the user-visible symptom from #270: tbbLibraryPath("tbb") returned nothing -# from a perfectly good installation. assert this only on Windows, where the -# installed layout is known exactly -- install.libs.R always builds the -# tbb.dll stub when TBB is enabled, and fails the install if it cannot. off -# Windows the library may legitimately be unresolvable, e.g. a system TBB -# whose distro package ships no unversioned 'libtbb.so' development symlink. -if (is_windows() && TBB_ENABLED) { +# from a perfectly good installation. assert this for the bundled build, where +# the installed layout is known exactly. a TBB supplied via TBB_LIB may +# legitimately be unresolvable, e.g. a distro package shipping no unversioned +# 'libtbb.so' development symlink. +if (TBB_ENABLED && !nzchar(TBB_LIB) && !is_sparc()) { tbb <- tbbLibraryPath("tbb") assert(!is.null(tbb)) assert(file.exists(tbb)) diff --git a/tools/config/cleanup.R b/tools/config/cleanup.R index 3cea7e18..1141a790 100644 --- a/tools/config/cleanup.R +++ b/tools/config/cleanup.R @@ -7,8 +7,6 @@ # install, so nothing is lost by removing these unlink("src/tbb/build", recursive = TRUE) unlink("src/tbb/build-tbb", recursive = TRUE) -unlink("src/tbb-compat/tbb.dll") -unlink(Sys.glob("src/tbb-compat/*.o")) unlink("inst/lib", recursive = TRUE) unlink("inst/libs", recursive = TRUE) unlink("inst/include/index.html", recursive = TRUE) diff --git a/tools/config/configure.R b/tools/config/configure.R index 0d6b8c43..d3d1311f 100644 --- a/tools/config/configure.R +++ b/tools/config/configure.R @@ -26,57 +26,14 @@ if (file.exists(makevars)) { } } -# on Windows, check for Rtools; if it exists, and it provides oneTBB, use it. -# -# older toolchains are deliberately passed over even though they do provide -# a TBB. Rtools42 ships TBB 2017, whose headers downstream packages cannot -# build against: StanHeaders uses tbb::this_task_arena::isolate, which that -# release still gates behind TBB_PREVIEW_TASK_ISOLATION, and its library -# doesn't export isolate_within_arena either, so there is nothing to link -# against even if the declaration were forced into view. we build the -# bundled copy of oneTBB in that case instead -- unless the user pointed -# TBB_LIB somewhere themselves, which is honoured as-is -if (.Platform$OS.type == "windows") { - - gccPath <- normalizePath(Sys.which("gcc"), winslash = "/") - - tbbLib <- Sys.getenv("TBB_LIB", unset = NA) - - # a TBB the user configured explicitly is honoured whatever its vintage; - # only one discovered next to gcc has to pass the oneTBB check below - tbbLibConfigured <- !is.na(tbbLib) - - if (is.na(tbbLib)) - tbbLib <- normalizePath(file.path(gccPath, "../../lib"), winslash = "/") - - tbbInc <- Sys.getenv("TBB_INC", unset = NA) - if (is.na(tbbInc)) - tbbInc <- normalizePath(file.path(gccPath, "../../include"), winslash = "/") - - # whenever we do adopt this tree, the library names have to come with it: - # a legacy TBB spells its archives 'libtbb_static.a' and - # 'libtbbmalloc_static.a', which the defaults below would not find - tbbFiles <- list.files(tbbLib, pattern = "^libtbb") - if (length(tbbFiles) && (tbbLibConfigured || file.exists(file.path(tbbInc, "oneapi")))) { - - tbbPattern <- "^lib(tbb\\d*(?:_static)?)\\.a$" - tbbName <- grep(tbbPattern, tbbFiles, perl = TRUE, value = TRUE) - tbbName <- gsub(tbbPattern, "\\1", tbbName, perl = TRUE) - - tbbMallocPattern <- "^lib(tbbmalloc\\d*(?:_static)?)\\.a$" - tbbMallocName <- grep(tbbMallocPattern, tbbFiles, perl = TRUE, value = TRUE) - tbbMallocName <- gsub(tbbMallocPattern, "\\1", tbbMallocName, perl = TRUE) - - Sys.setenv( - TBB_LIB = tbbLib, - TBB_INC = tbbInc, - TBB_NAME = tbbName, - TBB_MALLOC_NAME = tbbMallocName - ) - - } - -} +# NOTE: we deliberately do not look for a TBB provided by Rtools. Rtools +# ships static libraries only, and the TBB it provides varies with the +# toolchain: Rtools42 has Intel TBB 2017, whose headers downstream packages +# cannot build against, while later versions have oneTBB. Building the +# bundled copy instead gives every platform the same oneTBB, shipped as a +# shared library and linked the same way, and makes the TBB ABI a property +# of RcppParallel rather than of the user's toolchain. TBB_LIB / TBB_INC are +# still honoured for anyone wanting to supply their own. # try and figure out path to TBB tbbRoot <- Sys.getenv("TBB_ROOT", unset = NA) @@ -227,12 +184,6 @@ if (is.na(tbbLib)) { } -# oneTBB appends its binary version to the library name on Windows, so the -# bundled build produces 'libtbb12.a' rather than 'libtbb.a' there -# (tbbmalloc gets no such suffix, so its default name still applies) -if (.Platform$OS.type == "windows" && buildBundledTbb && !nzchar(Sys.getenv("TBB_NAME"))) - tbbName <- "tbb12" - # now, define TBB_LIB and TBB_INC as appropriate define( TBB_LIB = if (!is.na(tbbLib)) tbbLib else "", @@ -242,86 +193,43 @@ define( ) # set PKG_LIBS -pkgLibs <- if (.Platform$OS.type == "windows") { - - if (!is.na(tbbLib) && file.exists(file.path(tbbInc, "oneapi"))) { - - # downstream packages link with '-lRcppParallel' alone, so - # RcppParallel.dll must provide the tbbmalloc API (scalable_malloc - # and friends) even though RcppParallel itself never calls it; use - # --whole-archive so those objects are linked in, and re-exported - # via their '-export:' directives. tbbmalloc must precede tbb here: - # both archives bundle an itt_notify object defining the same - # symbols, and with tbbmalloc's copy already linked, tbb's is never - # pulled in, avoiding duplicate definition errors - c( - "-Wl,-L\"$(TBB_LIB)\"", - "-Wl,--whole-archive", - "-l$(TBB_MALLOC_NAME)", - "-Wl,--no-whole-archive", - "-l$(TBB_NAME)" - ) - - } else if (!is.na(tbbLib)) { - - # with an older (non-oneTBB) toolchain like Rtools42, tbb and - # tbbmalloc both define DllMain, so tbbmalloc cannot be linked - # wholesale; its objects also carry no '-export:' directives, so - # nothing would be re-exported anyhow -- just link as needed - c( - "-Wl,-L\"$(TBB_LIB)\"", - "-l$(TBB_NAME)", - "-l$(TBB_MALLOC_NAME)" - ) - - } else if (buildBundledTbb) { - - # the bundled oneTBB is built as a static library on Windows, so - # RcppParallel.dll takes exactly the same shape as it does with an - # Rtools oneTBB above -- see that branch for why tbbmalloc is linked - # wholesale, and why it must precede tbb - c( - "-Wl,-Ltbb/build/lib_release", - "-Wl,--whole-archive", - "-l$(TBB_MALLOC_NAME)", - "-Wl,--no-whole-archive", - "-l$(TBB_NAME)" - ) - - } - -} else if (!is.na(tbbLib)) { +pkgLibs <- if (!is.na(tbbLib)) { + # a TBB supplied via TBB_LIB / TBB_ROOT. an rpath is meaningless on Windows, + # where the loader has no equivalent -- see R/zzz.R for how we resolve there c( "-Wl,-L\"$(TBB_LIB)\"", - sprintf("-Wl,-rpath,%s", shQuote(tbbLib)), + if (.Platform$OS.type != "windows") + sprintf("-Wl,-rpath,%s", shQuote(tbbLib)), "-l$(TBB_NAME)", "-l$(TBB_MALLOC_NAME)" ) +} else if (!buildBundledTbb) { + + # no TBB to link at all; the tinythread fallback is used instead + NULL + } else if (R.version$os == "emscripten") { - + c( "-Wl,-Ltbb/build/lib_release", "-l$(TBB_NAME)" ) - + } else { - + c( "-Wl,-Ltbb/build/lib_release", "-l$(TBB_NAME)", "-l$(TBB_MALLOC_NAME)" ) - + } -# on Windows, we may need to link to ssp; otherwise, -# we see errors like -# -# C:\rtools43\x86_64-w64-mingw32.static.posix\bin/ld.exe: C:/rtools43/x86_64-w64-mingw32.static.posix/lib/libtbb12.a(allocator.cpp.obj):allocator.cpp:(.text+0x18b): undefined reference to `__stack_chk_fail' -# +# on Windows, link to ssp for the stack-protector helpers (__stack_chk_fail +# and friends), which mingw does not provide in libgcc if (.Platform$OS.type == "windows") { pkgLibs <- c(pkgLibs, "-lssp") }