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

Progress towards tier 1 support for FreeBSD x86_64 #1859

Merged
merged 23 commits into from
Dec 29, 2018
Merged

Conversation

mgxm
Copy link
Contributor

@mgxm mgxm commented Dec 27, 2018

Continuing the work already done by @tiehuis and @myfreeweb, I have completed
some missing points to finish the Tier 1 support. One of the
main points was that we get rid of all the syscalls and now we are using the
libc interface, which is the stable ABI.

Almost all tests are passing, missing only one to fix and which I'm working
on
a lot of fixes. The said test is the test-cli, which works on FreeBSD 12 but fails on
FreeBSD 11.

FreeBSD 11 Stack Trace
error: NameTooLong
/usr/home/marcio/build/zig/build/lib/zig/std/os/index.zig:1040:28: 0x234bff in ??? (cli)
            posix.EPERM => return error.AccessDenied,
                           ^
/usr/home/marcio/build/zig/build/lib/zig/std/os/index.zig:1014:9: 0x22bc9c in ??? (cli)
        return deleteFileC(&file_path_c);
        ^
/usr/home/marcio/build/zig/build/lib/zig/std/os/index.zig:513:42: 0x2202f9 in ??? (cli)
    if (file_path.len >= posix.PATH_MAX) return error.NameTooLong;
                                         ^
/usr/home/marcio/build/zig/build/lib/zig/std/os/index.zig:1013:29: 0x22bc1b in ??? (cli)
        const file_path_c = try toPosixPath(file_path);
                            ^
/usr/home/marcio/build/zig/build/lib/zig/std/os/index.zig:1442:16: 0x229355 in ??? (cli)
            => return err,
               ^
/usr/home/marcio/build/zig/build/lib/zig/std/os/index.zig:1484:17: 0x2297ad in ??? (cli)
                try deleteTree(allocator, full_entry_path);
                ^
/usr/home/marcio/build/zig/test/cli.zig:40:9: 0x228411 in ??? (cli)
        try os.deleteTree(a, dir_path);

mgxm and others added 21 commits December 19, 2018 18:41
Since the stable kernel ABI is through libc ziglang#1759
All supported versions of FreeBSD have libc++ in the base system.
Remove all syscalls references

 * dup2()
 * chdir()
 * execve()
 * fork()
 * getcwd()
 * isatty()
 * readlink()
 * mkdir()
 * mmap()
 * munmap()
 * read()
 * rmdir()
 * symlink()
 * pread()
 * write()
 * pwrite()
 * rename()
 * open()
 * close()
 * lseek()
 * exit()
 * unlink()
 * waitpid()
 * nanosleep()
 * setreuid()
 * setregid()
 * raise()
 * fstat()
 * pipe()
 * added pipe2() extern c fn
The system call getrandom(2) just landed on FreeBSD 12, so if we want to
support some earlier version or at least FreeBSD 11, we can't depend on
the system call.
FreeBSD doesn't mount procfs as default on the base system, so we can't
depend on it to get the current path, In this case, we use sysctl(3) to
retrieves the system information and get the same information.

 - CTL_KERN: High kernel limits

 - KERN_PROC: Return selected information about specific running
   processes.

 - KERN_PROC_PATHNAME: The path of the process

 - Process ID: a process ID of -1 implies the current process.
Stack trace is partially working, with only a few symbols missing. Uses
the same functions that we use in Linux to get the necessary debug info.
Fix dirent/stat, add preadv
Use libc interface for:
 - getdents
 - kill
 - openat
 - setgid
 - setuid
Prior to this fix, the compare-outputs test suite was showing a strange
behavior, the tests always stopped between tests 6-8 and had a stack track
similar to each other.

```
Test 8/68 compare-output multiple files with private function (ReleaseSmall)...OK
/usr/home/mgxm/dev/zig/zig-cache/source.zig:7:2: error: invalid token: '&'
}&(getStdOut() catch unreachable).outStream().stream;
 ^
The following command exited with error code 1:

```

With the wrong O_* flags, the source code was being written in append mode which
resulted in an invalid file

```zig
use @import("foo.zig");
use @import("bar.zig");

pub fn main() void {
	foo_function();
	bar_function();
}&(getStdOut() catch unreachable).outStream().stream;
	stdout.print("OK 2\n") catch unreachable;
}

fn privateFunction() void {
	printText();
}
```
@mgxm mgxm changed the title Add Tier 1 FreeBSD Support for x86_64 [WIP] Add Tier 1 FreeBSD Support for x86_64 Dec 27, 2018
@andrewrk
Copy link
Member

Looks like the problem is in trying to cross compile, targeting FreeBSD.

lld: error: cannot open /usr/lib/Scrt1.o: No such file or directory
lld: error: cannot open /usr/lib/crti.o: No such file or directory
lld: error: cannot open /usr/lib/crtbegin.o: No such file or directory
lld: error: unable to find library -lgcc
lld: error: unable to find library -lgcc_s
lld: error: unable to find library -lc
lld: error: unable to find library -lm
lld: error: unable to find library -lgcc
lld: error: unable to find library -lgcc_s
lld: error: cannot open /usr/lib/crtend.o: No such file or directory
lld: error: cannot open /usr/lib/crtn.o: No such file or directory

We'll have to figure out how to target FreeBSD on non-FreeBSD operating systems to continue zig's promise of "use any target to build for any target".

