Iterate over readmeta results (#1308)

* Iterate over `readmeta` results

* Correct get_soname

* Require ObjectFile 0.4

* Update Manifest.toml

Author: eschnett

Manifest.toml

@@ -41,7 +41,7 @@ JLD2 = "0.1.6, 0.2, 0.3, 0.4"
 JLLWrappers = "1.2.0"
 JSON = "0.21"
 LoggingExtras = "0.4, 1"
-ObjectFile = "0.3.6, 0.4"
+ObjectFile = "0.4"
 OutputCollectors = "0.1"
 PkgLicenses = "0.2"
 Registrator = "1.1"

Project.toml

@@ -81,38 +81,40 @@ function audit(prefix::Prefix, src_name::AbstractString = "";
 
         # Peel this binary file open like a delicious tangerine
         try
-            readmeta(f) do oh
-                if !is_for_platform(oh, platform)
-                    if verbose
-                        @warn("Skipping binary analysis of $(relpath(f, prefix.path)) (incorrect platform)")
-                    end
-                else
-                    # Check that the ISA isn't too high
-                    all_ok &= check_isa(oh, platform, prefix; verbose, silent)
-                    # Check that the OS ABI is set correctly (often indicates the wrong linker was used)
-                    all_ok &= check_os_abi(oh, platform; verbose)
-                    # Make sure all binary files are executables, if libraries aren't
-                    # executables Julia may not be able to dlopen them:
-                    # https://github.com/JuliaLang/julia/issues/38993.  In principle this
-                    # should be done when autofix=true, but we have to run this fix on MKL
-                    # for Windows, for which however we have to set autofix=false:
-                    # https://github.com/JuliaPackaging/Yggdrasil/pull/922.
-                    all_ok &= ensure_executability(oh; verbose, silent)
-
-                    # If this is a dynamic object, do the dynamic checks
-                    if isdynamic(oh)
-                        # Check that the libgfortran version matches
-                        all_ok &= check_libgfortran_version(oh, platform; verbose, has_csl)
-                        # Check whether the library depends on any of the most common
-                        # libraries provided by `CompilerSupportLibraries_jll`.
-                        all_ok &= check_csl_libs(oh, platform; verbose, has_csl)
-                        # Check that the libstdcxx string ABI matches
-                        all_ok &= check_cxxstring_abi(oh, platform; verbose)
-                        # Check that this binary file's dynamic linkage works properly.  Note to always
-                        # DO THIS ONE LAST as it can actually mutate the file, which causes the previous
-                        # checks to freak out a little bit.
-                        all_ok &= check_dynamic_linkage(oh, prefix, bin_files;
-                                                        platform, silent, verbose, autofix, src_name)
+            readmeta(f) do ohs
+                foreach(ohs) do oh
+                    if !is_for_platform(oh, platform)
+                        if verbose
+                            @warn("Skipping binary analysis of $(relpath(f, prefix.path)) (incorrect platform)")
+                        end
+                    else
+                        # Check that the ISA isn't too high
+                        all_ok &= check_isa(oh, platform, prefix; verbose, silent)
+                        # Check that the OS ABI is set correctly (often indicates the wrong linker was used)
+                        all_ok &= check_os_abi(oh, platform; verbose)
+                        # Make sure all binary files are executables, if libraries aren't
+                        # executables Julia may not be able to dlopen them:
+                        # https://github.com/JuliaLang/julia/issues/38993.  In principle this
+                        # should be done when autofix=true, but we have to run this fix on MKL
+                        # for Windows, for which however we have to set autofix=false:
+                        # https://github.com/JuliaPackaging/Yggdrasil/pull/922.
+                        all_ok &= ensure_executability(oh; verbose, silent)
+                    
+                        # If this is a dynamic object, do the dynamic checks
+                        if isdynamic(oh)
+                            # Check that the libgfortran version matches
+                            all_ok &= check_libgfortran_version(oh, platform; verbose, has_csl)
+                            # Check whether the library depends on any of the most common
+                            # libraries provided by `CompilerSupportLibraries_jll`.
+                            all_ok &= check_csl_libs(oh, platform; verbose, has_csl)
+                            # Check that the libstdcxx string ABI matches
+                            all_ok &= check_cxxstring_abi(oh, platform; verbose)
+                            # Check that this binary file's dynamic linkage works properly.  Note to always
+                            # DO THIS ONE LAST as it can actually mutate the file, which causes the previous
+                            # checks to freak out a little bit.
+                            all_ok &= check_dynamic_linkage(oh, prefix, bin_files;
+                                                            platform, silent, verbose, autofix, src_name)
+                        end
                     end
                 end
             end

src/Auditor.jl

@@ -120,8 +120,8 @@ function detect_libstdcxx_version(oh::ObjectHandle, platform::AbstractPlatform)
 
     # Extract all pieces of `.gnu.version_d` from libstdc++.so, find the `GLIBCXX_*`
     # symbols, and use the maximum version of that to find the GLIBCXX ABI version number
-    version_symbols = readmeta(first(libstdcxx_libs)) do oh
-        unique(vcat((x -> x.names).(ELFVersionData(oh))...))
+    version_symbols = readmeta(first(libstdcxx_libs)) do ohs
+        unique(vcat((x -> x.names).(vcat(ELFVersionData.(ohs)...))...))
     end
     version_symbols = filter(x -> startswith(x, "GLIBCXX_"), version_symbols)
     if isempty(version_symbols)

src/auditor/compiler_abi.jl

@@ -68,14 +68,14 @@ function platform_for_object(oh::ObjectHandle)
 end
 
 function _rpaths(file::AbstractString)
-    readmeta(file) do oh
-        rpaths(RPath(oh))
+    readmeta(file) do ohs
+        vcat(rpaths.(RPath.(ohs))...)
     end
 end
 
 function _canonical_rpaths(file::AbstractString)
-    readmeta(file) do oh
-        canonical_rpaths(RPath(oh))
+    readmeta(file) do ohs
+        vcat(canonical_rpaths.(RPath.(ohs))...)
     end
 end
 

src/auditor/dynamic_linkage.jl

@@ -4,7 +4,7 @@ get_soname(oh::ObjectHandle) = nothing
 # Auto-open a path into an ObjectHandle
 function get_soname(path::AbstractString)
     try
-        readmeta(get_soname, path)
+        only(readmeta(ns -> get_soname.(ns), path))
     catch e
         @warn "Could not probe $(path) for an SONAME!" exception=(e, catch_backtrace())
         return nothing

src/auditor/soname_matching.jl

@@ -34,15 +34,16 @@ function step4(state::WizardState, ur::Runner, platform::AbstractPlatform,
 
     # Check if we can load them as an object file
     files = filter(files) do f
-        readmeta(f) do oh
-            return Auditor.is_for_platform(oh, platform)
+        readmeta(f) do ohs
+            return any(Auditor.is_for_platform(oh, platform) for oh in ohs)
         end
     end
 
     # Strip out the prefix from filenames
     state.files = map(file->replace(file, "$(destdir_path)/" => ""), files)
     state.file_kinds = map(files) do f
-        readmeta(f) do oh
+        readmeta(f) do ohs
+            oh = first(ohs)
             if isexecutable(oh)
                 return :executable
             elseif islibrary(oh)

src/wizard/interactive_build.jl

@@ -85,7 +85,7 @@ from `ObjectFile`.
 function filter_object_files(files)
     return filter(files) do f
         try
-            readmeta(f) do oh
+            readmeta(f) do ohs
                 return true
             end
         catch e
@@ -121,8 +121,8 @@ function match_files(state::WizardState, prefix::Prefix,
     prefix_files = filter_object_files(prefix_files)
     # Check if we can load them as an object file
     prefix_files = filter(prefix_files) do f
-        readmeta(f) do oh
-            if !Auditor.is_for_platform(oh, platform)
+        readmeta(f) do ohs
+            if !any(Auditor.is_for_platform(oh, platform) for oh in ohs)
                 if !silent
                     @warn("Skipping binary `$f` with incorrect platform")
                 end

src/wizard/utils.jl

@@ -86,9 +86,11 @@ end
             prefix = Prefix(testdir)
 
             # Run ISA test
-            readmeta(locate(product, prefix)) do oh
-                detected_isa = Auditor.analyze_instruction_set(oh, platform; verbose=true)
-                @test detected_isa == "avx512"
+            readmeta(locate(product, prefix)) do ohs
+                foreach(ohs) do oh
+                    detected_isa = Auditor.analyze_instruction_set(oh, platform; verbose=true)
+                    @test detected_isa == "avx512"
+                end
             end
         end
     end
@@ -133,9 +135,11 @@ end
             prefix = Prefix(testdir)
 
             # Run ISA test
-            readmeta(locate(product, prefix)) do oh
-                detected_march = Auditor.analyze_instruction_set(oh, platform; verbose=true)
-                @test detected_march == "avx"
+            readmeta(locate(product, prefix)) do ohs
+                foreach(ohs) do oh
+                    detected_march = Auditor.analyze_instruction_set(oh, platform; verbose=true)
+                    @test detected_march == "avx"
+                end
             end
         end
     end
@@ -182,14 +186,16 @@ end
             prefix = Prefix(testdir)
 
             # Run ISA test
-            readmeta(locate(product, prefix)) do oh
-                detected_march = Auditor.analyze_instruction_set(oh, platform; verbose=true)
-                if march == "avx2"
-                    # Detecting the ISA isn't 100% reliable and it's even less
-                    # accurate when looking for AVX2 features
-                    @test_broken march == detected_march
-                else
-                    @test march == detected_march
+            readmeta(locate(product, prefix)) do ohs
+                foreach(ohs) do oh
+                    detected_march = Auditor.analyze_instruction_set(oh, platform; verbose=true)
+                    if march == "avx2"
+                        # Detecting the ISA isn't 100% reliable and it's even less
+                        # accurate when looking for AVX2 features
+                        @test_broken march == detected_march
+                    else
+                        @test march == detected_march
+                    end
                 end
             end
         end
@@ -243,9 +249,11 @@ end
                 prefix = Prefix(testdir)
 
                 # Ensure that the library detects as the correct cxxstring_abi:
-                readmeta(locate(libcxxstringabi_test, prefix)) do oh
-                    detected_cxxstring_abi = Auditor.detect_cxxstring_abi(oh, platform)
-                    @test detected_cxxstring_abi == cxxstring_abi(platform)
+                readmeta(locate(libcxxstringabi_test, prefix)) do ohs
+                    foreach(ohs) do oh
+                        detected_cxxstring_abi = Auditor.detect_cxxstring_abi(oh, platform)
+                        @test detected_cxxstring_abi == cxxstring_abi(platform)
+                    end
                 end
 
                 # Explicitly test cxx string abi mismatches
@@ -404,11 +412,13 @@ end
         prefix = Prefix(testdir)
 
         # Helper to extract the dylib id of a path
-        function get_dylib_id(path)
-            return readmeta(path) do oh
-                dylib_id_lcs = [lc for lc in MachOLoadCmds(oh) if isa(lc, MachOIdDylibCmd)]
-                @test !isempty(dylib_id_lcs)
-                return dylib_name(first(dylib_id_lcs))
+        function get_dylib_ids(path)
+            return readmeta(path) do ohs
+                map(ohs) do oh
+                     dylib_id_lcs = [lc for lc in MachOLoadCmds(oh) if isa(lc, MachOIdDylibCmd)]
+                     @test !isempty(dylib_id_lcs)
+                     return dylib_name(first(dylib_id_lcs))
+                end
             end
         end
 
@@ -419,7 +429,7 @@ end
         right_id_path = locate(right_id, prefix; platform=platform)
         for p in (no_id_path, abs_id_path, right_id_path)
             @test any(startswith.(p, libdirs(prefix)))
-            @test get_dylib_id(p) == "@rpath/$(basename(p))"
+            @test all(get_dylib_ids(p) .== "@rpath/$(basename(p))")
         end
 
         # Only if it already has an `@rpath/`-ified ID, it doesn't get touched.
@@ -534,10 +544,12 @@ end
         # audit should warn us.
         libgfortran_versions = (3, 4, 5)
         other_libgfortran_version = libgfortran_versions[findfirst(v -> v != our_libgfortran_version.major, libgfortran_versions)]
-        @test_logs (:warn, Regex("but we are supposedly building for libgfortran$(other_libgfortran_version)")) (:warn, r"Linked library libgfortran.so.5") (:warn, r"Linked library libquadmath.so.0") (:warn, r"Linked library libgcc_s.so.1") readmeta(hello_world_path) do oh
-            p = deepcopy(platform)
-            p["libgfortran_version"] = "$(other_libgfortran_version).0.0"
-            @test !Auditor.audit(Prefix(testdir); platform=p, autofix=false)
+        @test_logs (:warn, Regex("but we are supposedly building for libgfortran$(other_libgfortran_version)")) (:warn, r"Linked library libgfortran.so.5") (:warn, r"Linked library libquadmath.so.0") (:warn, r"Linked library libgcc_s.so.1") readmeta(hello_world_path) do ohs
+            foreach(ohs) do oh
+                p = deepcopy(platform)
+                p["libgfortran_version"] = "$(other_libgfortran_version).0.0"
+                @test !Auditor.audit(Prefix(testdir); platform=p, autofix=false)
+            end
         end
     end
 end