Skip to content
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

Relocation breaks Qt installs on Linux #13922

Closed
2 tasks done
carlocab opened this issue Sep 24, 2022 · 17 comments · Fixed by #14094
Closed
2 tasks done

Relocation breaks Qt installs on Linux #13922

carlocab opened this issue Sep 24, 2022 · 17 comments · Fixed by #14094
Assignees
Labels
bug Reproducible Homebrew/brew bug in progress Maintainers are working on this outdated PR was locked due to age

Comments

@carlocab
Copy link
Member

brew config output

HOMEBREW_VERSION: 3.6.2
ORIGIN: https://github.com/Homebrew/brew
HEAD: b8b195cc64a29595797651720ebb2ea09affb682
Last commit: 5 days ago
Core tap ORIGIN: https://github.com/Homebrew/homebrew-core
Core tap HEAD: 55e393266248529b58c2ff6df23485a90ba8a81e
Core tap last commit: 5 days ago
Core tap branch: master
HOMEBREW_PREFIX: /home/linuxbrew/.linuxbrew
HOMEBREW_CASK_OPTS: []
HOMEBREW_MAKE_JOBS: 4
Homebrew Ruby: 2.6.8 => /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/vendor/portable-ruby/2.6.8_1/bin/ruby
CPU: quad-core 64-bit icelake
Clang: N/A
Git: 2.37.3 => /bin/git
Curl: 7.81.0 => /bin/curl
Kernel: Linux 5.10.109-0-virt x86_64 GNU/Linux
OS: Unknown
Host glibc: 2.35
/usr/bin/gcc: 11.2.0
/usr/bin/ruby: N/A
glibc: N/A
gcc@11: N/A
gcc@12: N/A
xorg: N/A

brew doctor output

Your system is ready to brew.

Verification

  • I ran brew update and am still able to reproduce my issue.
  • I have resolved all warnings from brew doctor and that did not fix my problem.

What were you trying to do (and why)?

Test Qt after pouring a just-built bottle for distribution.

What happened (include all command output)?

Running the test fails with

  ==> ./test
  ASSERT: "list.contains(fmt)" in file /tmp/qt-test-20220920-224400-u60d5/main.cpp, line 31

This is because relocation breaks the bottle. See Homebrew/homebrew-core#111031.

What did you expect to happen?

Successful test.

Step-by-step reproduction instructions (by running brew commands)

brew install --build-bottle qt
brew bottle --json qt --keep-old --only-json-tab
brew install --only-dependencies ./qt--6.3.1_4.x86_64_linux.bottle.tar.gz
brew install ./qt--6.3.1_4.x86_64_linux.bottle.tar.gz
brew test qt
@carlocab
Copy link
Member Author

CC @Homebrew/linux

@carlocab carlocab added the help wanted We want help addressing this label Sep 24, 2022
@danielnachun
Copy link
Member

I don't know if this is helpful, but some openj9 binaries also seem to break on relocation and fails during its build process, hence why it remains unbottled on Linux.

@MikeMcQuaid
Copy link
Member

@carlocab Is this a (or: are these) cellar :any bottle that is broken when poured outside the default prefix or a bottle that never works ever?

@carlocab
Copy link
Member Author

The bottle seems to never work ever. I think the bug is in patchelf.rb. It would be good for someone in @Homebrew/linux to diagnose this and report it there.

@cho-m
Copy link
Member

cho-m commented Sep 27, 2022

