--- Log opened Sat Jun 27 00:00:50 2009 00:48 -!- tgrabiec [n=tomekg@rev-189-107.ramtel.pl] has quit [Remote closed the connection] 01:04 -!- tgrabiec [n=tomekg@rev-189-107.ramtel.pl] has joined #jato 01:43 -!- ahuillet_ [n=user@79.83.34.239] has quit ["Leaving"] 04:30 -!- tgrabiec [n=tomekg@rev-189-107.ramtel.pl] has quit [Remote closed the connection] 08:36 -!- penberg [n=penberg@cs181039060.pp.htv.fi] has joined #jato 08:42 < vegard> *yawn* 08:44 < vegard> http://folk.uio.no/vegardno/trace.txt 08:44 < vegard> that's the full trace you requested 08:45 < vegard> actually that's more that we want 08:46 < vegard> because it throws a NullPointerException instead of crashing 08:50 < penberg> blah 08:50 < penberg> iinc sucks 08:51 < vegard> ah I'll just try to reproduce it with a small example 08:51 < penberg> vegard: yeah, that's usually the best approach 08:51 < penberg> so the trace doesn't get totally out of control. 08:52 < penberg> vegard: hmm, so that trace doesn't have the faulting EIP 08:52 < vegard> yeah, true, but it's not hard to find 08:53 < vegard> search for getProperty 08:53 < penberg> but there's no register dump either 08:53 < vegard> and you'll see the next thing it loads is NullPointerException 08:53 < vegard> yes, true 08:53 < penberg> I'll just wait for a simpler trace 08:53 < penberg> ;) 08:53 < penberg> we need some -Xcrashhard flag 08:54 < penberg> that dumps everything _even_ if it's a NullPointerException 08:54 < vegard> or arithmeticexception 08:54 < penberg> yup 08:54 < penberg> -Xdump:all? 08:54 < penberg> -Xdebug, perhaps? 08:54 < penberg> -Xdebug:jit ? 08:58 < vegard> -Xdump:signal ? 09:01 < penberg> hmm 09:01 < penberg> maybe 09:03 < penberg> vegard: so no patches from you yet? 09:03 < vegard> tried a one-liner now: new Properties().put("hi", "ho"); 09:03 < penberg> end result? 09:03 < vegard> it deadlocks :-( 09:03 < vegard> in jit_magic_trampoline :-( 09:03 < vegard> apparently it's trying to compile Object.hashCode() from within itself..!? 09:04 < penberg> probably not 09:04 < penberg> the thing is 09:04 < penberg> in compile(), it's pretty dangerous to start _calling_ java methods 09:05 < penberg> because if we try to run the same method 09:05 < penberg> we'll deadlock 09:05 < penberg> so I guess class initialization just causes Object.hashCode() to be called? 09:06 < vegard> http://folk.uio.no/vegardno/gdb-trace.txt 09:06 < penberg> yup 09:07 < penberg> #83 0x0805bcb9 in jit_magic_trampoline (cu=0x81b9d98) at jit/trampoline.c:81 09:07 < penberg> #4 0x0805bbfc in jit_magic_trampoline (cu=0x81b9d98) at jit/trampoline.c:102 09:08 < vegard> btw, I switched the lock/trace lines in jit_magic_trampoline so we'd know which one that deadlocks 09:08 < penberg> the only question is what cu that is? 09:08 < penberg> vegard: makes sense. 09:18 < vegard> so what's the solution (in general) to this? 09:18 < vegard> it seems like a pretty tough problem 09:18 < penberg> lazy class loading. 09:19 < vegard> compiling a method may require loading more classes -- loading the class means initializing it -- means compiling its 09:19 < vegard> meaning we don't load until we need it. 09:19 < vegard> but we need it for compilation, don't we? 09:19 < vegard> or did you mean lazy class initialisation? 09:20 < penberg> well yes 09:20 < penberg> hmm 09:20 < penberg> otoh 09:20 < penberg> when the class is loaded 09:20 < penberg> it needs to be initialized too 09:27 < vegard> I think it's weird that this even shows up 09:28 < vegard> doesn't that tell us there's something wrong with the _java_ code? 09:28 < vegard> not necessarily, I guess. 09:31 < vegard> if we could delay the s... 09:38 < vegard> can we delay execution until the compilation is finished 09:38 < vegard> and just before exiting the jit_magic_trampoline (after unlocking the cu), we call it? 09:39 < vegard> synchronize_all_clinits() or smoething 09:43 < vegard> because I don't think we need them during compilation? 09:44 < vegard> and it would help with the recursion 09:47 < vegard> or is that just another ugly hack? 09:58 < penberg> wrong with java code? 09:58 < penberg> not really. 09:58 < penberg> but yeah, something like that makes sense 09:58 < penberg> but you need to be careful 09:59 < penberg> not to allow any threads to run the jit'd code before classes are initialized 09:59 < vegard> hm, true 09:59 < penberg> but what bothers me is 09:59 < penberg> if a method depends on a class 09:59 < penberg> and that class initialization depends on the method 09:59 < penberg> how is it supposed to work? 09:59 < penberg> so I'm not convinced we fully understand what's going on here. 10:00 < vegard> the method usually doesn't depend on the class being initialized 10:00 < vegard> so while it depends on having the definition of the class 10:00 < vegard> or rather: the _compilation_ of that method doesn't depend on anything but the class definition 10:02 < vegard> the method itself may depend on the class being initialized 10:03 < vegard> but that's a different thing, and is perfectly ok 10:08 < vegard> ok, this one actually ran successfully. 10:11 < vegard> BTW, one question: will our NullPointerException signal magic stuff work under valgrind? 10:12 < vegard> I guess so. 10:34 -!- penberg [n=penberg@cs181039060.pp.htv.fi] has quit ["leaving"] 10:36 <@ahuillet> hi 10:37 < vegard> hi :) 10:38 <@ahuillet> Subject: [PATCH 9/11] x86: add OP_XXX(reg, reg) monoburg rules 10:38 <@ahuillet> oh, he did it for me 10:38 <@ahuillet> They will be needed when EXPR_LOCAL will be replaced with EXPR_TEMPORARY 10:38 <@ahuillet> but ?? 10:52 <@ahuillet> Subject: [PATCH 10/11] jit: do not push EXPR_LOCAL onto mimic-stack 10:52 <@ahuillet> I don't understand this 10:52 <@ahuillet> Instruction at pc=3 modifies value of local variable. 10:52 <@ahuillet> Instruction at pc=6 generates STMT_STORE with source popped 10:52 <@ahuillet> from mimic-stack (EXPR_LOCAL) which has invalid value - it was 10:52 <@ahuillet> modified while in mimic-stack. 10:52 <@ahuillet> when he says this 10:52 <@ahuillet> why hasn't the variable been modified? 10:52 <@ahuillet> its value is on the stack, we read it from the stack, and it has in fact changed, so what is the problem? 10:56 < penberg_home> ahuillet: check the regression test 10:57 < penberg_home> ahuillet: but yeah, i think we want a better solution here 10:58 < penberg_home> ahuillet: 10:58 < penberg_home> 2: iload_1 10:58 < penberg_home> 3: iinc 1, 1 10:58 < penberg_home> the point here is that (2) loads the _value_ of the local on the stack 10:58 < penberg_home> and then (3) changes the _local variable_ 10:58 < penberg_home> not the _value_ on the stack. 10:58 <@ahuillet> I understand that at bytecode level 10:59 < penberg_home> so 10:59 < penberg_home> when we push EXPR_LOCAL on the mimic stack 10:59 < penberg_home> that means we're deferring the load 10:59 < penberg_home> which means that whoever comes _after_ (3) 10:59 < penberg_home> will load the new value, not the value at (2) 11:00 <@ahuillet> yes, is that wrong? 11:00 < penberg_home> yes! 11:00 <@ahuillet> ok 11:00 <@ahuillet> then we have to fix convert_iinc 11:00 < penberg_home> let me repeat this: at (2), we _semantically_ load a _value_ 11:00 < penberg_home> in the JVM 11:00 < penberg_home> ahuillet: yes agreed 11:00 <@ahuillet> yeah, I understand the difference now 11:00 < penberg_home> did you see my mail to tgrabiec? 11:00 <@ahuillet> I have a RPN calculator :) 11:01 < penberg_home> :) 11:01 <@ahuillet> "I did apply this as a bug fix but I'm wonder if this is too generic fix. 11:01 <@ahuillet> Isn't the only problematic bytecode iinc? Can we fix that instead? Doing 11:01 <@ahuillet> this at EXPR_LOCAL level generates tons of unnecessary copies." 11:01 < penberg_home> ahuillet: I'm not sure how to fix that, though? 11:01 < penberg_home> ahuillet: because basically, at iinc time 11:01 <@ahuillet> browse the operand stack 11:01 <@ahuillet> find all occurences of our EXPR_LOCAL 11:02 < penberg_home> yup. 11:02 < penberg_home> makes sens. 11:02 <@ahuillet> turn them into EXPR_TEMPORARY 11:02 < penberg_home> sense. 11:02 < penberg_home> patches welcome ;) 11:02 <@ahuillet> pretty much like what I did to fix the mimic stack at block boundaries 11:02 < penberg_home> ahuillet: yup, ack for the approach 11:02 <@ahuillet> however since this isn't a correctness issue given the tgrabiec fixed the bug, I guess it's better to wait a bit 11:03 <@ahuillet> given the, as you pointed out, deferring the load is semantically not correct 11:03 <@ahuillet> (sometimes) 11:03 <@ahuillet> *that 11:03 < penberg_home> hmm 11:03 < penberg_home> but iinc is the only problematic one here, isn't it? 11:04 < penberg_home> and I don't like the extra loads at all 11:04 < penberg_home> ahuillet: when tgrabiec appears 11:04 < penberg_home> lets just propose the fix to him 11:04 < penberg_home> if you dont' want to take a stab at it 11:04 <@ahuillet> any bytecode instruction that touches locals is potentially dangerous 11:05 < penberg_home> store 11:05 < penberg_home> hmm 11:05 < penberg_home> maybe you're right 11:06 <@ahuillet> besides... store variable -> reg, op reg reg, op reg reg, op reg reg, op reg reg 11:06 <@ahuillet> vs. op mem reg, op mem reg, op mem reg, op mem reg 11:06 <@ahuillet> I would not bet on the first one to be slower. 11:09 -!- tgrabiec [n=tomekg@rev-189-107.ramtel.pl] has joined #jato 11:10 < tgrabiec> ahuillet: thera are already regression tests for the new monoburg rules, because the operate on 2 temporaries now, not on a temporary and a local. 11:10 < tgrabiec> so they are testes (that's why i found that sbb was broken) 11:11 <@ahuillet> you cannot add regression tests for this arbitrarily anyway, the best technique is to remove the non-(reg,reg) variants 11:11 <@ahuillet> so yeah, *regression tests*, that's ok 11:11 <@ahuillet> I believe you lack a unit test or tswo 11:11 <@ahuillet> *two 11:11 <@ahuillet> xor reg, reg or I don't remember what 11:12 < tgrabiec> ok, i'll check that 11:12 <@ahuillet> then we have to fix convert_iinc 11:12 <@ahuillet> browse the operand stack 11:12 <@ahuillet> find all occurences of our EXPR_LOCAL 11:12 <@ahuillet> yup. 11:12 <@ahuillet> makes sens. 11:12 <@ahuillet> turn them into EXPR_TEMPORARY 11:12 <@ahuillet> however since this isn't a correctness issue given the tgrabiec fixed the bug, I guess it's better to wait a bit 11:13 <@ahuillet> anyway just a thought, 'cause I don't think touching that right now is a priority 11:13 < tgrabiec> yes, i was thinking about that too 11:14 < penberg_home> ahuillet, tgrabiec: i have been wondering if we should remove the non reg, reg variants completely 11:14 < penberg_home> to make sure we get proper testing coverage 11:15 < penberg_home> s/proper/better/g 11:15 <@ahuillet> penberg_home : honestly, I don't know why you wrote them in the first place. :) 11:15 < penberg_home> ahuillet: historical reasons probably. 11:15 < penberg_home> no idea. 11:15 <@ahuillet> lag... grmbl 11:15 < penberg_home> but 11:15 <@ahuillet> I can't do QoS on the router here, but my sister's usage of the line would warrant it 11:16 < penberg_home> on x86 isa, it can produce smaller code 11:16 < penberg_home> which is good 11:16 < penberg_home> which is why i haven't removed them yet 11:16 <@ahuillet> as I said above, I am not completely convinced this is an advantage in all cases 11:16 <@ahuillet> not to do reg,reg I mean 11:17 < penberg_home> maybe 11:17 < penberg_home> it's all hand-waving until we start to measure things 11:17 < penberg_home> but I guess I'm fine with getting rid of them 11:17 < penberg_home> from testing pov 11:25 < vegard> how do I get patch numbers with git-send-email ? 11:26 < vegard> ah, I need to do it in git-format-patch 11:26 < vegard> thx. 11:27 < penberg_home> :) 11:27 < penberg_home> -n 11:27 < vegard> patch series coming up! 11:27 < penberg_home> woohoo 11:28 -!- penberg [n=penberg@cs181039060.pp.htv.fi] has joined #jato 11:28 < penberg> vegard: so now I am on the merge machine 11:28 < penberg_home> penberg: oh hi there! 11:28 < penberg_home> nice to finally meet you 11:28 < penberg> penberg_home: thnx. 11:29 < penberg> you too 11:29 < vegard> now I actually realized "penberg" would be an english word as well 11:29 < vegard> mountain of pens..? 11:29 < penberg> berg is an English word? 11:30 < vegard> well, you have iceberg 11:31 < penberg_home> true 11:31 < vegard> apparently the english "berg" is just an abbreviation for "iceberg" 11:31 < vegard> oh 11:31 < vegard> I forgot to Cc the maling list 11:31 * vegard Waaaah 11:31 < vegard> do I resend? 11:32 < penberg_home> vegard: sure 11:32 < penberg_home> i don't see them here either 11:32 < vegard> aha 11:32 < vegard> I forgot to send them at all :D 11:32 < penberg_home> :-) 11:32 < penberg_home> I see you are an experienced patch submitter 11:32 < penberg_home> ;) 11:33 < vegard> ah I prefer git request-pull 11:33 < vegard> so harder to screw something up 11:33 < penberg> :) 11:33 -!- mode/#jato [+o penberg] by ChanServ 11:36 < vegard> ahuillet: if you want to see how to implement missing VM classes, see patches 3-5 that I just sent to the mailing list. it's very easy 11:37 <@penberg> hmm 11:37 <@penberg> indentation in java code is 4 spaces 11:37 < vegard> well, it's easy to set it up so that the VM recognizes them, it's harder to actually implement them ;) 11:37 <@penberg> but what the hell, i'll just let it slide 11:37 < vegard> it is!? 11:37 <@penberg> the vm_register_native() thing seriously needs a table-driven approach 11:37 < vegard> 4 spaces? so I need to press backspace 4 times instead of 1 if I want to remove an indentation? 11:38 <@penberg> but what the hell, I'll let that slide too 11:38 <@penberg> it's Saturday, after all! 11:38 <@penberg> vegard: that's the way it is 11:38 < vegard> :-( 11:38 <@penberg> tabs in java are just madness. 11:38 < vegard> *sulk* 11:38 <@penberg> i don't know what that means 11:38 <@penberg> configure your idez 11:39 <@penberg> I was thinking of going for 2 spaces 11:39 <@penberg> but then I dropped the idea because I'm too lazy to reindent everything 11:39 <@penberg> vegard: would 2 spaces be better? 11:39 <@ahuillet> + NOT_IMPLEMENTED; 11:39 <@ahuillet> this macro print __func__ among other things right? 11:39 <@ahuillet> better not use that in the monoburg rules then, 'cause it will not make sense 11:40 <@ahuillet> use a explicit "EXPR_FVALUE not implemented" 11:40 <@penberg> yup 11:40 <@penberg> I'll merge the patch anyway, though. 11:40 <@penberg> so make an incremental one on top of it 11:40 <@penberg> i'll fix the java indentation issues by hand 11:40 <@ahuillet> Subject: [PATCH 2/6] insn-selector: don't use fixed var for EXPR_ARRAYLENGTH 11:40 <@ahuillet> what about that? 11:40 <@penberg> in the diff, like real men do. 11:40 <@ahuillet> what's the reason for the change? 11:41 < vegard> um, it reuses eax 11:41 < vegard> when you pass x.length as an argument to a function 11:41 <@ahuillet> so what? 11:42 <@ahuillet> your patch seems correct, but I'd like to know what was broken before 11:42 < vegard> 22:24 < tgrabiec> i know what's wrong :) 11:42 < vegard> 22:24 < tgrabiec> rule: reg: EXPR_ARRAYLENGTH(reg) is broken 11:42 < vegard> 22:24 < tgrabiec> it should not put the result in EAX ! 11:42 <@ahuillet> is it a bugfix, acleanup... 11:42 <@penberg> bugfix probably? 11:42 < vegard> see the log from yesterday: http://folk.uio.no/vegardno/jato-irc-logs/2009-06-26.txt 11:42 <@penberg> vegard: summary, pls. 11:42 <@penberg> so I can amend that to the patch description 11:42 <@penberg> tgrabiec: can I have your signoff for the patch too? 11:42 < vegard> we were producing an "idiv %eax" instruction 11:42 < vegard> which is nonsensical, it divides eax by eax 11:43 <@ahuillet> ok makes sense 11:43 <@ahuillet> in fact we should probably output no result in a fixed var 11:43 <@penberg> ahuillet: yes 11:43 <@ahuillet> but always make a MOV_REG_REG between the fixed var and a new var we get_var'd 11:43 <@penberg> that's historical too 11:43 <@penberg> from the time before a register allocator. 11:43 <@ahuillet> makes sense, and this gets by Acked-By then. :) 11:43 <@ahuillet> the register allocator behaves relatively poorly with fixed vars anyway 11:44 <@ahuillet> when we have finer liveness it will help a bit but still, we have to reduce fixed var usage to the bare minimum 11:44 <@penberg> ok, I need to eat now 11:44 <@penberg> vegard: patch description, pls 11:44 <@penberg> tgrabiec: signoff pls 11:45 <@penberg> will merge after lunch 11:45 <@penberg> the series looks good to me 11:45 <@ahuillet> same here 11:45 <@ahuillet> (about the series, not lunch, it's too early for lunch) 11:46 <@ahuillet> vegard : so what's the situation with PrintTest, are we back to pre-cafebabe situation? 11:48 < vegard> mh, not quite yet 11:48 < vegard> and not with that patch series anyway 11:48 < vegard> I have an additional two patches here 11:48 < vegard> they gets us much further than before 11:48 <@ahuillet> :) 11:48 < vegard> but something still goes wrong... 11:49 <@ahuillet> I love how jato is about getting as far as possible 11:49 < vegard> :) 11:51 < vegard> so the ones that fail now are ExceptionsTest and PrintTest 11:51 <@ahuillet> for what reason? 11:51 <@ahuillet> maybe we do need some FPU ;) 11:52 < vegard> compile_error: Failed to compile method `getChars' in class `java/lang/String', error: -22 11:52 < vegard> I wonder why? 11:53 < vegard> 22 is EINVAL 12:00 < vegard> we do have || operator working, right? 12:02 <@ahuillet> no idea. :) 12:02 <@ahuillet> I have never tested it myself, that I am sure of 12:03 < penberg_home> vegard: add a regression test. 12:03 < penberg_home> probably not working 12:03 < penberg_home> i don't think we support all branch conditions 12:05 < penberg_home> vegard: so what's the status of your jamvm removal branch? 12:05 < penberg_home> i'd like to pull it 12:06 < vegard> penberg_home: status is: priority is getting ahuillet back to work :) 12:06 < penberg_home> ok 12:06 < penberg_home> i'll just nuke jamvm myself then 12:06 < penberg_home> if you're okay with that 12:06 < vegard> sure. the main problem is getting rid of include/vm/vm.h 12:06 < vegard> just so you know. 12:06 < vegard> I think the rest is pretty much git rm -rf jamvm 12:07 <@penberg> vegard: sure, I'll let you deal with that 12:07 <@penberg> I'll just nuke the bulk of it 12:12 < vegard> from what I can see, the branch stuff works properly 12:12 <@penberg> ok nuked 12:13 <@penberg> vegard, ahuillet, tgrabiec patches against AUTHORS would be appreciated. 12:14 < tgrabiec> penberg: what do i do to sign-off ? 12:15 < tgrabiec> is it enoght for me to tell you that or should i reply the email? 12:15 < tgrabiec> *is it enough 12:16 <@penberg> tgrabiec: yup, but no need to worry about that 12:16 <@penberg> I merged the patch already ;) 12:16 <@penberg> and it's vegard's fault anyway for taking a patch without a signoff :-X 12:17 < vegard> um 12:17 < vegard> I didn't take it 12:17 < vegard> I just wrote it down 12:17 < vegard> or, more like, tgrabiec said what was wrong and I changed the code accordingly 12:19 < tgrabiec> it's fine for me 12:20 <@penberg> yup 12:23 <@penberg> tgrabiec: ok, I moved some code under lib/ now. 12:23 < tgrabiec> penberg: i'm confused, what code ? 12:24 <@penberg> radix-tree.c and such 12:24 <@penberg> we talked about this a month ago or so 12:24 < vegard> list ? 12:24 <@penberg> vegard: optimizing compiler? 12:24 <@penberg> vegard: that too 12:24 < vegard> penberg: yeah, if I put if(true && false) it is likely that javac does constant-expression folding, no? 12:25 <@penberg> no it's not 12:25 < vegard> no..? 12:25 <@penberg> javac doesn't do optimizations traditionally 12:25 < vegard> won't it turn that into if (false) and remove the branch entirely? 12:25 <@penberg> to keep bytecode as close to the java code as possible. 12:25 <@penberg> for debugging reasons. 12:25 < vegard> oh. really..? O.o 12:25 <@penberg> i'm not saying it doesn't do it 12:25 <@penberg> I'm just saying, don't expect javac to do it. 12:26 < vegard> oh right. but this is probably safer, no? 12:29 < vegard> convert() for OPC_INVOKESTATIC fails... 12:29 < penberg_home> btw, I figured out how to do a really simple gc 12:29 < penberg_home> use malloc + free 12:29 < penberg_home> so the only thing we need to do is the garbage collection part. 12:30 < penberg_home> not the actual _allocator_ part. 12:51 < vegard> hm. 12:51 < vegard> did we never invoke static native methods before? 12:52 < tgrabiec> vegard: i think we did 12:52 < tgrabiec> i'm sure we did 12:52 < tgrabiec> for example vmthrowable_fill_in_stack_trace 12:53 < vegard> INVOKESTATIC is calling into cafebabe to find the bytecode for java/lang/VMSystem.arraycopy() 12:53 < vegard> but it doesn't exist of course, since it's a native 12:54 < penberg_home> static native methods? 12:54 < penberg_home> surely we call those. 12:55 < vegard> why does jit want to compile a native method :-( 12:55 < penberg_home> do we have VMSystem class now? 12:55 < vegard> yes, I do 12:55 < penberg_home> and it's flagged as 'native' 12:56 < vegard> yes 12:56 < penberg_home> mine doesn't have arraycopy() method there 12:57 < vegard> vm_register_native("java/lang/VMSystem", "arraycopy", 12:57 < vegard> &native_vmsystem_arraycopy); 12:57 < vegard> static native void arraycopy(Object src, int srcStart, Object dest, int destStart, int len); 12:58 < vegard> I compiled it and classloader loads it 12:58 < penberg_home> hmm 12:58 < penberg_home> hmm 12:59 < penberg_home> well, as you know, we do support that 12:59 < penberg_home> so it has to be some bug of yours :) 12:59 < vegard> oh. 12:59 < vegard> maybe it needs to be public as well? 12:59 < penberg_home> hmm 12:59 < penberg_home> so if it's private 13:00 < vegard> yes! that was it. 13:00 < penberg_home> it's invokespecial, not invokestatic, I suppose? 13:00 < vegard> it's invokestatic 13:00 < penberg_home> in the bytecode? 13:00 < vegard> yes 13:00 < penberg_home> so that's a bug in jato then 13:01 < penberg_home> a bug in vm_method_is_native(), I suppose? 13:02 < penberg_home> no idea how that's possible 13:02 < penberg_home> i think it's a bug in cafebabe, actually. 13:02 < penberg_home> ooh 13:02 < penberg_home> "#pragma once" 13:02 < penberg_home> does that actually help? 13:02 < vegard> where? cafebabe includes? 13:03 < penberg_home> yes. 13:03 < penberg_home> vegard: but do you agree it has to be a bug in cafebabe? 13:03 < vegard> does it actually help? don't know. 13:03 < vegard> I think I had some reason for using it. don't remember now. 13:04 < vegard> I don't think it's a bug in cafebabe 13:05 <@penberg> so how do you explain the difference? 13:06 <@penberg> does the _classfile_ have ACC_NATIVE for the method? 13:06 < vegard> public vs. non-public? 13:06 < vegard> let me generate the HIR again 13:06 <@penberg> yes. 13:06 <@penberg> vegard: it _has_ to be a bug in cafebabe 13:06 <@penberg> there's no way jato tries to compile a method with ACC_NATIVE set. 13:07 < vegard> but why would it be? we shouldn't even be calling cafebabe if it's not native 13:07 < vegard> sorry, if it's native* 13:07 <@penberg> what are you talking about 13:07 <@penberg> of course we load the _class_ with cafebabe 13:07 <@penberg> and use vm_is_native() to determine whether the method is native or not 13:08 <@penberg> and if that returns true, jato will _not_ attempt to compile the damn thing! 13:08 <@penberg> check jit_magic_trampoline 13:08 <@penberg> that is, unless someone is using java_trampoline directyl... 13:08 <@penberg> oh but that's static 13:08 <@penberg> so that cannot happen 13:08 < vegard> the HIR.. :) 13:09 <@penberg> so it absolutely must be a bug during classloading 13:09 <@penberg> the hir doesn't matter 13:09 <@penberg> the first call goes through the trampoline 13:09 < vegard> but we load the flags directly from the bytecode. 13:09 < vegard> and the test is simply an & 13:09 <@penberg> yes, I do realize that. 13:09 < vegard> so the bug has no place to be :P 13:10 <@penberg> (1) does the _classfile_ hava ACC_NATIVE or not 13:10 <@penberg> (2) can you trace cafebabe and prove that it loads the flag properly 13:10 <@penberg> (3) can you see that the same flag is set in the trampoline 13:11 <@penberg> one possibility is that the fixup code is broken 13:11 <@penberg> but I don't really see how that's possible either 13:11 <@penberg> because in order for jato to compile(), we must go through a trampoline 13:11 <@penberg> and the fixup code doesn't inject trampolines to the call-sites 13:12 <@penberg> so it has to be either that cafebabe never reads the ACC_NATIVE flag properly 13:12 <@penberg> or that there's some memory corruption somewhere that clears the flag. 13:12 < vegard> ok, so _with_ the public, we get a bytecode that is INVOKE 13:12 <@penberg> there's no "INVOKE" bytecode ;) 13:12 < vegard> eh? 13:12 <@penberg> EXPR_INVOKE? 13:12 < vegard> yes 13:13 <@penberg> yes, that would be invokestatic 13:13 <@penberg> (bytecode) 13:13 < vegard> seems to be exactly the same 13:13 < vegard> I'll make a proper diff 13:14 <@penberg> or invokespecial 13:14 <@penberg> invokespecial and invokestatic *both* use EXPR_INVOKE 13:14 <@penberg> vegard: well, I'd rather you investigate classloading phase first to see if ACC_NATIVE appears or not. 13:15 < vegard> ok 13:15 <@penberg> don't get me wrong, I'm happy if you can prove me wrong with this one 13:15 <@penberg> but I seriously doubt I am wrong 13:15 <@penberg> vegard: are you sure it's trying to compile the right method? 13:15 <@penberg> maybe the method resolution part is wrong? 13:15 <@penberg> I'd suspect in the package private case we use invokespecial 13:15 <@penberg> which could explain the difference 13:16 <@penberg> so are you absolutely certain jato tries to compile the _native_ method, and not something completely different? 13:18 < vegard> method is native? arraycopy yes 13:18 < vegard> method is native? arraycopy yes 13:18 <@penberg> ok. 13:18 < vegard> that's what I inserted in vm_method_is_native() 13:18 < vegard> _without_ the public 13:18 <@penberg> ok. 13:18 < vegard> exactly the same with the public 13:18 <@penberg> and you can see in compile() that jato tries to compile 'arraycopy'? 13:19 < vegard> yes 13:21 <@penberg> ... 13:21 <@penberg> that's _impossible_ :) 13:21 <@penberg> just read what jit_magic_trampoline() does. 13:23 < vegard> I'll call abort() and get a stack trace... 13:23 <@penberg> thnx 13:23 <@penberg> I'm not sure why we don't _always_ abort in case of compilation error. 13:23 <@penberg> it's not as we can recover from it or anything. 13:24 < vegard> um well, we can 13:24 < vegard> we need to throw some exception or error 13:25 <@penberg> uhm? 13:25 <@penberg> if _compilation fails_ 13:25 <@penberg> it's obviously not safe at all to throw an exception. 13:25 < vegard> we have to do it 13:25 <@penberg> yes, _out of memory_ is a separate issue. 13:25 < vegard> bytecode can be loaded from anywhere 13:26 <@penberg> I'm not following you at all 13:26 <@penberg> we should abort() 13:26 < vegard> we shouldn't abort() on invalid bytecode sequence..? 13:26 <@penberg> yes we should 13:26 < tgrabiec> i think it depends on the cause, but basically we should throw an exception 13:26 <@penberg> invalid bytecode sequence should be rejected by the _verifier_ 13:26 <@penberg> which we don't have. 13:26 <@penberg> no! 13:27 <@penberg> the _verifier_ should take care of it 13:27 <@penberg> if you let invalid bytecode to the _compiler_ you're dead anyway 13:27 <@penberg> i think the verifier semantics are explained in the jvm spec. 13:28 <@penberg> furthermore, if the operand stack is messed up 13:28 <@penberg> we already _abort()_ 13:28 < tgrabiec> penberg: but what about multithreaded environments, shouldn't we allow other threads to continue ? 13:29 <@penberg> tgrabiec: if _compilation_ fails? 13:29 <@penberg> why would we want to do that? 13:29 <@penberg> tgrabiec, vegard: you do realize that if compilation fails, that's a serious bug in the vm? 13:29 <@penberg> yes, we happen to fail often at the moment 13:30 <@penberg> (because we don't support the instruction set fully) 13:30 <@penberg> but as long as we have no way to fallback to interpretation or anything 13:30 <@penberg> compilation errors should just stop the program. 13:30 <@penberg> tgrabiec: there's on guarantee what happens if one thread dies 13:30 <@penberg> as a matter of fact 13:31 <@penberg> letting the rest of the program continue could even result in even more serious problems 13:31 <@penberg> data loss, etc. 13:31 < tgrabiec> ok, i agree with you - we can die if we have a strong bytecode verifier 13:31 <@penberg> so in short: yes, invalid bytecode must be rejected by the verifier. 13:31 < vegard> yes. 13:31 <@penberg> but no, we must not attempt to recover from compilation errors 13:31 < vegard> but no invalid (user) input should ever make the program crash :) (in principle) 13:31 <@penberg> unless we have a fallback mechanism. 13:31 <@penberg> vegard: yes, but you're talking about the verifier now. 13:32 < vegard> yes 13:32 < vegard> which we don't have :-p 13:32 <@penberg> yes, and that's fine for the time being. 13:33 <@penberg> vegard: btw, AFAIK, writing the verifier is tricky indeed. 13:34 < vegard> ok 13:34 < vegard> just discovered something weird 13:35 < vegard> I'm compiling runtime/classpath/java/lang/VMSystem.class 13:35 <@penberg> vegard: yeah? 13:35 < vegard> but the output goes to regression/java/lang/VMSystem.clas 13:35 <@penberg> :) 13:35 < vegard> and I'm in top-level jato/ 13:35 < vegard> :-( 13:35 <@penberg> -d ? 13:35 < vegard> aha. what's that? 13:36 < vegard> AHA 13:36 < vegard> grrr :) 13:36 <@penberg> destination 13:36 < vegard> that needs to be fixed... 13:36 < vegard> in mainline. 13:37 < vegard> ok so it actually finds the method regardless of public and non-public... 13:38 < vegard> it was just a messed up classpath with two (potentially different) .class files in different locations 13:38 < vegard> ugh, so ugly. 13:39 <@penberg> :) 13:48 <@penberg> vegard: http://github.com/penberg/jato/commit/473bb65c6a61a487da6c1356bb92b170e2a445a1 13:48 <@penberg> vegard,tgrabiec: can either of you send me a patch that makes compilation failures abort() ? 13:51 < tgrabiec> vegard: shall you? 13:59 <@ahuillet> back 14:23 < vegard> I won't 14:23 < vegard> I'm trying to unblock the rest of you :) 14:24 < vegard> in other words: I'm trying with all I can to make PrintTest not fail 14:24 < vegard> and if you have something else to do meanwhile, that's also great ;) 14:24 < tgrabiec> vegard: ok, i'll send it then 14:26 < tgrabiec> penberg: hmm, i think we already call die() in compile_error() don; we ? 14:27 < penberg_home> tgrabiec: but apparently we don't die 14:27 < penberg_home> vegard: ? 14:27 < tgrabiec> how come ? 14:28 < tgrabiec> vegard: can you tell me how to reproduce ? 14:34 < vegard> no :/ 14:34 < vegard> not anymore 14:34 < vegard> actually 14:34 < vegard> no, not anymore 14:34 < tgrabiec> so, case solved ? 14:34 < vegard> pretty much, yeah. get a different crash now :) 14:45 < vegard> hm. 14:46 < vegard> we seem to have a problem with glibj.zip 14:46 < vegard> it already includes many of the VM* classes. 14:46 < vegard> so 14:46 < vegard> it looks like we actually shouldn't be including them ourselves. 14:47 < vegard> I need to study the classpath vm integration guide. 14:47 < tgrabiec> makes sense. we want to override some though, like VMthrowable for example - we add a new 'backtrace' array field for storing the stack trace and it's VM specific thing 14:48 < vegard> yes. ok, that's what the integration guide said as well 14:49 < vegard> we can override the ones we want/need to override. 14:49 < tgrabiec> yes, but no need to override all 14:49 < vegard> *nod* 14:55 <@penberg> vegard: btw, I reformatted VMSystem 14:55 < vegard> ah, yes 14:55 < vegard> um, we should actually chuck them. 14:55 < vegard> since I was being stupid and didn't realize they'd already be defined in glibj.zip 14:57 <@penberg> vegard, tgrabiec: so as far as I can tell 14:57 <@penberg> the only real regression from the cafebabe merge is the fact that PrintTest doesn't work 14:58 < vegard> minus all the ones we already fixed :) 14:58 <@penberg> and that's really just because the JIT doesn't support enough bytecodes to initialize everything properly? 14:58 < tgrabiec> penberg: end array store check 14:58 <@penberg> tgrabiec: right. 14:58 < tgrabiec> (we can't get array element class yet) 14:58 <@penberg> I guess that's simple enough to fix, though? 14:58 < vegard> and displaying exceptions 14:58 <@penberg> displaying exceptions is related to S.o.println() 14:58 < tgrabiec> displaying exceptions is blocked by obj->class == NULL now 14:58 <@penberg> ok ok 14:58 <@penberg> just saying, that from my point of view, the major regressions are fixed 14:59 <@penberg> I don't really consider PrintTest breakage a regression. 14:59 <@penberg> because the problem is in Jato 14:59 <@penberg> not cafebabe 14:59 <@penberg> and Jato never supported PrintTest properly anyway 14:59 <@penberg> (yes, it did work because it got support from jamvm) 14:59 * penberg is a happy camper! 14:59 <@penberg> excellent work guys 15:00 <@penberg> I'm sure ahuillet doesn't see it quite the same way 15:00 <@penberg> but lets not worry about that too much ;) 15:00 <@ahuillet> so, let me read what I have to disagree about 15:00 <@ahuillet> and that's really just because the JIT doesn't support enough bytecodes to initialize everything properly? 15:00 < vegard> penberg: maybe I can bring you back to earth with a new, nice trace.txt? :) 15:00 <@ahuillet> which ones? 15:01 <@penberg> ahuillet: that's for you and vegard to find out. 15:01 <@penberg> vegard: sure. 15:01 <@ahuillet> penberg : ok, I don't quite agree, indeed. :) 15:01 <@penberg> ahuillet: :) 15:02 <@penberg> well, you can't please everyone, can you?-) 15:02 < vegard> http://folk.uio.no/vegardno/trace.txt 15:02 <@ahuillet> so we crash in write() 15:02 <@ahuillet> mmh 15:02 <@penberg> vegard: you probably want to narrow that bug down? 15:03 <@ahuillet> that's definitely a regression from my PoV :) 15:03 < vegard> penberg: the sad thing is that the code is just a Number n = new Integer(64) :( 15:03 < vegard> but I can try to narrow it down further 15:04 <@penberg> so if you turn that into a simple test that just does "Number n = new Integer(64)" 15:04 <@penberg> it crashes? 15:04 < vegard> it _is_ that simple test 15:04 <@penberg> aah I see 15:04 <@penberg> yeah, you do need to narrow it down further 15:05 <@penberg> vegard: but it's crashing in synchronize_clinit()? 15:06 < vegard> yes 15:06 <@penberg> vegard: I don't see any trampolines there 15:06 < vegard> running the of some class, don't know which 15:06 < vegard> oh, it calls vm_class_run_clinit() 15:16 < vegard> ok, I managed to narrow it down somewhat 15:16 < vegard> by extending Number 15:17 < vegard> the only method that ever compiles is java/lang/Number. 15:17 <@ahuillet> what's going wrong? 15:17 < vegard> it crashes 15:17 < vegard> and I have no idea _where_ it crashes 15:18 < vegard> because the addresses in the backtrace are not jit code and not C code... 15:18 <@penberg> trampoline 15:18 < vegard> http://folk.uio.no/vegardno/trace2.txt 15:18 <@penberg> trace the trampolines 15:18 < vegard> I have the "jit_magic_trampoline:" output if that's what you mean? 15:18 <@penberg> yup 15:18 <@penberg> 0x0827eda1 15:18 <@penberg> isn't that in text? 15:19 <@ahuillet> sure looks like it 15:19 <@penberg> yup 15:19 < vegard> uh, what? where'd you get that number? 15:19 <@ahuillet> maybe it's bit far away though 15:19 < vegard> ah. 15:19 <@penberg> SIGSEGV at EIP 0827eda1 while accessing memory address 0827eda1. 15:19 <@ahuillet> it's far from main that's sure 15:19 <@penberg> ok, so it's a bogus address 15:20 <@penberg> jump to some bad address 15:20 <@ahuillet> it can't be in .text if it SIGSEGV on it anyway 15:20 < vegard> but even the caller, 0x441fcdf is nowhere to be found! 15:20 <@penberg> ok so 0x441FCDF 15:20 <@ahuillet> vegard : disassemble it in GDB, see if it makes sense 15:20 < vegard> hang on. 15:21 < vegard> oh! 15:21 < vegard> gdb gives a better stacktrace. it says it crashed in free() 15:21 < vegard> #0 0xb7c4c572 in free () from /lib/tls/i686/cmov/libc.so.6 15:21 < vegard> #1 0x08061dd5 in cafebabe_class_init (c=0x1, s=0x973f408) 15:21 <@penberg> vegard: I think valgrind took over free() 15:21 <@ahuillet> that's not the same place, man 15:21 <@ahuillet> but yes, valgrind wraps free() 15:22 <@penberg> hmm 15:22 < vegard> http://pastebin.com/m9a5e1ff 15:22 <@penberg> so the value of 'c' is bogus too? 15:23 < vegard> yes, but valgrind should still be able to tell us that it _was_ free (it should say something like VG_replace_free() or something in the stacktrace) 15:23 < vegard> penberg: hm. 15:24 < vegard> that looks seriously wrong 15:24 < vegard> why would it call cafebabe_class_init() using a trampoline? 15:24 < vegard> it just doesn't make sense... 15:24 <@ahuillet> I assume it wants to jump higher 15:25 < vegard> the only places where we even call cafebabe_class_init() is from the classloader (load_from_file() and load_from_zip()) 15:26 <@penberg> well, I don't see synchronize_clinit() 15:26 <@penberg> vegard: but anyway, I'd try to narrow it down to specific part of 15:27 < vegard> ah, yes. second problem :) 15:27 < vegard> I have no clue why java.lang.Number. is so huge 15:27 < vegard> my .java file says it should be 4-5 lines tops 15:27 <@penberg> vegard: look at gnu classpath code. 15:27 <@penberg> vegard: array initializerw. 15:27 <@penberg> static final char[] digits = { 15:28 < vegard> oh. it's just a long series of stores... 15:28 < vegard> cool. 15:28 <@penberg> yes 15:28 < vegard> then this could be quite easy to reproduce after all. 15:28 <@penberg> so write an ArrayTest or something? 15:29 <@penberg> interesting. Jato has 27 KLOC 15:30 < vegard> cool! now I have a 10-line test case that crashes in the _exact_ same way, _without_ loading any classes but the test-case class! 15:31 <@penberg> :-) 15:31 <@penberg> arrays are not widely tested 15:31 <@penberg> so I wouldn't be surprised there are some bugs there. 15:31 < vegard> yeah. it's probably my array allocation code :-B 15:31 <@penberg> :-X 15:32 <@penberg> yeah that would make sense 15:32 <@penberg> some nasty buffer overrun or something 15:32 <@penberg> yesterday this was the food channel 15:32 <@penberg> now this is the collaborative debugging channel 15:32 < vegard> so going from 29 to 30 array elements in that digits array 15:32 < vegard> makes it crash. 15:33 < vegard> /* XXX: Use the right size... */ 15:33 < vegard> res = zalloc(sizeof(*res) + 8 * count); 15:33 <@penberg> vegard: do we support the -jar option? 15:34 < vegard> penberg: no. 15:34 <@penberg> ok 15:34 <@penberg> probably not too difficult to implement? 15:35 < vegard> don't think so 15:35 <@ahuillet> interronegative questions are one of the worst offenders to quality of communication. :) 15:36 <@penberg> ahuillet: :) 15:36 <@penberg> how come? 15:36 <@ahuillet> because the correct answer to agree with the question is "no" 15:36 <@ahuillet> probably not too difficult to implement? 15:36 <@ahuillet> don't think so 15:36 <@ahuillet> theorically this means vegard thinks it's not too difficult to implement 15:37 <@ahuillet> but it's very confusing 15:37 < vegard> no, that's what I meant 15:37 <@ahuillet> I know it is, it's just not possible to be 100% certain in that case 15:37 <@ahuillet> we had this problem you and I yesterday too IIRC 15:37 < vegard> hehe 15:37 <@penberg> well 15:37 < vegard> I went through the same thing with my girlfriend 15:37 <@penberg> I'll try harder next time ;) 15:38 <@penberg> aah 15:38 <@penberg> load class from zip not implemented 15:38 < vegard> apparently in norwegian we reply the opposite of what you'd do in english to these kinds of questions 15:39 <@ahuillet> ah ? 15:39 <@ahuillet> "isn't it difficult to write computer programs ?" 15:39 <@ahuillet> what would you answer in norwegian to say it is not difficult to write computer programs ? 15:40 < vegard> I'd say no... 15:40 < vegard> hm. 15:40 <@ahuillet> same in english, french, german, ... 15:40 < vegard> let me find the example :) 15:41 <@penberg> http://pastebin.com/m7530d23f 15:41 <@penberg> yay! 15:45 < vegard> ok so it's probably EXPR_ARRAY_DEREF that's wrong? 15:46 < tgrabiec> just to let you know, putfield/getfield/putstatic/getstatic are also affected by the 'iinc' bug. and it is not only about x++, basically any expression can be executed between EXPR_CLASS_FIELD (or others) are pushed onto and popped from mimic-stack. I'm working on patches now 15:47 <@penberg> vegard: hmm, store to array probably? 15:48 <@penberg> i think DEREF is the load part? 15:48 < vegard> there's no ARRAY_STORE ? 15:49 < vegard> the HIR has STORE: store_dest: ARRAY_DEREF: ... 15:50 <@penberg> ok ok 15:50 <@ahuillet> mmh 15:50 < tgrabiec> ARRAY_DEREF is used both for store and load 15:50 <@ahuillet> store into a deref? 15:50 <@penberg> so there's a STMT_STORE(ARRAY_DEREF, ...) somewhere? 15:50 <@ahuillet> it really doesn't feel good to me :) 15:50 <@penberg> agreed, s/ARRAY_DEREF/ARRAY_REF/ perhaps? 15:50 < vegard> maybe this is why we crash, then? :) 15:51 <@ahuillet> penberg : we have to check whether it's our bytecode->HIR conversion that is wrong 15:51 <@ahuillet> or if it's just the HIR naming 15:51 <@penberg> it's the hir naming probably 15:51 < vegard> there's a STMT_STORE(array_deref) rule in insn-selector.. 15:51 <@penberg> tgrabiec: ok, cool. 15:51 <@penberg> yup 16:05 < vegard> mov %ebx,(%edi,%esi,1) 16:05 <@ahuillet> omg not that 16:05 < vegard> that 1 is the multiplier, right? 16:05 <@ahuillet> yes 16:05 <@ahuillet> scale, it's called 16:05 < vegard> sorry. 16:05 < vegard> why the "omg not that" ? :) 16:06 <@ahuillet> you don't imagine how many crashes I debugged on lines like this. 16:06 <@ahuillet> I had a great mov %ebx,(%edi, %ebx, 1) (regallocator bug) that ate several days of my life 16:06 < vegard> lol :) 16:06 < vegard> so this particular line is output by the STMT_STORE(array_deref, reg) 16:07 <@penberg> vegard: yeah, stay away from ahuillet unless you're prepared to debug some crazy bugs :) 16:07 <@ahuillet> vegard : any problem with it? 16:07 < vegard> order of the regs are src, base, index, scale 16:07 <@ahuillet> edi should contain the base address of the array, and esi the scaled index 16:07 < vegard> this is a byte array, so scale = 1 is correct at least 16:07 <@ahuillet> (IIRC we always scale = 1) 16:07 < vegard> yup 16:08 < vegard> actually the scale can be bigger for ints and such 16:09 <@ahuillet> k 16:09 <@ahuillet> so what's wrong with this? 16:10 < vegard> array stores with large indexes seems to crash 16:11 <@ahuillet> + /* Test for expression double evaluation bug */ 16:11 <@ahuillet> tgrabiec : so what is it? 16:12 < vegard> ahuillet: this is my testcase: http://pastebin.com/m389f89e8 16:13 <@penberg> vegard: how large is large? 16:13 <@ahuillet> SIGSEGV at EIP b80f17c4 while accessing memory address 0000001c. 16:13 <@ahuillet> that? 16:13 < vegard> penberg: around 30 seems to be enough to trigger it 16:14 < vegard> it seems to cause some kind of corruption 16:14 < vegard> ahuillet: hm, not quite what I get here, but it coudl very well be the same. 16:14 <@ahuillet> it crashes at +28, so that's the 29th number 16:14 <@penberg> ok so the indexing is probably just wrong then? 16:14 <@ahuillet> assuming the base address is 0 16:14 < vegard> it crashes in different ways depending on how many elements the array has ;) 16:14 <@ahuillet> man the code is huge 16:14 < vegard> ahuillet: yep :) 16:15 <@ahuillet> 2200 LIR instructions !! 16:15 < vegard> think it could be a reg-alloc bug? ;) 16:15 < vegard> (oops, I just suspected your code again. sorry!) 16:15 <@penberg> hey 16:15 <@ahuillet> [ 597 ] 0x08ca07f5: 89 5d f8 mov %ebx,-0x8(%ebp) 16:15 <@penberg> I did write parts of the register allocator too! 16:15 <@ahuillet> here is some code 16:15 <@ahuillet> SIGSEGV at EIP b7fd57c4 while accessing memory address 0000001c. 16:15 <@ahuillet> here's the crash 16:15 <@ahuillet> 'bit strange, no? 16:15 <@penberg> ebp corrupted? 16:16 < vegard> um, that's a different address..? 16:16 <@ahuillet> vegard : yeah..; 16:16 <@penberg> :) 16:16 <@ahuillet> (gdb) where 16:16 <@ahuillet> #0 0xb7f087c4 in ?? () 16:16 <@ahuillet> #1 0x00000005 in ?? () 16:16 <@ahuillet> #2 0x00000000 in ?? () 16:16 <@ahuillet> now that's stack corruption. 16:16 <@penberg> yes! 16:16 < vegard> yes. it crashes in different ways depending on the size of the array :) 16:16 < vegard> ok, so I know about at least one thign that is wrong: 16:17 < vegard> array_deref: EXPR_ARRAY_DEREF(reg, reg) 16:17 <@ahuillet> yeah? 16:17 < vegard> - select_insn(s, tree, imm_reg_insn(INSN_ADD_IMM_REG, sizeof(struct vm_object) + sizeof(uint32_t), base)); 16:17 < vegard> + select_insn(s, tree, imm_reg_insn(INSN_ADD_IMM_REG, sizeof(struct vm_object), base)); 16:17 <@penberg> relic from jamvm? 16:17 < vegard> yes 16:17 <@penberg> right. 16:17 < vegard> but it doesn't really fix the crash :) 16:17 <@penberg> well again 16:18 <@ahuillet> I would not expect it to suffice :) 16:18 <@penberg> if we treat the array always as 32-bit 16:18 <@penberg> then big enough indices just overwrite random parts of memory 16:18 <@ahuillet> valgrind's gonna take ages on this... 16:18 <@penberg> so does it work if you make it 'int' ? 16:18 <@penberg> the array 16:18 <@ahuillet> where's the array stored btw? 16:18 <@ahuillet> does it go on the stack like I'd expect it to? 16:18 < vegard> penberg: it doesn't work for int either. 16:19 < vegard> ahuillet: no, no. we allocate it with malloc() 16:19 <@ahuillet> oh. 16:19 <@ahuillet> so writing slightly past the bounds won't corrupt the stack 16:19 < vegard> res = zalloc(sizeof(*res) + 8 * count); 16:20 < vegard> that's the allocation 16:20 < vegard> the sizeof(*res) == sizeof(struct vm_object) 16:20 <@penberg> 8 * count? so you're always allocating enough memory for 64-bits? 16:20 <@penberg> vegard: where is the array length stored, btw? 16:20 < vegard> penberg: yes. hence the comment: XXX: Use the right size... :) 16:20 <@penberg> ok, that's fine then 16:20 < vegard> res->array_length = count; 16:20 <@penberg> so 16:20 < vegard> in struct vm_object 16:21 <@penberg> what if this is not related to arrays at all? 16:21 <@penberg> if the code size is above PAGE_SIZE? 16:21 <@ahuillet> penberg : ? 16:21 <@penberg> so can you crash jato if you simply generate tons of adds or something? 16:21 < vegard> hm, I can try. 16:21 <@penberg> oh forget it 16:21 <@penberg> we handle that properly 16:22 <@penberg> i think we had a page_size limit in the past 16:22 <@penberg> but __alloc_exec() seems to do the right thing here 16:22 <@penberg> hmm 16:22 <@penberg> spills? 16:22 <@penberg> would explain stack corruption, wouldn't it? 16:22 <@ahuillet> ?? 16:23 <@ahuillet> could, yes 16:23 <@ahuillet> would... why? 16:23 <@penberg> not enough memory for spill slots? 16:23 <@penberg> or we're spilling to wrong location? 16:23 <@penberg> vegard: (and yes, this is "arthur's code" again) 16:23 <@penberg> ;) 16:24 <@ahuillet> not enough memory for spill slots? 16:24 <@ahuillet> we sure have a lot of intervals. 16:24 < vegard> yup 16:25 < vegard> it crashed with an array of size 1 and just a lot of adds 16:25 <@penberg> ok, so it's not related to arrays. 16:25 <@penberg> just big code. 16:25 <@ahuillet> good to know 16:25 <@penberg> I am putting my cents on spill code. 16:25 <@ahuillet> grmbl 16:25 <@penberg> ahuillet: well, hey, I don't _know_ what's going on 16:25 <@penberg> but 16:25 < vegard> it could be still related to arrays 16:26 <@penberg> if we fail to allocate enough memory on the stack 16:26 <@penberg> for all spill slots 16:26 <@penberg> we would corrupt the stack 16:26 <@ahuillet> why is it always my fault? 16:26 <@penberg> likewise 16:26 <@ahuillet> why do you hate me so much? 16:26 <@ahuillet> I'm not talking to you 16:26 <@ahuillet> ever 16:26 <@ahuillet> again 16:26 <@penberg> if we calculate the wrong index somehow 16:26 <@penberg> we corrupt the stack 16:26 <@penberg> ahuillet: :) 16:26 <@penberg> well 16:26 <@penberg> it's not that I hate you 16:26 <@penberg> I just like to see you suffer. 16:26 < vegard> http://nopaste.com/p/aKEaUI4cD 16:26 <@penberg> :-) 16:26 < vegard> that's the one that crashes as well 16:27 <@ahuillet> vegard : what if you drop the array ? 16:27 < vegard> but if I replace x[0] with just 'x' (and make x a plain int), it doesn't crash after all 16:27 <@ahuillet> okkk 16:27 <@ahuillet> I would have expected that 16:27 <@ahuillet> vegard : even with *a lot* of adds? 16:27 < vegard> I'll try, one sec :) 16:28 <@penberg> ahuillet: not sure if a lot of immediate adds generate any register pressure though? 16:28 <@ahuillet> [ ? ] 0x0a6e2006: 81 ec b0 44 00 00 sub $0x44b0,%esp 16:28 < vegard> lol :) that's big.. 16:28 <@ahuillet> 17kB 16:28 <@ahuillet> big but most definitely not huge 16:28 <@penberg> wow 16:28 <@penberg> vegard surely has some monster programs to run 16:28 <@ahuillet> [ 2 ] 0x0a6e2025: 65 8b 35 f4 ff ff ff mov %gs:0xfffffff4,%esi 16:29 < vegard> ok, 8 * 256 adds coming up... 16:29 <@penberg> ahuillet: exception handling 16:29 <@ahuillet> ??!! 16:29 <@ahuillet> what the hell does that even mean? 16:29 <@penberg> loads from gs segment are per-thread 16:29 <@penberg> load from gs segment at address 0xff... 16:29 <@penberg> it's a per-thread variable. 16:29 <@ahuillet> oh 16:29 <@penberg> that's what GCC generates too 16:29 <@ahuillet> ok, TLS is implemented like this 16:29 <@ahuillet> interesting 16:29 <@penberg> if you put "__thread" attribute to a variable. 16:29 <@penberg> ahuillet: yup 16:30 < vegard> ok, the 2048 adds still ran fine. 16:30 <@penberg> the gs segment is mapped per-thread 16:30 <@penberg> vegard: how much does it need to reserve on stack? 16:30 < vegard> vegard@ubuntu:~/jato/regression$ du -sh trace3.txt 16:30 < vegard> 39M trace3.txt 16:30 < vegard> one sec, it takes a bit to find ;) 16:31 * ahuillet laughs 16:31 <@penberg> aww 16:31 < vegard> [ ? ] 0x0c4e9006: 81 ec 50 80 00 00 sub $0x8050,%esp 16:31 <@penberg> don't we have -Xtrace:asm or something? 16:31 <@penberg> interesting. 16:31 < vegard> and it generated virtual registers up to r6171 afaics 16:32 <@ahuillet> hurray for linear-scan.c! 16:32 < vegard> no, wrong. r2061 16:32 < vegard> 6171 was the number of IR instructions 16:32 <@ahuillet> hurray for linear-scan.c! 16:32 <@ahuillet> still. 16:32 < vegard> haha, I look forward to doing jato stress testing ;-) 16:33 < vegard> generating invalid bytecodes... and huge programs 16:33 <@ahuillet> eax: 00000064 ebx: 0a6e3016 ecx: 0a6e1b08 edx: 0a6e15a8 16:33 <@ahuillet> esi: 0a6e1b08 edi: 0000001c ebp: bfbc2924 esp: bfbc2844 16:33 < vegard> but for now we need to tackle a 30-big array initialisation that is needed for java.lang.Number to work ;) 16:33 <@ahuillet> all those 0xa6xxxxx addresses are code addresses 16:33 <@ahuillet> something's really wrong here. 16:34 < vegard> we could diff the asm output of a 29 vs a 30-long array? 16:36 <@ahuillet> technically feasible, but what are you going to get, I don't know 16:36 < vegard> well, something changes there 16:36 < vegard> doing the diff will highlight the differences 16:36 < vegard> um 16:37 < vegard> ...Mr. Tautology ;) 16:37 <@ahuillet> :) 16:38 <@ahuillet> man the HIR for this initialization is so big 16:38 <@ahuillet> we can do it in like 30 instructions 16:39 <@ahuillet> array_index: [value int 0x62] 16:39 <@ahuillet> what is that? 16:39 <@ahuillet> the array is of size 30 < 0x20 16:39 <@ahuillet> and we index 0x62 16:40 <@ahuillet> the array we create has size 0x64 apparently 16:40 <@ahuillet> in the code I mean 16:40 <@ahuillet> for 30 bytes... 16:42 < vegard> oh 16:42 < vegard> is it possible that we don't allocate enough space for the instructions? 16:42 <@ahuillet> ok I shut up 16:42 < tgrabiec> ahuillet: the regression test you asked about tests for the bug fixed in the commit that it follows 16:42 < vegard> machine code I mean 16:42 <@ahuillet> the array is 100bytes 16:42 <@ahuillet> so everything's fine here 16:42 <@ahuillet> vegard : that's what penberg suggested earlier 16:42 < vegard> ah right 16:42 < vegard> oops :) 16:42 <@ahuillet> I'll try to disassemble my "stack" 16:43 <@ahuillet> see if he was right 16:44 <@ahuillet> (gdb) disassemble 0xbf92ad64 16:44 <@ahuillet> No function contains specified address. 16:44 <@ahuillet> ?!?! 16:44 <@ahuillet> I did not ask him for a function! 16:44 <@ahuillet> ah ok 16:44 <@ahuillet> I need a range 16:45 <@ahuillet> nope, there's no code on my "stack" 16:45 <@ahuillet> 0xbf92ad64: 0x00000005 0x00000000 0x00000001 0x00000000 16:45 <@ahuillet> 0xbf92ad74: 0x00000001 0xb7e969ac 0xbf92ae64 0xb7f327c4 16:45 <@ahuillet> 0xbf92ad84: 0x00000005 0x00000000 0x00000001 0xbf92ae50 16:46 < vegard> bah, too much changed for that diff to really be useful.. 16:47 <@ahuillet> I'm not surprised 16:48 <@ahuillet> mmh 16:50 < vegard> it could still be the monoburg rules, right? 16:50 <@penberg> vegard: code addresses change which makes the diff unusable 16:50 <@penberg> i usually just sed them away 16:51 < vegard> that's what I did 16:51 < vegard> but register assignments changed too much too 16:51 <@penberg> aah 16:53 <@ahuillet> I'm doing step by step execution. 16:54 <@ahuillet> Breakpoint 3, vm_object_check_array (obj=0x950eb08, index=27) at include/vm/class.h:51 16:54 <@ahuillet> 51 return vmc->name && vmc->name[0] == '['; 16:54 <@ahuillet> (gdb) 16:54 <@ahuillet> Continuing. 16:54 <@ahuillet> Program received signal SIGSEGV, Segmentation fault. 16:55 <@ahuillet> so between two calls to vm_object_check_array index 27 16:57 < tgrabiec> does it help to disable convertion of array check ? 16:57 <@ahuillet> 0x0805c7f3 in vm_class_get_field_recursive (vmc=0x1c, name=0x9b05b08 "P^°\t", type=0x1c
) at vm/class.c:346 16:57 <@ahuillet> 346 } 16:57 <@ahuillet> (gdb) finish 16:57 <@ahuillet> Run till exit from #0 0x0805c7f3 in vm_class_get_field_recursive (vmc=0x1c, name=0x9b05b08 "P^°\t", type=0x1c
) at vm/class.c:346 16:57 <@ahuillet> here is your crash guys. 16:57 < vegard> vmc=0x1c..? 16:57 <@ahuillet> don't ask why but that's where it crashes. :) 16:57 <@penberg> tgrabiec: what's -Xtrace:bytecode-offset btw? 16:58 < tgrabiec> it enable bytecode offsets in LIR and assembly 16:58 <@penberg> tgrabiec: isn't it enabled by default? 16:58 < tgrabiec> i think it's in -Xtrace:jit 16:58 < tgrabiec> but no, not by default, at least last time i checked 16:58 < tgrabiec> it could be 16:58 <@penberg> but hmm 16:59 <@penberg> it's not useful without -Xtrace:, right? 16:59 < vegard> I don't think it is. -Xtrace:jit and -Xtrace:asm gives different assembly listings 16:59 <@penberg> i think we should just nuke it. 16:59 <@penberg> vegard: aah 16:59 <@penberg> wonder why 16:59 < vegard> probably because of -Xtrace:bytecode-offset? 16:59 < vegard> I mean the asm is the same, but the output format.. :) 17:00 < vegard> ahuillet: so why is the vmc=0x1c? 17:00 < tgrabiec> vegard: yes - i think we could enable -Xtrace:bytecode-offset unconditionally 17:00 <@ahuillet> I don't know, I'm still running step by step to find where the problem is 17:00 <@ahuillet> Breakpoint 6, vm_object_check_array (obj=0x9af5b08, index=27) at include/vm/class.h:51 17:00 < vegard> 0x1c = 28, btw. 17:00 <@ahuillet> I make it up to here, that's for sure 17:01 <@penberg> vegard: so now you can learn from the master. 17:01 <@ahuillet> 0x09af7011: call 0x805c7f3 17:01 <@penberg> no, I am not talking about ahuillet. 17:01 <@ahuillet> how many times do we call this function? 17:01 <@penberg> I am talking about myself, obviously. 17:01 <@ahuillet> when do we call it? what does it do? 17:01 <@penberg> secret sauce: (1) blame ahuillet's code, (2) be convicing enough, (3) wait for a bug fix in a totally unrelated piece of code to arrive in your inbox 17:01 < vegard> check_array? 17:02 < vegard> penberg: lol... 17:02 <@ahuillet> because this function crashes. 17:02 <@ahuillet> the crash is in here, clearly 17:02 < tgrabiec> i don't know if it's relevant in your case, but convert_array_store/load is affected with the expression-double-evaluation bug that was in checkcast 17:02 <@penberg> well, in all fairness, you might be just seeing the side effects of the smashed stack 17:02 < vegard> ahuillet: check_array is called from STMT_ARRAY_CHECK 17:03 <@ahuillet> penberg : I trace the code till this function 17:03 <@ahuillet> and once in this function my stack goes wild 17:03 <@penberg> tgrabiec: this is a smashed stack. 17:03 <@penberg> ahuillet: ok. 17:03 <@penberg> which function is that 17:03 <@penberg> vm_class_get_field_recursive? 17:03 <@ahuillet> yes 17:03 <@ahuillet> 0x09af7011: call 0x805c7f3 17:03 <@ahuillet> mmh 17:03 <@ahuillet> +44 17:03 <@ahuillet> wth 17:04 < vegard> heh 17:04 < vegard> that looks a tad odd. 17:04 <@ahuillet> omfg 17:04 <@ahuillet> http://paste.pocoo.org/show/O0Ah1p8UdV2yC8iAkrdy/ 17:04 <@ahuillet> no wonder the stack is corrupted. 17:05 <@ahuillet> 0x0805c7f3 : pop %ebx 17:05 <@ahuillet> look where it jumps 17:05 < vegard> this is gcc code 17:05 < vegard> are you suggesting a broken gcc, in this rather straight-forward code? 17:05 <@penberg> ahuillet: jumping or calling? 17:05 <@ahuillet> no 17:06 <@ahuillet> penberg : same thing, we never return :) 17:06 <@ahuillet> vegard : it's a problem with jato 17:06 <@ahuillet> vegard : we jump/call to a pop instruction 17:06 <@ahuillet> stack gets all funky 17:06 < vegard> ahuillet: from where? 17:06 <@penberg> ahuillet: from where? 17:06 <@penberg> haha 17:06 <@ahuillet> hence the corruption of the registers with code addresses 17:06 <@ahuillet> we do that from the generated assembly 17:06 < vegard> okay. that's pretty cool. 17:06 <@ahuillet> for the array vegard gave me the code for 17:07 <@ahuillet> no it's not 17:07 <@ahuillet> we jump in the middle of nowhere. 17:07 <@ahuillet> it's not cool :) 17:07 < vegard> it's pretty cool that you figured it out so far :) 17:07 <@penberg> hmm 17:07 <@penberg> we generated the code? 17:07 <@ahuillet> yes 17:07 <@ahuillet> I'm going to run it once again and look at the assembly 17:08 <@penberg> vm_object_alloc() ? 17:08 <@penberg> array_size_check() ? 17:08 <@ahuillet> ? 17:08 <@penberg> vm_object_alloc_native_array() 17:08 <@penberg> these are the functions we call from generated code 17:08 <@penberg> multiarray_size_check... 17:08 <@penberg> see insnselector 17:09 <@ahuillet> 0x08c8b011: call 0x805c7f3 17:09 <@ahuillet> that's after storing the 27th element ok 17:09 <@ahuillet> 0x08c8af81: call 0x805e7f3 17:09 <@ahuillet> now look at this line 17:09 <@ahuillet> that's after storing the 26th 17:09 <@ahuillet> and, no doubt, the 25 elements before 17:09 <@ahuillet> we do -0x2000 on the jump address somehow 17:10 < vegard> the code itself changes? 17:10 <@ahuillet> don't ask me why 17:10 < vegard> at run-time? 17:10 <@penberg> ahuillet: fixup code? 17:10 < vegard> or are these _different_ call instructions? 17:10 <@ahuillet> but we do call 0x805c7f3 and should do call 0x805e7f3 17:10 <@ahuillet> penberg : want me to patch it in gdb and see if things run? 17:11 <@penberg> ahuillet: I mean fixup_direct_calls() in jit_magic_trampoline() 17:11 <@penberg> maybe that's broken? 17:11 <@penberg> that _changes_ call insns at run time 17:11 <@ahuillet> select_insn(s, tree, rel_insn(INSN_CALL_REL, (unsigned long) vm_object_check_array)); 17:11 <@ahuillet> is there any change going on for this? 17:11 <@ahuillet> 'cause we're obviously doing something very wrong here 17:12 < tgrabiec> ahuillet: what's wrong with that ? 17:12 <@ahuillet> tgrabiec : it works for the 26th first elements. 17:12 <@ahuillet> starting at the 27th, the address is corrupted in the generated code 17:12 <@ahuillet> I don't know why, I'm just stating facts here 17:13 <@penberg> ahuillet: so did you try disabling the fixup_direct_calls() ? 17:13 <@ahuillet> waaaait a minute :=) 17:13 < tgrabiec> well, i'd suggest you try to disable _convertion_ of array check code, it's broken, i think it breaks asm code and who knows what else 17:13 <@penberg> ahuillet: ok ! 17:13 <@penberg> ;) 17:13 <@ahuillet> penberg : fixup_direct_calls has nothing to do with this if I understand correctly 17:13 <@ahuillet> * This fixes relative calls generated by EXPR_INVOKE. 17:13 <@ahuillet> we're not using EXPR_INVOKE here 17:13 <@ahuillet> select_insn(s, tree, rel_insn(INSN_CALL_REL, (unsigned long) vm_object_check_array)); 17:13 <@penberg> yes, you're right. 17:14 <@ahuillet> tgrabiec : breaks by removing 0x2000 at some random address ? 17:14 <@ahuillet> tgrabiec : that's an interesting way of breaking code :) 17:14 < vegard> if it isn't what you think it is, it has be what you don't think it is... 17:14 < tgrabiec> hmm, perhaps i should join the conquest :) how to reproduce ? 17:14 <@ahuillet> tgrabiec : http://pastebin.com/m389f89e8 17:14 <@ahuillet> grab this 17:15 <@ahuillet> now, I'm gonna help you a bit with gdb 'cause it took me some time 17:15 <@ahuillet> you need one breakpoint on compile() 17:15 <@ahuillet> 6 breakpoint keep y 0x0805e802 in vm_object_check_array at include/vm/class.h:51 17:15 <@ahuillet> stop only if index==27 17:15 <@ahuillet> and this one 17:15 <@ahuillet> b vm_object_check_array if index==27 17:16 <@ahuillet> then, use "si" and "disassemble $eip $eip+100" 17:16 <@ahuillet> and have fun watching your stack explode 17:16 <@ahuillet> it breaks "some time" after the breakpoint 17:16 <@ahuillet> (gdb) disassemble $eip $eip+150 17:16 <@ahuillet> at about +150 after the return 17:17 < vegard> can't you put a data breakpoint on the CALL instruction? 17:17 <@ahuillet> you'll see a borked call 17:17 <@ahuillet> vegard : yes, but what for? 17:17 < vegard> to be notified of when that instruction changes? 17:17 < vegard> so you can do a backtrace? 17:17 <@ahuillet> hahaha, nice try 17:17 <@ahuillet> how do I know the address of the call instruction? 17:17 <@ahuillet> how do I know it before it's too late I mean 17:17 < vegard> it's in the assembly listing 17:17 <@ahuillet> mmh, I need a bigger scrollback buffer 17:18 < vegard> try screen :) 17:18 <@ahuillet> bigger scrollback? 17:19 < vegard> :defscrollback 100000 17:19 < vegard> withuot the :, in .screenrc should do it :) 17:19 <@ahuillet> [ 171 ] 0x0a865011: e8 dd 77 7f fd call 0x000000000805c7f3 17:20 <@ahuillet> it's already fsckedup in the assembly listing. 17:20 <@ahuillet> [ 171 ] 633: call_rel $0x805e7f3 17:20 <@ahuillet> but it's ok in the LIR output 17:21 <@ahuillet> so something goes wrong in between 17:21 < vegard> heh, that's serious weird. 17:23 < tgrabiec> hmm, the byte listing shows the offset, and the address after call is current + offset 17:23 <@ahuillet> tgrabiec : ? 17:23 < vegard> 0x0a865011 vs. $0x805e7f3 17:23 < vegard> ? 17:23 < vegard> oops 17:23 < vegard> 0x000000000805c7f3 vs. $0x805e7f3 17:24 < vegard> that's the $2000 difference, right? 17:24 < tgrabiec> aha 17:24 < vegard> that's pretty hard to spot :) ahuillet must have good eyes. 17:24 * penberg is unable to decide whether to watch a documentary on tv or you guys debugging this sucker. 17:24 <@ahuillet> 0x2000 17:24 < vegard> oh, he saw it with the +44 thing 17:24 < vegard> right. 17:24 <@penberg> (both are equally entertaining) 17:24 <@ahuillet> 0x805c807 vs e807 here 17:25 <@penberg> so the address changes from HIR to the assembly? 17:25 <@ahuillet> one address changes 17:25 <@ahuillet> by 0x2000 17:25 <@penberg> EXPR_INVOKE? 17:26 <@penberg> in the HIR? 17:26 <@ahuillet> oh crap 17:26 <@ahuillet> after a recompilation things have changed.. 17:26 <@ahuillet> there's no INVOKE here penberg 17:26 <@penberg> so where does the address appear on HIR? 17:27 <@ahuillet> it does not !!! 17:27 <@ahuillet> it's not a call made by the java ode 17:27 <@ahuillet> *code 17:27 <@ahuillet> it's a call we emit in jato 17:27 * penberg is confused 17:27 <@ahuillet> select_insn(s, tree, rel_insn(INSN_CALL_REL, (unsigned long) vm_object_check_array)); 17:27 <@ahuillet> it's this call 17:27 <@penberg> the the address does not appear in the HIR 17:27 <@ahuillet> that goes fubar 17:27 <@penberg> so it changes from where to where? 17:27 <@penberg> LIR to assembly? 17:28 <@ahuillet> yup 17:28 <@ahuillet> errr no 17:28 <@ahuillet> errr yes 17:28 <@penberg> :) 17:28 <@ahuillet> LIR OK, assembly NOK 17:29 <@penberg> so 17:29 < vegard> ahuillet: beware, though. gcc is dumber than you'd expect. the arguments printed in a function can change if that function doesn't need those parameters anymore and you called some functino that modified them in-between 17:29 < vegard> s/gcc/gdb/ 17:29 <@ahuillet> ?! 17:29 <@penberg> can you confirm that with a printf() as well? 17:29 <@penberg> in emit_call? 17:29 <@ahuillet> vegard : I don't use gdb here for anything else than disassembling the code I am running 17:29 < vegard> ahuillet: oh ok. 17:29 <@ahuillet> penberg : that's what I tried to do but adding the printf changed all addresses 17:29 <@ahuillet> so I have to find what's wrong this time again 17:30 <@penberg> right 17:30 <@penberg> ahuillet: unfortunately 17:30 <@ahuillet> ok, it still crashes after the 27th element 17:30 <@penberg> ahuillet: after insn selector 17:31 <@penberg> ahuillet: the only things we do is liveness analysis, reg alloc, spilling, and then emission ;) 17:31 <@ahuillet> 0x098bd011: call 0x805c833 17:31 <@ahuillet> ok, definitely the same problem 17:31 <@ahuillet> only the address changes 17:31 < vegard> 0x0a9c7011: call 0x805deae 17:31 < vegard> I got that.. is it the same problem? 17:31 <@ahuillet> static void __emit_call(struct buffer *buf, void *call_target) 17:31 <@ahuillet> printf("call target %#x\n", call_target); 17:31 <@ahuillet> see this? 17:32 <@ahuillet> I added it in __emit_call 17:32 <@ahuillet> 100 call target 0x805e833 17:32 <@ahuillet> I don't see 0x805c833 here. 17:32 <@ahuillet> so the call target is ok 17:32 <@ahuillet> int disp = call_target - buffer_current(buf) - CALL_INSN_SIZE; 17:32 <@ahuillet> maybe this is wrong now 17:33 <@penberg> int hmm 17:33 <@penberg> that's 32-bits, so it's probably fine 17:33 <@ahuillet> can't be :) 17:34 <@penberg> ahuillet: how so? 17:34 < vegard> oh! 17:34 <@ahuillet> penberg : because it's the only explanation 17:34 <@ahuillet> we do nothing after this 17:34 <@ahuillet> I know the correct address makes it to __emit_call 17:34 <@ahuillet> and then in memory it's wrong 17:35 <@ahuillet> so __emit_call does something wrong 17:35 <@penberg> or the address is changed after the emission 17:35 <@penberg> branch fixup? 17:35 <@ahuillet> by whom? 17:35 <@ahuillet> doing a -0x2000 in there? 17:35 <@ahuillet> it's a constant offset 17:35 <@ahuillet> we're not writing an arbitrary value where clearly substracting 0x2000 17:36 <@penberg> ok 17:36 <@penberg> but i can't see the bug here. 17:37 < vegard> maybe it's because the output code crosses a page in size? 17:37 <@ahuillet> 0x99fa011: 0xe8 0x25 0x28 0x66 0xfe 17:37 <@ahuillet> 0xfe662825 is the displacement we are writing 17:38 <@penberg> so that's a negative offset 17:38 <@penberg> text comes before heap, no? 17:38 <@ahuillet> 0805c836 17:38 <@ahuillet> the sum yields this 17:38 <@ahuillet> see the c836 ? 17:38 <@ahuillet> should be e836 17:38 <@ahuillet> the disp value is wrong. :) 17:39 <@penberg> so emit_imm32() is broken? 17:39 <@penberg> hmm 17:39 <@penberg> or the calculation hmm 17:39 <@penberg> ok 17:39 <@penberg> something pretty curious here 17:40 <@penberg> call_target is "void *" 17:40 <@penberg> buffer_current returns "void "* 17:40 < vegard> buffer is reallocated 17:40 <@penberg> but we store it in "int" 17:40 < vegard> in generic_buffer_expand() 17:40 < vegard> could that be it? 17:40 <@ahuillet> it's reallocated? that explains why I could not find it in my __emit_call traces 17:40 < vegard> it would change the output of buffer_current() 17:40 <@penberg> OUCH 17:40 <@penberg> yes, as soon as the buffer grows big enough 17:40 <@penberg> all the previous offsets are wrong. 17:41 <@ahuillet> errr 17:41 <@penberg> I wonder who fucking idiot wrote that function. 17:41 <@ahuillet> we need to fix up all offsets 17:41 <@penberg> ...oh. 17:41 <@penberg> it was me 17:41 <@penberg> :) 17:41 < vegard> ...lol 17:41 <@ahuillet> I'm surprised it works at all :) 17:41 <@penberg> ahuillet: how come? 17:41 < vegard> that was one hard bug to find... 17:41 <@penberg> realloc() doesn't _always_ relocate. 17:41 <@ahuillet> yeah, but here it reallocated 17:41 <@penberg> yup. 17:41 <@ahuillet> and it still worked for the 26 first elements 17:41 < vegard> what's the size threshold that triggers a realloc? 17:42 <@ahuillet> with lots of function calls as well. 17:42 <@ahuillet> ok so 17:42 < vegard> you realloc it for _every_ byte you add!? 17:42 <@ahuillet> we can go the intel DRI way and keep a list of places where to add the offset 17:42 <@ahuillet> (relocation) 17:42 <@ahuillet> or we can find another way (allocate a second chunk and do a jmp) 17:43 <@penberg> vegard: actually the bug is in expand_buffer_exec() 17:43 < vegard> penberg: aha. right. sorry :-P 17:44 <@penberg> what the hell 17:44 <@penberg> no one is calling alloc_exec() 17:44 <@ahuillet> confirmed 17:44 < vegard> aw 17:44 < vegard> what a waste of nice documentation ;) 17:45 <@ahuillet> vegard : so once it's fixed we have println working? 17:45 < vegard> the only documentated function in the project is unused ;) 17:45 <@penberg> cu->objcode = __alloc_buffer(&exec_buf_ops); 17:45 <@penberg> page_size = getpagesize(); 17:45 <@penberg> size = ALIGN(buf->size + 1, page_size); 17:46 <@penberg> so the reallocation happens when we cross page boundary 17:46 < vegard> ahuillet: if I knew where to set the initial allocation size I coudl just up that as a workaround and see if it works now :) 17:46 <@penberg> so I was right all along. 17:46 <@ahuillet> erm... ? 17:47 <@ahuillet> penberg : fix the bug :) 17:47 <@ahuillet> now I watch you suffer 17:47 <@penberg> hmmm. 17:47 * ahuillet reaches for a coke 17:48 <@ahuillet> penberg : so, how do we fix it? 17:49 <@penberg> two options: (1) no relocation and (2) backpatching. 17:49 < vegard> SIGSEGV at EIP 0806fdcc while accessing memory address 0806fdcc. 17:49 < vegard> sorry, nvm. 17:49 <@penberg> I don't think we can avoid relocation. 17:50 <@penberg> so I guess we can just first generate all the code 17:50 <@penberg> then go through the list of insn a second time 17:50 <@penberg> and only then emit all the CALLs 17:51 <@penberg> and branches. 17:51 <@ahuillet> penberg : all EIP-relative instructions 17:51 <@penberg> not even EIP relative. 17:51 <@penberg> absolute branches too 17:52 <@penberg> hmmmmmmmmm 17:52 <@ahuillet> oh yes, because we emit crap 17:52 <@penberg> any way to avoid relocation 17:52 <@ahuillet> all EIP relative CPU instructions. 17:52 <@ahuillet> so call rel, jmp rel, ... not too many, I think 17:52 <@ahuillet> this takes place at machine code level, not LIR 17:52 <@penberg> that's two passes, though. 17:52 <@ahuillet> emission in two passes yes 17:53 <@penberg> yup. 17:53 <@ahuillet> the other way is to look at the LIR 17:53 <@ahuillet> calculate an upper bound for the size 17:53 <@ahuillet> allocate that without zeroing out the pages 17:53 <@penberg> well, the thing is 17:53 <@ahuillet> and eventually realloc (shrink) the allocated block 17:54 < vegard> I multiplied size by 64 and it passes now :) 17:54 <@ahuillet> vegard :þ 17:54 < vegard> let's look at S.o.p 17:54 <@penberg> i'd like to be more space efficient for the allocated text. 17:54 <@penberg> so I wonder if we should just mmap() a huge buffer. 17:55 <@penberg> that will not be backed up by physical memory unless it's being used 17:55 <@penberg> then make sure only one thread at a time does code emission 17:55 <@ahuillet> penberg : same would occur for malloc() wouldn't it? 17:55 <@ahuillet> or has our teacher lied to us.? 17:55 <@penberg> and simply increment a pointer as we go on 17:55 <@penberg> ahuillet: we can't use malloc() here. 17:55 <@ahuillet> why? 17:55 <@penberg> we need physical pages so we can turn on the executable bit. 17:55 <@ahuillet> we can do that after code emission, can't we? 17:56 <@penberg> we now use posix_memalign() + mprotect() 17:56 <@ahuillet> so we emit in a malloc(654654654654654654) buffer 17:56 <@ahuillet> then shrink with realloc, and then place the x bit 17:56 <@ahuillet> bad idea? 17:56 <@penberg> then you make random parts of the heap executable as well. 17:56 < vegard> ugh, I really hate that solution :) 17:56 <@ahuillet> no, because you know the exact size of the buffer 17:56 <@penberg> ahuillet: but you waste memory at the end of the page 17:57 <@ahuillet> isn't memory allocated with page granularity anyway? 17:57 < vegard> why not do what ahuillet said, and create a linked-list "chain" of exec pages, by putting "jmp " at the end of each page? 17:57 <@ahuillet> and we're talking about 2kB wasted on average 17:57 <@ahuillet> on ARM or Microblaze it would be a problem, but here..? 17:57 <@penberg> vegard: because if it happens to be in a hotpath there's nothing you can do? 17:57 <@penberg> and it's a freaking branch there? 17:57 <@penberg> ahuillet: yes, I am thinking embedded specifically here. 17:58 <@ahuillet> ok, what page size do they have? 17:58 <@penberg> 4 K 17:58 <@ahuillet> how does the current approach not waste half of a page? 17:58 <@penberg> ahuillet: the current approach is crap 17:58 <@ahuillet> how does my suggestion change anything since in the end, you do a realloc() to the final exact size just like the current approach 17:58 <@ahuillet> I'd like to understand what I'm missing, here :) 17:58 <@penberg> yes, it doesn't change anything. 17:59 <@penberg> but it doesn't change the fundamental fact that the current approach is crap too. 17:59 <@ahuillet> you can't allocate half of a page anyway can you? 17:59 <@ahuillet> my suggestion aims at improving things. :) 17:59 <@penberg> yes, and my mmap() suggestion fixes the problem once and for all 17:59 <@ahuillet> so how does that work? 18:00 < vegard> I don't like just allocating a huge memory area. 18:00 < vegard> the size of that would be an arbitrary decision 18:00 <@ahuillet> vegard : you have two ways, allocate small and enlarge 18:01 <@ahuillet> or allocate big and shrink 18:01 <@ahuillet> in the first case you relocate 18:01 <@ahuillet> in the second you don't 18:01 <@ahuillet> and the decision is not arbitrary, there is an upper bound to a size of machine code emitted by one LIR instruction 18:01 <@ahuillet> not theorically, but there is in fact 18:01 < vegard> but standard realloc() or even mremap() isn't guaranteed to not relocate even if you shrink it? 18:01 <@ahuillet> I don't understand the question :) 18:02 < vegard> "or allocate a big and shrink" "in the second you don't [relocate]" 18:02 < vegard> how do you know that? 18:02 < vegard> besides, how do you know what's "big enough"? 18:02 <@ahuillet> and the decision is not arbitrary, there is an upper bound to a size of machine code emitted by one LIR instruction 18:02 <@ahuillet> number of LIR instructions * 20 bytes 18:03 <@ahuillet> or malloc(1GB), that works too :] 18:03 < vegard> it's disgusting :) 18:03 <@ahuillet> it surely is, but I don't even know if it works for now 18:03 <@ahuillet> note that relocating is maybe more appealing to our senses 18:03 <@ahuillet> but it's really disgusting as well 18:05 < vegard> why? 18:05 <@ahuillet> because you have to rewrite potentially a lot of addreses 18:05 <@ahuillet> that you have to keep in a list 18:05 <@ahuillet> so you build the list, browse the list, rewrite 18:06 <@ahuillet> performance wise it's not exactly nice, and very prone to breaking if you forget to add one of the values 18:06 <@ahuillet> anyway, what is the mmap solution? mmap /dev/zero? 18:06 <@ahuillet> IIRC that's what glibc does for big calloc() calls 18:06 < vegard> you can mmap anonymous memory, no need for /dev/zero :) 18:08 < vegard> hm. well, the performance overhead happens only if you have to relocate 18:08 <@ahuillet> which you do have to do 18:08 <@ahuillet> unless you overestimate 18:08 <@ahuillet> and we're back to my disgusting idea 18:09 < vegard> and if we re_al_locate that's probably going to be expensive already 18:09 <@ahuillet> anyway the way it gets fixed is pretty orthogonal to our work :) 18:09 <@penberg> I'm fixing it with mmap() 18:10 <@penberg> allocate a fixed region of 256 MB 18:10 < vegard> UGH. that's not going to be good for embedded you know :) 18:11 <@penberg> why? 18:12 < vegard> because virtual memory isn't free either 18:12 <@penberg> it's not as if they need to make it 256 MB 18:12 <@penberg> vegard: ? 18:12 < vegard> as in 18:12 < vegard> free of cost 18:12 <@penberg> which cost are you talking about? 18:12 <@ahuillet> as in everyone doesn't have 32bit addressing for virtual memory? 18:13 <@penberg> yes, I can see the problem with cases where there's no mpu 18:13 < vegard> it's just ugly... 18:13 <@penberg> vegard: ....... 18:13 <@penberg> what are the proposed alternatives? 18:13 <@penberg> branch at the end of a page? 18:13 <@ahuillet> ugliness isn't an argument in favor or against anything in computer programming 18:13 < vegard> back-patching! relocation. 18:13 <@ahuillet> nor in any field FWIW 18:14 <@penberg> vegard: that's fragile. 18:14 <@penberg> and adds a second pass to the code emission. 18:14 <@penberg> and 18:14 <@penberg> it doesn't fix the brain-damage of the current approach 18:14 <@ahuillet> did you know that liveness analysis was slower than register allocation btw? 18:14 <@penberg> and if you think about it more carefully 18:14 <@penberg> you will see that we _want_ functions to be tightly packed 18:15 <@penberg> preferably under few huge pages 18:15 <@penberg> to reduce tlb pressure 18:15 <@penberg> and make sure instruction cache contains relevant data. 18:15 <@penberg> vegard: so really, I would like some concrete arguments why my approach sucks 18:15 <@penberg> because converting it to use huge tables is straight-forward 18:15 <@ahuillet> it's ugly \o/ 18:16 <@penberg> I don't see the ugliness. 18:16 < vegard> what if we need more than 256MB for a huge app, then? 18:16 < vegard> it's an arbitrary limit 18:16 <@ahuillet> I don't have an ugliness measurement device for programs 18:16 <@ahuillet> all my beauty appreciation neurons are dedicated solely to women 18:17 < vegard> a scalable solution is less ugly 18:17 <@penberg> ok 18:17 <@penberg> so you do realize we're talking about _code_ here 18:17 <@penberg> so you're saying 18:17 <@penberg> what if we have an executable that's bigger than 256 MB 18:18 <@penberg> then we bump it up to 1 GB 18:18 <@penberg> there's _always_ a limit to text _anyway_ 18:18 <@penberg> so I don't see the point 18:18 <@penberg> hell, we can even tune that constant according to _physical_ memory on the machine 18:18 < vegard> I think it's a nice principle to not impose arbitrary limitations 18:18 <@penberg> hey 18:19 < vegard> of course you can say oh, but we have arbitrary limitations everywhere. the address space is 32 bits wide, isn't that arbitrary? 18:19 <@penberg> there _is_ a _arbitrary_ limitation _anyway_ 18:19 <@penberg> ahuillet: we're talking about _text_ here! 18:19 < vegard> Nice Programs Don't Allocate More Than They Need. 18:20 <@penberg> we don't _allocate_ anything 18:20 <@penberg> we mmap it 18:20 < vegard> we allocate virtual addresses 18:20 < vegard> it's possible to run out of those too. 18:20 <@penberg> hmm 18:21 <@penberg> that's not an issue, really. 18:21 <@penberg> we can always change my scheme to allocate, say 32 MB at a time 18:21 <@ahuillet> ahuillet: we're talking about _text_ here! 18:21 <@ahuillet> don't bring me into this :) 18:21 <@penberg> or whatever the size of a huge page is 18:21 <@penberg> ahuillet: oh, sorry ;) 18:23 < vegard> also remember, the more memory you allocate from the heap, the less is available for the stack 18:23 < vegard> um. not sure if that argument is valid 18:24 <@ahuillet> might I suggest for now we focus on being able to print things on screen and running small programs? 18:24 <@ahuillet> and worry about big programs and embedded platforms the day someone comes up with a use case combining both? 18:24 <@ahuillet> (and wants to write a patch) 18:25 < vegard> btw 18:25 < vegard> ah never mind. 18:26 < vegard> ahuillet: I'm working on it. 18:29 < vegard> we're crashing on a NULL deref now :) 18:29 <@ahuillet> not sure if this is good. :) 18:29 < vegard> gnu/classpath/SystemProperties.getProperty(Ljava/lang/String;Ljava/lang/S 18:29 < vegard> tring;)Ljava/lang/String; 18:31 < tgrabiec> omg, our mimic-stack & expression handling is sooo wrong 18:31 <@ahuillet> what have you found? 18:31 < tgrabiec> when something is pushed on mimic-stack, it should evaluate always the same 18:32 < tgrabiec> now, every arithmetic op overrides a temporary that is one of it's arguments 18:33 -!- ahuillet_ [n=user@239.34.83-79.rev.gaoland.net] has joined #jato 18:45 <@penberg> tgrabiec: http://www.laughingpanda.org/~penberg/jato-mmap-text 18:45 <@penberg> tgrabiec: seems to break exception handling code 18:45 <@penberg> tgrabiec: any idea why? 18:47 <@penberg> vegard: and the more i think about this, the more convinced i am of this approach 18:47 <@penberg> vegard: your suggested backpatching doesn't solve the inherent problem with relocation 18:47 <@penberg> vegard: that text is scattered all around heap 18:48 <@penberg> vegard: making instruction cache more likely to contain crap from time to time 18:48 <@penberg> (i.e. _data_ in the heap) 18:48 <@penberg> and makes using huge tables impossible 18:48 <@penberg> I'm not saying my solution is perfect 18:48 < vegard> huge tables? 18:48 <@penberg> but it's a step in the right direction. 18:48 <@penberg> vegard: yup, 1 MB pages on x86 IIRC 18:49 < vegard> um, you're not talking about TLB, are you? :P 18:49 <@penberg> vegard: which generates less tlb pressure 18:49 <@penberg> vegard: yes I am 18:49 < ahuillet_> ok, so when you mmap 256MB like you do, what happens? you actually eat 256MB of virtual memory and no physical memory until you write to it, right? 18:49 <@penberg> vegard: there's "hugetlbfs" or something in linux 18:49 < vegard> translation look-aside buffers? :) 18:49 <@penberg> vegard: where you can map huge pages in userspace 18:49 < ahuillet_> simply out of curiosity, how does that differ from the behavior of malloc? 18:49 <@penberg> ahuillet: yes. 18:49 <@penberg> vegard: yes. 18:50 <@penberg> ahuillet: malloc works that way as well 18:50 < ahuillet_> k 18:50 <@penberg> ahuillet: the point here is that we're packing text in sequential memory locations 18:50 < ahuillet_> yes yes, I understand 18:50 <@penberg> ahuillet: making sure all the data cached in that memory region is _text_ 18:50 <@penberg> ahuillet: yup, ok 18:50 <@penberg> tgrabiec: ping? 18:50 < ahuillet_> we grab a pool of memory and put all the code there 18:50 <@penberg> ahuillet: indeed. 18:50 < ahuillet_> that's what I had in mind since I don't like relocations 18:51 < ahuillet_> based on the little experience of it I had in nouveau :) 18:51 <@penberg> ahuillet,vegard: the biggest weakness is _releasing_ that memory 18:51 < vegard> but the pool has to be relocatable anyway, right? 18:51 <@penberg> if we decide to recompile methods at some point 18:51 <@penberg> vegard: nope. 18:51 < ahuillet_> recompile ? 18:51 <@penberg> ahuillet: yes, if we decide to recompile methods during runtime 18:51 <@penberg> ahuillet: because of optimizations or what not 18:51 < tgrabiec> penberg: i'm looking at it, but i need go in few minutes or some girl's gonna kill me :) 18:51 < ahuillet_> so it means throw away the code for the method and rewrite it 18:51 <@penberg> tgrabiec: oh that's fine ;) 18:51 < ahuillet_> so we have to... ok, I see the problem 18:51 < ahuillet_> either we don't free at all and it sucks 18:51 <@penberg> tgrabiec: I'm eating atm and will debug that then 18:52 <@penberg> ahuillet: exactly. 18:52 <@penberg> ahuillet: or we generate holes there 18:52 <@penberg> ahuillet: and have difficulties reusing the code 18:52 < ahuillet_> or we try to collect the memory previously eaten by the method but we will end up with fragmentation 18:52 < ahuillet_> and since we cannot move things around inside the pool 18:52 <@penberg> ahuillet: because we don't know the size up front 18:52 < ahuillet_> we're screwed 18:52 < vegard> isn't that a killer argument against the "big pool" idea? 18:52 < ahuillet_> vegard : killer argument is something that applies now or soon 18:53 <@penberg> vegard: not really. 18:53 < vegard> I mean, we're going to want to unload classes if they're no longer needed, no? 18:53 < ahuillet_> recompiling methods for optimization... we probably have 20 months before thinking about it 18:53 <@penberg> ahuillet: indeed 18:53 < vegard> not recompiling. unloading 18:53 <@penberg> vegard: when is class unneeded? 18:53 < ahuillet_> how do you know you have to unload a class? 18:54 < vegard> garbage collection? 18:54 <@penberg> vegard: ? 18:54 <@penberg> vegard: we just generated code that references the class 18:54 <@penberg> so no, we probably won't be doing class unloading any time soon either 18:54 < ahuillet_> you garbage collect data, not text?! 18:54 <@penberg> ahuillet: i think hotspot uses a gc for text too 18:54 <@penberg> I'm not 100% sure though 18:55 < vegard> well, it's possible that a class becomes unused 18:55 < ahuillet_> correctness = feature completeness >> optimization 18:55 < ahuillet_> this doesn't prevent us from thinking ahead, but garbage collecting text is too far away for us to see clearly I guess 18:55 < vegard> it's not primarily about garbage collecting _text_ 18:55 < vegard> it's about garbage collecting _classes_ 18:56 <@penberg> ahuillet, vegard: ok, well the patch is there, I am eating 18:56 <@penberg> whoever debugs it first wins a fabulous prize 18:56 < ahuillet_> what, it's buggy? 18:56 <@penberg> yeah breaks exception handling 18:56 < ahuillet_> apart from the exception stuff that tgrabiec is playing his relationship on right now 18:56 <@penberg> :) 18:57 <@penberg> I didn't mean tgrabiec this time 18:57 <@penberg> ahuillet, vegard: but you guys! 18:57 <@penberg> now I seriously must eat 18:57 <@penberg> getting evil eye from my gf 18:57 < ahuillet_> hey I debugged the issue 18:57 < ahuillet_> it was not fun 18:57 < vegard> :D 18:57 < ahuillet_> now I'm debugging MESA's SSE assembly code 18:57 < ahuillet_> it's even less funny 19:00 <@penberg> tgrabiec: probably is_native() is just broken onw 19:02 <@penberg> tgrabiec: yup, that fixed it 19:02 <@penberg> thanks for the help anyway 19:04 < vegard> O.o 19:04 < vegard> he's invisible now? 19:05 < vegard> penberg: and what about eating? :p 19:05 <@penberg> almost done 19:05 <@penberg> I am a multitasker, remember 19:06 <@penberg> vegard: about to push, pls test 19:06 <@penberg> ahuillet, vegard, tgrabiec: thanks for debugging this! 19:06 <@penberg> that's a nice bug from 2005! ;) 19:07 <@penberg> vegard: http://github.com/penberg/jato/commit/3724f41f598cef0ed2a5c1b3fbf00b241221e0df 19:08 < vegard> it sure was a tough one :) 19:08 < ahuillet_> whow, we even have a picture of penberg ! :) 19:08 < vegard> ahuillet is pretty good with gdb :) 19:08 < ahuillet_> yeah fortunately all bugs aren't like this 19:10 <@penberg> ahuillet_: if you register on gravatar.com, you'll face will show up in the github logs too :) 19:12 < vegard> you look a bit different from the pic on your homepage 19:12 < vegard> oh 19:12 < vegard> you switched that one too 19:12 <@penberg> it's a pretty recent photo actually 19:12 <@penberg> the one on my home page was from 2006 i think 19:13 <@penberg> and my gf always said i looked retarded in that one ;) 19:13 < vegard> but did you manage to look so different in short time? :) 19:13 < ahuillet_> oh, so penberg has a homepage 19:13 <@penberg> vegard: it's just the hair color 19:13 < vegard> no the shape of the face as well 19:13 <@penberg> haha 19:13 <@penberg> so you're saying I look fat, eh? 19:14 <@penberg> whaddya mean short time frame, btw? 19:14 <@penberg> something like three years? 19:14 < vegard> no, you changed that picture just a few months ago. 19:14 <@penberg> vegard: http://www.cs.helsinki.fi/u/penberg/pekka-pretty.png 19:14 <@penberg> vegard: which picture? 19:14 <@penberg> vegard: that's from 2006 19:14 < vegard> the gravatar one 19:15 < vegard> yup, that's the one 19:15 <@penberg> yeah yeah 19:15 <@penberg> but that's from 2006 19:15 < vegard> one day it was that one, and then another day it was the new one 19:15 <@penberg> yeah sure 19:15 < ahuillet_> vegard : what do you want, crossfading ? :P 19:16 < vegard> let's see.. in the 2006 one you have ears 19:16 < vegard> and teeth 19:16 <@penberg> i still do 19:16 <@penberg> they're just hidden under the hair now 19:17 < vegard> but seriously, it looks like you have puffier cheeks now. not saying you look fat. 19:18 <@penberg> thnx vegard 19:18 <@penberg> pekka the puff 19:19 < vegard> you won't find a guy as tough 19:19 <@penberg> probably 5 kg difference. 19:19 <@penberg> or something 19:19 <@penberg> vegard: but hey 19:19 <@penberg> vegard: just look linus' pictures 19:20 <@penberg> vegard: from early 1990s to later 1990s when he moved to states 19:20 <@penberg> so I think I am on track here. 19:20 <@penberg> right on track that is 19:27 < vegard> http://folk.uio.no/vegardno/jato-irc-logs/pisg.html 19:28 <@penberg> The loudest one was penberg_home, who yelled 3.0% of the time! 19:28 <@penberg> Another old yeller was penberg, who shouted 3.0% of the time! 19:28 <@penberg> bastard! 19:29 < vegard> you're not exactly improving your statistics either ;) 19:31 <@penberg> why do I appear on all of the crappy statistics? 19:31 <@penberg> penberg talks to him/herself a lot. He/She wrote over 5 lines in a row 120 times! 19:32 < vegard> because you wrote the largest number of lines in general, probably 19:35 < ahuillet_> ok, found a bug in MESA code 19:35 < ahuillet_> (yet another one...) 19:41 -!- penberg [n=penberg@cs181039060.pp.htv.fi] has quit ["leaving"] 19:50 < penberg_home> vegard: vm_object_alloc_string() 19:50 < penberg_home> vegard: signedness warnings 19:50 < vegard> yes. 19:50 < penberg_home> vegard: any suggestions how to fix it? 19:51 < vegard> cast to uint8_t* ? 19:51 < penberg_home> hmm 19:51 < vegard> alternatively, add another function that hides the cast :) 19:51 < penberg_home> i don't like casting. 19:51 < penberg_home> why not pass a void pointer there? 19:51 < penberg_home> it seems like you pass different kinds of buffers there? 19:51 < penberg_home> that is 19:51 < penberg_home> change the function signature to accept void * 19:51 < penberg_home> vegard: are you okay with that? 19:52 < vegard> no :) 19:52 < penberg_home> ok, why? 19:52 < vegard> we need unsigned because that's what the utf8 bytes from the classfile are 19:52 < penberg_home> you're passing a uint32_t pointer there 19:52 < vegard> no, uint8_t * 19:53 < vegard> we need the type safety, that's why I don't want void *. 19:54 < vegard> we need to get the warning if somebody (somewhat involuntarily) passes a vm_object* or something 19:54 < penberg_home> hmmm 19:54 < vegard> it's kind of ugly. 19:55 < penberg_home> well 19:55 < penberg_home> the point is 19:55 < penberg_home> we have "char *" _strings_ 19:55 < penberg_home> and pass those to the function 19:55 < penberg_home> why can't we use char * there then? 19:56 < penberg_home> for the utf-8 buffer too? 19:56 < penberg_home> oh well 19:56 < penberg_home> i think I'll call it a night 19:56 < penberg_home> but I want those warnings fixed 19:56 < penberg_home> obviously not high priority compared to the other things ;) 19:59 -!- ahuillet_ [n=user@239.34.83-79.rev.gaoland.net] has quit ["Leaving"] 19:59 -!- ahuillet_ [n=user@239.34.83-79.rev.gaoland.net] has joined #jato 20:12 -!- ahuillet_ [n=user@239.34.83-79.rev.gaoland.net] has quit ["Leaving"] 20:20 * tgrabiec is back 20:26 < penberg_home> tgrabiec: I figured it out. 20:26 < penberg_home> tgrabiec: just needed to fix is_native() 20:26 < tgrabiec> i can see :) 20:26 < tgrabiec> nice 20:26 < penberg_home> http://github.com/penberg/jato/commit/f61c9374a29aa0bc0fa5f983e506488bb36be88e 20:26 < penberg_home> I couldn't resist 20:27 < tgrabiec> oh, it's a good thing. 20:27 <@ahuillet> ok, I'm done with $ hexedit /usr/lib/xorg/modules/dri/libdricore.so 20:27 <@ahuillet> is there something for me here? less hardcore? 20:30 < penberg_home> vegard: something simple for ahuillet maybe? 20:30 < penberg_home> ahuillet: floating point support? 20:30 < penberg_home> ;) 20:32 <@ahuillet> manger 20:34 < vegard> um 20:34 < vegard> I could send you my latest patch queue 20:35 < vegard> if you want to figure out why IntegerArithmeticTest fails :) 20:39 < penberg_home> vegard: how are you sending your patches? 20:39 < vegard> huh? 20:39 < penberg_home> git am doesn't like the fact that you have the email header and git generated header 20:39 < vegard> mutt, how so? 20:40 < penberg_home> vegard: export them and try git am 20:40 < penberg_home> and you will see what i mean 20:40 < vegard> maybe I should use -H instead of -i ? 20:40 < penberg_home> vegard: am says "patch empty" 20:40 < penberg_home> vegard: no ide 20:40 < penberg_home> idea 20:40 < penberg_home> but the current setup is wrong apparently 20:40 < vegard> I've always done it 20:41 < penberg_home> don't shoot the messenger 20:42 < penberg_home> vegard: patch applied 20:42 < vegard> thx 20:45 < vegard> ==13173== Warning: set address range perms: large range [0x886a000, 0x1886a000) (defined) 20:45 < penberg_home> vegard: http://www.mjmwired.net/kernel/Documentation/vm/hugetlbpage.txt 20:45 < vegard> new warning from valgrind 20:45 < penberg_home> vegard: what's that? 20:45 < penberg_home> vegard: my mmap() ? 20:45 < vegard> probably yeah :) 20:45 < penberg_home> crap 20:46 < penberg_home> maybe we should make it bit smaller then 20:47 < vegard> btw 20:47 < vegard> you can probably nuke the test/jamvm directory as well 20:47 < penberg_home> aha 20:47 < vegard> without getting into trouble 20:50 < penberg_home> vegard: done 20:50 < vegard> (y) 20:51 < penberg_home> vegard: that warning 20:51 < penberg_home> Diagnostic message, mostly for benefit of the Valgrind developers, to do with memory permissions. 20:51 < penberg_home> vegard: http://valgrind.org/docs/manual/manual-core.html 20:51 < vegard> right. dang :) 20:51 < vegard> and here I thought I might fool you into thinking it was something serious 20:52 < penberg_home> vegard: i know you're trying to dig a hole for me 20:52 < penberg_home> yes 20:52 < penberg_home> you almost got me 20:52 < vegard> that could only be fixed by relocation 20:52 < penberg_home> which is why i checked 20:52 < vegard> I 20:53 < vegard> 'll just have to try harder. 20:58 < vegard> btw 20:58 < vegard> throw_from_native() 20:58 < vegard> it probably needs a compiler barrier 20:59 < vegard> oh, __cleanup_args already is asm volatile, so nvm. 21:07 <@ahuillet> the valgrind warning is nothing 21:09 < vegard> yes, thank you, we know :) 21:11 -!- ahuillet_ [n=user@239.34.83-79.rev.gaoland.net] has joined #jato 21:12 < ahuillet_> so, the array store, does it work now? 21:12 < ahuillet_> seems like everything is fine? 21:12 < vegard> yes 21:13 < ahuillet_> know what bothers me? 21:13 < ahuillet_> you're the one who's going to get println to print something for the first time 21:13 < ahuillet_> this should have been my pleasure. :) 21:13 < vegard> hehe 21:13 < vegard> I've been using jato.internal.VM.println() for a long time ;) 21:13 < vegard> right now I'm trying to figure out why testIntegerDivisionRegReg() of IntegerArithmeticTest fails... 21:13 < ahuillet_> wth is that, a printf()? 21:14 < vegard> yup 21:14 < vegard> disguised as a native method 21:14 < ahuillet_> DivisionRegReg? mmh 21:14 < ahuillet_> it fails since when? 21:14 < vegard> not sure exactly. seems that it doesn't catch the ArithmeticException that is being thrown :) 21:14 < ahuillet_> I'll check if it fails here 21:14 < ahuillet_> an exception in IntegerArithmeticTest?! 21:15 < vegard> it tries a deliberate division-by-zero 21:15 < ahuillet_> grmbl 21:15 < vegard> the method name seems like a misnomer, though 21:15 < ahuillet_> I don't love the idea 21:15 < vegard> it should be testIntegerDivisionByZero() I guess 21:15 < penberg_home> ahuillet_: why is that 21:15 < ahuillet_> this is an arithmetic test, a basic one, not an exception test 21:15 < penberg_home> ahuillet_: disagreed 21:15 < penberg_home> ahuillet_: it's a test for iadd 21:15 < ahuillet_> ? 21:15 < penberg_home> and part of idiv specification is div by zero 21:15 < penberg_home> s/iadd/idiv/g 21:16 < vegard> penberg_home: does it fail for you too? :) 21:16 < penberg_home> vegard: no? 21:16 < ahuillet_> Tests OK here 21:16 < penberg_home> just disagreeing with ahuillet's disagreement 21:16 < vegard> ...weeird 21:16 < ahuillet_> penberg_home : my problem is, I like simple stuff when I test out modifications 21:16 < ahuillet_> exception handlers are a pain to trace etc. 21:17 < ahuillet_> though I'll complain when it actually gets on my nerve, ie. when it makes me waste time 21:17 < ahuillet_> vegard : show me the trace 21:17 < vegard> a huillet to the rescue! 21:17 < penberg_home> tgrabiec: hmm 21:18 < penberg_home> tgrabiec: should we create a separate IntegerArithmeticExceptionsTest ? 21:18 < penberg_home> tgrabiec: ahuillet has a point there. 21:18 < vegard> ahuillet_: http://folk.uio.no/vegardno/trace.txt 21:19 < ahuillet_> vegard :something's fishy in the HIR anyway 21:19 < ahuillet_> the first store is completely useless 21:19 < tgrabiec> penberg_home: how about moving it to ExceptionsTest ? there is a test for each bytecode for exceptions it can trow 21:19 < vegard> ahuillet_: it's in the source code...? 21:20 < ahuillet_> vegard : and what's it supposed to test?! 21:20 < ahuillet_> ah, ok I see 21:20 < ahuillet_> ok makes sense 21:20 < penberg_home> tgrabiec: the problem is that most bytecodes can throw an exception 21:20 < penberg_home> I'd really prefer to keep ExceptionsTest for just _exception handling_ 21:21 < penberg_home> so basically athrow et al 21:21 < penberg_home> yes, I do realize we can't isolate that completely 21:21 < tgrabiec> penberg_home: ok, so you suggest creating a per-exception-class test classes, or per-bytecode classes ? 21:21 < ahuillet_> oh, I still haven't renamed INSN_CALL_REL 21:21 < ahuillet_> it's an absolute call 21:22 < penberg_home> tgrabiec: well, I suggest just reflecting the current grouping 21:22 < penberg_home> tgrabiec: so IntegerArithmetic and so on 21:22 < tgrabiec> penberg_home: aha, ok, i'll take core of it when i fix the bugs 21:23 < penberg_home> ahuillet_: sounds ok to you as well? 21:23 < ahuillet_> what? 21:24 < ahuillet_> I don't believe exceptions in IntegerArithmeticTest to be a good thing just for the sake of speed of testing 21:25 < vegard> hm. it seems that this particular integerarithmetictest fails on latest mainline as well :-/ 21:25 < vegard> for me. 21:25 < ahuillet_> vegard : strange. 21:25 < ahuillet_> how do we do the arithmetic exception stuff? catch SIGFPE? 21:25 < vegard> yes 21:25 < penberg_home> ahuillet_: we're moving them _out_ of the class. 21:26 < ahuillet_> penberg_home : oh, then I'm fine with it 21:26 < penberg_home> ahuillet_: to a IntegerArithmeticExceptionsTest. 21:26 < ahuillet_> though it seriously is no big deal 21:26 < ahuillet_> ThisIsARidiculouslyLongNameAndILoveJava :] 21:27 < tgrabiec> vegard: Tests OK here with latest mainline 21:27 < vegard> tgrabiec: try running it with GDB=valgrind 21:28 < ahuillet_> Reading...==9266== Invalid write of size 4 21:28 < ahuillet_> ==9266== at 0x8060D80: install_signal_bh (signal.c:70) 21:28 < ahuillet_> ==9266== by 0x806A99D: sigfpe_handler (signal.c:59) 21:28 < ahuillet_> Invalid read of size 4 21:28 < ahuillet_> ==9266== at 0x806AA32: throw_arithmetic_exception (signal.c:44) 21:28 < vegard> and it fails in the end, right? 21:28 < ahuillet_> and the test *does* fail sorry 21:28 < ahuillet_> I don't know why I did not see it the previous time.... 21:29 < ahuillet_> ok, it only fails with valgrind 21:29 < vegard> it only fails when you run it under valgrind. 21:29 < tgrabiec> ok, same here, so what does valgrind change ? 21:29 < ahuillet_> tgrabiec : if it fails under valgrind, something is wrong 21:29 < ahuillet_> valgrind is right until proven wrong :) 21:30 < vegard> that first invalid write of size 4 is a write to "thread 1 stack" 21:31 < vegard> are signal handlers per-thread? 21:31 < tgrabiec> vegard: what do you mean ? 21:32 < tgrabiec> afaik signal handlers are per process 21:32 < tgrabiec> but sigsegv and sigfpe are delivered to the thread that triggered it 21:34 < tgrabiec> oh, strange, when i run: valgrind jato.sh -cp regression jvm/IntegerArithmeticTest ; echo $? 21:34 < tgrabiec> it passes 21:34 < tgrabiec> (jato.sh is a wrapper for scripts/java) 21:34 < vegard> http://markmail.org/message/l7gje5chdifb3lvs <-- similar problem 21:35 < ahuillet_> vegard : crap and unrelated i'd say 21:36 < vegard> ok. 21:36 < vegard> I feel (fear?) we'll have many nice bugs to blog about in the next few days :) 21:40 < vegard> --vex-iropt-precise-memory-exns=yes 21:41 < vegard> didn't help one bit. 21:43 < tgrabiec> vegard: the first 'invalid write' is supposed to occure, 'Address 0xbec87c04 is on thread 1's stack' - because install_signal_bh() modifies the stack ('pushes' return address on it) 21:44 < tgrabiec> and the other messages about addresses on stack come from __cleanup_args() and __override_return_address() i believe 21:45 < tgrabiec> but it looks like our modifications are ignored 21:45 < tgrabiec> so as if valgrind idn't let us change the ucontext_t of a signal 21:46 < tgrabiec> oh, no sorry 21:46 < tgrabiec> we _did_ changed the ucontext (because exception is thrown) 21:48 < vegard> but it is not caught 21:49 < tgrabiec> yes, as if throw_from_native() didn't work 21:50 < vegard> maybe valgrind simply doesn't write back the ucontext? 21:50 < vegard> after the signal handler has executed 21:50 < vegard> (to registers, I mean) 21:51 < tgrabiec> no no ucontext is saved, because throw_arithmetic_exception() is called 21:52 < tgrabiec> it is installed by signal_install_bh() 21:55 < vegard> heh. I tried to run jato under strace... and it seems to just loop on time(NULL)/stat64("/etc/localtime") 21:56 < vegard> oh, it did eventually finish. wonder where that came from. 21:58 < tgrabiec> oh that's weird - exception is not catched because no handler is found in the table 21:58 * tgrabiec diggs deeper 22:02 < vegard> hm. I sense that this is quickly becoming my fault :) 22:04 < vegard> exception_covers() returns 0 under valgrind... 22:04 < tgrabiec> vegard: yes, that;s because bytecode offset is 0 22:04 < tgrabiec> so there is a bug in native_ptr_to_bytecode_offset() 22:04 < vegard> ah. do you know why that is? :) 22:05 < tgrabiec> not yet :) 22:09 < tgrabiec> no, mapping is ok. the address is wrong. it thinks exceptions comes from 0x0886cdee but IDIV instruction is at 0x0886cdfe 22:10 < vegard> http://valgrind.org/docs/manual/manual-core.html#manual-core.signals 22:10 < vegard> it says: "If you're using signals in clever ways (for example, catching SIGSEGV, modifying page state and restarting the instruction), you're probably relying on precise exceptions. In this case, you will need to use --vex-iropt-precise-memory-exns=yes. " 22:11 < vegard> but it doesn't make a difference to me. 22:11 < tgrabiec> neither for me 22:11 < vegard> but it seems like this is a problem of valgrind's 22:11 < vegard> it's 16 bytes off? 22:12 < tgrabiec> yes... 22:12 < tgrabiec> hmm 22:14 < tgrabiec> yes, the ucontext_t is corrupted at signal entry 22:14 < tgrabiec> well, at lest it holds not the idiv's address 22:16 < vegard> it's 16 off here as well 22:16 -!- Netsplit leguin.freenode.net <-> irc.freenode.net quits: @ahuillet 22:16 < vegard> you could do an incredibly ugly hack, that is to check RUNNING_ON_VALGRIND from valgrind.h and add 16 to the address if that's the case 22:16 -!- Netsplit over, joins: @ahuillet 22:16 < tgrabiec> well, but i'm not sure it's always 16 bytes 22:17 < vegard> no.. 22:17 < tgrabiec> (i printed the memory content that the signal source address points to and it matches the content printed by ASM output) 22:17 < tgrabiec> i have no idea why would it be this way 22:18 < vegard> it matches the output? 22:18 < vegard> so the signal handler gets the right address? 22:18 < vegard> oh, the memory content 22:18 < tgrabiec> i mean, the address is wrong - there's no idiv, but there is what is supposed to be there 22:19 < tgrabiec> i thought that maybe there was idiv at that address, but no 22:19 -!- Netsplit leguin.freenode.net <-> irc.freenode.net quits: @ahuillet 22:19 < ahuillet_> grmbl, what's my clone doing 22:19 < vegard> http://osdir.com/ml/debugging.valgrind/2004-08/msg00178.html 22:19 < ahuillet_> oh it's just a netsplit 22:21 < vegard> apparently valgrind is JITing our code, so it doesn't know exactly which instruction that caused it... 22:22 -!- Netsplit over, joins: @ahuillet 22:22 < vegard> this e-mail thread is from 2004 :( 22:22 < vegard> wonder why they didn't fix it? 22:23 -!- penberg_home [n=penberg@cs149038.pp.htv.fi] has quit [] 22:23 < vegard> http://osdir.com/ml/debugging.valgrind/2004-08/msg00198.html 22:23 < tgrabiec> vegard: by i checked the memory content pointed by return address 22:23 < vegard> that reply from the author of valgrind 22:23 < tgrabiec> *but i checke 22:23 < vegard> yes, but the address reporting is inaccurate 22:25 < tgrabiec> hmm so i guess we can't fix it 22:25 < vegard> :((( 22:25 < vegard> it certainly seems like that is the case. 22:25 < tgrabiec> well, we could disable some exception tests when run under valgrind 22:26 < vegard> but 22:26 < vegard> I find it VERY odd 22:26 < vegard> because.. how can we return if the return address is inexact? 22:26 < tgrabiec> it's strange that it's just 16 bytes off 22:27 < tgrabiec> i can imagine that valgrind takes over signals and runs signal handlers over it 22:27 -!- penberg_home [n=penberg@cs149038.pp.htv.fi] has joined #jato 22:27 < vegard> is it because normally we don't recover from SIGFPE by returning to the same instruction? 22:30 < vegard> and can we rely on accurate reporting when we're outside of valgrind? 22:32 < tgrabiec> vegard: when i disable throwing from sigfpe, it enters infinite loop 22:33 < tgrabiec> but it does not prove that the address is good, because it's 16 bytes before idiv 22:33 < vegard> hehe, yeah 22:33 < tgrabiec> anyway, i think i'll leave it now 22:34 < vegard> yeah, good idea :) 22:34 < vegard> another day, another chance 22:34 < tgrabiec> vegard: did you ever run these tests with valgrind before ? 22:35 < vegard> yes, I believe so 22:35 < tgrabiec> because this idiv test is there for a couple of days/weeks i believe 22:35 < vegard> it was probably before we got the exception code 22:35 < tgrabiec> vegard: no, i wrote that test 22:36 < vegard> because I was working on my own repository, I didn't see those changes until later 22:36 < vegard> and after we merged I thought it was just one more of those things that I'd broken :) 22:36 < tgrabiec> aha, so it always failed ? 22:37 < vegard> I guess it did. a few of the tests have always failed, I guess I concentrated on the PrintTest since we merged 22:37 < ahuillet_> anyway that's one more reason to move exception stuff out of IntegerArithmeticTest :)) 22:37 < ahuillet_> vegard : let's keep concentrated on it :P 22:38 < tgrabiec> ahuillet_, i agree 23:00 -!- penberg_home [n=penberg@cs149038.pp.htv.fi] has quit [] 23:03 < tgrabiec> ahuillet_: so i'm thinking how to solve this ostack bug, and i have a question. can we make EXPR_TEMPORARY to be only R-value (unscratchable) ? It would require new register allocation in OP_XXX rules, can you see any other problems? 23:04 < ahuillet_> ok, I'm gonna need more information 23:04 < ahuillet_> like, what bug 23:05 < tgrabiec> ok, wait a sec.. 23:05 < ahuillet_> we cannot make EXPR_TEMPORARY to be R-value, since we have to assign something to it 23:05 < ahuillet_> a temporary has no reason to exist if you can't give it a value 23:06 < tgrabiec> right 23:06 < tgrabiec> here's example: http://pastebin.com/d228d4f01 23:07 < ahuillet_> yes? 23:08 < tgrabiec> the point is that both ADDs operate on the same temporary 23:08 < tgrabiec> and the result of first add modifies the input of the second, which should not occure 23:08 < tgrabiec> well, a quick solution would be to make dup create a separate temporary for each 23:09 < ahuillet_> isn't that simply a bug in DUP? 23:09 < ahuillet_> convert_dup that is 23:10 < ahuillet_> or maybe convert_istore 23:10 < ahuillet_> do we properly pop the temporary? 23:10 < tgrabiec> yes it's a bug in dup, but i'm not sure that it's the only case 23:10 < ahuillet_> well, the thing is, we are not handling ostack/memory correctly 23:11 < ahuillet_> semantically, we are doing incorrect things 23:11 < ahuillet_> we have to fix this by doing correct things, like your patch of this morning except it wasn't the classiest way of doing it 23:11 < ahuillet_> so no need to play with how EXPR_TEMPORARY can be used, this should be fixed in convert_* 23:11 < tgrabiec> right 23:12 < tgrabiec> i'm just thinking how to reorganize things that it won't be possible to write bad convertion code like this 23:12 < tgrabiec> it's very error prone now 23:12 < ahuillet_> let's just write good conversion code ;) 23:12 < ahuillet_> implementing a stack based language on a register based machine is error prone anyway 23:14 < tgrabiec> ok, i guess i'll fix converters now 23:15 < ahuillet_> I'm thinking 23:15 < ahuillet_> 'cause they don't seem obviously wrong 23:15 < ahuillet_> that said you've found bugs today and yesterday that I did not find in one year 23:15 < ahuillet_> 'cause of my misunderstanding of the semantics 23:16 < ahuillet_> for example in our case 23:16 < ahuillet_> how is it wrong to use the same temporary for both additions? 23:25 < tgrabiec> it's wrong because they override this temporary 23:25 < tgrabiec> it would be ok if they put the result somewhere else 23:25 < ahuillet_> override? 23:25 < ahuillet_> the result is stored into local variables 23:25 < tgrabiec> ahuillet_, yes, AFAIK OP_XXX put the result in state->left 23:26 < ahuillet_> what's wrong in the piece of code you showed me? 23:26 < ahuillet_> why does it compute something wrong? 23:26 < ahuillet_> yes, the result goes into the left operand 23:26 < tgrabiec> so, in our case it is the register associated with EXPR_TEMPORAY 23:27 < ahuillet_> are we compute anything wrong here or not? 23:27 < tgrabiec> so output from one add becomes silently input to the second 23:28 < ahuillet_> omg yes we do 23:28 < ahuillet_> except in your particular example we don't 23:28 < ahuillet_> because we add 0 23:28 < ahuillet_> (pay attention to that when showing code to someone.. :p) 23:28 < ahuillet_> ok, I understand, we should be using two copies 23:28 < ahuillet_> or output in a new register 23:29 < ahuillet_> but in x86 this is not really an option 23:29 < tgrabiec> yes 23:29 < tgrabiec> we need to do store anyway 23:29 < ahuillet_> (we don't have OP op1, op2, dest instructions... one of the operands is the destination) 23:29 < ahuillet_> so we need an extra copy 23:29 < ahuillet_> now the question is 23:29 < ahuillet_> dup() is supposed to do this extra copy 23:30 < tgrabiec> if we assume that EXPR_TEMPORARY can be overwritten (any how), then dup must push different temporaries back 23:30 < ahuillet_> but that's what it does, doesn't it? 23:30 < ahuillet_> I mean dup is clearly supposed to turn one value into two 23:30 < tgrabiec> no, it currently pushes the same temporary 23:30 < ahuillet_> as in, clone it 23:31 < ahuillet_> ?? 23:31 < tgrabiec> i tpushes twice the same expr 23:31 < ahuillet_> god. 23:31 < ahuillet_> I wrote it. :) 23:31 < ahuillet_> or maybe is that penberg 23:31 < ahuillet_> anyway the fix is trivial isn't it? 23:31 < ahuillet_> dup = dup_expr(ctx, value); 23:31 < ahuillet_> call this twice 23:31 < ahuillet_> no? 23:31 < tgrabiec> yeah, it's trivial 23:33 < tgrabiec> i was just thinking whether there is something else. how about EXPR_MIMIC_STACK_SLOT, is it handled ok regarding this story ? 23:33 < tgrabiec> (i didn't check it) 23:34 < ahuillet_> mimic stack slot is something "special" 23:34 < ahuillet_> I believe it might have the same problem 23:34 < ahuillet_> one given mimic_stack_slot is replaced by the same temporary all over the function 23:34 < ahuillet_> is it wrong, I am not sure 23:34 < ahuillet_> can't prove it to myself 23:35 < ahuillet_> if you can suggest use cases we can think together 23:37 < tgrabiec> can briefly describe my what's the purpose of EXPR_MIMIC_STACK_SLOT ? 23:37 < tgrabiec> err: can you briefly... 23:38 < ahuillet_> so, at basic block boundaries, the mimic stack is not necessarily empty 23:38 < ahuillet_> so we have to "propagate" the mimic stack to other blocks 23:38 < ahuillet_> basically you have two painful cases : one block with two children, noted A -> (B, C) 23:38 < ahuillet_> and one block with two predecessors, noted (A, B) -> C 23:39 < ahuillet_> so, the algorithm to spill and reload the mimic stack isn't trivial because of those two cases 23:39 < ahuillet_> for each element of the mimic stack we store it to a temporary 23:40 < ahuillet_> (at the exit of a block) 23:40 < ahuillet_> and we reload each element from the right temporary at the start of a block 23:40 < ahuillet_> problem is we can't directly pick temporaries, because of the two cases I mentionned 23:40 < ahuillet_> so we take EXPR_MIMIC_STACK_SLOT 23:40 < ahuillet_> a mimic stack slot is identified by a "side" (entry or exit) and a slot number 23:41 < ahuillet_> so basically if you have an expression "E" on the mimic stack at the end of a block 23:41 < ahuillet_> it will be stored into a temporary 23:42 < ahuillet_> and when successors do a stack_pop(), they will get this temporary 23:43 < ahuillet_> am I clear enough ? 23:44 < tgrabiec> so far yes 23:44 < ahuillet_> well that's pretty much it 23:44 < ahuillet_> so there are no "dup" issues here 23:44 < ahuillet_> I believe. :) 23:45 < ahuillet_> can you remind me the problem with iinc? 23:46 < ahuillet_> iconst, dup, store, iinc, and both values on top of the stack and in the local would have been incremented, right? 23:46 < tgrabiec> it was because iinc overrides local variable, and EXPR_LOCAL for this variable is already on mimic-stack 23:46 < ahuillet_> err.. I don't understand that 23:46 < ahuillet_> what do you call "override"? 23:47 < tgrabiec> override = assign a new value (possibly different) 23:47 < ahuillet_> ok, but why did it do that, I don't remember 23:47 < ahuillet_> any use case including "dup" will be fixed when dup is fixed 23:47 < ahuillet_> which I hope is soon :) 23:48 < tgrabiec> the problem with iinc was: iload_1; iinc 1,1; istore_2 23:48 < ahuillet_> ok, in that case we would end up with local *1* being incremented, right? 23:48 < tgrabiec> before the bug was fixed local variable 2 would hold incremented value after that 23:48 < ahuillet_> ah oh sorry iinc doesn't touch the stack 23:49 < ahuillet_> yeah, it's a corner issue, not related to our topic 23:49 < ahuillet_> EXPR_TEMPORARY issues are tied to our bug in convert_dup I believe 23:49 < ahuillet_> unless you can expose other problems I believe convert_dup will fix everything. 23:50 < tgrabiec> yes, apparently that's it 23:50 < tgrabiec> but i think we have to put jasmin tests into mainline to test such quirks 23:51 < ahuillet_> yeah, I did not to it when committing EXPR_MIMIC_STACK_SLOT stuff because of hesitations with the build system 23:53 < ahuillet_> *do it 23:53 < ahuillet_> anyway, you're doing the convert_dup patch? 23:53 < tgrabiec> yes 23:58 < ahuillet_> great 23:59 < tgrabiec> ah, i've just realized that i found that bug when i was working on putstatic/getstatic/putfield/getfield bugfix 23:59 < tgrabiec> ok, so convert_dup goes first --- Log closed Sun Jun 28 00:00:56 2009