[vmkit-commits] [PATCH] Fix symbol search order to fix detection of symbols from vmkit or loaded libs

[Nicolas Geoffray]

Hi Will,

The approach looks good to me. We could definitely do better by listing all
the methods vmkit defines, but for now what you're doing is fine.

Nicolas

On Wed, Nov 16, 2011 at 4:19 AM, Will Dietz <wdietz2 at illinois.edu> wrote:

> On Tue, Nov 15, 2011 at 3:58 PM, Will Dietz <wdietz2 at illinois.edu> wrote:
> > On Tue, Nov 15, 2011 at 2:41 PM, Will Dietz <wdietz2 at illinois.edu>
> wrote:
> >> On Tue, Nov 15, 2011 at 2:24 PM, Nicolas Geoffray
> >> <nicolas.geoffray at gmail.com> wrote:
> >>> Hi Will,
> >>>
> >>> I'm not sure that's right. With GNU Classpath, I also want to define
> my own
> >>> methods and not use the ones implemented in Classpath. If we reorder
> things,
> >>> the method defined in GNU Classpath will be used. Not mine.
> >>> vmkit is using RTLD_LOCAL when loading, so symbol resolution on a
> dlsym must
> >>> provide the loaded library. If you do dlsym(SELF_HANDLE, 'mysymbol'),
> you'll
> >>> only get symbols loaded in the j3 executable.
> >>> What is it that does not work for you?
> >>> Cheers,
> >>> Nicolas
> >>>
> >>
> >
> > Hmm, you're right that regardless of my discussion below, this changes
> > the behavior for symbols that are defined in both places.
> >
> > Unfortunately, it appears that OpenJDK likes to dlopen() with
> > RTLD_GLOBAL, which complicates things.
> >
>
> Inlined below is an updated patch that works here, and should
> correctly handle all existing cases (except that we're doing more
> dlsym's for each lookup than before).
>
> Let me know if you like it, else there are a number of other options :).
>
> ~Will
>
> From ba8355a4b4b3bb39aaf64764b0b6998297419fbd Mon Sep 17 00:00:00 2001
> From: Will Dietz <w at wdtz.org>
> Date: Mon, 14 Nov 2011 09:45:35 -0600
> Subject: [PATCH] Fix detection of whether or not symbols are from j3 or
> not.
>
> Unfortunately, native code that's part of OpenJDK likes to dlopen things
> with
> RTLD_GLOBAL, resulting in the existing mechanism for determining where a
> symbol
> came from inaccurate.
>
> Instead, search both our process (and any global libraries), but also
> search our
> loaded libraries.
>
> Using this, we can determine which symbols are in vmkit: they must be
> in in the self
> search AND be a different symbol from what was found in the library search.
> ---
>  lib/J3/VMCore/JnjvmClassLoader.cpp |   45
> ++++++++++++++++++++++++-----------
>  1 files changed, 31 insertions(+), 14 deletions(-)
>
> diff --git a/lib/J3/VMCore/JnjvmClassLoader.cpp
> b/lib/J3/VMCore/JnjvmClassLoader.cpp
> index bf825a8..98abde4 100644
> --- a/lib/J3/VMCore/JnjvmClassLoader.cpp
> +++ b/lib/J3/VMCore/JnjvmClassLoader.cpp
> @@ -977,22 +977,39 @@ const UTF8*
> JnjvmClassLoader::constructArrayName(uint32 steps,
>  }
>
>  word_t JnjvmClassLoader::loadInLib(const char* buf, bool& j3) {
> -  word_t res =
> (word_t)TheCompiler->loadMethod(mvm::System::GetSelfHandle(), buf);
> -
> -  if (!res) {
> -    for (std::vector<void*>::iterator i = nativeLibs.begin(),
> -              e = nativeLibs.end(); i!= e; ++i) {
> -      res = (word_t)TheCompiler->loadMethod((*i), buf);
> -      if (res) break;
> -    }
> -  } else {
> -    j3 = true;
> +  // Check 'self'.  Should only check our process, however it's possible
> native
> +  // code dlopen'd something itself (with RTLD_GLOBAL; OpenJDK does this).
> +  // To handle this, we search both ourselves and the libraries we loaded.
> +  word_t sym =
> +    (word_t)TheCompiler->loadMethod(mvm::System::GetSelfHandle(), buf);
> +
> +  // Search loaded libraries as well, both as fallback and to determine
> +  // whether or not the symbol in question is defined by vmkit.
> +  word_t symFromLib = 0;
> +  for (std::vector<void*>::iterator i = nativeLibs.begin(),
> +      e = nativeLibs.end(); i!= e; ++i) {
> +    symFromLib = (word_t)TheCompiler->loadMethod((*i), buf);
> +    if (symFromLib) break;
>    }
> -
> -  if (!res && this != bootstrapLoader)
> -    res = bootstrapLoader->loadInLib(buf, j3);
>
> -  return (word_t)res;
> +  if (sym) {
> +    // Always use the definition from 'self', if it exists.
> +    // Furthermore, claim it's defined in j3 iff it wasn't found in one
> of our
> +    // libraries.  This might be wrong if we do a lookup on a symbol
> that's
> +    // neither in vmkit nor a VM-loaded library (but /is/ in a
> different library
> +    // that has been dlopen'd by native code), but that should never
> +    // be called from java code anyway.
> +    j3 = (sym != symFromLib);
> +    return sym;
> +  }
> +
> +  // Otherwise return what we found in the libraries, if anything
> +  if (symFromLib) return symFromLib;
> +
> +  if (this != bootstrapLoader)
> +    return bootstrapLoader->loadInLib(buf, j3);
> +
> +  return 0;
>  }
>
>  void* JnjvmClassLoader::loadLib(const char* buf) {
> --
> 1.7.5.1
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/vmkit-commits/attachments/20111116/a695e53f/attachment.html>