Also see an issue in gopass (Homebrew/homebrew-core#111620) which could be due to same root cause. At least brew install --build-bottle has working binary but brew bottle ... ; brew uninstall ... ; brew install ./<bottle> has broken binary with ELF issues seen via readelf and gdb.

Needs further analysis to know if all these issues (qt, openj9, gopass) are same.

@Bo98 Bo98 self-assigned this Sep 30, 2022
@Bo98
Copy link
Member

Bo98 commented Oct 1, 2022

Good news is I've got a fix for the issue: Bo98/patchelf.rb@8dc5568 (paired with Bo98/rbelftools@63e0d32).

Bad news is we get patching errors on existing bottles that have already been corrupted by the bug ("cannot normalize PT_NOTE segment: non-contiguous SHT_NOTE sections"). The patchelf binary behaves the same way, so the behaviour is correct. Not sure the best way to deal with it beyond maybe inserting a hack to make it silently give up fixing rather than error out.

@carlocab
Copy link
Member Author

carlocab commented Oct 1, 2022

Not sure the best way to deal with it beyond maybe inserting a hack to make it silently give up fixing rather than error out.

Maybe do this when HOMEBREW_DEVELOPER is off?

@Bo98
Copy link
Member

Bo98 commented Oct 1, 2022

The tricky bit is doing something that's upstreamable, which would mean upstreaming a conditional.

I've looked at it a bit closer and the main issue seems to be empty NOTE program segments being left under the bugged version. I could probably make it a warning under that specific condition with the reasoning of backwards compatibility, and still error if there's other junk (non-empty). I think that's all we would need for our cases.

@danielnachun
Copy link
Member

vgrep is probably also affected by this as it works fine when built directly from source but segfaults after being poured from a bottle.

@carlocab
Copy link
Member Author

carlocab commented Oct 2, 2022

The tricky bit is doing something that's upstreamable, which would mean upstreaming a conditional.

Does it not throw a specific error that we can catch? If so we can probably do that here. Or, if not, we can upstream the throwing a specific error with the justification that downstream projects should be able to decide how to handle the previous erroneous behaviour.

I suspect lots of formulae are affected by this -- I think strongswan is one of them. (Or maybe strongswan just doesn't like getting patchelfed; I forget.)

@Bo98
Copy link
Member

Bo98 commented Oct 2, 2022

vgrep is probably also affected by this as it works fine when built directly from source but segfaults after being poured from a bottle.

Might be different than the note section issue but I've fixed a couple of other corruption bugs too in that commit so it could cover this - I'll have a look as I'll need to see the readelf dump of that binary (both directly from the bottle and the installed file after patching).

Does it not throw a specific error that we can catch? If so we can probably do that here. Or, if not, we can upstream the throwing a specific error with the justification that downstream projects should be able to decide how to handle the previous erroneous behaviour.

It throws a generic PatchError, but with a specific message.

However the problem is this raising the error will mean nothing is written to disk when we actually want a "best-effort" patch since patching nothing will result in a non-functional binary.

So the only real solution beyond a conditional is to make it a warning for the specific scope we need it to be, which I think can make sense as a backwards compatibility with older patchelf case.

@Bo98
Copy link
Member

Bo98 commented Oct 10, 2022

Submitted a bunch of patches to upstream of upstream (grand upstream?):

The second one in particular should resolve the concerns I mentioned previously. I'll port these to Ruby shortly.

@Bo98
Copy link
Member

Bo98 commented Oct 10, 2022

Ruby port: Bo98/patchelf.rb@3a4a39d

(in addition to Bo98/patchelf.rb@8dc5568 and Bo98/rbelftools@63e0d32 from before)

Not sure what the best way to test all this is.

@Bo98
Copy link
Member

Bo98 commented Oct 11, 2022

Not exactly the correct way to do it (we'll do it properly and get a new upstream release), but if someone wants to try Bo98/brew@0a16d77 and install a bunch of bottles then please do. Just make sure you've run brew install-bundler-gems recently enough that it probably won't overwrite changes.

I'm not just interested in problematic bottles - I'm also interested in general compatibility of these changes with all the bottles we currently have available.

@Bo98
Copy link
Member

Bo98 commented Oct 11, 2022

david942j/patchelf.rb#39

@Bo98
Copy link
Member

Bo98 commented Oct 28, 2022

Submitted a bunch of patches to upstream of upstream (grand upstream?):

These have now made it into a release: https://github.com/NixOS/patchelf/releases/tag/0.16.0

@Bo98
Copy link
Member

Bo98 commented Oct 28, 2022

Ruby port has been merged now too.

@github-actions github-actions bot added the outdated PR was locked due to age label Dec 5, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Reproducible Homebrew/brew bug in progress Maintainers are working on this outdated PR was locked due to age
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants