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

OpenBSD: fix integration and broken specs #15118

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bin/crystal
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ esac
if [ -x "$CRYSTAL_DIR/${CRYSTAL_BIN}" ]; then
__warning_msg "Using compiled compiler at ${CRYSTAL_DIR#"$PWD/"}/${CRYSTAL_BIN}"
exec "$CRYSTAL_DIR/${CRYSTAL_BIN}" "$@"
elif !($PARENT_CRYSTAL_EXISTS); then
elif (! $PARENT_CRYSTAL_EXISTS); then
__error_msg 'You need to have a crystal executable in your path! or set CRYSTAL env variable'
exit 1
else
Expand Down
6 changes: 5 additions & 1 deletion spec/compiler/loader/unix_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,11 @@ describe Crystal::Loader do
exc = expect_raises(Crystal::Loader::LoadError, /no such file|not found|cannot open/i) do
Crystal::Loader.parse(["-l", "foo/bar.o"], search_paths: [] of String)
end
exc.message.should contain File.join(Dir.current, "foo", "bar.o")
{% if flag?(:openbsd) %}
exc.message.should contain "foo/bar.o"
{% else %}
exc.message.should contain File.join(Dir.current, "foo", "bar.o")
{% end %}
end
end

Expand Down
17 changes: 11 additions & 6 deletions spec/std/exception/call_stack_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,19 @@ describe "Backtrace" do
error.to_s.should contain("IndexError")
end

it "prints crash backtrace to stderr", tags: %w[slow] do
sample = datapath("crash_backtrace_sample")
{% if flag?(:openbsd) %}
# FIXME: the segfault handler doesn't work on OpenBSD
pending "prints crash backtrace to stderr"
{% else %}
it "prints crash backtrace to stderr", tags: %w[slow] do
sample = datapath("crash_backtrace_sample")

_, output, error = compile_and_run_file(sample)
_, output, error = compile_and_run_file(sample)

output.to_s.should be_empty
error.to_s.should contain("Invalid memory access")
end
output.to_s.should be_empty
error.to_s.should contain("Invalid memory access")
end
{% end %}

# Do not test this on platforms that cannot remove the current working
# directory of the process:
Expand Down
8 changes: 4 additions & 4 deletions spec/std/io/io_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -425,9 +425,9 @@ describe IO do
str.read_fully?(slice).should be_nil
end

# pipe(2) returns bidirectional file descriptors on FreeBSD and Solaris,
# pipe(2) returns bidirectional file descriptors on some platforms,
# gate this test behind the platform flag.
{% unless flag?(:freebsd) || flag?(:solaris) %}
{% unless flag?(:freebsd) || flag?(:solaris) || flag?(:openbsd) %}
it "raises if trying to read to an IO not opened for reading" do
IO.pipe do |r, w|
expect_raises(IO::Error, "File not open for reading") do
Expand Down Expand Up @@ -574,9 +574,9 @@ describe IO do
io.read_byte.should be_nil
end

# pipe(2) returns bidirectional file descriptors on FreeBSD and Solaris,
# pipe(2) returns bidirectional file descriptors on some platforms,
# gate this test behind the platform flag.
{% unless flag?(:freebsd) || flag?(:solaris) %}
{% unless flag?(:freebsd) || flag?(:solaris) || flag?(:openbsd) %}
it "raises if trying to write to an IO not opened for writing" do
IO.pipe do |r, w|
# unless sync is used the flush on close triggers the exception again
Expand Down
89 changes: 47 additions & 42 deletions spec/std/kernel_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -251,55 +251,60 @@ describe "at_exit" do
end
end

describe "hardware exception" do
it "reports invalid memory access", tags: %w[slow] do
status, _, error = compile_and_run_source <<-'CRYSTAL'
puts Pointer(Int64).null.value
CRYSTAL

status.success?.should be_false
error.should contain("Invalid memory access")
error.should_not contain("Stack overflow")
end

{% if flag?(:netbsd) %}
# FIXME: on netbsd the process crashes with SIGILL after receiving SIGSEGV
pending "detects stack overflow on the main stack"
pending "detects stack overflow on a fiber stack"
{% else %}
it "detects stack overflow on the main stack", tags: %w[slow] do
# This spec can take some time under FreeBSD where
# the default stack size is 0.5G. Setting a
# smaller stack size with `ulimit -s 8192`
# will address this.
{% if flag?(:openbsd) %}
# FIXME: the segfault handler doesn't work on OpenBSD
pending "hardware exception"
{% else %}
describe "hardware exception" do
it "reports invalid memory access", tags: %w[slow] do
status, _, error = compile_and_run_source <<-'CRYSTAL'
def foo
y = StaticArray(Int8, 512).new(0)
foo
end
foo
puts Pointer(Int64).null.value
CRYSTAL

