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

Use apt clang instead of manually installing clang #12

Merged
merged 4 commits into from
Feb 22, 2024

Conversation

mschwager
Copy link
Member

@mschwager mschwager commented Feb 20, 2024

Using -lstdc++ fixes issues like:

$ ruby -e 'require "ruzzy"; print Ruzzy::ASAN_PATH'
<internal:/usr/lib/ruby/vendor_ruby/rubygems/core_ext/kernel_require.rb>:85:in `require': /var/lib/gems/3.1.0/gems/ruzzy-0.6.0/lib/cruzzy/cruzzy.so: undefined symbol: _ZNSt6thread6_StateD2Ev - /var/lib/gems/3.1.0/gems/ruzzy-0.6.0/lib/cruzzy/cruzzy.so (LoadError)
	from <internal:/usr/lib/ruby/vendor_ruby/rubygems/core_ext/kernel_require.rb>:85:in `require'
	from /var/lib/gems/3.1.0/gems/ruzzy-0.6.0/lib/ruzzy.rb:7:in `<module:Ruzzy>'
	from /var/lib/gems/3.1.0/gems/ruzzy-0.6.0/lib/ruzzy.rb:6:in `<top (required)>'
	from <internal:/usr/lib/ruby/vendor_ruby/rubygems/core_ext/kernel_require.rb>:160:in `require'
	from <internal:/usr/lib/ruby/vendor_ruby/rubygems/core_ext/kernel_require.rb>:160:in `rescue in require'
	from <internal:/usr/lib/ruby/vendor_ruby/rubygems/core_ext/kernel_require.rb>:149:in `require'
	from -e:1:in `<main>'
<internal:/usr/lib/ruby/vendor_ruby/rubygems/core_ext/kernel_require.rb>:85:in `require': cannot load such file -- ruzzy (LoadError)
	from <internal:/usr/lib/ruby/vendor_ruby/rubygems/core_ext/kernel_require.rb>:85:in `require'
	from -e:1:in `<main>'

Changing the dummy code was necessary because older versions of clang do not set detect_stack_use_after_return=1 by default. Instead of adding this to the environment, I decided to use different ASAN violation code so we don't require ASAN_OPTIONS modification.

There's some debate as to whether we should use -lstdc++ or -lc++. I'm not sure what the right answer is, but -lstdc++ works for me while -lc++ doesn't. These links may help give some context or recommendations:

@AdvenamTacet AdvenamTacet self-assigned this Feb 20, 2024
ext/dummy/dummy.c Outdated Show resolved Hide resolved
Copy link
Contributor

@AdvenamTacet AdvenamTacet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to check it Tomorrow, but based on reading without running, LGTM.

ext/cruzzy/extconf.rb Show resolved Hide resolved
ext/dummy/dummy.c Show resolved Hide resolved
Copy link
Contributor

@AdvenamTacet AdvenamTacet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I want to suggest future changes, but I can do it in another PR. Those changes fix the issue we know.
I didn't find any new problem after applying that PR.

There's some debate as to whether we should use -lstdc++ or -lc++. I'm not sure what the right answer is, but -lstdc++ works for me while -lc++ doesn't. These links may help give some context or recommendations:

I was able to compile with with libc++, so it should work as well, but I can look at it later. If there is no reason to not use libc++, I think it's better to use libc++.

@mschwager mschwager merged commit 351e712 into main Feb 22, 2024
5 checks passed
@mschwager mschwager deleted the mschwager-apt-clang branch February 22, 2024 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants