Failure with clang and -O2 optimisation level about libmill HOT 39 CLOSED

Which OS, which commit ?

from libmill.

Linux. I don't remember the exact commit but I can try to reproduce with the current head if you are interested.

from libmill.

eglibc 2.19, llvm 3.5

from libmill.

i wonder if this failing test is related? https://gist.github.com/reqshark/f1d3ae8fe505d14b9c50

from libmill.

btw the LLVM I was using:

Apple LLVM version 6.1.0 (clang-602.0.53) (based on LLVM 3.6.0svn)
Target: x86_64-apple-darwin14.3.0
Thread model: posix

/usr/lib/libc.dylib:
Mach header
      magic cputype cpusubtype  caps    filetype ncmds sizeofcmds      flags
MH_MAGIC_64  X86_64        ALL  0x00       DYLIB    50       3656   NOUNDEFS DYLDLINK TWOLEVEL APP_EXTENSION_SAFE

from libmill.

Yes, that's the one. Just use -O1 for now.

from libmill.

The workaround added to the build system. It will use -O1 with clang automatically.

from libmill.

great, thanks. That fixes this issue now on my machine

from libmill.

This looks like a clang bug. Here's the minimal test case:

#include <assert.h>
#include <stdio.h>
#include "../libmill.h"

void sender(chan ch, int val) {
    printf("chs val=%p\n", &val);
    chs(ch, int, val);
    printf("chs end\n");
}

int main() {

    chan ch1 = chmake(int, 10);
    chs(ch1, int, 0);
    chs(ch1, int, 0);
    chr(ch1, int);
    chr(ch1, int);
    chclose(ch1);

    chan ch3 = chmake(int, 0);
    go(sender(ch3, 0));
    go(sender(ch3, 0));

    return 0;
}

The output looks like this:

chs val=0x7ffc629e941c
chs val=0x7ffc629e941c

Which means that the coroutine is invoked twice in parallel, yet the pointer to a local variable is same in both cases.

from libmill.

Which file is this failing?

➜  build git:(cmake) ✗ ./tests/chan
➜  build git:(cmake) ✗ echo $?
0

Compiled with -O3 llvm3.6

...
[ 79%] Building C object CMakeFiles/test_chan.dir/tests/chan.c.o
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc   -Wall -Wpedantic -Wextra -O3 -DNDEBUG   -o CMakeFiles/test_chan.dir/tests/chan.c.o   -c /Users/Nicholas/code/c/libmill/tests/chan.c
...

➜  build git:(cmake) ✗ /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc -v
Apple LLVM version 6.1.0 (clang-602.0.53) (based on LLVM 3.6.0svn)
Target: x86_64-apple-darwin14.3.0
Thread model: posix

OSX 10.10.3

from libmill.

oh, well -DNDEBUG inserted by cmake when configuring with cmake -DCMAKE_BUILD_TYPE=Release .. removes all assert statements, so nvm.

from libmill.

Indeed, if I build with:

cmake -DCMAKE_C_FLAGS='-O3' ..
make
./tests/chan
Assertion failed: (val == 888), function main, file /Users/Nicholas/code/c/libmill/tests/chan.c, line 87.

from libmill.

Yes, that's the one.

from libmill.

yet the pointer to a local variable is same in both cases.

pointer aliasing? maybe we need a well placed restrict keyword?

from libmill.

    go(sender(ch3, 0));
    go(sender(ch3, 0));

what happens when you do an optimizing build not using rvalues?

    int a = 0;
    int b = 0;
    go(sender(ch3, a));
    go(sender(ch3, b));

Does the pointer aliasing happen as well? What about if you keep the rvalues, but invoke sender without go (not sure if go is a magical macro or fn)?

    sender(ch3, 0);
    sender(ch3, 0);

from libmill.

In the optimized build, you could also try checking the emitted assembly; I wonder if the rvalue 0 is being placed into the static memory segment and the address is being taken from the same spot twice?

from libmill.

It seems to be more complex than that: If I delete all the tests in tests/chan.c beyond the one that fails, it succeeds. Botched inlining, maybe?

from libmill.

Minimal test case:

#include <assert.h>
#include <stdio.h>
#include "../libmill.h"

