[Auditor] Fix path normalisation for rpaths (#1186)

* [Auditor] Fix relative path normalisation for new runpaths

* [Audit] Remove type piracy

* [Audit] Strip out absolute rpaths

* [Audit] Add test for rpaths

Author: giordano

src/auditor/dynamic_linkage.jl

@@ -1,5 +1,4 @@
 using ObjectFile.ELF
-import ObjectFile: rpaths, canonical_rpaths
 
 """
     platform_for_object(oh::ObjectHandle)
@@ -68,13 +67,13 @@ function platform_for_object(oh::ObjectHandle)
     end
 end
 
-function rpaths(file::AbstractString)
+function _rpaths(file::AbstractString)
     readmeta(file) do oh
         rpaths(RPath(oh))
     end
 end
 
-function canonical_rpaths(file::AbstractString)
+function _canonical_rpaths(file::AbstractString)
     readmeta(file) do oh
         canonical_rpaths(RPath(oh))
     end
@@ -406,12 +405,14 @@ function update_linkage(prefix::Prefix, platform::AbstractPlatform, path::Abstra
             if rp == "."
                 return "\$ORIGIN"
             end
-            if startswith(rp, ".")
+            if startswith(rp, ".") || !startswith(rp, "/")
+                # Relative paths starting with `.`, or anything which isn't an absolute
+                # path.  It may also be a relative path without the leading `./`
                 return "\$ORIGIN/$(rp)"
             end
             return rp
         end
-        current_rpaths = [r for r in rpaths(path) if !isempty(r)]
+        current_rpaths = [r for r in _rpaths(path) if !isempty(r)]
         add_rpath = rp -> begin
             # Join together RPaths to set new one
             rpaths = unique(vcat(current_rpaths, rp))
@@ -425,6 +426,9 @@ function update_linkage(prefix::Prefix, platform::AbstractPlatform, path::Abstra
                 return path
             end
             rpaths = chomp_slashdot.(rpaths)
+            # Remove paths starting with `/workspace`: they will not work outisde of the
+            # build environment and only create noise when debugging.
+            filter!(rp -> !startswith(rp, "/workspace"), rpaths)
 
             rpath_str = join(rpaths, ':')
             return `$patchelf $(patchelf_flags(platform)) --set-rpath $(rpath_str) $(rel_path)`
@@ -435,7 +439,7 @@ function update_linkage(prefix::Prefix, platform::AbstractPlatform, path::Abstra
     # If the relative directory doesn't already exist within the RPATH of this
     # binary, then add it in.
     new_libdir = abspath(dirname(new_libpath) * "/")
-    if !(new_libdir in canonical_rpaths(path))
+    if !(new_libdir in _canonical_rpaths(path))
         libname = basename(old_libpath)
         cmd = add_rpath(normalize_rpath(relpath(new_libdir, dirname(path))))
         with_logfile(prefix, "update_rpath_$(basename(path))_$(libname).log"; subdir) do io

test/auditing.jl

@@ -571,6 +571,55 @@ end
     end
 end
 
+@testset "Auditor - rpaths" begin
+    @testset "$platform" for platform in (Platform("x86_64", "linux"; libc="glibc"), Platform("x86_64", "macos"))
+        mktempdir() do build_path
+            build_output_meta = nothing
+            @test_logs (:info, "Building for $(triplet(platform))") match_mode=:any begin
+                build_output_meta = autobuild(
+                    build_path,
+                    "rpaths",
+                    v"1.0.0",
+                    # No sources
+                    FileSource[],
+                    # Build two libraries, `libbar` in `${libdir}/qux/` and `libfoo` in
+                    # `${libdir}`, with the latter linking to the former.
+                    raw"""
+                    mkdir -p ${libdir}/qux
+                    echo "int bar(){return 38;}" | gcc -x c -shared -fPIC - -o ${libdir}/qux/libbar.${dlext}
+                    echo "extern int bar(); int foo(){return bar() + 4;}" | gcc -x c -shared -fPIC - -o ${libdir}/libfoo.${dlext} -L${libdir}/qux -lbar -Wl,-rpath,${libdir}/qux
+                    """,
+                    [platform],
+                    # Ensure our library products are built
+                    [LibraryProduct("libbar", :libbar, "\$libdir/qux"), LibraryProduct("libfoo", :libfoo)],
+                    # No dependencies
+                    Dependency[];
+                    require_license = false
+                )
+            end
+            # Extract our platform's build
+            @test haskey(build_output_meta, platform)
+            tarball_path, tarball_hash = build_output_meta[platform][1:2]
+            # Ensure the build products were created
+            @test isfile(tarball_path)
+
+            # Unpack it somewhere else
+            @test verify(tarball_path, tarball_hash)
+            testdir = joinpath(build_path, "testdir")
+            mkdir(testdir)
+            unpack(tarball_path, testdir)
+            # Make sure rpath of libbar is empty
+            @test Auditor._rpaths(joinpath(testdir, "lib", "qux", "libbar.$(platform_dlext(platform))")) == []
+            # Make sure the rpath of libfoo contains only `$ORIGIN/qux`, with the relative
+            # path handled correctly.
+            libfoo_rpaths = Auditor._rpaths(joinpath(testdir, "lib", "libfoo.$(platform_dlext(platform))"))
+            @test (Sys.isapple(platform) ? "@loader_path" : "\$ORIGIN") * "/qux" in libfoo_rpaths
+            # Currently we don't filter out absolute rpaths for macOS libraries, no good.
+            @test length(libfoo_rpaths) == 1 broken=Sys.isapple(platform)
+        end
+    end
+end
+
 @testset "Auditor - execution permission" begin
     mktempdir() do build_path
         build_output_meta = nothing