From 0484bfe8200832a17444336f4abcd3284e01aa5f Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Tue, 12 Oct 2021 13:11:23 +0800 Subject: [PATCH 1/4] mac/formula_cellar_checks: check for flat namespace libraries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are at least five instances where a formula has libraries compiled with `-flat_namespace` due to a bug in detecting the macOS version (cf. Homebrew/homebrew-core#87103, Homebrew/homebrew-core#85974, Homebrew/homebrew-core#85973). I think it makes sense to check for this more generally. It is sometimes intentional, so I've added a check for an allowlist for those instances. Running this on the current `util-linux` bottle produces ❯ brew audit --strict util-linux util-linux: * Libraries were compiled with a flat namespace. This can cause linker errors due to name collisions, and is often due to a bug in detecting the macOS version. /usr/local/Cellar/util-linux/2.37.2/lib/libblkid.1.dylib /usr/local/Cellar/util-linux/2.37.2/lib/libfdisk.1.dylib /usr/local/Cellar/util-linux/2.37.2/lib/libsmartcols.1.dylib /usr/local/Cellar/util-linux/2.37.2/lib/libuuid.1.dylib Error: 1 problem in 1 formula detected Some things that still need to be done here: - fix this check for universal binaries - check if we want to restrict this audit check to newer versions of macOS - fix false positives (try `brew audit --strict llvm` and compare the output of `otool -hV` on the identified files) While we're here, let's fix the formatting of the output of these other audits (cf. #12217). --- .../extend/os/mac/formula_cellar_checks.rb | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb index 1bb8f462eb..e2fbb294ea 100644 --- a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb +++ b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb @@ -22,7 +22,7 @@ module FormulaCellarChecks <<~EOS Header files that shadow system header files were installed to "#{formula.include}" The offending files are: - #{files * "\n "} + #{files * "\n "} EOS end @@ -41,7 +41,7 @@ module FormulaCellarChecks These object files were linked against the deprecated system OpenSSL or the system's private LibreSSL. Adding `depends_on "openssl"` to the formula may help. - #{system_openssl * "\n "} + #{system_openssl * "\n "} EOS end @@ -58,7 +58,7 @@ module FormulaCellarChecks These python extension modules were linked directly to a Python framework binary. They should be linked with -undefined dynamic_lookup instead of -lpython or -framework Python. - #{framework_links * "\n "} + #{framework_links * "\n "} EOS end @@ -88,12 +88,37 @@ module FormulaCellarChecks end end + def check_flat_namespace(formula) + return unless formula.prefix.directory? + return if formula.tap.present? && tap_audit_exception(:flat_namespace_allowlist, formula.name) + + keg = Keg.new(formula.prefix) + flat_namespace_files = keg.mach_o_files.reject do |file| + next true unless file.dylib? + # FIXME: macho.header.flag? is not defined when macho + # is a universal binary. + next true if file.universal? + + macho = MachO.open(file) + macho.header.flag?(:MH_TWO_LEVEL) + end + return if flat_namespace_files.empty? + + <<~EOS + Libraries were compiled with a flat namespace. + This can cause linker errors due to name collisions, and + is often due to a bug in detecting the macOS version. + #{flat_namespace_files * "\n "} + EOS + end + def audit_installed generic_audit_installed problem_if_output(check_shadowed_headers) problem_if_output(check_openssl_links) problem_if_output(check_python_framework_links(formula.lib)) check_linkage + problem_if_output(check_flat_namespace(formula)) end def valid_library_extension?(filename) From 7223f8ef74f273366d1bd7c2aa7e6a2023ecf04f Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Tue, 12 Oct 2021 13:49:53 +0800 Subject: [PATCH 2/4] Fix universal binary handling in `check_flat_namespace` --- Library/Homebrew/extend/os/mac/formula_cellar_checks.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb index e2fbb294ea..515cbbdd0d 100644 --- a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb +++ b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb @@ -95,12 +95,13 @@ module FormulaCellarChecks keg = Keg.new(formula.prefix) flat_namespace_files = keg.mach_o_files.reject do |file| next true unless file.dylib? - # FIXME: macho.header.flag? is not defined when macho - # is a universal binary. - next true if file.universal? macho = MachO.open(file) - macho.header.flag?(:MH_TWO_LEVEL) + if file.universal? + macho.machos.map(&:header).all? { |h| h.flag? :MH_TWO_LEVEL } + else + macho.header.flag?(:MH_TWO_LEVEL) + end end return if flat_namespace_files.empty? From 4fbe0a2b1bd35eb76467a9decee526382fe9ac5b Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Tue, 12 Oct 2021 15:35:46 +0800 Subject: [PATCH 3/4] Fix false positives in audit There was a typo that made it so that all libraries were being included in `flat_namespace_files`. --- Library/Homebrew/extend/os/mac/formula_cellar_checks.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb index 515cbbdd0d..ab9042331a 100644 --- a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb +++ b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb @@ -98,9 +98,9 @@ module FormulaCellarChecks macho = MachO.open(file) if file.universal? - macho.machos.map(&:header).all? { |h| h.flag? :MH_TWO_LEVEL } + macho.machos.map(&:header).all? { |h| h.flag? :MH_TWOLEVEL } else - macho.header.flag?(:MH_TWO_LEVEL) + macho.header.flag?(:MH_TWOLEVEL) end end return if flat_namespace_files.empty? From e8eb781470df7463ca2147862d257815f5062314 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Wed, 13 Oct 2021 16:54:02 +0800 Subject: [PATCH 4/4] mac/formula_cellar_checks: apply suggestions from code review - Fix missing space - use `MachO::Utils.fat_magic?` - call `#flag?` consistently. Co-authored-by: Bo Anderson --- Library/Homebrew/extend/os/mac/formula_cellar_checks.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb index ab9042331a..3c297e1849 100644 --- a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb +++ b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb @@ -41,7 +41,7 @@ module FormulaCellarChecks These object files were linked against the deprecated system OpenSSL or the system's private LibreSSL. Adding `depends_on "openssl"` to the formula may help. - #{system_openssl * "\n "} + #{system_openssl * "\n "} EOS end @@ -97,10 +97,10 @@ module FormulaCellarChecks next true unless file.dylib? macho = MachO.open(file) - if file.universal? + if MachO::Utils.fat_magic?(macho.magic) macho.machos.map(&:header).all? { |h| h.flag? :MH_TWOLEVEL } else - macho.header.flag?(:MH_TWOLEVEL) + macho.header.flag? :MH_TWOLEVEL end end return if flat_namespace_files.empty?