From fd44c0816f85ea0e4e75419a0784984bad796603 Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Wed, 30 Oct 2024 23:36:07 +0100 Subject: [PATCH 1/4] Add indirect branch tracking (#15122) Adds support for indirect branch tracking for X86[_64] (CET) and AArch64 targets through the following compile time flags (taken from gcc/clang/rust): - `-Dcf-protection=branch` (or `=return` or `=full`) for X86 - `-Dbranch-protection=bti` for AArch64 These flags are automatically set for OpenBSD, that enforces IBT or BTI on all user land applications. The patch also removes the `-Wl-znobtcfi` linker option since we don't need to disable it anymore. OpenBSD is the only OS I know to support _and_ enforce IBT or BTI in user land. Linux for example only supports it for kernel code (for the time being). I manually tested IBT in an OpenBSD VM on x86_64 with a supported CPU (Intel Raptor Lake). I can compile & recompile crystal as well as run `gmake std_spec` without running into IBT issues :+1: Notes: - I expected to have to add the ASM instructions to the fiber context switch ASM... but messing with the stack pointer isn't considered as a conditional jump apparently :shrug: - I'm using the genius idea from @straight-shoota that we can pass `-Dkey=value` then test for `flag?("key=value")` and it just worked :astonished: - I can't test BTI on AArch64: I have no hardware and there are no bindings for the `aarch64-unknown-openbsd` target; there are little reasons it wouldn't work though; - I added support for shadow stack (SHSTK) on X86 (`-Dcf-protection=return`). I'm not sure we really support it though, since fibers are messing with the stacks? --- src/compiler/crystal/codegen/codegen.cr | 32 +++++++++++++++++++------ src/compiler/crystal/codegen/debug.cr | 8 ++----- src/compiler/crystal/codegen/fun.cr | 3 +-- src/compiler/crystal/compiler.cr | 6 ----- src/compiler/crystal/semantic/flags.cr | 13 +++++++++- src/llvm/lib_llvm/core.cr | 7 +++++- src/llvm/module.cr | 4 ++++ 7 files changed, 50 insertions(+), 23 deletions(-) diff --git a/src/compiler/crystal/codegen/codegen.cr b/src/compiler/crystal/codegen/codegen.cr index c4844df9a5e8..7e15b1bdc385 100644 --- a/src/compiler/crystal/codegen/codegen.cr +++ b/src/compiler/crystal/codegen/codegen.cr @@ -274,7 +274,7 @@ module Crystal @llvm_context : LLVM::Context = LLVM::Context.new) @abi = @program.target_machine.abi # LLVM::Context.register(@llvm_context, "main") - @llvm_mod = @llvm_context.new_module("main_module") + @llvm_mod = configure_module(@llvm_context.new_module("main_module")) @main_mod = @llvm_mod @main_llvm_context = @main_mod.context @llvm_typer = LLVMTyper.new(@program, @llvm_context) @@ -345,8 +345,6 @@ module Crystal @unused_fun_defs = [] of FunDef @proc_counts = Hash(String, Int32).new(0) - @llvm_mod.data_layout = self.data_layout - # We need to define __crystal_malloc and __crystal_realloc as soon as possible, # to avoid some memory being allocated with plain malloc. codegen_well_known_functions @node @@ -367,6 +365,30 @@ module Crystal getter llvm_context + def configure_module(llvm_mod) + llvm_mod.data_layout = @program.target_machine.data_layout + + # enable branch authentication instructions (BTI) + if @program.has_flag?("aarch64") + if @program.has_flag?("branch-protection=bti") + llvm_mod.add_flag(:override, "branch-target-enforcement", 1) + end + end + + # enable control flow enforcement protection (CET): IBT and/or SHSTK + if @program.has_flag?("x86_64") || @program.has_flag?("i386") + if @program.has_flag?("cf-protection=branch") || @program.has_flag?("cf-protection=full") + llvm_mod.add_flag(:override, "cf-protection-branch", 1) + end + + if @program.has_flag?("cf-protection=return") || @program.has_flag?("cf-protection=full") + llvm_mod.add_flag(:override, "cf-protection-return", 1) + end + end + + llvm_mod + end + def new_builder(llvm_context) wrap_builder(llvm_context.new_builder) end @@ -419,10 +441,6 @@ module Crystal global.initializer = llvm_element_type.const_array(llvm_elements) end - def data_layout - @program.target_machine.data_layout - end - class CodegenWellKnownFunctions < Visitor @codegen : CodeGenVisitor diff --git a/src/compiler/crystal/codegen/debug.cr b/src/compiler/crystal/codegen/debug.cr index dd4b6c361905..870506377f7a 100644 --- a/src/compiler/crystal/codegen/debug.cr +++ b/src/compiler/crystal/codegen/debug.cr @@ -42,17 +42,13 @@ module Crystal if @program.has_flag?("msvc") # Windows uses CodeView instead of DWARF - mod.add_flag( - LibLLVM::ModuleFlagBehavior::Warning, - "CodeView", - mod.context.int32.const_int(1) - ) + mod.add_flag(LibLLVM::ModuleFlagBehavior::Warning, "CodeView", 1) end mod.add_flag( LibLLVM::ModuleFlagBehavior::Warning, "Debug Info Version", - mod.context.int32.const_int(LLVM::DEBUG_METADATA_VERSION) + LLVM::DEBUG_METADATA_VERSION ) end diff --git a/src/compiler/crystal/codegen/fun.cr b/src/compiler/crystal/codegen/fun.cr index 616b21b79d24..c56bde6e5c2a 100644 --- a/src/compiler/crystal/codegen/fun.cr +++ b/src/compiler/crystal/codegen/fun.cr @@ -626,8 +626,7 @@ class Crystal::CodeGenVisitor # LLVM::Context.register(llvm_context, type_name) llvm_typer = LLVMTyper.new(@program, llvm_context) - llvm_mod = llvm_context.new_module(type_name) - llvm_mod.data_layout = self.data_layout + llvm_mod = configure_module(llvm_context.new_module(type_name)) llvm_builder = new_builder(llvm_context) define_symbol_table llvm_mod, llvm_typer diff --git a/src/compiler/crystal/compiler.cr b/src/compiler/crystal/compiler.cr index ffd2b3e4a7d2..878a1ae4896a 100644 --- a/src/compiler/crystal/compiler.cr +++ b/src/compiler/crystal/compiler.cr @@ -507,12 +507,6 @@ module Crystal link_flags += " -L/usr/local/lib" end - if program.has_flag?("openbsd") - # OpenBSD requires Indirect Branch Tracking by default, but we're not - # compatible (yet), so we disable it for now: - link_flags += " -Wl,-znobtcfi" - end - {DEFAULT_LINKER, %(#{DEFAULT_LINKER} "${@}" -o #{Process.quote_posix(output_filename)} #{link_flags} #{program.lib_flags(@cross_compile)}), object_names} end end diff --git a/src/compiler/crystal/semantic/flags.cr b/src/compiler/crystal/semantic/flags.cr index d455f1fdb0c7..d4b0f265a3d1 100644 --- a/src/compiler/crystal/semantic/flags.cr +++ b/src/compiler/crystal/semantic/flags.cr @@ -49,7 +49,18 @@ class Crystal::Program flags.add "freebsd#{target.freebsd_version}" end flags.add "netbsd" if target.netbsd? - flags.add "openbsd" if target.openbsd? + + if target.openbsd? + flags.add "openbsd" + + case target.architecture + when "aarch64" + flags.add "branch-protection=bti" unless flags.any?(&.starts_with?("branch-protection=")) + when "x86_64", "i386" + flags.add "cf-protection=branch" unless flags.any?(&.starts_with?("cf-protection=")) + end + end + flags.add "dragonfly" if target.dragonfly? flags.add "solaris" if target.solaris? flags.add "android" if target.android? diff --git a/src/llvm/lib_llvm/core.cr b/src/llvm/lib_llvm/core.cr index 1796bd00a0ee..7137501fdb31 100644 --- a/src/llvm/lib_llvm/core.cr +++ b/src/llvm/lib_llvm/core.cr @@ -5,7 +5,12 @@ lib LibLLVM # counterparts (e.g. `LLVMModuleFlagBehavior` v.s. `LLVM::Module::ModFlagBehavior`) enum ModuleFlagBehavior - Warning = 1 + Error = 0 + Warning = 1 + Require = 2 + Override = 3 + Append = 4 + AppendUnique = 5 end alias AttributeIndex = UInt diff --git a/src/llvm/module.cr b/src/llvm/module.cr index 32b025bffee7..0e73e983358a 100644 --- a/src/llvm/module.cr +++ b/src/llvm/module.cr @@ -45,6 +45,10 @@ class LLVM::Module GlobalCollection.new(self) end + def add_flag(module_flag : LibLLVM::ModuleFlagBehavior, key : String, val : Int32) + add_flag(module_flag, key, @context.int32.const_int(val)) + end + def add_flag(module_flag : LibLLVM::ModuleFlagBehavior, key : String, val : Value) LibLLVM.add_module_flag( self, From d353bb42b2a3c7dcb5d03d169634313a856d1445 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Thu, 31 Oct 2024 18:16:14 +0800 Subject: [PATCH 2/4] Support deferencing symlinks in `make install` (#15138) On platforms without complete symbolic link support (e.g. native MSYS2 environments), `make install deref_symlinks=1` will dereference the individual directories under `src/lib_c` and copy the contents, instead of copying the directories as symbolic links. --- Makefile | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/Makefile b/Makefile index 51ab60bb40ec..d30db53464f7 100644 --- a/Makefile +++ b/Makefile @@ -24,18 +24,19 @@ all: CRYSTAL ?= crystal## which previous crystal compiler use LLVM_CONFIG ?= ## llvm-config command path to use -release ?= ## Compile in release mode -stats ?= ## Enable statistics output -progress ?= ## Enable progress output -threads ?= ## Maximum number of threads to use -debug ?= ## Add symbolic debug info -verbose ?= ## Run specs in verbose mode -junit_output ?= ## Path to output junit results -static ?= ## Enable static linking -target ?= ## Cross-compilation target -interpreter ?= ## Enable interpreter feature -check ?= ## Enable only check when running format -order ?=random ## Enable order for spec execution (values: "default" | "random" | seed number) +release ?= ## Compile in release mode +stats ?= ## Enable statistics output +progress ?= ## Enable progress output +threads ?= ## Maximum number of threads to use +debug ?= ## Add symbolic debug info +verbose ?= ## Run specs in verbose mode +junit_output ?= ## Path to output junit results +static ?= ## Enable static linking +target ?= ## Cross-compilation target +interpreter ?= ## Enable interpreter feature +check ?= ## Enable only check when running format +order ?=random ## Enable order for spec execution (values: "default" | "random" | seed number) +deref_symlinks ?= ## Deference symbolic links for `make install` O := .build SOURCES := $(shell find src -name '*.cr') @@ -167,7 +168,7 @@ install: $(O)/$(CRYSTAL_BIN) man/crystal.1.gz ## Install the compiler at DESTDIR $(INSTALL) -m 0755 "$(O)/$(CRYSTAL_BIN)" "$(BINDIR)/$(CRYSTAL_BIN)" $(INSTALL) -d -m 0755 $(DATADIR) - cp -av src "$(DATADIR)/src" + cp $(if $(deref_symlinks),-rvL --preserve=all,-av) src "$(DATADIR)/src" rm -rf "$(DATADIR)/$(LLVM_EXT_OBJ)" # Don't install llvm_ext.o $(INSTALL) -d -m 0755 "$(MANDIR)/man1/" From 8635bce731a58b55a7a95a1a21ed892e81590b5d Mon Sep 17 00:00:00 2001 From: Barney <86712892+BigBoyBarney@users.noreply.github.com> Date: Thu, 31 Oct 2024 11:17:43 +0100 Subject: [PATCH 3/4] Improve docs for `String#rindex!` (#15132) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Johannes Müller --- src/string.cr | 70 ++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 58 insertions(+), 12 deletions(-) diff --git a/src/string.cr b/src/string.cr index f0dbd1a1eae3..7507e3b7249e 100644 --- a/src/string.cr +++ b/src/string.cr @@ -3473,8 +3473,8 @@ class String # ``` # "Hello, World".rindex('o') # => 8 # "Hello, World".rindex('Z') # => nil - # "Hello, World".rindex("o", 5) # => 4 - # "Hello, World".rindex("W", 2) # => nil + # "Hello, World".rindex('o', 5) # => 4 + # "Hello, World".rindex('W', 2) # => nil # ``` def rindex(search : Char, offset = size - 1) # If it's ASCII we can delegate to slice @@ -3519,7 +3519,16 @@ class String end end - # :ditto: + # Returns the index of the _last_ appearance of *search* in the string, + # If *offset* is present, it defines the position to _end_ the search + # (characters beyond this point are ignored). + # + # ``` + # "Hello, World".rindex("orld") # => 8 + # "Hello, World".rindex("snorlax") # => nil + # "Hello, World".rindex("o", 5) # => 4 + # "Hello, World".rindex("W", 2) # => nil + # ``` def rindex(search : String, offset = size - search.size) : Int32? offset += size if offset < 0 return if offset < 0 @@ -3572,7 +3581,16 @@ class String end end - # :ditto: + # Returns the index of the _last_ appearance of *search* in the string, + # If *offset* is present, it defines the position to _end_ the search + # (characters beyond this point are ignored). + # + # ``` + # "Hello, World".rindex(/world/i) # => 7 + # "Hello, World".rindex(/world/) # => nil + # "Hello, World".rindex(/o/, 5) # => 4 + # "Hello, World".rindex(/W/, 2) # => nil + # ``` def rindex(search : Regex, offset = size, *, options : Regex::MatchOptions = Regex::MatchOptions::None) : Int32? offset += size if offset < 0 return nil unless 0 <= offset <= size @@ -3586,21 +3604,49 @@ class String match_result.try &.begin end - # :ditto: - # + # Returns the index of the _last_ appearance of *search* in the string, + # If *offset* is present, it defines the position to _end_ the search + # (characters beyond this point are ignored). # Raises `Enumerable::NotFoundError` if *search* does not occur in `self`. - def rindex!(search : Regex, offset = size, *, options : Regex::MatchOptions = Regex::MatchOptions::None) : Int32 - rindex(search, offset, options: options) || raise Enumerable::NotFoundError.new + # + # ``` + # "Hello, World".rindex!('o') # => 8 + # "Hello, World".rindex!('Z') # raises Enumerable::NotFoundError + # "Hello, World".rindex!('o', 5) # => 4 + # "Hello, World".rindex!('W', 2) # raises Enumerable::NotFoundError + # ``` + def rindex!(search : Char, offset = size - 1) : Int32 + rindex(search, offset) || raise Enumerable::NotFoundError.new end - # :ditto: + # Returns the index of the _last_ appearance of *search* in the string, + # If *offset* is present, it defines the position to _end_ the search + # (characters beyond this point are ignored). + # Raises `Enumerable::NotFoundError` if *search* does not occur in `self`. + # + # ``` + # "Hello, World".rindex!("orld") # => 8 + # "Hello, World".rindex!("snorlax") # raises Enumerable::NotFoundError + # "Hello, World".rindex!("o", 5) # => 4 + # "Hello, World".rindex!("W", 2) # raises Enumerable::NotFoundError + # ``` def rindex!(search : String, offset = size - search.size) : Int32 rindex(search, offset) || raise Enumerable::NotFoundError.new end - # :ditto: - def rindex!(search : Char, offset = size - 1) : Int32 - rindex(search, offset) || raise Enumerable::NotFoundError.new + # Returns the index of the _last_ appearance of *search* in the string, + # If *offset* is present, it defines the position to _end_ the search + # (characters beyond this point are ignored). + # Raises `Enumerable::NotFoundError` if *search* does not occur in `self`. + # + # ``` + # "Hello, World".rindex!(/world/i) # => 7 + # "Hello, World".rindex!(/world/) # raises Enumerable::NotFoundError + # "Hello, World".rindex!(/o/, 5) # => 4 + # "Hello, World".rindex!(/W/, 2) # raises Enumerable::NotFoundError + # ``` + def rindex!(search : Regex, offset = size, *, options : Regex::MatchOptions = Regex::MatchOptions::None) : Int32 + rindex(search, offset, options: options) || raise Enumerable::NotFoundError.new end # Searches separator or pattern (`Regex`) in the string, and returns From 4aac6f2ee3494a593d79eeea389c77037e025adc Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Thu, 31 Oct 2024 18:20:50 +0800 Subject: [PATCH 4/4] Protect constant initializers with mutex on Windows (#15134) `Crystal::System::FileDescriptor#@@reader_thread` is initialized before `Crystal::System::Fiber::RESERVED_STACK_SIZE` which creates a race condition. Regression from #14947 --- src/crystal/once.cr | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/crystal/once.cr b/src/crystal/once.cr index 1e6243669809..56eea2be693a 100644 --- a/src/crystal/once.cr +++ b/src/crystal/once.cr @@ -11,9 +11,6 @@ # :nodoc: class Crystal::OnceState @rec = [] of Bool* - {% if flag?(:preview_mt) %} - @mutex = Mutex.new(:reentrant) - {% end %} def once(flag : Bool*, initializer : Void*) unless flag.value @@ -29,7 +26,13 @@ class Crystal::OnceState end end - {% if flag?(:preview_mt) %} + # on Win32, `Crystal::System::FileDescriptor#@@reader_thread` spawns a new + # thread even without the `preview_mt` flag, and the thread can also reference + # Crystal constants, leading to race conditions, so we always enable the mutex + # TODO: can this be improved? + {% if flag?(:preview_mt) || flag?(:win32) %} + @mutex = Mutex.new(:reentrant) + def once(flag : Bool*, initializer : Void*) unless flag.value @mutex.synchronize do