void sender(chan ch, int val) {
    chs(ch, int, val);
}

int main() {
    chan ch3 = chmake(int, 0);
    go(sender(ch3, 888));
    go(sender(ch3, 999));
    int val = chr(ch3, int);
    assert(val == 888);
    chs(ch3, int, 333);
    chs(ch3, int, 444);
    return 0;
}

from libmill.

Replacing 888/999 by a/b has no effect.

from libmill.

LLVM transform passes: http://llvm.org/docs/Passes.html

and the code commentary all throughout this file: http://llvm.org/docs/doxygen/html/ArgumentPromotion_8cpp_source.html

from libmill.

Can I switch individual passes off?

from libmill.

maybe when this opt module is used: http://llvm.org/docs/CommandGuide/opt.html

looks like there's a -disable-opt flag or the -disable-inlining might do the trick. But nearly all of those transforms seem inherently unpredictable and susceptible to combinations of varrying unpredictability

from libmill.

Regarding:

The workaround added to the build system. It will use -O1 with clang automatically.

I'm trying to make a homebrew formula for libmill. Unfortunately the tests fail with recent HEAD.

I use 'make check' to ensure that the project's unit tests pass prior to allowing an install to proceed.

Output:

/Applications/Xcode.app/Contents/Developer/usr/bin/make  check-TESTS
PASS: tests/example
PASS: tests/cls
PASS: tests/go
PASS: tests/sleep
PASS: tests/tcp
PASS: tests/udp
PASS: tests/signals
PASS: tests/fdwait
PASS: tests/unix
./test-driver: line 107: 54542 Abort trap: 6           "$@" > $log_file 2>&1
FAIL: tests/chan
./test-driver: line 107: 54545 Abort trap: 6           "$@" > $log_file 2>&1
FAIL: tests/choose
============================================================================
Testsuite summary for libmill 92cac9d-dirty
============================================================================
# TOTAL: 11
# PASS:  9
# SKIP:  0
# XFAIL: 0
# FAIL:  2
# XPASS: 0
# ERROR: 0

The CFLAGS are set to be empty such that the optimization level is '-O1'. Example:

./doltlibtool --tag=CC --mode=link clang -O1 -o perf/chs perf/chs.o libmill.la

Clang version:

Apple LLVM version 6.1.0 (clang-602.0.49) (based on LLVM 3.6.0svn)
Target: x86_64-apple-darwin14.3.0
Thread model: posix

OS: Yosemite

I'm quite happy to yield the work-in-progress homebrew formula for libmill, if you need it.

from libmill.

When checking for the errors from the tests, look into tests/chan.log and tests/choose.log. What kind of errors are you seeing there?

As for homebrew formula, I don't have an OSX box so I can't test it myself. Maybe sending it to the mailing list would make sense?

from libmill.

Homebrew sets CFLAGS automatically to "-O2". By setting the environment CFLAGS to '', it allows ./configure to set the CFLAGS to O1 for clang.

