codegen assertion failure, problem with code_native

[yuyichao]

The actual output is too long to post here but it looks like

yuyichao% ./julia -f -e '@code_native 1 + 1' | wc
 129249  344502 2470759

Piping to tail suggest that the end is always the same so it seems that the disassembler works until it reaches the end of the file.

Bisect result. ad37021

ad370215297c3d78efd9b1135f7731b115c2ccc7 is the first bad commit
commit ad370215297c3d78efd9b1135f7731b115c2ccc7
Author: Jameson Nash <[email protected]>
Date:   Mon Apr 18 21:00:08 2016 -0400

    do a better job at generating code for known functions

    closes #15048

:040000 040000 2440f97408e9d97b3ab126ed69fc3483bd4d57b8 bd297c38e4c1fb7c8868e58d20e143682093c1df M      base
:040000 040000 c68c27e351cf0b24d74e6b52e5f041538d2051d8 df3434506f7936808c8baa292c92828d70bda7e9 M      src


git bisect start
# good: [b0e098aab77335c0b100c4515e5d63d07ac69c9a] Merge pull request #15921 from JuliaLang/teh/concat
git bisect good b0e098aab77335c0b100c4515e5d63d07ac69c9a
# bad: [637ac97def4d86ab2f7e6b65a40557ccc161a217] Merge pull request #15934 from JuliaLang/jn/precompile_plus
git bisect bad 637ac97def4d86ab2f7e6b65a40557ccc161a217
# good: [57a0c72ef66aba1acf05ff663d06ade6a3164afd] Merge pull request #15925 from JuliaLang/jn/no-stage0
git bisect good 57a0c72ef66aba1acf05ff663d06ade6a3164afd
# bad: [ad370215297c3d78efd9b1135f7731b115c2ccc7] do a better job at generating code for known functions
git bisect bad ad370215297c3d78efd9b1135f7731b115c2ccc7
# first bad commit: [ad370215297c3d78efd9b1135f7731b115c2ccc7] do a better job at generating code for known functions

@vtjnash

[JeffBezanson]

I get

julia: /home/jeff/src/julia-master/julia/src/codegen.cpp:1330: uint64_t compute_obj_symsize(const llvm::object::ObjectFile*, uint64_t): Assertion `hi == 0' failed.

[vtjnash]

perhaps an effect of #15617?

[Ismael-VC]

Do you think it's a good idea to add a test here, something kinda like this?

On v0.4.5:

julia> function test_native()
           orig = STDOUT
           temp = redirect_stdout()[1]
           @code_native 1 + 1
           println("END")
           lines = []
           while true
               line = readline(temp)
               if line == "END\n"
                   break
               end
               push!(lines, line)
           end
           redirect_stdout(orig)
           join(lines, "")
       end
test_native (generic function with 1 method)

julia> native_test = "\t.text\nFilename: int.jl\nSource line: 8\n\tpushq\t%rbp\n\tmovq\t%rsp, %rbp\nSource line: 8\n\taddq\t%rsi, %rdi\n\tmovq\t%rdi, %rax\n\tpopq\t%rbp\n\tret\n"
"\t.text\nFilename: int.jl\nSource line: 8\n\tpushq\t%rbp\n\tmovq\t%rsp, %rbp\nSource line: 8\n\taddq\t%rsi, %rdi\n\tmovq\t%rdi, %rax\n\tpopq\t%rbp\n\tret\n"

julia> using Base.Test

julia> @test test_native() == native_test

The expected output seems to be consistent except on ARM, but could be tailored, I don't know if it's a good idea, but I allways show off the @code_native 1 + 1 snippet, which is why I noticed.

[yuyichao]

That is the meaning of the needs-tests label. Adding a test for this is easy when this is fixed.

[Ismael-VC]

@yuyichao I understand that, but I mean is the way I'm testing is a good idea, that's the only way of testing this that came up to me, if it is I can send PR for review, thanks!

[yuyichao]

is the way I'm testing is a good idea

No, every optimization, debug option, line number change, different architecture, available instructions set is going to break this badly. Just test for the length should be good enough to notice the difference. code_native also accept io argument and we already have tests for it. It's a matter of adding a upper limit on the output in additional to the !isempty test.

I can send PR for review

I don't see the point of sending a PR with a failing test while there's already an open issue to track the problem.

[Ismael-VC]

No, every optimization, debug option, line number change, different architecture, available instructions set is going to break this badly.

That's what I wanted to know. I had the impression that the expected native output for 1 + 1 was somewhat consistent. It may seem obvious to you, but it's not for me, thanks for your input and your time!

I don't see the point of sending a PR with a failing test while there's already an open issue to track the problem.

Me neither, of course was I meant to send a PR with a working test (since there are any for this yet, even when there is an issue), not the snippet above (I just said something like that), which is why I'm asking.

code_native also accept io argument and we already have tests for it.

Oh, I didn't know that, it's not documented, except for methods(code_native) and the tests.

I'm still not good enough to help you here, so I'll go document that stuff.

[yuyichao]

Oh, I didn't know that, it's not documented
I'll go document that stuff.

I didn't realize it wasn't..... FYI both code_native and code_llvm accept that, would be nice to have that added to the doc.

[yuyichao]

It may seem obvious to you, but it's not for me

Try @code_native with debug build.

[Ismael-VC]

Adding a test for this is easy when this is fixed.

ade5a90 doesn't address the testing part of this issue.

• https://github.com/JuliaLang/julia/blob/master/test/reflection.jl#L14

function test_bin_reflection(freflect, f, types, n)
    iob = IOBuffer()                               
    freflect(iob, f, types)                        
    str = takebuf_string(iob)
    @test !isempty(str)                        
    lines = length(split(str, "\n"))                                         
    @test lines ≤ n                               
end                                                                                                                                    

❓

[vtjnash]

that looks fine too. there's an assertion failure that it was 50-50 likely to hit also, depending on how various functions got placed in the object file.

[tkelman]

and yet the assertion failure never seemed to happen on CI?

[vtjnash]

the assertion isn't enabled on CI

[tkelman]

FORCE_ASSERTIONS=1 doesn't cover this?

julia/.travis.yml

Line 69 in e8601e8

BUILDOPTS="-j3 VERBOSE=1 FORCE_ASSERTIONS=1";

We should build LLVM with assertions enabled on CI.