Zig: CI failure: macos translate-c regression

Created on 27 Oct 2020  路  43Comments  路  Source: ziglang/zig

c1ae9f40c728e769e804eabfcab0b40d6c0f67f2 fixes a different CI issue, but the macOS CI is still failing. The test case in question is "struct initializer - simple". The C code is 

struct {double x,y,z;} s0 = {1.2, 1.3};

The regression is (expected in green, regressed output in red):

 pub export var s0: struct_unnamed_2 = struct_unnamed_2{
+    .x = 1.2,
+    .y = 1.3,
-    .x = 0.;999999999999999,
-    .y = 0.<99999999999999:,
     .z = 0,
 };

I'm not sure why this is happening. On my linux machine, the test case runs valgrind-clean: valgrind ./zig translate-c test.c -target x86_64-macos-gnu (expected output)

bug os-macos translate-c
Source
picture andrewrk

Most helpful comment

OK, confirmed that the issue is solved by rebuilding, passing [the stage1 that fails the translate-c tests] for -DZIG_EXECUTABLE in order to rebuild zig1.o using stage1 rather than stage0. So this confirms @LemonBoy's hypothesis that stage0 is passing incorrect LLVM CPU features for "baseline", and it also leads us towards a fix, if we can just get cmake to cooperate.

picture andrewrk on 28 Oct 2020
🎉2

All 43 comments

output on macos Catalina:

const struct_unnamed_1 = extern struct {
    x: f64,
    y: f64,
    z: f64,
};
pub export var s0: struct_unnamed_1 = struct_unnamed_1{
    .x = 1.2,
    .y = 1.3,
    .z = 0,
};
prime31 picture prime31 on 27 Oct 2020

I can't reproduce it either :/

jedisct1 picture jedisct1 on 27 Oct 2020

The leading .x = and trailing , parts look fine so the problem is either with the f64 value or the formatting code.
Since the std.fmt tests are passing I'd say the latter is unlikely to be the problem, maybe ZigClangAPFloat_getValueAsApproximateDouble (the correct name should be ZigClangFloatingLiteral_getValueAsApproximateDouble) is returning some bogus value (or Zig is messing up at the ABI boundary).

I can't reproduce it either :/

Have you tried with a clean build (nuke the whole build folder) using -native as OS?

LemonBoy picture LemonBoy on 27 Oct 2020

Have you tried with a clean build (nuke the whole build folder) using -native as OS?

Yes. The output is fine (on BigSur).

jedisct1 picture jedisct1 on 27 Oct 2020

Yes. The output is fine (on BigSur).

Hmm, can you try with the following cmake options:

  -DZIG_TARGET_TRIPLE="$ARCH-native-gnu" \
  -DZIG_TARGET_MCPU="baseline" \
  -DZIG_STATIC=ON

The MCPU parameter may play a role here, not really sure.

LemonBoy picture LemonBoy on 27 Oct 2020

Different issue, but -DZIG_STATIC=ON fails with clang: error: unsupported option '-static-libgcc'.

jedisct1 picture jedisct1 on 27 Oct 2020

Different issue, but -DZIG_STATIC=ON fails with clang: error: unsupported option '-static-libgcc'.

You really can't win with build systems...
Try following the CI script then, using the same pre-built llvm/clang tarball may increase the chances of reproducing the problem.

LemonBoy picture LemonBoy on 27 Oct 2020

ziglang.org doesn't allow downloads of the llvm/clang tarball from arbitrary IP addresses :) scratch that, wrong URL :)

jedisct1 picture jedisct1 on 27 Oct 2020

