Patches 6 and 7 look good. Isn't the +1  in GetStringUTFLength due to the trailing '\0'?<br><br><div class="gmail_quote">On Tue, Oct 18, 2011 at 7:30 AM, Will Dietz <span dir="ltr"><<a href="mailto:willdtz@gmail.com">willdtz@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div><div></div><div class="h5">On Mon, Oct 17, 2011 at 5:41 PM, Will Dietz <<a href="mailto:willdtz@gmail.com">willdtz@gmail.com</a>> wrote:<br>

> Hi,<br>
><br>
> Attached are various patches that I've refactored out from the OpenJDK<br>
> port, as they aren't necessarily part of switching runtimes and help<br>
> the Classpath implementation too.<br>
><br>
> Hopefully OpenJDK patches will be coming in the semi-near future...<br>
><br>
> Please let me know if you have any questions or comments, I'm rather<br>
> happy to discuss :).<br>
><br>
> Thanks, and have a good one!<br>
><br>
> ~Will<br>
><br>
<br>
</div></div>Some notes on the patches:<br>
<br>
0001 Minor typo fixes.<br>
<br>
0002 This fixes an assert that incorrectly bails when resolving a<br>
method through an array--if you look at j3VirtualTableLookup for<br>
example it expects 'cl' can be an array class (see the line after the<br>
resolveMethod() invocation), which suggests to me that this was the<br>
correct fix.  The code that triggers this is buried somewhere in the<br>
bootstrap process of the OpenJDK port, not very reproducible.....yet<br>
:).  If this doesn't immediately seem right/wrong to you let me know<br>
and we'll just leave this as-is (we can re-evaluate once the OpenJDK<br>
stuff is on the table).  Finally, if this patch *is* correct, we might<br>
want to do the same for resolveField for consistency, although I<br>
haven't run into a case where that's needed so that's not included<br>
here.<br>
<br>
0003 Adds some sun.misc.Unsafe.* field accessors.  I'm not sure what<br>
kind of error-handling one is supposed to do while implementing<br>
"unsafe" methods, but these are fairly simple methods.  Due to some<br>
win patch-organization on my part, the pieces that make these usable<br>
by defining staticFieldOffset are in the next patch.  Sorry about<br>
that.<br>
<br>
0004 More sun.misc.Unsafe.* implementations, although these are a bit<br>
weaker robustness-wise.  In short, it's the minimum functionality to<br>
get what OpenJDK's bootstrapping process needs to run and seems to<br>
work well enough in my indirect testing via mauve and the like. YMMV,<br>
etc, and if anything seems particularly egregious let me know and I'll<br>
do what I can to clean them up some more.<br>
<br>
0005 This is simply driven by a desire to improve the develop-debug<br>
cycle, hopefully not too controversial :).  Mostly I got tired of<br>
hitting pieces of JNI that weren't implemented and having to wait for<br>
the extraordinarily slow Debug build to run just to see *what* wasn't<br>
implemented.<br>
<br>
0006/0007 Implement a touch more of misc JNI methods--hopefully these<br>
are straightforward, if still lacking some love in the "throw what<br>
exception when" department.  They also begin to make a case for some<br>
abstractions to help with code-duplication (and this gets much worse<br>
with the OpenJDK port stuff), but I'm aiming to have all these patches<br>
almost entirely *add* LOC until the big picture is on the table--and<br>
then we can figure out how to best architect code-sharing across<br>
Unsafe/JNI/OpenJDK/GNUClasspath code.<br>
<br>
0008 This is where I'd expect the most discussion.  This is an<br>
incomplete support of registerNatives, but handles the kinds of<br>
registerNatives-ing that OpenJDK likes to do (mostly of the "this java<br>
method is really JVM_FooBar", or same thing with "this method is just<br>
an alias for a particular JNI method..."--all an external library<br>
redefining our Java methods with pointers it got from us either via<br>
symbol resolution of exported through the JNI interface), and should<br>
be AOT-compatible (in the cases it's not, it'll be caught with a<br>
runtime error).  However, the dependency on symbols and in particular<br>
dladdr isn't the best--probably breaks on Darwin, for example.<br>
<br>
The upside is that in theory we handle more cases, which might help<br>
with JNI and the like.. I'm not sure.<br>
<br>
Thinking about it more, there are a few options/ideas that might be better:<br>
--Check function pointers we're given in registerNatives against a<br>
whitelisted table of functions that we allow/expect to be called on.<br>
Since this is primarily motivated with getting OpenJDK working, this<br>
table could simply consist of all the JVM_* and JNI Interface methods.<br>
The benefit here is that we can encode the mapping with some simpler<br>
identifier that refers to that entry (index in the table, for example)<br>
and that would be completely AOT-compatible since we can just look<br>
them up in the same table.  This is essentially a more hard-coded<br>
version of the above patch that doesn't require dladdr(), and has a<br>
potentially simpler mapping to encode in the JavaMethod definition,<br>
but has no potential for anything beyond this OpenJDK-based use case.<br>
I'm curious to hear your thoughts, but I suppose this is my favorite<br>
at the moment--I'd forgotten amidst everything else that I was using<br>
dladdr and that that would break Darwin support.<br>
<br>
--Ignore registerNatives() altogether and statically encode the<br>
mapping (either by defining<br>
Java_java_MyClass_MyNativeMethod_Signature(...)-style methods, or<br>
something more programmatic/table-driven).  This would work, and is<br>
most similar to what is done elsewhere.  That said, it's the least<br>
flexible and the adds the an implementation/maintenance/completeness<br>
burden that the other ideas above don't have.  Well, and minor<br>
correctness issue if someone calls one of the mapped functions before<br>
the corresponding registerNatives is invoked (or after<br>
UnregisterNatives, not implemented yet).<br>
<br>
Anyway, hopefully that gives you a bit more context for these patches,<br>
and look forward to your thoughts :).<br>
<br>
Thanks for your time, and have a good one!<br>
<br>
~Will<br>
_______________________________________________<br>
vmkit-commits mailing list<br>
<a href="mailto:vmkit-commits@cs.uiuc.edu">vmkit-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/vmkit-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/vmkit-commits</a><br>
</blockquote></div><br>