status.success?.should be_false
error.should contain("Stack overflow")
error.should contain("Invalid memory access")
error.should_not contain("Stack overflow")
end

it "detects stack overflow on a fiber stack", tags: %w[slow] do
status, _, error = compile_and_run_source <<-'CRYSTAL'
def foo
y = StaticArray(Int8, 512).new(0)
{% if flag?(:netbsd) %}
# FIXME: on netbsd the process crashes with SIGILL after receiving SIGSEGV
pending "detects stack overflow on the main stack"
pending "detects stack overflow on a fiber stack"
{% else %}
it "detects stack overflow on the main stack", tags: %w[slow] do
# This spec can take some time under FreeBSD where
# the default stack size is 0.5G. Setting a
# smaller stack size with `ulimit -s 8192`
# will address this.
status, _, error = compile_and_run_source <<-'CRYSTAL'
def foo
y = StaticArray(Int8, 512).new(0)
foo
end
foo
end
CRYSTAL

spawn do
foo
end
status.success?.should be_false
error.should contain("Stack overflow")
end

sleep 60.seconds
CRYSTAL
it "detects stack overflow on a fiber stack", tags: %w[slow] do
status, _, error = compile_and_run_source <<-'CRYSTAL'
def foo
y = StaticArray(Int8, 512).new(0)
foo
end

status.success?.should be_false
error.should contain("Stack overflow")
end
{% end %}
end
spawn do
foo
end

sleep 60.seconds
CRYSTAL

status.success?.should be_false
error.should contain("Stack overflow")
end
{% end %}
end
{% end %}
4 changes: 2 additions & 2 deletions spec/std/socket/addrinfo_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ describe Socket::Addrinfo, tags: "network" do
end

it "raises helpful message on getaddrinfo failure" do
expect_raises(Socket::Addrinfo::Error, "Hostname lookup for badhostname failed: ") do
Socket::Addrinfo.resolve("badhostname", 80, type: Socket::Type::DGRAM)
expect_raises(Socket::Addrinfo::Error, "Hostname lookup for badhostname.unknown failed: ") do
Socket::Addrinfo.resolve("badhostname.unknown", 80, type: Socket::Type::DGRAM)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I didn't mean to push this fix in this PR but to make it it's own PR.

The problem here is that within my vagrant/libvirt VM resolving a hostname without an extension leads to 30 seconds (freeds, netbsd) to 2.5 minutes (openbsd) time to fail to resolve the name 😲

Using a domain with an extension, however, fails to resolve immediately 🤷

end
end

