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

LLVM < 3.8 generates incorrect binary in release mode #4104

Closed
asterite opened this issue Mar 3, 2017 · 8 comments
Closed

LLVM < 3.8 generates incorrect binary in release mode #4104

asterite opened this issue Mar 3, 2017 · 8 comments

Comments

@asterite
Copy link
Member

asterite commented Mar 3, 2017

We have #4057, #3013 and #3695 as examples. We should stop supporting LLVM 3.5 (I'm not sure this happens in 3.6, but it's a relatively old version).

@luislavena
Copy link
Contributor

Hello,

I've performed the following tests:

Using latest release (0.21.0) under Linux, I've compiled latest master [812223f] and then compiled the compiler with that build: first compile the compiler with omnibus version, then compile it again with .build/crystal one.

The following is the sample command line used:

# stage 0
$ CC=clang-3.5 CXX=clang++-3.5 LLVM_CONFIG=llvm-config-3.5 make clean crystal

# stage 1
$ touch src/compiler/crystal.cr
$ CC=clang-3.5 CXX=clang++-3.5 LLVM_CONFIG=llvm-config-3.5 make crystal

Once the second compiler was ready, the following script was used for testing:

require "digest/sha1"

puts Digest::SHA1.digest("This is a test")
puts Digest::SHA1.digest("This is a test")
puts Digest::SHA1.digest("This is a test")
puts Digest::SHA1.digest("This is a test")
puts Digest::SHA1.digest("This is a test")
puts Digest::SHA1.digest("This is a test")
puts Digest::SHA1.digest("This is a test")
puts Digest::SHA1.digest("This is a test")
puts Digest::SHA1.digest("This is a test")

And here are the results of each different version of LLVM:

  • OS: Ubuntu 16.10 (x86_64, kernel 4.8.0-39-generic)
  • Crystal: 0.21.0+16 [812223f]
LLVM Result
3.5.2 BAD
3.6.2 BAD
3.7.1 Error compiling llvm_ext
3.8.1 OK
3.9.0 OK

So should be safe to assume that any LLVM prior to 3.8 is not recommended.

It appears omnibus-crystal has been updated to use 3.8.1, but still uses 3.5.0 to bootstrap itself the build process. Not sure if this has any effect.

Cheers.

@asterite
Copy link
Member Author

asterite commented Mar 4, 2017

@luislavena Thank you for the detailed report!

In omnibus we are still using a patched LLVM 3.5.1 (we are using the current branch for releases, not master), which ends up being used in the .deb and .rpm packages we distribute, and also in every downloaded provided in the release page. So this basically affects all distributed Crystal binaries except when you download it from homebrew, because that is built against LLVM 3.9.

We need to update the LLVM version in omnibus but it's pretty tricky (some issues with libgc versions)

@ysbaddaden
Copy link
Contributor

I'll make a patch to remove support for LLVM 3.6 and below in the next few days, so it can be merged once omnibus is updated. That shall cleanup things up.

@refi64
Copy link
Contributor

refi64 commented Mar 8, 2017

FWIW, right now I'm using the Crystal Ubuntu binaries, and I just hit this bug. Is the LLVM version used there going to be upgraded soon?

@bararchy
Copy link
Contributor

@kirbyfan64 Same issue, @ysbaddaden and progress with this ?

@mverzilli
Copy link

@kirbyfan64 @bararchy, we're working on that. Next release should come with 3.8 or higher.

@sdogruyol
Copy link
Member

Are we building with 3.8 or higher yet?

@asterite
Copy link
Member Author

I think so, yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants