[vmkit-commits] [PATCH] Various patches

Tue Oct 18 10:36:14 PDT 2011

Patches 6 and 7 look good. Isn't the +1  in GetStringUTFLength due to the
trailing '\0'?

On Tue, Oct 18, 2011 at 7:30 AM, Will Dietz <willdtz at gmail.com> wrote:

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