Skip to content

[Auditor] Use executables from Binutils_jll when possible #1365

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Project.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ version = "0.6.4"
[deps]
ArgParse = "c7e460c6-2fb9-53a9-8c5b-16f535851c63"
BinaryBuilderBase = "7f725544-6523-48cd-82d1-3fa08ff4056e"
Binutils_jll = "489e263e-5428-50b0-a723-147a141b401e"
Dates = "ade2ca70-3891-5945-98fb-dc099432e06a"
Downloads = "f43a241f-c20a-4ad4-852c-f6b1247861c6"
GitHub = "bc5e4493-9b4d-5f90-b8aa-2b2bcaad7a26"
Expand Down Expand Up @@ -34,6 +35,7 @@ ghr_jll = "07c12ed4-43bc-5495-8a2a-d5838ef8d533"

[compat]
ArgParse = "1.1"
Binutils_jll = "2"
BinaryBuilderBase = "1.35.2"
Downloads = "1"
GitHub = "5.1"
Expand Down
4 changes: 2 additions & 2 deletions src/Auditor.jl
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ const AUDITOR_LOGGING_LOCK = ReentrantLock()

# Helper function to run a command and print to `io` its invocation and full
# output (mimim what the sandbox does normally, but outside of it).
function run_with_io(io::IO, cmd::Cmd)
function run_with_io(io::IO, cmd::Cmd; wait::Bool=true)
println(io, "---> $(join(cmd.exec, " "))")
run(pipeline(cmd; stdout=io, stderr=io))
run(pipeline(cmd; stdout=io, stderr=io); wait)
end

include("auditor/instruction_set.jl")
Expand Down
26 changes: 18 additions & 8 deletions src/auditor/compiler_abi.jl
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Base.BinaryPlatforms: detect_libstdcxx_version, detect_cxxstring_abi
using ObjectFile
using Binutils_jll: Binutils_jll

csl_warning(lib) = @lock AUDITOR_LOGGING_LOCK @warn(
"""
Expand Down Expand Up @@ -171,15 +172,24 @@
seekstart(input)

output = IOBuffer()
mktempdir() do dir
# No need to acquire a sandbox lock here because we use a (hopefully)
# different temporary directory for each run.
ur = preferred_runner()(dir; cwd="/workspace/", platform=platform)
cmd = Cmd(`/opt/bin/$(triplet(ur.platform))/c++filt`; ignorestatus=true)
if strip_underscore
cmd = `$(cmd) --strip-underscore`
cmd = if Binutils_jll.is_available()
ignorestatus(Binutils_jll.cxxfilt())
else
Cmd(`/opt/bin/$(triplet(platform))/c++filt`; ignorestatus=true)
end
if strip_underscore
cmd = `$(cmd) --strip-underscore`
end

if Binutils_jll.is_available()
run(pipeline(cmd; stdin=input, stdout=output))
else
mktempdir() do dir

Check warning on line 187 in src/auditor/compiler_abi.jl

View check run for this annotation

Codecov / codecov/patch

src/auditor/compiler_abi.jl#L187

Added line #L187 was not covered by tests
# No need to acquire a sandbox lock here because we use a (hopefully)
# different temporary directory for each run.
ur = preferred_runner()(dir; cwd="/workspace/", platform=platform)
run_interactive(ur, cmd; stdin=input, stdout=output)

Check warning on line 191 in src/auditor/compiler_abi.jl

View check run for this annotation

Codecov / codecov/patch

src/auditor/compiler_abi.jl#L190-L191

Added lines #L190 - L191 were not covered by tests
end
run_interactive(ur, cmd; stdin=input, stdout=output)
end

return filter!(s -> !isempty(s), split(String(take!(output)), "\n"))
Expand Down
24 changes: 11 additions & 13 deletions src/auditor/soname_matching.jl
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using Patchelf_jll: patchelf

# Not everything has an SONAME
get_soname(oh::ObjectHandle) = nothing

Expand Down Expand Up @@ -63,20 +65,16 @@
end

# Otherwise, set the SONAME
ur = preferred_runner()(prefix.path; cwd="/workspace/", platform=platform)
set_soname_cmd = ``

if Sys.isapple(platform)
install_name_tool = "/opt/bin/$(triplet(ur.platform))/install_name_tool"
set_soname_cmd = `$install_name_tool -id $(soname) $(rel_path)`
elseif Sys.islinux(platform) || Sys.isbsd(platform)
patchelf = "/usr/bin/patchelf"
set_soname_cmd = `$patchelf $(patchelf_flags(platform)) --set-soname $(soname) $(rel_path)`
end

# Create a new linkage that looks like @rpath/$lib on OSX,
# Create a new linkage that looks like @rpath/$lib on OSX,
retval = with_logfile(prefix, "set_soname_$(basename(rel_path))_$(soname).log"; subdir) do io
@lock AUDITOR_SANDBOX_LOCK run(ur, set_soname_cmd, io; verbose=verbose)
if Sys.isapple(platform)
ur = preferred_runner()(prefix.path; cwd="/workspace/", platform=platform)
install_name_tool = "/opt/bin/$(triplet(ur.platform))/install_name_tool"
set_soname_cmd = `$install_name_tool -id $(soname) $(rel_path)`
@lock AUDITOR_SANDBOX_LOCK run(ur, set_soname_cmd, io; verbose=verbose)

Check warning on line 74 in src/auditor/soname_matching.jl

View check run for this annotation

Codecov / codecov/patch

src/auditor/soname_matching.jl#L71-L74

Added lines #L71 - L74 were not covered by tests
elseif Sys.islinux(platform) || Sys.isbsd(platform)
success(run_with_io(io, `$(patchelf()) $(patchelf_flags(platform)) --set-soname $(soname) $(realpath(path))`; wait=false))
end
end

if !retval
Expand Down
8 changes: 6 additions & 2 deletions test/auditing.jl
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,12 @@ using BinaryBuilder.Auditor: compatible_marchs, valid_library_path
"__ZNSt8_Rb_treeINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESt4pairIKS5_S5_ESt10_Select1stIS8_ESt4lessIS5_ESaIS8_EE7_M_copyINSE_11_Alloc_nodeEEEPSt13_Rb_tree_nodeIS8_EPKSI_PSt18_Rb_tree_node_baseRT_",
]
unmangled_symbol_names = Auditor.cppfilt(mangled_symbol_names, Platform("x86_64", "macos"); strip_underscore=true)
@test all(unmangled_symbol_names .== [
"std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (.isra.41)",
# binutils and llvm c++filt slightly disagree on the formatting of final annotation of
# this symbol (`[clone .isra.41]` for the former, `(.isra.41)` for the latter), so we
# only compare the rest.
@test startswith(unmangled_symbol_names[1],
"std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&)")
@test all(unmangled_symbol_names[2:end] .== [
"std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > std::operator+<char, std::char_traits<char>, std::allocator<char> >(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&&, char const*)",
"std::vector<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > > > >::~vector()",
"std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >* std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >::_M_copy<std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >::_Alloc_node>(std::_Rb_tree_node<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const*, std::_Rb_tree_node_base*, std::_Rb_tree<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::_Select1st<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >::_Alloc_node&)",
Expand Down
Loading