For all purposes, the build should be identical to the Travis OSX build, which it isn't. Perhaps Travis's OSX is older than Yosemite, or I have an out of date clang (I've not updated to the latest XCode)

I'll do some more prodding

On Aug 4, 2015, at 10:07 PM, sustrik [email protected] wrote:

Are you sure that empty CFLAGS result in -01? On my (Linux) box it's -02 in such case.

As for homebrew formula, I don't have an OSX box so I can't test it myself. Maybe sending it to the mailing list would make sense?

—
Reply to this email directly or view it on GitHub.

from libmill.

And I'll extract out the failed test logs when I can

from libmill.

One more thing: Does the failure happen even without homebrew involved?

from libmill.

chan.log:
https://gist.github.com/benjolitz/27d60d787eeaa39bd948

choose.log:
https://gist.github.com/benjolitz/5baf671add4bb7e72bd8

Strangely enough, compiling without homebrew passes the tests correctly.

Here is the brew formula:
https://gist.github.com/benjolitz/086ddcd3534d6cc8c63d
It can be installed/tested using
brew install https://gist.githubusercontent.com/benjolitz/086ddcd3534d6cc8c63d/raw/146d4af710e95fe8844f988641cde5ddf90a564e/libmill.rb

I can't figure out the differences between with and without brew cases -- their config logs and CFLAGS/LDFLAGS all appear the same. :/

from libmill.

Yes. That's exactly the error you get when compiling with -O2. Here's what I think happens:

Here's how it works: gcc uses the last occurence of -O flag. By auotmake's default it's -O2:

gcc ... -O2

However, configure.ac figures out that it's using clang and appends -O1 to the command line:

gcc ... -O2 -O1

Then, homebrew defines CFLAGS=-O2 which is added to the command line:

gcc ... -O2 -O1 -O2

And given that gcc uses the last occurence of the flag, you end up with -O2.

Try either setting CFLAGS to empty string, or, if not possible in homebrew, adding -O1 to it, so that we end up with:

gcc ... -O2 -O1 -O2 -O1

from libmill.

Confirmed. Adding env :std into the formula allowed libmill to define the CFLAGS and pass the test vector.

from libmill.

Bingo!

from libmill.

I have a stable homebrew recipe now for git HEAD and libmill 0.16 beta. They both pass the tests.

HEAD requires env :std, while 0.16 beta requires ENV.append_to_cflags "-01"

from libmill.

Wasted 2hrs debugging the original (-O2) issue. What's happening is that two coroutines are launched, each one declares 'mill_val' local variable (as expansion of chs macro) and compiler the reuses address of one of them in the other coroutine instead of using its own instance.

This doesn't even look related to the fact that longjmp() is assumed to always jump up the stack: If that was the case, the local variable in one coroutine would be assumed to be out of scope after a longjmp() and wouldn't be re-used.

from libmill.

That sounds like a clang bug (miscompilation).

http://llvm.org/docs/HowToSubmitABug.html#miscompilations

from libmill.

Here's the minimal test case:

#include <stdio.h>
#include <stdlib.h>

#include "../libmill.h"

void foo() {
    int x = random();
    goredump();
    printf("pointer to x: %p\n", &x);
    msleep(now() + 1000);
}

int main() {

    chan ch1 = chmake(int, 2);
    chs(ch1, int, 0);
    chs(ch1, int, 0);

    go(foo());
    go(foo());

    return 0;
}

And here's the output. Notice that pointer to x is the same, although a different coroutine is running, according to goredump:

COROUTINE  state                                      current                                  created
------------------------------------------------------------------------------------------------------------------------
{0}        ready                                      tests/chan.c:19                          <main>
{1}        RUNNING                                    ---                                      tests/chan.c:21

CHANNEL  msgs/max    senders/receivers                          refs  done  created
------------------------------------------------------------------------------------------------------------------------
<1>      2/2                                                    1     no    tests/chan.c:17

pointer to x: 0x7ffe72dc3234

COROUTINE  state                                      current                                  created
------------------------------------------------------------------------------------------------------------------------
{0}        ready                                      tests/chan.c:19                          <main>
{1}        msleep()                                   tests/chan.c:12                          tests/chan.c:21
{2}        RUNNING                                    ---                                      tests/chan.c:22

CHANNEL  msgs/max    senders/receivers                          refs  done  created
------------------------------------------------------------------------------------------------------------------------
<1>      2/2                                                    1     no    tests/chan.c:17

pointer to x: 0x7ffe72dc3234

from libmill.

@benjolitz: I am afraid that no compiler developer would even bother looking at it once they realise what kind of black magic is happening inside go().

from libmill.

For the record: I think I know what's happening here, although I cannot check the theory until I get to my computer.

It looks like the body of the coroutine is inlined and thus treated as single function together with the function that does go(). That being the case, optimiser is free to rearrange local variables in such a way that those declared in the coroutine precede the filler variable that shifts the stack pointer to the coroutine's stack, i.e. they are allocated on the caller's stack.

Unfortunately, I am not aware of a way to disable inlining at caller's side (using a function pointer won't work because libmill isn't aware of the coroutine's prototype) so the only option left seems to require disabling inlining when declaring the coroutine. This can be hidden in a macro though:

coroutine void foo(int a, int b, int c) {
}

from libmill.

Confirmed. Declaring coroutines with __attribute__((noinline)) fixes the problem.

from libmill.

Fixed by introducing 'coroutine' keyword.

from libmill.