However you don't have to solve this problem in this PR. One step at a time is OK, and this PR already does a lot.

@mgxm
Copy link
Contributor Author

mgxm commented Dec 27, 2018

We'll have to figure out how to target FreeBSD on non-FreeBSD operating systems to continue zig's promise of "use any target to build for any target".

I completely forgot about that, I will work on it too

There's anything I can do?
should I remove Freebsd from the test matrix?

@andrewrk
Copy link
Member

There's anything I can do?
should I remove Freebsd from the test matrix?

Yeah, for now, remove it from the test matrix, and manually add the native tests in the CI script:

  • bin/zig test ../test/behavior.zig
  • bin/zig test ../std/index.zig
  • bin/zig test ../std/special/compiler_rt/index.zig

And then do these with --release-fast, --release-small, and --library c variants.

@valpackett
Copy link
Contributor

works on FreeBSD 12 but fails on FreeBSD 11

Don't expect anything that touches the filesystem to work on both when the filesystem structures are defined for 12 :) I think it's fine to not support versions older than 12 in a young language.

target FreeBSD on non-FreeBSD operating systems

You can't cross-link without having the libraries. Try extracting the system https://download.freebsd.org/ftp/snapshots/amd64/12.0-STABLE/base.txz into some directory and passing --sysroot=/that/directory to lld.

(You can only extract lib directories: bsdtar -xf ... -C /that/directory ./lib ./usr/lib ./usr/libdata ./usr/include)

(btw do you target macOS from non-macOS operating systems?)

@@ -116,7 +116,7 @@ pub fn getRandomBytes(buf: []u8) !void {
else => return unexpectedErrorPosix(errno),
}
},
Os.macosx, Os.ios => return getRandomBytesDevURandom(buf),
Os.macosx, Os.ios, Os.freebsd => return getRandomBytesDevURandom(buf),
Copy link
Contributor

Choose a reason for hiding this comment

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

note: FreeBSD 12 has getrandom, I used it on purpose

Copy link
Member

Choose a reason for hiding this comment

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

Why not use getrandom?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because getrandom just landed on FreeBSD 12 and I was considering we were going to support version 11, but as @myfreeweb said if it's ok to no support, we can bring it back

Copy link
Member

Choose a reason for hiding this comment

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

I see, thanks.

I think it's fine to not support versions older than 12 in a young language.

I agree with this statement.

@andrewrk
Copy link
Member

(btw do you target macOS from non-macOS operating systems?)

Yes, and it works, although LLD is not great at mach-o files. The newest darwin versions do not require the crt1.o startup files or anything like that.

You can't cross-link without having the libraries.

We're going to have to provide these in source form (probably ported to zig, just like we do with compiler-rt) and have the ability to build them for any target.

@valpackett
Copy link
Contributor

provide these in source form (probably ported to zig, just like we do with compiler-rt) and have the ability to build them for any target

A whole libc for any target? That sounds… very ambitious (and how does it align with "libc is the stable ABI"?)

@andrewrk
Copy link
Member

A whole libc for any target is #514. But that's not needed for FreeBSD cross compilation. Only crt1.o & friends, compatible with the FreeBSD libc is necessary. That's a much smaller component. These .o files are statically linked into executables when building against FreeBSD libc. Try it for yourself with the system compiler and look at the linker line. So your ABI question would apply to using C with the system compiler as much as it would apply to Zig here.

- Manually add all the native tests to CI manifest.
@mgxm
Copy link
Contributor Author

mgxm commented Dec 28, 2018

I removed the FreeBSD from the test matrix and added the native's tests as
recommended. For this last one, please tell me if it's the correct way or I got
it wrong

@andrewrk
Copy link
Member

Looks good! I'm ready to merge this if the tests passed for you locally. Looks like the CI failure was a temporary hiccup.

@mgxm
Copy link
Contributor Author

mgxm commented Dec 28, 2018

test ../std/index.zig will fail, clock-related functions need to be implemented yet.

@andrewrk
Copy link
Member

That's ok, let's disable that test for this PR and then part of tier 1 support is working on those tests.

@andrewrk
Copy link
Member

I'm ready to merge this, do you consider it ready @mgxm?

@mgxm
Copy link
Contributor Author

mgxm commented Dec 29, 2018

Yes, but because I made a mess running the tests in the beginning, I think we
should rename the PR to "Initial Tier 1 support" or something like that. I was
using all the tests running with ./bin/zig build --build-file ../build.zig test as a reference and these tests are all passing on FreeBSD 12 as I didn't
know about and didn't go through the tests with bin/zig test ../std/index.zig,
I could not verify all the missing fixes, so the cross-compilation is not yet
done and the std library has some work to be done.

My apologies for the confusion.

@mgxm
Copy link
Contributor Author

mgxm commented Dec 29, 2018

Issue #1759 have a checklist that you could ticked off.

  • switch to libc
  • stack traces

The issue #1217 I could not reproduce on the master branch

@andrewrk
Copy link
Member

No worries! This patch still represents significant progress. Thank you for your contribution.

@andrewrk andrewrk changed the title [WIP] Add Tier 1 FreeBSD Support for x86_64 Progress towards tier 1 support for FreeBSD x86_64 Dec 29, 2018
@andrewrk andrewrk merged commit d8b6fa9 into ziglang:master Dec 29, 2018
@mgxm mgxm deleted the fbsd2 branch January 4, 2019 21:54
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.

3 participants