dmajor-2016-11-18.log

--- Log opened Fri Nov 18 12:51:34 2016
12:51 -!- Irssi: Starting query in mozilla with dmajor
12:51 <acrichto> ping
12:51 <dmajor> pong
12:51 <acrichto> hey got a sec to chat about that msvc win64 unwind rust bug?
12:51 <dmajor> yep, sure
12:52 <acrichto> ok so I just spoke a bit with ted in #rust-internals
12:52 <acrichto> unfortunately not a lot of progres though
12:52 <acrichto> I was curious, though, is there an easy way to test out something like:
12:52 <acrichto> C++ calls C which executes ud2?
12:52 <acrichto> just to be 100% sure that it works as expected
12:52 <acrichto> and hopefully we can work backwards to see what rust is doing differently
12:53 <dmajor> I can stick a __ud2() into some necko C code locally if needed
12:53 <acrichto> that'd at least confirm the suspicion that it's all on Rust's side
12:54 <dmajor> but I think what it comes down to is: why aren't we emitting an entry in xul.dll's .pdata section for |intentional_panic|?
12:54 <acrichto> (which I think it is)
12:54 <acrichto> ah so I should also start out by saying I'm not *super* familiar with all the nitty gritty details of unwinding on msvc
12:54 <acrichto> I've mostly just done things like "make sure we tell LLVM we want unwinding"
12:54 <acrichto> so I could rephrase your question, a bit actually
12:55 <acrichto> if you're compiling C code, what emits the .pdata section for intentional_panic ?
12:55 <acrichto> e.g. is it a flag to the C compiler?
12:56 <dmajor> it shouldn't have to be anything that the developer does
12:56 <acrichto> right yeah
12:56 <dmajor> for win64 binaries, all functions should have a .pdata entry, that's just part of the rules for producing a binary
12:56 <acrichto> hm
12:57 <acrichto> does gecko support compiling with LLVM on windows?
12:58 <dmajor> we can compile using LLVM via clang-cl, but that still uses MSVC's link.exe to do the linking
12:58 <dmajor> and I imagine it's the linker that produces the .pdata
12:58 <acrichto> ah it's ok, rustc has the same architecture
12:58 <acrichto> so presumably we're not passing *something* to LLVM ?
12:58 <dmajor> could be. (this is where my knowledge breaks down)
12:59 <acrichto> hm
12:59 <acrichto> so there's no special flag for this in C?
12:59 <acrichto> it's jsut "if you compile it should do the right thing"
13:00 <dmajor> right
13:00 <acrichto> hm so in theory rust is using the same backend as clang-cl
13:00 <acrichto> a pretty up-to-date one as well
13:00 <acrichto> and we never say "llvm, please don't generate pdata"
13:01 <acrichto> so now I'm confused why it's not happening for us...
13:01 <dmajor> ok, that's good to know. (I thought maybe you were deliberately not generating pdata as part of the "extern functions are never unwound" assumption.)
13:01 <acrichto> oh no definitely not
13:02 <acrichto> so we do this in the LLVM IR:
13:02 <acrichto> attributes #0 = { norecurse nounwind readnone uwtable }
13:02 <acrichto> so we tag functions as "nounwind"
13:02 <acrichto> if what you're saying is true though
13:02 <acrichto> LLVM can't consider that as "don't generate pdata"
13:02 <acrichto> b/c all functions need that
13:05 <dmajor> hmm
13:06 <acrichto> is there tooling to witness the lack of pdata for Rust functions?
13:06 <dmajor> not really. basically, me crawling the PE image in my debugger
13:06 <acrichto> lol
13:07 <dmajor> so, it seems that some rust functions _do_ have pdata -- the first three frames in #c7 were fine
13:07 <dmajor> it's just the function at the language boundary broke
13:08 <acrichto> just the `intentional_panic` function?
13:08 <dmajor> right
13:08 <acrichto> weird!
13:08  * acrichto tests
13:08 <dmajor> which I assumed was because of the extern declaration, but that was just a guess
13:10 <acrichto> maybe?
13:10 <acrichto> the codegen looks very similar
13:10 <acrichto> the only difference I see is the ABI
13:11 <acrichto> but that's expected...
13:11 <dmajor> btw, apparently |dumpbin -pdata xul.dll| will list all the entries
13:13 <acrichto> oh nice
13:17 <acrichto> hm ok I'll try to do some digging locally
13:18 <acrichto> and see if I can repro various things with this info
--- Log closed Fri Nov 18 13:23:46 2016
--- Log opened Fri Nov 18 13:54:12 2016
13:54 <dmajor> https://pastebin.mozilla.org/8929441
13:54 <dmajor> basically, what we'd expect
13:55 <acrichto> ok cool
13:55 <acrichto> so something fishy is going in the C entry points into Rust
13:56 <acrichto> need to figure out why they're not getting pdata sections
13:56 <dmajor> (and for bonus fun, that was on my asan build, compiled with clang-cl)
13:57 <acrichto> hm, do you know how to get LLVM IR out of clang-cl?
13:58 <dmajor> I want to say it will accept -Xclang or some-such and pass it through. Do you know how to get it out of regular clang?
13:59 <acrichto> -emit-llvm -S
14:02 <dmajor> what does -S do?
14:02 <acrichto> "emit human readable assesmbly"
14:02 <acrichto> (I think at leaset)
14:02 <acrichto> that may not be required?
14:04 <dmajor> ; Function Attrs: nounwind optsize sanitize_address sspstrong uwtable
14:05 <dmajor> that's the earliest C function on the stack
14:06 <acrichto> aaaaaargh
14:06 <acrichto> that's exactly what we have
14:06 <acrichto> minus the non relevant ones
14:07 <acrichto> the entry point doesn't look funy?
14:07 <acrichto> it's just like
14:07 <acrichto> declare i32 @foo() #N { ... }
14:07 <dmajor> define void @jpeg_CreateDecompress(%struct.jpeg_decompress_struct* %cinfo, i32 %version, i64 %structsize) local_unnamed_addr #0 !dbg !630 {
14:08 <acrichto> yeah we use `unnamed_addr` instead of `local_unnamed_addr` but otherwise these look identical
14:08 <acrichto> what's the command line for compiling the C object?
14:08 <dmajor> (I used "-Xclang -emit-llvm" btw)
14:08 <dmajor> https://pastebin.mozilla.org/8929445
14:09 <acrichto> lol I recognize like none of these arguments
14:09 <dmajor> yeah it's clang-cl's job to translate them into clang-ese :)
14:09 <acrichto> ware any of those exception-related ?
14:10 <acrichto> I expected to see something like /EHxxxxx but didn't see
14:10 <dmajor> what function are you looking at, btw? did you apply ted's intentional_panic patch?
14:11 <acrichto> oh I'm not actually looking at gecko, just locally what we emit from something like
14:11 <acrichto> #[no_mangle] pub extern fn foo() {}
14:11 <dmajor> mmmm
14:11 <acrichto> unfortunately I don't have access to a windows computer right now so I can only generate IR on linux
14:12 <dmajor> maybe things get more complicated when you try to link it together with gecko?
14:12 <acrichto> maybe yeah but that sounds highly suspicious
14:12 <acrichto> unless gecko's like post-modifying object files
14:12 <acrichto> it's just ferrying them over to the linker
14:12 <acrichto> which involves no funny business
14:13 <dmajor> does the situation change if your foo() contains a panic! ? 
14:13 <acrichto> whoa
14:13 <acrichto> oh nmvd
14:13 <acrichto> nah same thing
14:14 <acrichto> ; Function Attrs: noreturn nounwind uwtable
14:14 <dmajor> well that's interesting
14:14 <dmajor> does noreturn somehow affect things?
14:14 <acrichto> nah I just made it go away and looks the same
14:14 <dmajor> hm?
14:14 <acrichto> e.g. `if foo { panic!(); }`
14:15 <acrichto> oh wait
14:15 <acrichto> hm
14:15 <dmajor> but in ted's case there's no if
14:15 <acrichto> right
14:15 <acrichto> how easily can you test that out?
14:15 <dmajor> you mean, apply ted's patch like I did earlier, but add an if?
14:16 <acrichto> yeah add a boolean parameter to the function
14:16 <acrichto> do `if parameter_is_true { panic!() }`
14:16 <acrichto> and then just pass true all the time on the C++ side of things
14:17 <dmajor> shouldn't be hard, it'll just take some time to build
14:17 <acrichto> man if that's it...
14:17 <acrichto> I guess llvm could be trying to be clever
14:17 <acrichto> and if a function can't return no need to unwind from it
14:17 <acrichto> although that doesn't entirely make sense...
14:18 <dmajor> in about an hour I need to disappear for another hour or so. I'll use that time to run a build.
14:18 <acrichto> ok cool
14:18 <acrichto> you're not in SF, are you?
14:18 <dmajor> nope
14:18 <acrichto> bah
--- Log closed Fri Nov 18 14:24:46 2016
--- Log opened Fri Nov 18 15:06:20 2016
15:06 <dmajor> I added `if !message.is_null()` and it still can't unwind
15:06 <acrichto> bah, so not related to noreturn
15:11 <dmajor> now that I have this build, I suppose I could play around and write different kinds of functions and see which ones get .pdata
15:11 <dmajor> I'll look into that after I get back
15:12 <acrichto> hehe ok, hopfully I'll have a windows computer again by next week...
--- Log closed Fri Nov 18 15:17:46 2016
--- Log opened Fri Nov 18 17:06:46 2016
17:06 <dmajor> I just compiled ted's sample program anew, and now it too lacks .pdata at the boundary function
17:06 <dmajor> not sure if this is good news or bad news :)
17:07 <dmajor> I suppose I've updated my rustc since the last time I tried in september
--- Log closed Fri Nov 18 17:12:46 2016