Expand Down
6 changes: 3 additions & 3 deletions spec/std/socket/tcp_server_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe TCPServer, tags: "network" do
end
error.os_error.should eq({% if flag?(:win32) %}
WinError::WSATYPE_NOT_FOUND
{% elsif flag?(:linux) && !flag?(:android) %}
{% elsif (flag?(:linux) && !flag?(:android)) || flag?(:openbsd) %}
Errno.new(LibC::EAI_SERVICE)
{% else %}
Errno.new(LibC::EAI_NONAME)
Expand Down Expand Up @@ -96,7 +96,7 @@ describe TCPServer, tags: "network" do
# FIXME: Resolve special handling for win32. The error code handling should be identical.
{% if flag?(:win32) %}
[WinError::WSAHOST_NOT_FOUND, WinError::WSATRY_AGAIN].should contain err.os_error
{% elsif flag?(:android) || flag?(:netbsd) %}
{% elsif flag?(:android) || flag?(:netbsd) || flag?(:openbsd) %}
err.os_error.should eq(Errno.new(LibC::EAI_NODATA))
{% else %}
[Errno.new(LibC::EAI_NONAME), Errno.new(LibC::EAI_AGAIN)].should contain err.os_error
Expand All @@ -110,7 +110,7 @@ describe TCPServer, tags: "network" do
# FIXME: Resolve special handling for win32. The error code handling should be identical.
{% if flag?(:win32) %}
[WinError::WSAHOST_NOT_FOUND, WinError::WSATRY_AGAIN].should contain err.os_error
{% elsif flag?(:android) || flag?(:netbsd) %}
{% elsif flag?(:android) || flag?(:netbsd) || flag?(:openbsd) %}
err.os_error.should eq(Errno.new(LibC::EAI_NODATA))
{% else %}
[Errno.new(LibC::EAI_NONAME), Errno.new(LibC::EAI_AGAIN)].should contain err.os_error
Expand Down
6 changes: 3 additions & 3 deletions spec/std/socket/tcp_socket_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe TCPSocket, tags: "network" do
end
error.os_error.should eq({% if flag?(:win32) %}
WinError::WSATYPE_NOT_FOUND
{% elsif flag?(:linux) && !flag?(:android) %}
{% elsif (flag?(:linux) && !flag?(:android)) || flag?(:openbsd) %}
Errno.new(LibC::EAI_SERVICE)
{% else %}
Errno.new(LibC::EAI_NONAME)
Expand Down Expand Up @@ -79,7 +79,7 @@ describe TCPSocket, tags: "network" do
# FIXME: Resolve special handling for win32. The error code handling should be identical.
{% if flag?(:win32) %}
[WinError::WSAHOST_NOT_FOUND, WinError::WSATRY_AGAIN].should contain err.os_error
{% elsif flag?(:android) || flag?(:netbsd) %}
{% elsif flag?(:android) || flag?(:netbsd) || flag?(:openbsd) %}
err.os_error.should eq(Errno.new(LibC::EAI_NODATA))
{% else %}
[Errno.new(LibC::EAI_NONAME), Errno.new(LibC::EAI_AGAIN)].should contain err.os_error
Expand All @@ -93,7 +93,7 @@ describe TCPSocket, tags: "network" do
# FIXME: Resolve special handling for win32. The error code handling should be identical.
{% if flag?(:win32) %}
[WinError::WSAHOST_NOT_FOUND, WinError::WSATRY_AGAIN].should contain err.os_error
{% elsif flag?(:android) || flag?(:netbsd) %}
{% elsif flag?(:android) || flag?(:netbsd) || flag?(:openbsd) %}
err.os_error.should eq(Errno.new(LibC::EAI_NODATA))
{% else %}
[Errno.new(LibC::EAI_NONAME), Errno.new(LibC::EAI_AGAIN)].should contain err.os_error
Expand Down
3 changes: 3 additions & 0 deletions spec/std/socket/udp_socket_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ describe UDPSocket, tags: "network" do
elsif {{ flag?(:netbsd) }} && family == Socket::Family::INET6
# FIXME: fails with "setsockopt: EADDRNOTAVAIL"
pending "joins and transmits to multicast groups"
elsif {{ flag?(:openbsd) }}
# FIXME: fails with "setsockopt: EINVAL (ipv4) or EADDRNOTAVAIL (ipv6)"
pending "joins and transmits to multicast groups"
else
it "joins and transmits to multicast groups" do
udp = UDPSocket.new(family)
Expand Down
13 changes: 13 additions & 0 deletions src/compiler/crystal/compiler.cr
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,19 @@ module Crystal
else
link_flags = @link_flags || ""
link_flags += " -rdynamic"

if program.has_flag?("freebsd") || program.has_flag?("openbsd")
# pkgs are installed to usr/local/lib but it's not in LIBRARY_PATH by
# default; we declare it to ease linking on these platforms:
link_flags += " -L/usr/local/lib"
end
Comment on lines +504 to +508
Copy link
Member

Choose a reason for hiding this comment

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

thought: I was wondering about the motivation for this. Seems like the default configuration is just sub-optimal. Apparently compilers in ports do add the folder, only the system compiler does not (https://wiki.freebsd.org/WarnerLosh/UsrLocal).
Maybe we should add some reference for justification here? Not sure if that link would do, it was just the first relevant result on Google.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you build from ports, then a port can patch it in, yeah, but when you compile or cross compile your own crystal binary then it breaks and you must think to export LIBRARY_PATH.

It ain't much, but that's nice. We did it before, but that got lost and I couldn't find why.


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
Expand Down
1 change: 1 addition & 0 deletions src/lib_c/x86_64-openbsd/c/netdb.cr
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ lib LibC
EAI_FAIL = -4
EAI_FAMILY = -6
EAI_MEMORY = -10
EAI_NODATA = -5
EAI_NONAME = -2
EAI_SERVICE = -8
EAI_SOCKTYPE = -7
Expand Down
Loading