Stop counting stale ELF strings when bottling

[MikeMcQuaid]

• Growing an ELF RPATH or interpreter makes patchelf move the dynamic
  string table or interpreter elsewhere in the file, leaving the old
  prefix string behind as dead bytes that strings-based scanning in
  brew bottle still found, wrongly pinning bottles (e.g. harfbuzz
  and the glib/gobject-introspection cluster) whose live linkage is
  fully placeholdered.
• Keg.text_matches_in_file now maps each match offset back to the
  ELF structures with the vendored elftools and drops matches the
  loader cannot see: unreferenced strings in loader-owned regions
  (.dynstr, .interp and the DT_STRTAB table) and bytes outside
  every section.
• Strings still referenced by dynamic tags (including references into
  the middle of a suffix-merged string), the live PT_INTERP value
  and strings compiled into ordinary sections keep pinning bottles.
• These formulae flip to cellar :any on their next rebottle.

This change is part of plans/relocatable-bottles.md

---

• Have you followed our Contributing guidelines?
• Have you checked for other open Pull Requests for the same change?
• Have you explained what your changes do? Performance claims (e.g. "this is faster") must include brew benchmark results.
• Have you explained why you'd like these changes included, not just what they do?
• For bug fixes, have you given step-by-step brew commands to reproduce the bug?
• Have you written new tests (excluding integration tests)? Here's an example.
• Have you successfully run brew lgtm (style, typechecking and tests) locally?

---

• I did not use AI/LLM to create this PR, or I disclosed the tool/model below and reviewed its output; I did not attribute commits to AI and will answer maintainer questions and review comments myself without AI/LLM.

Claude Fable 5 with local review and testing.

---

[github-actions]

Thanks for your pull request. This has been closed because it appears to be missing the pull request template, perhaps because this was written by an AI not a human. We require humans to read and fill in these templates.

Please edit this pull request to fill in the current pull request template. This workflow will reopen this pull request automatically once the template is complete. Do not open a new pull request for this.

[Copilot]

Pull request overview

Updates bottling scans to ignore stale ELF prefix strings left by patchelf while preserving live references.

Changes:

• Adds ELF-aware match filtering.
• Adds regression tests for stale and live strings.
• Removes the completed relocation-plan item.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Summary	Findings
Library/Homebrew/test/keg_relocate/elf_checker_spec.rb	Adds ELF filtering regression coverage.	None
Library/Homebrew/plans/relocatable-bottles.md	Removes the completed plan item.	None
Library/Homebrew/keg_relocate.rb	Implements ELF-aware filtering.	Two moderate findings (4 votes each): account for DT_CONFIG, DT_DEPAUDIT, and DT_AUDIT; rescue parser truncation errors such as IOError/EOFError.

Suppressed comments (3)

Library/Homebrew/keg_relocate.rb:540

• The vendored elftools defines DT_FILTER as 0x7ffffffe, the same value as DT_USED, while the ELF ABI value for DT_FILTER is 0x7fffffff. Thus an actual DT_FILTER tag is not added to live_string_offsets (and a DT_USED value is treated as a string offset), so a live filter path can be discarded. Correct the dependency constant or account for the ABI value here.

          ELFTools::Constants::DT::DT_FILTER,

Library/Homebrew/keg_relocate.rb:572

• match_range covers the entire printable run returned by strings, not the specific prefix occurrence being reported. For example, if a dynamic tag points at the interior libfoo.so of /old/prefix/libfoo.so, the live offset lies in this range and the unreferenced /old/prefix is retained, so this still pins a stale suffix-merged prefix. Track the byte range of the actual matched occurrence before deciding it is live.

        # A string some dynamic tag still points into is live, including
        # references into the middle of a suffix-merged string.
        next true if live_string_offsets.any? { |live| match_range.cover?(live) }

Library/Homebrew/keg_relocate.rb:536

• The dynamic string table is also indexed by loader-visible structures such as .dynsym symbol st_name entries (and version tables), not only by the string-valued dynamic tags listed here. A prefix-containing live symbol name will therefore be treated as an unreferenced .dynstr string and dropped. Preserve offsets referenced by the dynamic symbol/version tables before classifying the remainder as dead.

        # Dynamic tags whose value is an offset into the dynamic string
        # table, i.e. the strings the loader can actually see.
        string_tags = [
          ELFTools::Constants::DT::DT_NEEDED,
          ELFTools::Constants::DT::DT_SONAME,

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[carlocab]

I don't know if we can justify treating every match outside the section table as dead, which seems to be what this approach does implicitly.

ELFFile#sections reflects only the section headers, while strings - scans the whole file. ELF execution is described by program headers, not just sections, so I think we can pick up text matches in non-section data but mistakenly classify them as non-live.

Also: I think Copilot's suppressed comments contain correctness issues that shouldn't have been suppressed.

[carlocab]

I think we want to keep the exception narrow here. We only need to identify strings that are demonstrably stale artefacts left behind by patchelf.rb; treating every match outside the current ELF structure as non-live seems broader than necessary, and risks suppressing strings we don't actually know are dead.

[MikeMcQuaid]

I think we want to keep the exception narrow here. We only need to identify strings that are demonstrably stale artefacts left behind by patchelf.rb; treating every match outside the current ELF structure as non-live seems broader than necessary, and risks suppressing strings we don't actually know are dead.

I disagree. As the person who wrote the original strings implementation: it's far too broad and we don't have a rock solid approach that allows us to narrow it sufficiently. It's a trade-off, ultimately, and the benefit to users on non-default prefixes is much higher when we nudge things closer towards assuming relocation rather than assuming not.

I'll look at all the comments here including the suppressed one but just want to be explicit that the there is an active goal here to be less "safe" and prioritise removing false negatives over false positives which we can trivially fix if we get bug reports.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.