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

Dump EmittedFunctionDetails to sysimage #6490

Merged
merged 2 commits into from
Apr 16, 2014

Conversation

ihnorton
Copy link
Member

This implements serialization and deserialization of line number information. Ref #5354.

@mbauman
Copy link
Member

mbauman commented Apr 10, 2014

I don't have any concrete suggestions for the deserialization, but keep in mind that LLVM's emitting API will be changing with 3.5/MCJIT. I started toying with this myself (here), but didn't get very far. The biggest issue is that (I think) the function pointers are now offsets within each anonymous module and no longer unique. But you probably already know this…

@JeffBezanson
Copy link
Member

This is so great. This is arguably the most important 0.3 blocker.

@ihnorton
Copy link
Member Author

Almost works, just need to fix the fptr round trip.

@ihnorton
Copy link
Member Author

Now it works.
(with sys.so on linux):

julia> include("notfound")
ERROR: could not open file /cmn/julia/notfound
 in include at boot.jl:244
 in include_from_node1 at loading.jl:125

(I was saving the function pointer correctly, but the line numbers were zero'd out when each DebugLoc was constructed, because I did not pass a scope MDNode. Now I pass a dummy MDNode. it works...but meh)

I added some minor contortions to call NotifyFunctionEmitted to set up the stackwalk stuff on Windows. @vtjnash could you please review? In particular, do you know if the UNWIND_INFO stanza added in NotifyFunctionEmitted is actually saved to sys.dll? (if it is, then I think I can avoid calling the notification and just grab pointer to tbl directly)

@mbauman thanks - hopefully this will go away when we move to 3.5 by getting info directly from DWARF.

@staticfloat
Copy link
Member

I can confirm that on OSX, this works just fine:

$ ./julia 
               _
   _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: http://docs.julialang.org
   _ _   _| |_  __ _   |  Type "help()" to list help topics
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 0.3.0-prerelease+2607 (2014-04-13 05:03 UTC)
 _/ |\__'_|_|_|\__'_|  |  (detached from ihnorton/dump_linedebug)/6f7163e (fork: 2 commits, 0 days)
|__/                   |  x86_64-apple-darwin13.1.0

julia> include("notfound")
ERROR: could not open file /Users/sabae/src/julia/notfound
 in include at boot.jl:244

julia> 

nagata:julia sabae [23:33:27]$ julia
               _
   _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: http://docs.julialang.org
   _ _   _| |_  __ _   |  Type "help()" to list help topics
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 0.3.0-prerelease+2600 (2014-04-12 02:36 UTC)
 _/ |\__'_|_|_|\__'_|  |  Commit 0382fd4* (1 day old master)
|__/                   |  x86_64-apple-darwin13.1.0

julia> include("notfound")
ERROR: could not open file /Users/sabae/src/julia/notfound

julia> 

@tkelman
Copy link
Contributor

tkelman commented Apr 13, 2014

On Windows this gives a WARNING: failed to insert function name into debug info then segfaults (builds the system image okay, repl doesn't start, neither does running a file or test command)

julia-debug.exe runs without segfaulting, but hasn't solved the line numbers problem.

@ihnorton
Copy link
Member Author

@staticfloat great, thanks!

@tkelman thanks - any idea where the crash is?

@ihnorton
Copy link
Member Author

Nevermind, I see where it is. Not sure what to do about it though; and it's a moot point because the epilogue insertion is failing.

@vtjnash
Copy link
Member

vtjnash commented Apr 14, 2014

sorry, i couldn't respond sooner.

no, the sys.dll is generated in a separate step, with different code from the original call to NotifyFunctionEmitted (we JIT slightly different code than we save for the static compile). I'm doing some really hairy stuff for windows too: I modify the LLVM Function JIT allocator module to always overallocate by 48 bytes, then when NotifyFunctionEmitted is called, I fill in that "reserved" space. In the pre-compiled module, we might be able to actually do this somewhat more efficiently and somewhat simpler (this just occured to me):

  • in the sys.dll image, configure llvm to emit stack frames (-fno-omit-stack-frame), like we do for code in src
  • also in the sys.dll image, emit the assembly instructions (and relocation information) for jmp _seh_exception_handler to some location catchjmp
  • also in the sys.dll image, emit a UnwindData struct once (it is the same for all functions), where the last argument is the module-relative address of catchjmp
  • also in the sys.dll image, emit a RUNTIME_FUNCTION[n] array (3 dwords per function), where each argument is the module-relative offset and size

@ihnorton
Copy link
Member Author

Some notes from IRC discussion w/ vtjnash:

  • meant -fno-omit-frame-pointer rather than -fno-omit-stack-frame, which is equivalent to TargetOptions.NoFramePointerElim = true
  • unfortunately we're not seeing a way to do relative addresses in 3.3 -

@vtjnash wants to punt on sys.dll for 0.3. We don't distribute it now anyway, and the (relatively few) people who build on Windows can just be warned that backtraces won't work if they generate sys.dll (maybe we turn it off and provide an overrideable setting for Make.user)

@ihnorton
Copy link
Member Author

So seeing that Windows support is not really tractable, and this works well on linux and mac, I'll merge tomorrow if there are no objections.

(I took out the NotifyFunctionEmitted foolishness).

@tkelman
Copy link
Contributor

tkelman commented Apr 15, 2014

Not sure what you changed, but now this branch doesn't build on Windows.

    CC src/codegen.o
In file included from codegen.cpp:683:0:
debuginfo.cpp: In function 'void jl_dump_linedebug_info()':
debuginfo.cpp:295:91: warning: cast from type 'const llvm::Function*' to type 'llvm::Constant*' casts away qualifiers [-Wcast-qual]
         Constant *info_data[6] = { ConstantExpr::getBitCast((Constant*)(*infoiter).second.func, T_psize),
                                                                                           ^
debuginfo.cpp: In function 'void jl_restore_linedebug_info(uv_lib_t*)':
debuginfo.cpp:318:62: warning: cast from type 'const char*' to type 'char*' casts away qualifiers [-Wcast-qual]
     uintptr_t *infoptr = (uintptr_t*)jl_dlsym(handle, (char*)"jl_linedebug_info");
                                                              ^
debuginfo.cpp:341:97: error: cannot convert 'std::vector<llvm::JITEvent_EmittedFunctionDetails::LineStart>' to 'PRUNTIME_FUNCTION {aka _RUNTIME_FUNCTION*}' in initialization
         FuncInfo info = { NULL, lengthAdr, std::string(name), std::string(filename), linestarts };
                                                                                                 ^
debuginfo.cpp:321:21: warning: unused variable 'dummyfunc' [-Wunused-variable]
     const Function& dummyfunc = *Function::Create(jl_func_sig, Function::ExternalLinkage);
                     ^
Makefile:54: recipe for target 'codegen.o' failed
make[1]: *** [codegen.o] Error 1

The FuncInfo struct has an extra entry in Win64. If you put #ifndef _OS_WINDOWS_ around the definition of jl_restore_linedebug_info then it appears to work (already not using it, may as well not define it).

I'm personally fine with sys.dll being disabled by default. The faster startup isn't worth losing backtraces.

- Fiddles with FuncInfo struct to optionally store name and filename info
- sysimg_handle -> jl_sysimg_handle (not exported)
- adds jl_restore_linedebug_info(uv_lib_t*)
@ihnorton
Copy link
Member Author

Fixed on windows, and eliminated warnings.

@ihnorton ihnorton changed the title RFC: Dump EmittedFunctionDetails to sysimage Dump EmittedFunctionDetails to sysimage Apr 16, 2014
ihnorton added a commit that referenced this pull request Apr 16, 2014
Dump EmittedFunctionDetails to sysimage
@ihnorton ihnorton merged commit c77e098 into JuliaLang:master Apr 16, 2014
@ihnorton ihnorton deleted the dump_linedebug branch April 16, 2014 04:14
@pao pao mentioned this pull request Apr 17, 2014
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.

6 participants