Unfortunately, the pre-built zig+llvm+clang doesn't work on Big Sur and I don't have previous versions of macOS to test this on :(

jedisct1 picture jedisct1 on 27 Oct 2020

I am on macos High Sierrra right now, and it works fine. I could upgrade my computer to Catalenia, and try again. Would that help? Do you know what version the CI uses?

g-w1 picture g-w1 on 27 Oct 2020

Looks like this is Catalina.

jedisct1 picture jedisct1 on 27 Oct 2020

Unfortunately, the pre-built zig+llvm+clang doesn't work on Big Sur

This is actually unexpected - does it give you any clues why?

I could upgrade my computer to Catalenia, and try again. Would that help?

I have Catalina and just tested it, and it didn't repro (with homebrew). I'm about to try it with the pre-built tarball.

andrewrk picture andrewrk on 27 Oct 2020

I was able to repro with the pre-built tarball, using equivalent cmake options as the CI script uses. The issue repros with -DCMAKE_BUILD_TYPE=Release. With -DCMAKE_BUILD_TYPE=Debug I get a link error:

Undefined symbol: zigcpp/libzigcpp.a(zig_clang_cc1_main.cpp.o): llvm::cfg::Update<llvm::BasicBlock*>::dump() const

This is strange because in the tarball we are only using zig as a C++ compiler to build stage1 - not a zig compiler - ultimately the zig that runs the translate-c tests is built from source.

On my linux machine, this still runs valgrind clean and gets the correct output with a release build of zig: valgrind ./zig translate-c test.c -target x86_64-macos-gnu

andrewrk picture andrewrk on 27 Oct 2020

So I just upgraded my mac to Catalenia, and tested it:

[[email protected] ~D/zig]$ ls
zig-macos-x86_64-0.6.0+0088efc4b.tar.xz
[[email protected] ~D/zig]$ tar xf zig-macos-x86_64-0.6.0+0088efc4b.tar.xz
[[email protected] ~D/zig]$ ls
zig-macos-x86_64-0.6.0+0088efc4b  zig-macos-x86_64-0.6.0+0088efc4b.tar.xz
[[email protected] ~D/zig]$ vim test.c
[[email protected] ~D/zig]$ ./zig-macos-x86_64-0.6.0+0088efc4b/zig translate-c test.c
error: unable to create compilation: CCompilerExitCode
[[email protected] ~D/zig]$
[[email protected] ~D/zig]$ cat test.c
struct {double x,y,z;} s0 = {1.2, 1.3};
[[email protected] ~D/zig]$ vim test.zig
[[email protected] ~D/zig]$ ./zig-macos-x86_64-0.6.0+0088efc4b/zig build-exe test.zig
error: unable to create compilation: CCompilerExitCode
[[email protected] ~D/zig]$ cat test.c test.zig
struct {double x,y,z;} s0 = {1.2, 1.3};
pub fn main() void {}
[[email protected] ~D/zig]$

I just downloaded the macos tarball and ran it. It ran perfectly before upgrading, but now doesn't work. I think this is a different bug though.

g-w1 picture g-w1 on 27 Oct 2020

The tarball is not being used to run translate-c, it's actually only being used as the C/C++ compiler to build stage1/stage2. The resulting binary is then tested, and that is what is failing.

andrewrk picture andrewrk on 27 Oct 2020

Should I open up a new issue for the CCompilerExitCode error then? It seems like my libc installation is broken (I dont have xcode), but I thought zig didn't depend on libc?

g-w1 picture g-w1 on 27 Oct 2020

Nope that issue is already fixed now that we have macos headers. That zig+llvm tarball is an old version of zig, but that's ok because it's only being used as a C/C++ compiler.

andrewrk picture andrewrk on 27 Oct 2020

Got it! Thanks.

g-w1 picture g-w1 on 27 Oct 2020

FWIW I tried really hard at replicating on my Catalina and failed... It beats me what causes this, but taking all comments from here, it seems like something is off with the Catalina install on the CI itself.

kubkon picture kubkon on 27 Oct 2020

One more data point: zig-bootstrap repository with x86_64-linux-musl does not repro the translate-c failure.

andrewrk picture andrewrk on 27 Oct 2020

I'm gonna make everyone sad right now:

build/bin/zig build test-translate-c

passes on BigSur and aarch64.

kubkon picture kubkon on 27 Oct 2020

Let me know if you get aarch64-macos-gnu built using zig-bootstrap. That is the equivalent test case here, and it's what we would use for CI runs on that hardware if we ever got it set up.

andrewrk picture andrewrk on 28 Oct 2020

the correct name should be ZigClangFloatingLiteral_getValueAsApproximateDouble

e1ca6946bee3acf9cbdf6e5ea30fa2d55304365d

maybe ZigClangAPFloat_getValueAsApproximateDouble

OK so this is weird, I made this patch:

--- a/src/translate_c.zig
+++ b/src/translate_c.zig
@@ -3641,6 +3641,7 @@ fn transBreak(rp: RestorePoint, scope: *Scope) TransError!*ast.Node {
 fn transFloatingLiteral(rp: RestorePoint, scope: *Scope, stmt: *const clang.FloatingLiteral, used: ResultUsed) TransError!*ast.Node {
     // TODO use something more accurate
     const dbl = stmt.getValueAsApproximateDouble();
+    std.debug.print("float literal = {d}\n", .{dbl});
     const node = try rp.c.arena.create(ast.Node.OneToken);
     node.* = .{
         .base = .{ .tag = .FloatLiteral },

And then run

./zig translate-c test.c

I get correct translate-c output, but the expected std.debug.print lines did not print.

andrewrk picture andrewrk on 28 Oct 2020

Oops, I forgot the zig-cache for zig1.o no longer takes into account dirty working changes. OK so with this patch we get:

float literal = 0.;999999999999999
float literal = 0.<99999999999999:
andrewrk picture andrewrk on 28 Oct 2020

This updated patch 

--- a/src/translate_c.zig
+++ b/src/translate_c.zig
@@ -3641,6 +3641,7 @@ fn transBreak(rp: RestorePoint, scope: *Scope) TransError!*ast.Node {
 fn transFloatingLiteral(rp: RestorePoint, scope: *Scope, stmt: *const clang.FloatingLiteral, used: ResultUsed) TransError!*ast.Node {
     // TODO use something more accurate
     const dbl = stmt.getValueAsApproximateDouble();
+    std.debug.print("float literal = {d} {} 0x{x}\n", .{ dbl, dbl, @bitCast(u64, dbl) });
     const node = try rp.c.arena.create(ast.Node.OneToken);
     node.* = .{
         .base = .{ .tag = .FloatLiteral },

on my linux machine gives:

float literal = 1.2 1.2e+00 0x3ff3333333333333
float literal = 1.3 1.3e+00 0x3ff4cccccccccccd

on macos gives:

float literal = 0.;999999999999999 0.;999999999999999e+00 0x3ff3333333333333
float literal = 0.<99999999999999: 0.<99999999999999:e+00 0x3ff4cccccccccccd

So this appears to be some kind of miscompilation in zig's std lib float printing code. Since it only happens for some builds of the compiler and not others, I'd wager it's UB in either stage1 or LLVM that is getting activated with zig-bootstrap on macos, but not anywhere else.

andrewrk picture andrewrk on 28 Oct 2020

OK when I said valgrind clean above, I hadn't realized the cache was in play. Wiping the cache this time, it is still valgrind clean for me on linux when built from source in both debug and release mode, however the zig-bootstrap built zig produces many valgrind warnings:

==12808== Command: /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig translate-c test.c -target x86_64-macos-gnu
==12808== 
==12808== Conditional jump or move depends on uninitialised value(s)
==12808==    at 0x67682E0: calloc (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0xC0: ???
==12808==    by 0x6E6125F: ???
==12808==    by 0x6E66FFF: ???
==12808==    by 0x6676BD5: llvm::StringMapImpl::RehashTable(unsigned int) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66205F8: std::__1::pair<llvm::StringMapIterator<llvm::cl::Option*>, bool> llvm::StringMap<llvm::cl::Option*, llvm::MallocAllocator>::try_emplace<llvm::cl::Option*>(llvm::StringRef, llvm::cl::Option*&&) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66206B7: (anonymous namespace)::CommandLineParser::addOption(llvm::cl::Option*, llvm::cl::SubCommand*) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66143DA: llvm::cl::Option::addArgument() (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x4B99551: _GLOBAL__sub_I_PPCTargetTransformInfo.cpp (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C3A: libc_start_init (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C80: (below main) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808== 
==12808== Conditional jump or move depends on uninitialised value(s)
==12808==    at 0x67682F2: calloc (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0xC0: ???
==12808==    by 0x6E6125F: ???
==12808==    by 0x6E66FFF: ???
==12808==    by 0x6676BD5: llvm::StringMapImpl::RehashTable(unsigned int) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66205F8: std::__1::pair<llvm::StringMapIterator<llvm::cl::Option*>, bool> llvm::StringMap<llvm::cl::Option*, llvm::MallocAllocator>::try_emplace<llvm::cl::Option*>(llvm::StringRef, llvm::cl::Option*&&) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66206B7: (anonymous namespace)::CommandLineParser::addOption(llvm::cl::Option*, llvm::cl::SubCommand*) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66143DA: llvm::cl::Option::addArgument() (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x4B99551: _GLOBAL__sub_I_PPCTargetTransformInfo.cpp (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C3A: libc_start_init (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C80: (below main) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808== 
==12808== Conditional jump or move depends on uninitialised value(s)
==12808==    at 0x6768304: calloc (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0xC0: ???
==12808==    by 0x6E6125F: ???
==12808==    by 0x6E66FFF: ???
==12808==    by 0x6676BD5: llvm::StringMapImpl::RehashTable(unsigned int) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66205F8: std::__1::pair<llvm::StringMapIterator<llvm::cl::Option*>, bool> llvm::StringMap<llvm::cl::Option*, llvm::MallocAllocator>::try_emplace<llvm::cl::Option*>(llvm::StringRef, llvm::cl::Option*&&) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66206B7: (anonymous namespace)::CommandLineParser::addOption(llvm::cl::Option*, llvm::cl::SubCommand*) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66143DA: llvm::cl::Option::addArgument() (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x4B99551: _GLOBAL__sub_I_PPCTargetTransformInfo.cpp (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C3A: libc_start_init (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C80: (below main) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808== 
==12808== Conditional jump or move depends on uninitialised value(s)
==12808==    at 0x6768316: calloc (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0xC0: ???
==12808==    by 0x6E6125F: ???
==12808==    by 0x6E66FFF: ???
==12808==    by 0x6676BD5: llvm::StringMapImpl::RehashTable(unsigned int) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66205F8: std::__1::pair<llvm::StringMapIterator<llvm::cl::Option*>, bool> llvm::StringMap<llvm::cl::Option*, llvm::MallocAllocator>::try_emplace<llvm::cl::Option*>(llvm::StringRef, llvm::cl::Option*&&) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66206B7: (anonymous namespace)::CommandLineParser::addOption(llvm::cl::Option*, llvm::cl::SubCommand*) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66143DA: llvm::cl::Option::addArgument() (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x4B99551: _GLOBAL__sub_I_PPCTargetTransformInfo.cpp (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C3A: libc_start_init (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C80: (below main) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808== 
==12808== Conditional jump or move depends on uninitialised value(s)
==12808==    at 0x6676C5E: llvm::StringMapImpl::RehashTable(unsigned int) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66205F8: std::__1::pair<llvm::StringMapIterator<llvm::cl::Option*>, bool> llvm::StringMap<llvm::cl::Option*, llvm::MallocAllocator>::try_emplace<llvm::cl::Option*>(llvm::StringRef, llvm::cl::Option*&&) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66206B7: (anonymous namespace)::CommandLineParser::addOption(llvm::cl::Option*, llvm::cl::SubCommand*) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66143DA: llvm::cl::Option::addArgument() (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x4B99551: _GLOBAL__sub_I_PPCTargetTransformInfo.cpp (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C3A: libc_start_init (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C80: (below main) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808== 
==12808== Conditional jump or move depends on uninitialised value(s)
==12808==    at 0x6676C71: llvm::StringMapImpl::RehashTable(unsigned int) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66205F8: std::__1::pair<llvm::StringMapIterator<llvm::cl::Option*>, bool> llvm::StringMap<llvm::cl::Option*, llvm::MallocAllocator>::try_emplace<llvm::cl::Option*>(llvm::StringRef, llvm::cl::Option*&&) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66206B7: (anonymous namespace)::CommandLineParser::addOption(llvm::cl::Option*, llvm::cl::SubCommand*) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66143DA: llvm::cl::Option::addArgument() (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x4B99551: _GLOBAL__sub_I_PPCTargetTransformInfo.cpp (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C3A: libc_start_init (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C80: (below main) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808== 
==12808== Conditional jump or move depends on uninitialised value(s)
==12808==    at 0x66765EF: llvm::StringMapImpl::LookupBucketFor(llvm::StringRef) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x662056B: std::__1::pair<llvm::StringMapIterator<llvm::cl::Option*>, bool> llvm::StringMap<llvm::cl::Option*, llvm::MallocAllocator>::try_emplace<llvm::cl::Option*>(llvm::StringRef, llvm::cl::Option*&&) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66206B7: (anonymous namespace)::CommandLineParser::addOption(llvm::cl::Option*, llvm::cl::SubCommand*) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66143DA: llvm::cl::Option::addArgument() (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x4B996A9: _GLOBAL__sub_I_PPCTargetTransformInfo.cpp (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C3A: libc_start_init (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C80: (below main) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808== 
==12808== Conditional jump or move depends on uninitialised value(s)
==12808==    at 0x662057F: std::__1::pair<llvm::StringMapIterator<llvm::cl::Option*>, bool> llvm::StringMap<llvm::cl::Option*, llvm::MallocAllocator>::try_emplace<llvm::cl::Option*>(llvm::StringRef, llvm::cl::Option*&&) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66206B7: (anonymous namespace)::CommandLineParser::addOption(llvm::cl::Option*, llvm::cl::SubCommand*) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66143DA: llvm::cl::Option::addArgument() (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x4B996A9: _GLOBAL__sub_I_PPCTargetTransformInfo.cpp (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C3A: libc_start_init (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C80: (below main) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808== 
==12808== Conditional jump or move depends on uninitialised value(s)
==12808==    at 0x6620584: std::__1::pair<llvm::StringMapIterator<llvm::cl::Option*>, bool> llvm::StringMap<llvm::cl::Option*, llvm::MallocAllocator>::try_emplace<llvm::cl::Option*>(llvm::StringRef, llvm::cl::Option*&&) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66206B7: (anonymous namespace)::CommandLineParser::addOption(llvm::cl::Option*, llvm::cl::SubCommand*) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66143DA: llvm::cl::Option::addArgument() (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x4B996A9: _GLOBAL__sub_I_PPCTargetTransformInfo.cpp (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C3A: libc_start_init (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C80: (below main) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808== 
==12808== Conditional jump or move depends on uninitialised value(s)
==12808==    at 0x662057F: std::__1::pair<llvm::StringMapIterator<llvm::cl::Option*>, bool> llvm::StringMap<llvm::cl::Option*, llvm::MallocAllocator>::try_emplace<llvm::cl::Option*>(llvm::StringRef, llvm::cl::Option*&&) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66206B7: (anonymous namespace)::CommandLineParser::addOption(llvm::cl::Option*, llvm::cl::SubCommand*) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66143DA: llvm::cl::Option::addArgument() (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x4BA1FDC: _GLOBAL__sub_I_PPCExpandISEL.cpp (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C3A: libc_start_init (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C80: (below main) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808== 
==12808== Conditional jump or move depends on uninitialised value(s)
==12808==    at 0x6620584: std::__1::pair<llvm::StringMapIterator<llvm::cl::Option*>, bool> llvm::StringMap<llvm::cl::Option*, llvm::MallocAllocator>::try_emplace<llvm::cl::Option*>(llvm::StringRef, llvm::cl::Option*&&) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66206B7: (anonymous namespace)::CommandLineParser::addOption(llvm::cl::Option*, llvm::cl::SubCommand*) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66143DA: llvm::cl::Option::addArgument() (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x4BA1FDC: _GLOBAL__sub_I_PPCExpandISEL.cpp (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C3A: libc_start_init (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C80: (below main) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808== 
==12808== Conditional jump or move depends on uninitialised value(s)
==12808==    at 0x662057F: std::__1::pair<llvm::StringMapIterator<llvm::cl::Option*>, bool> llvm::StringMap<llvm::cl::Option*, llvm::MallocAllocator>::try_emplace<llvm::cl::Option*>(llvm::StringRef, llvm::cl::Option*&&) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66206B7: (anonymous namespace)::CommandLineParser::addOption(llvm::cl::Option*, llvm::cl::SubCommand*) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66143DA: llvm::cl::Option::addArgument() (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x4BA355C: _GLOBAL__sub_I_PPCPreEmitPeephole.cpp (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C3A: libc_start_init (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C80: (below main) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808== 
==12808== Conditional jump or move depends on uninitialised value(s)
==12808==    at 0x6620584: std::__1::pair<llvm::StringMapIterator<llvm::cl::Option*>, bool> llvm::StringMap<llvm::cl::Option*, llvm::MallocAllocator>::try_emplace<llvm::cl::Option*>(llvm::StringRef, llvm::cl::Option*&&) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66206B7: (anonymous namespace)::CommandLineParser::addOption(llvm::cl::Option*, llvm::cl::SubCommand*) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x66143DA: llvm::cl::Option::addArgument() (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x4BA355C: _GLOBAL__sub_I_PPCPreEmitPeephole.cpp (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C3A: libc_start_init (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)
==12808==    by 0x6765C80: (below main) (in /home/andy/dev/bootstrap-zig/out/zig-x86_64-linux-musl-baseline/bin/zig)

and many more.

Based on the fact that they all come from libc_start_init I'm starting to wonder if this is the C++ Static Initialization Order Fiasco.

andrewrk picture andrewrk on 28 Oct 2020

So this appears to be some kind of miscompilation in zig's std lib float printing code. Since it only happens for some builds of the compiler and not others, I'd wager it's UB in either stage1 or LLVM that is getting activated with zig-bootstrap on macos, but not anywhere else.

Holy cow, we keep getting all sorts of weird-ass bugs.
One more hypothesis: the CI now builds the stage1 with mcpu=baseline and, while Zig defaults to a pretty simple x86_64 machine with a small set of features, clang defaults to core2duo or something like that (no Intel mac was ever sold with an older cpu).

LemonBoy picture LemonBoy on 28 Oct 2020

That sounds like something worthwhile to explore regardless of whether it fixes this bug - adjusting zig's "baseline" CPU features when the selected OS is macos to include features that are present in all hardware sold by apple.

andrewrk picture andrewrk on 28 Oct 2020

New data:

const std = @import("std");

pub fn main() !void {
    var x: f64 = 1.2;
    var y: f64 = 1.3;
    std.debug.print("x={d} {} 0x{x}\n", .{ x, x, @bitCast(u64, x) });
    std.debug.print("y={d} {} 0x{x}\n", .{ y, y, @bitCast(u64, y) });
}

output (Debug, ReleaseFast)

x=1.2 1.2e+00 0x3ff3333333333333
y=1.3 1.3e+00 0x3ff4cccccccccccd
andrewrk picture andrewrk on 28 Oct 2020

Commenting out that one translate-c test gives more failures:

========= Expected to find: ===================
pub export var a: f32 = @floatCast(f32, 3.1415);
pub export var b: f64 = 3.1415;
pub export var c: c_int = @floatToInt(c_int, 3.1415);
pub export var d: f64 = @intToFloat(f64, @as(c_int, 3));
========= But file does not contain it: =======
pub export var a: f32 = @floatCast(f32, 2.:=;);
pub export var b: f64 = 2.:=;;
pub export var c: c_int = @floatToInt(c_int, 2.:=;);
pub export var d: f64 = @intToFloat(f64, @as(c_int, 3));
andrewrk picture andrewrk on 28 Oct 2020

Can you reproduce the corrupted floating-point printing with a small stand-alone example?
My new theory is that the mcpu=generic is making LLVM emit some new calls to compile_rt helpers that may explain the wrong results.
Is the number still ok in formatFloatDecimal, when errol.errol3 is called?

LemonBoy picture LemonBoy on 28 Oct 2020

In https://github.com/ziglang/zig/issues/6830#issuecomment-718057619 I also tried it with -target x86_64-native -mcpu=baseline, same output. I haven't been able to repro the corrupted floating-point printing with a small example yet, and I'm out of ideas to try.

andrewrk picture andrewrk on 28 Oct 2020

You know what else is weird? Why did the tests pass for 0088efc4b22645698faf328369a1deca2dc9070f and others?

The difference is: in the passing commits, the zig binary used to build zig1.o was the binary from the tarball. This issue started happening after a0f4606f32bab6df88d0afc1bf9ff479470b68cc, which now rebuilds the zig executable from source using the tarball only for zig cc and then the fresh-from-source zig executable is used to build zig1.o.

andrewrk picture andrewrk on 28 Oct 2020

I can reproduce a suspiciously similar problem with the following code:

const std = @import("std");

pub fn main() !void {
    var x: f64 = 1.2;
    var y: f64 = 3.1415;
    std.debug.print("x={d} {} 0x{x}\n", .{ x, x, @bitCast(u64, x) });
    std.debug.print("y={d} {} 0x{x}\n", .{ y, y, @bitCast(u64, y) });
}

When compiled with zig run -mcpu=generic -target i386-linux it prints

x=0.< 0.<e+00 0x3ff3333333333333
y=2.:>2 2.:>2e+00 0x400921cac083126f

because it's fucking things due to the use of x87, in fact adding -mcpu=generic+sse2 makes the correct results appear.

What I suspect is that some part of the bootstrapped binary is somehow passing the wrong set of flags.

LemonBoy picture LemonBoy on 28 Oct 2020

Thanks, that's a great clue. I will double check that we pass the correct CPU feature sets at all stages of compilation.

It's confusing because for x86_64, baseline is this CPU model, which does have sse2:

https://github.com/ziglang/zig/blob/1a171a143aaa1be843b6f028cbb8e82aaec79e2c/lib/std/target/x86.zig#L2909-L2927

This seems like 2 bugs then, because that should still work even with -mcpu=generic -target i386-linux.

andrewrk picture andrewrk on 28 Oct 2020

Thanks, that's a great clue. I will double check that we pass the correct CPU feature sets at all stages of compilation.

You could quickly check if that's the case by running the faulty binary trough objdump.

This seems like 2 bugs then, because that should still work even with -mcpu=generic -target i386-linux.

Nope, without SSE most of the standard library is broken due to x87 being weird (missing/extra rounding, NaNs becoming quiet and other weirdness). That's why I changed the i386 default to pentium4 when I upstreamed the i386-linux port.

LemonBoy picture LemonBoy on 28 Oct 2020
👍1

OK, so I got the issue replicated locally on my x86_64 Catalina. Fire away if you want something checked quickly and whatnot.

kubkon picture kubkon on 28 Oct 2020

Hmm OK I think @LemonBoy's clues are starting to lead somewhere:

https://github.com/ziglang/zig/blob/1a171a143aaa1be843b6f028cbb8e82aaec79e2c/src/stage1/zig0.cpp#L172-L176

Looks like this zig0 code is too simple; my changes caused a regression from this code we used to have:

https://github.com/ziglang/zig/blob/cae93c860bc2c599618482a4190daf619a0c69e2/src/codegen.cpp#L8766-L8795

So my mistake here was in thinking we could rely on stage2 code to handle the concept of "baseline" when actually if we want zig0 to be responsible for building zig1.o, we have to duplicate the logic in zig0.cpp.

Edit: hmm this code applies to RISC-V and i386, not x86_64, but let me try a few things...

andrewrk picture andrewrk on 28 Oct 2020

No dice with this patch:

--- a/src/stage1/zig0.cpp
+++ b/src/stage1/zig0.cpp
@@ -172,8 +172,20 @@ static Error target_parse_triple(struct ZigTarget *target, const char *zig_tripl
         } else if (strcmp(mcpu, "baseline") == 0) {
             target->is_native_os = false;
             target->is_native_cpu = false;
-            target->llvm_cpu_name = "";
-            target->llvm_cpu_features = "";
+            // The goal here is to match logic from std/Target.zig
+            if (target_is_riscv(target)) {
+                target->llvm_cpu_name = "";
+                target->llvm_cpu_features = "+a,+c,+d,+f,+m,+relax";
+            } else if (target->arch == ZigLLVM_x86) {
+                target->llvm_cpu_name = "pentium4";
+                target->llvm_cpu_features = "";
+            } else if (target->arch == ZigLLVM_x86_64) {
+                target->llvm_cpu_name = "x86_64";
+                target->llvm_cpu_features = "";
+            } else {
+                target->llvm_cpu_name = "";
+                target->llvm_cpu_features = "";
+            }
         } else {
             const char *msg = "stage0 can't handle CPU/features in the target";
             stage2_panic(msg, strlen(msg));

I'm waiting on a build to finish where I pass -DZIG_EXECUTABLE with the path to the produced zig so that zig1.o gets built with stage1 zig instead of stage0 zig and we'll see what happens there.

andrewrk picture andrewrk on 28 Oct 2020

OK, confirmed that the issue is solved by rebuilding, passing [the stage1 that fails the translate-c tests] for -DZIG_EXECUTABLE in order to rebuild zig1.o using stage1 rather than stage0. So this confirms @LemonBoy's hypothesis that stage0 is passing incorrect LLVM CPU features for "baseline", and it also leads us towards a fix, if we can just get cmake to cooperate.

andrewrk picture andrewrk on 28 Oct 2020
🎉2

Note that my patch was wrong: target->llvm_cpu_name = "x86_64"; needed a hyphen instead of an underscore: x86-64 but that did not ultimately have any effect.

Here is an objdump difference of a bad and good (respectively) zig1.o:

andy@Andrews-MacBook-Air:~/dev/zig/build-static$ objdump -d zig1.o | grep 'fld'
   2dc38: d9 eb                         fldpi
   a1120: db ac ff ff f7 ac ff          fldt    -5441537(%rdi,%rdi,8)
  157e74: d9 a4 ff ff c7 a4 ff          fldenv  -5978113(%rdi,%rdi,8)
  158518: d9 86 ff ff 9b 4d             flds    1302069247(%rsi)
  17c0ec: d9 c1                         fld %st(1)
  1cff44: db ab ff ff 2c ae             fldt    -1372782593(%rbx)
  20f0c4: db a8 ff ff db a8             fldt    -1461977089(%rax)
  241fa0: d9 ee                         fldz
  241fb4: d9 ee                         fldz
  318bd4: d9 e9                         fldl2t
andy@Andrews-MacBook-Air:~/dev/zig/build-static$ objdump -d ../build-static-ok/zig1.o | grep 'fld'
  159348: d9 47 ff                      flds    -1(%rdi)
  165fd4: db a9 ff ff db a9             fldt    -1445199873(%rcx)
  165fdc: db a9 ff ff 06 c1             fldt    -1056505857(%rcx)
  2e8dcc: d9 ea                         fldl2e
andrewrk picture andrewrk on 28 Oct 2020

I tried this new patch:

--- a/src/stage1/zig0.cpp
+++ b/src/stage1/zig0.cpp
@@ -154,6 +154,23 @@ static void get_native_target(ZigTarget *target) {
     }
 }

+static void set_baseline_cpu(struct ZigTarget *target) {
+    // The goal here is to match logic from std/Target.zig
+    if (target_is_riscv(target)) {
+        target->llvm_cpu_name = "";
+        target->llvm_cpu_features = "+a,+c,+d,+f,+m,+relax";
+    } else if (target->arch == ZigLLVM_x86) {
+        target->llvm_cpu_name = "pentium4";
+        target->llvm_cpu_features = "";
+    } else if (target->arch == ZigLLVM_x86_64) {
+        target->llvm_cpu_name = "x86-64";
+        target->llvm_cpu_features = "-3dnow,-3dnowa,+64bit,-adx,-aes,-amx-bf16,-amx-int8,-amx-tile,-avx,-avx2,-avx512bf16,-avx512bitalg,-avx512bw,-avx512cd,-avx512dq,-avx512er,-avx512f,-avx512ifma,-avx512pf,-avx512vbmi,-avx512vbmi2,-avx512vl,-avx512vnni,-avx512vp2intersect,-avx512vpopcntdq,-bmi,-bmi2,-branchfusion,-cldemote,-clflushopt,-clwb,-clzero,+cmov,-cx16,+cx8,-enqcmd,-ermsb,-f16c,-false-deps-lzcnt-tzcnt,-false-deps-popcnt,-fast-11bytenop,-fast-15bytenop,-fast-7bytenop,-fast-bextr,-fast-gather,-fast-hops,-fast-lzcnt,-fast-scalar-fsqrt,-fast-scalar-shift-masks,-fast-shld-rotate,-fast-variable-shuffle,-fast-vector-fsqrt,-fast-vector-shift-masks,-fma,-fma4,-fsgsbase,+fxsr,-gfni,-idivl-to-divb,+idivq-to-divl,-invpcid,-lea-sp,-lea-uses-ag,-lvi-cfi,-lvi-load-hardening,-lwp,-lzcnt,+macrofusion,-merge-to-threeway-branch,+mmx,-movbe,-movdir64b,-movdiri,-mpx,-mwaitx,+nopl,-pad-short-functions,-pclmul,-pconfig,-pku,-popcnt,-prefer-128-bit,-prefer-256-bit,-prefer-mask-registers,-prefetchwt1,-prfchw,-ptwrite,-rdpid,-rdrnd,-rdseed,-retpoline,-retpoline-external-thunk,-retpoline-indirect-branches,-retpoline-indirect-calls,-rtm,-sahf,-serialize,-seses,-sgx,-sha,-shstk,+slow-3ops-lea,+slow-incdec,-slow-lea,-slow-pmaddwd,-slow-pmulld,-slow-shld,-slow-two-mem-ops,-slow-unaligned-mem-16,-slow-unaligned-mem-32,-soft-float,+sse,-sse-unaligned-mem,+sse2,-sse3,-sse4.1,-sse4.2,-sse4a,-ssse3,-tbm,-tsxldtrk,-use-aa,-use-glm-div-sqrt-costs,-vaes,-vpclmulqdq,+vzeroupper,-waitpkg,-wbnoinvd,+x87,-xop,-xsave,-xsavec,-xsaveopt,-xsaves";
+    } else {
+        target->llvm_cpu_name = "";
+        target->llvm_cpu_features = "";
+    }
+}
+
 static Error target_parse_triple(struct ZigTarget *target, const char *zig_triple, const char *mcpu,
         const char *dynamic_linker)
 {
@@ -172,8 +189,7 @@ static Error target_parse_triple(struct ZigTarget *target, const char *zig_tripl
         } else if (strcmp(mcpu, "baseline") == 0) {
             target->is_native_os = false;
             target->is_native_cpu = false;
-            target->llvm_cpu_name = "";
-            target->llvm_cpu_features = "";
+            set_baseline_cpu(target);
         } else {
             const char *msg = "stage0 can't handle CPU/features in the target";
             stage2_panic(msg, strlen(msg));
@@ -214,6 +230,7 @@ static Error target_parse_triple(struct ZigTarget *target, const char *zig_tripl
             const char *msg = "stage0 can't handle CPU/features in the target";
             stage2_panic(msg, strlen(msg));
         }
+        set_baseline_cpu(target);
     }

     return ErrorNone;

I'm back to being confused. I got this value by executing:

./zig build-exe hello.zig --verbose-llvm-cpu-features -target x86_64-native -mcpu=baseline

So the idea here is to hardcode the baseline LLVM values into zig0.cpp, leaving nothing up to chance. I think this is a reasonable path forward, especially considering that the eventual plan for bootstrapping is to take advantage of the stage2 C backend, eliminating LLVM as a the bootstrapping dependency entirely. Then this zig0 code gets simplified more and this can be deleted.

But it doesn't work, and I don't know why. We still have the path forward of regenerating zig1.o with stage1 but that leaves this unsolved mystery.

andrewrk picture andrewrk on 28 Oct 2020

I think that commit should do it.

andrewrk picture andrewrk on 29 Oct 2020
🎉1
Was this page helpful?
0 / 5 - 0 ratings

Related issues

andrewrk picture andrewrk  路  3Comments
andrewrk picture andrewrk  路  3Comments
bronze1man picture bronze1man  路  3Comments
andrewrk picture andrewrk  路  3Comments
jorangreef picture jorangreef  路  3Comments