Skip to content

Commit 5d1ca39

Browse files
committed
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.
1 parent c72c09e commit 5d1ca39

1 file changed

Lines changed: 50 additions & 29 deletions

File tree

.github/scripts/tbb-downstream-check.R

Lines changed: 50 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -104,38 +104,59 @@ dllName <- paste0("check", .Platform$dynlib.ext)
104104
# too rather than carrying a copy of its own. Two runtimes would be a silent
105105
# failure -- an observer registered with one would never fire for arenas owned
106106
# by the other -- so it is worth asserting rather than assuming.
107-
tbbImports <- function(dll) {
108107

109-
objdump <- Sys.which("objdump")
110-
if (!nzchar(objdump)) {
111-
writeLines("** objdump not found; skipping the import table check")
112-
return(NULL)
113-
}
108+
# objdump has to match the target architecture, and the first one on the PATH
109+
# may well not: the aarch64 runner has an x86_64 objdump from C:/mingw64 ahead
110+
# of Rtools' own, and it exits non-zero having read nothing. Look beside the
111+
# compilers first, since those are necessarily right for the target
112+
objdumpCandidates <- function() {
114113

115-
output <- suppressWarnings(
116-
system2(objdump, c("-p", shQuote(dll)), stdout = TRUE, stderr = TRUE)
117-
)
118-
status <- attr(output, "status")
114+
compilers <- Sys.which(c("gcc", "clang", "cc"))
115+
beside <- file.path(dirname(compilers[nzchar(compilers)]), "objdump.exe")
116+
117+
candidates <- unique(c(beside, Sys.which("objdump")))
118+
candidates[nzchar(candidates) & file.exists(candidates)]
119+
120+
}
119121

120-
# keep only the import tables. the export table follows them, and lists the
121-
# library's own inlined TBB instantiations -- left in, it would be absorbed
122-
# into the last 'DLL Name:' block and credit that library with TBB symbols
123-
# it never imported
124-
exports <- grep("export table|The Export Tables", output)
125-
if (length(exports))
126-
output <- output[seq_len(exports[[1L]] - 1L)]
122+
tbbImports <- function(dll) {
127123

128124
# each imported library opens a 'DLL Name:' block listing the symbols taken
129-
# from it, and runs until the next such block
130-
starts <- grep("DLL Name:", output, fixed = TRUE)
131-
132-
# don't let an objdump that couldn't read the file pass as 'no imports': the
133-
# runner's objdump may be built for another target (an x86_64 one cannot
134-
# read an aarch64 PE, and exits non-zero having printed nothing)
135-
if (!length(starts) || (is.numeric(status) && status != 0L)) {
136-
fmt <- "** '%s' could not read the import table of '%s'; skipping the check"
125+
# from it; finding at least one is how we know objdump really read the file,
126+
# rather than failing in a way that would look like 'imports nothing'
127+
starts <- integer()
128+
output <- character()
129+
130+
for (objdump in objdumpCandidates()) {
131+
132+
output <- suppressWarnings(
133+
system2(objdump, c("-p", shQuote(dll)), stdout = TRUE, stderr = TRUE)
134+
)
135+
status <- attr(output, "status")
136+
137+
# keep only the import tables. the export table follows them, and lists
138+
# the library's own inlined TBB instantiations -- left in, it would be
139+
# absorbed into the last 'DLL Name:' block and credit that library with
140+
# TBB symbols it never imported
141+
exports <- grep("export table|The Export Tables", output)
142+
if (length(exports))
143+
output <- output[seq_len(exports[[1L]] - 1L)]
144+
145+
starts <- grep("DLL Name:", output, fixed = TRUE)
146+
if (length(starts) && !(is.numeric(status) && status != 0L))
147+
break
148+
149+
fmt <- "** '%s' could not read the import table of '%s'"
137150
writeLines(sprintf(fmt, objdump, basename(dll)))
138151
writeLines(output)
152+
153+
starts <- integer()
154+
155+
}
156+
157+
if (!length(starts)) {
158+
fmt <- "** no usable objdump for '%s'; skipping the import table check"
159+
writeLines(sprintf(fmt, basename(dll)))
139160
return(NULL)
140161
}
141162

@@ -180,10 +201,10 @@ if (.Platform$OS.type == "windows") {
180201
}
181202

182203
# and RcppParallel itself must be a client of that same runtime
183-
rcppParallelDll <- file.path(
184-
system.file(paste0("libs", .Platform$r_arch), package = "RcppParallel"),
185-
"RcppParallel.dll"
186-
)
204+
rcppParallelDll <- RcppParallel:::archSystemFile("libs", "RcppParallel.dll")
205+
if (!file.exists(rcppParallelDll))
206+
stop("could not locate RcppParallel.dll within the installed package")
207+
187208
providers <- tbbImports(rcppParallelDll)
188209
if (!is.null(providers) && !identical(providers, "tbb.dll"))
189210
stop("RcppParallel.dll does not take its tbb symbols from 'tbb.dll' ",

0 commit comments

Comments
 (0)