From 514d22ff9a4f247a0a04e2c9e6fcd258ec5d4410 Mon Sep 17 00:00:00 2001 From: Kevin Ushey Date: Tue, 28 Jul 2026 00:43:07 -0700 Subject: [PATCH 1/6] build the bundled onetbb as a shared library on windows Windows was the only platform where TBB was not a shared library loaded at runtime, and nearly every Windows-specific wart traced back to that. Use the bundled oneTBB there too, built shared, shipped as tbb.dll / tbbmalloc.dll and linked exactly as on other platforms. Rtools ships static libraries only, so adopting its TBB meant linking it into RcppParallel.dll. That made the TBB version -- and ABI -- a property of the user's toolchain (Intel TBB 2017 on Rtools42, oneTBB later), and left downstream packages with no TBB library to link against. The workarounds accumulated: re-exporting the whole tbbmalloc archive so the allocator could be resolved (#262), a tbb.dll stub so packages linking -ltbb could load (#249, #250), building that stub differently for legacy toolchains (#258), offering it from RcppParallelLibs() so rstan could link at all (#269), and a library lookup that searched for archives never installed with the package (#270). Worse, the stub linked the TBB archives itself, so it was a second complete copy of the oneTBB scheduler. Both it and RcppParallel.dll export the same 81 tbb::detail::r1:: entry points, both are loaded in every session, and which one a downstream package binds to is up to the linker. Two schedulers with separate thread pools and separate global state is a correctness problem, not a linking inconvenience. This removes: - the Rtools TBB detection in configure.R, and with it the TBB_NAME derivation from archive names and the tbb12 special case - the --whole-archive tbbmalloc re-export, which only existed because there was no DLL to link - BUILD_SHARED_LIBS=0 for Windows - src/tbb-compat/ and buildTbbStub() entirely, along with the #ifndef _WIN32 guard in observer_proxy.cpp and the __TBB_LEGACY_..._PROVIDED macro that let the stub tell which headers it was compiling against - the Windows branches in .install.libs(), loadTbbLibrary(), tbbLdFlags() and tbbLibraryPath(), and the Windows-first load order in .onLoad() Notes on the pieces that are not just deletions: - mingw CMake would name a shared build libtbb.dll; patch PREFIX to "" so the runtime is tbb.dll, the name binaries built against 5.1.11 and earlier import. The import library keeps its lib prefix, so -ltbb still resolves. - the legacy task_scheduler_observer_v3::observe definition now applies on Windows too. mingw has no .def file, so its export comes from a dllexport directive: the declaration gains TBB_EXPORT, which is empty when consuming the headers. - .onLoad loads tbb before RcppParallel on every platform now, since RcppParallel.dll imports from tbb.dll and the package library directory is not on the DLL search path. - tbbLdFlags() keeps -lRcppParallel on Windows: isProcessForkedChild is compiled into RcppParallel.dll, and Windows has no lazy binding to resolve it at load time. The downstream CI check now asserts the property this is all for: the downstream library's TBB symbols come from exactly one module, that module is tbb.dll, and RcppParallel.dll imports from it rather than carrying its own copy. --- .github/scripts/tbb-downstream-check.R | 109 ++++++++----- NEWS.md | 48 ++++-- R/tbb.R | 58 +++---- R/zzz.R | 46 ++---- patches/legacy_tbb_abi.diff | 75 ++++----- patches/unversioned_libraries.diff | 42 ++++- src/install.libs.R | 143 +++------------- src/tbb-compat/tbb-compat.cpp | 152 ------------------ .../oneapi/tbb/task_scheduler_observer.h | 17 +- src/tbb/src/tbb/CMakeLists.txt | 7 +- src/tbb/src/tbb/observer_proxy.cpp | 14 +- src/tbb/src/tbbmalloc/CMakeLists.txt | 5 + src/tbb/src/tbbmalloc_proxy/CMakeLists.txt | 5 + tests/test-tbb-library-path.R | 19 ++- tools/config/cleanup.R | 2 - tools/config/configure.R | 140 +++------------- 16 files changed, 286 insertions(+), 596 deletions(-) delete mode 100644 src/tbb-compat/tbb-compat.cpp diff --git a/.github/scripts/tbb-downstream-check.R b/.github/scripts/tbb-downstream-check.R index 214d6df2..89caaaa6 100644 --- a/.github/scripts/tbb-downstream-check.R +++ b/.github/scripts/tbb-downstream-check.R @@ -97,24 +97,27 @@ 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") { +# 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. +tbbImports <- function(dll) { objdump <- Sys.which("objdump") - output <- if (nzchar(objdump)) - suppressWarnings(system2(objdump, c("-p", shQuote(dllName)), stdout = TRUE, stderr = TRUE)) + if (!nzchar(objdump)) { + writeLines("** objdump not found; skipping the import table check") + return(NULL) + } + 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 this + # 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 @@ -125,49 +128,71 @@ if (.Platform$OS.type == "windows") { # 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) - if (!nzchar(objdump)) { + # 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) + if (!length(starts) || (is.numeric(status) && status != 0L)) { + fmt <- "** '%s' could not read the import table of '%s'; skipping the check" + writeLines(sprintf(fmt, objdump, basename(dll))) + writeLines(output) + return(NULL) + } - writeLines("** objdump not found; skipping the import table check") + imported <- sub(".*DLL Name:[[:space:]]*", "", output[starts]) + ends <- c(starts[-1L], length(output) + 1L) - } else if (!readable) { + # 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]]) + } - # 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) + 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 <- file.path( + system.file(paste0("libs", .Platform$r_arch), package = "RcppParallel"), + "RcppParallel.dll" + ) + 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..a0a1e744 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 + provided since Rtools42; toolchains without it (e.g. Rtools40) 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..fb57159b 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,28 @@ 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))) - - return(ldFlags) + libsPath <- archSystemFile("libs") + tbbPath <- tbbLibraryPath() + + fmt <- "-L%s -lRcppParallel -L%s -l%s -l%s" + return(sprintf( + fmt, + asBuildPath(libsPath), + asBuildPath(tbbPath), + TBB_NAME, + TBB_MALLOC_NAME + )) } - + # shortcut if TBB_LIB defined tbbLib <- Sys.getenv("TBB_LINK_LIB", Sys.getenv("TBB_LIB", unset = TBB_LIB)) if (nzchar(tbbLib)) { @@ -150,15 +139,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/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/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") } From 89b8c7cc9ed6c642aec963149890166b27f100fd Mon Sep 17 00:00:00 2001 From: Kevin Ushey Date: Tue, 28 Jul 2026 00:54:45 -0700 Subject: [PATCH 2/6] fix the shared tbb link on rtools42 and aarch64 windows Two failures from the first CI run, both only reachable now that TBB is linked as a shared library on Windows -- a static build has no link step. Rtools42 (gcc 10): the TBB link failed with undefined references to __stack_chk_fail, __strncpy_chk and __strncat_chk. TBB compiles with -fstack-protector-strong and _FORTIFY_SOURCE, and on mingw those helpers live in libssp rather than libgcc; newer toolchains pull it in implicitly, Rtools42 does not. Pass -lssp via CMAKE_SHARED_LINKER_FLAGS, mirroring the -lssp that configure.R already adds for RcppParallel's own link. aarch64 Windows: 'lld: error: unknown argument: -z'. Rtools45-aarch64 drives clang with lld, and Clang.cmake adds -Wl,-z,relro,-z,now for any non-Apple target. GNU.cmake already guards this with NOT MINGW; give Clang.cmake the equivalent guard. windows-latest (release) passed on the first run, so the shared mingw build, the PREFIX "" naming and the reordered load in .onLoad() all work. --- patches/mingw_clang_cmake.diff | 17 +++++++++++++++++ src/install.libs.R | 6 ++++++ src/tbb/cmake/compilers/Clang.cmake | 8 ++++++-- 3 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 patches/mingw_clang_cmake.diff diff --git a/patches/mingw_clang_cmake.diff b/patches/mingw_clang_cmake.diff new file mode 100644 index 00000000..dffaed11 --- /dev/null +++ b/patches/mingw_clang_cmake.diff @@ -0,0 +1,17 @@ +--- a/src/tbb/cmake/compilers/Clang.cmake ++++ b/src/tbb/cmake/compilers/Clang.cmake +@@ -65,8 +65,12 @@ 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() + diff --git a/src/install.libs.R b/src/install.libs.R index 1e56e968..7f958d84 100644 --- a/src/install.libs.R +++ b/src/install.libs.R @@ -315,6 +315,12 @@ useBundledTbb <- function() { if (.Platform$OS.type == "windows") { cmakeFlags <- c( "-G", "MSYS Makefiles", + # TBB compiles with -fstack-protector-strong and _FORTIFY_SOURCE, and + # on mingw the helpers those need (__stack_chk_fail, __strncpy_chk and + # friends) live in libssp rather than in libgcc. Newer toolchains pull + # it in implicitly; Rtools42 (gcc 10) does not, and the TBB link then + # fails with undefined references to them + "-DCMAKE_SHARED_LINKER_FLAGS=-lssp", cmakeFlags ) } diff --git a/src/tbb/cmake/compilers/Clang.cmake b/src/tbb/cmake/compilers/Clang.cmake index dcd66634..645c6fe1 100644 --- a/src/tbb/cmake/compilers/Clang.cmake +++ b/src/tbb/cmake/compilers/Clang.cmake @@ -65,8 +65,12 @@ 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() From c72c09ec1e1206b9fce2798d37b5a80853278ca6 Mon Sep 17 00:00:00 2001 From: Kevin Ushey Date: Tue, 28 Jul 2026 09:56:34 -0700 Subject: [PATCH 3/6] link libssp via TBB_COMMON_LINK_LIBS rather than the shared linker flags TBB compiles with -fstack-protector-strong and _FORTIFY_SOURCE=2, and on mingw the helpers those need (__stack_chk_fail, __stack_chk_guard, __strncpy_chk, ...) live in libssp rather than libgcc. Rtools42 (gcc 10) does not pull it in implicitly, so the link fails with undefined references to them. Passing -lssp through CMAKE_SHARED_LINKER_FLAGS was not enough: those flags are emitted before the objects, and GNU ld only resolves symbols that are already undefined when it reaches a library, so it was discarded. tbb and tbbmalloc happened to survive that because tbb_handle_ipo gives them LTO; tbbmalloc_proxy has no such call and failed to link. Putting ssp in TBB_COMMON_LINK_LIBS routes it through target_link_libraries instead, which lands after the objects, and covers all three targets. --- patches/mingw_clang_cmake.diff | 17 ++++++++++++++++- patches/mingw_gnu_cmake.diff | 28 ++++++++++++++++++++++------ src/install.libs.R | 6 ------ src/tbb/cmake/compilers/Clang.cmake | 10 ++++++++++ src/tbb/cmake/compilers/GNU.cmake | 10 ++++++++++ 5 files changed, 58 insertions(+), 13 deletions(-) diff --git a/patches/mingw_clang_cmake.diff b/patches/mingw_clang_cmake.diff index dffaed11..df983c66 100644 --- a/patches/mingw_clang_cmake.diff +++ b/patches/mingw_clang_cmake.diff @@ -1,6 +1,6 @@ --- a/src/tbb/cmake/compilers/Clang.cmake +++ b/src/tbb/cmake/compilers/Clang.cmake -@@ -65,8 +65,12 @@ endif() +@@ -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>) @@ -15,3 +15,18 @@ 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/src/install.libs.R b/src/install.libs.R index 7f958d84..1e56e968 100644 --- a/src/install.libs.R +++ b/src/install.libs.R @@ -315,12 +315,6 @@ useBundledTbb <- function() { if (.Platform$OS.type == "windows") { cmakeFlags <- c( "-G", "MSYS Makefiles", - # TBB compiles with -fstack-protector-strong and _FORTIFY_SOURCE, and - # on mingw the helpers those need (__stack_chk_fail, __strncpy_chk and - # friends) live in libssp rather than in libgcc. Newer toolchains pull - # it in implicitly; Rtools42 (gcc 10) does not, and the TBB link then - # fails with undefined references to them - "-DCMAKE_SHARED_LINKER_FLAGS=-lssp", cmakeFlags ) } diff --git a/src/tbb/cmake/compilers/Clang.cmake b/src/tbb/cmake/compilers/Clang.cmake index 645c6fe1..bc39e8f1 100644 --- a/src/tbb/cmake/compilers/Clang.cmake +++ b/src/tbb/cmake/compilers/Clang.cmake @@ -76,6 +76,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() + 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) From 5d1ca39675d23efb3ec52d61df1e754e5eff6e90 Mon Sep 17 00:00:00 2001 From: Kevin Ushey Date: Tue, 28 Jul 2026 10:08:42 -0700 Subject: [PATCH 4/6] fix the RcppParallel.dll half of the downstream import check Two reasons it was never actually running, both visible in the CI log even though the job passed: - the path was built with paste0("libs", .Platform$r_arch), giving 'libsx64' rather than 'libs/x64'. system.file() returned "" for that, so objdump was handed '/RcppParallel.dll' and reported "No such file". Use archSystemFile(), which the package already has for this, and fail loudly if the library still can't be found. - on aarch64 the runner has an x86_64 objdump from C:/mingw64 ahead of Rtools' own on the PATH, and it cannot read an aarch64 PE, so both halves of the check skipped there. Try the objdump sitting beside the compiler first, and fall back through the remaining candidates. --- .github/scripts/tbb-downstream-check.R | 79 ++++++++++++++++---------- 1 file changed, 50 insertions(+), 29 deletions(-) diff --git a/.github/scripts/tbb-downstream-check.R b/.github/scripts/tbb-downstream-check.R index 89caaaa6..2e4a6b02 100644 --- a/.github/scripts/tbb-downstream-check.R +++ b/.github/scripts/tbb-downstream-check.R @@ -104,38 +104,59 @@ dllName <- paste0("check", .Platform$dynlib.ext) # 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. -tbbImports <- function(dll) { - objdump <- Sys.which("objdump") - if (!nzchar(objdump)) { - writeLines("** objdump not found; skipping the import table check") - return(NULL) - } +# objdump has to match the target architecture, and the first one on the PATH +# may well not: the aarch64 runner has an x86_64 objdump from C:/mingw64 ahead +# of Rtools' own, and it exits non-zero having read nothing. Look beside the +# compilers first, since those are necessarily right for the target +objdumpCandidates <- function() { - output <- suppressWarnings( - system2(objdump, c("-p", shQuote(dll)), stdout = TRUE, stderr = TRUE) - ) - status <- attr(output, "status") + compilers <- Sys.which(c("gcc", "clang", "cc")) + beside <- file.path(dirname(compilers[nzchar(compilers)]), "objdump.exe") + + candidates <- unique(c(beside, Sys.which("objdump"))) + candidates[nzchar(candidates) & file.exists(candidates)] + +} - # 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)] +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) - - # 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) - if (!length(starts) || (is.numeric(status) && status != 0L)) { - fmt <- "** '%s' could not read the import table of '%s'; skipping the check" + # 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) + + starts <- integer() + + } + + if (!length(starts)) { + fmt <- "** no usable objdump for '%s'; skipping the import table check" + writeLines(sprintf(fmt, basename(dll))) return(NULL) } @@ -180,10 +201,10 @@ if (.Platform$OS.type == "windows") { } # and RcppParallel itself must be a client of that same runtime - rcppParallelDll <- file.path( - system.file(paste0("libs", .Platform$r_arch), package = "RcppParallel"), - "RcppParallel.dll" - ) + 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' ", From a54538d4de425b1e7cdfad32948914f497ccc1a9 Mon Sep 17 00:00:00 2001 From: Kevin Ushey Date: Tue, 28 Jul 2026 10:20:39 -0700 Subject: [PATCH 5/6] locate objdump via the compiler R actually builds with The previous attempt looked beside Sys.which("gcc"/"clang"), but on the aarch64 runner those resolve to C:/mingw64 -- the same x86_64 toolchain that leads the PATH -- so both candidates were the one objdump that cannot read an aarch64 PE, and the check still skipped there. They were also not deduplicated, differing only in slash direction. 'R CMD config CC' names the compiler that produced the library, so its objdump can read it by construction. Try that first, and normalize the candidates so one objdump is not tried under two spellings. --- .github/scripts/tbb-downstream-check.R | 30 ++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/.github/scripts/tbb-downstream-check.R b/.github/scripts/tbb-downstream-check.R index 2e4a6b02..5df99e22 100644 --- a/.github/scripts/tbb-downstream-check.R +++ b/.github/scripts/tbb-downstream-check.R @@ -106,16 +106,34 @@ dllName <- paste0("check", .Platform$dynlib.ext) # 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 has an x86_64 objdump from C:/mingw64 ahead -# of Rtools' own, and it exits non-zero having read nothing. Look beside the -# compilers first, since those are necessarily right for the target +# 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() { - compilers <- Sys.which(c("gcc", "clang", "cc")) + 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 <- unique(c(beside, Sys.which("objdump"))) - candidates[nzchar(candidates) & file.exists(candidates)] + 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)) } From ec75f865c2485d5c6f2682a2de62eae5c7517a29 Mon Sep 17 00:00:00 2001 From: Kevin Ushey Date: Tue, 28 Jul 2026 10:37:50 -0700 Subject: [PATCH 6/6] don't emit -ltbb when no tbb was installed The Windows branch of tbbLdFlags() named the TBB libraries unconditionally, dropping the file.exists() guard the previous stub-based code had. On a build that fell back to tinythread -- configure.R sets TBB_ENABLED = FALSE only there, when cmake is missing and TBB_LIB is unset -- RcppParallelLibs() then emitted -L -lRcppParallel -L -ltbb -ltbbmalloc for libraries that were never built or shipped, so a downstream package would fail to link with "cannot find -ltbb". CxxFlags() correctly reports -DRCPP_PARALLEL_USE_TBB=0 in that configuration, so the package is not even using TBB. Name them only when TBB is enabled and the library actually resolves. Also drop an unverified aside from NEWS: Rtools42 and Rtools45 are confirmed to ship cmake, Rtools40 was not checked. --- NEWS.md | 4 ++-- R/tbb.R | 27 +++++++++++++++++---------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/NEWS.md b/NEWS.md index a0a1e744..99d10cf8 100644 --- a/NEWS.md +++ b/NEWS.md @@ -17,8 +17,8 @@ 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 - provided since Rtools42; toolchains without it (e.g. Rtools40) continue to - use the tinythread fallback. + 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 diff --git a/R/tbb.R b/R/tbb.R index fb57159b..7a773684 100644 --- a/R/tbb.R +++ b/R/tbb.R @@ -94,16 +94,23 @@ tbbLdFlags <- function() { if (is_windows()) { libsPath <- archSystemFile("libs") - tbbPath <- tbbLibraryPath() - - fmt <- "-L%s -lRcppParallel -L%s -l%s -l%s" - return(sprintf( - fmt, - asBuildPath(libsPath), - asBuildPath(tbbPath), - TBB_NAME, - TBB_MALLOC_NAME - )) + 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) }