Re: Implementation details of VMStackWalker

2005-07-26 Thread Tom Tromey
 Andrew == Andrew Haley [EMAIL PROTECTED] writes:

Andrew IMO trying to unify low-level stack walker code is unnecessary and
Andrew leads to too many compromises; it's a merge too far.

It would be preferable from a maintenance point of view if we could
find a way to merge this.  A fair number of our divergences come from
this particular area.

Tom


___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


RE: Implementation details of VMStackWalker

2005-07-25 Thread Jeroen Frijters
Ingo Prötel wrote:
 I just implemented VMStackWalker for our VM and have some questions.
 
 The reference implementation of 'getCallingClass()' and
 'getCallingClassLoader()' just look at the third entry in the class
 context. Would it not be better to return the first class that is not
 assignable to the class in context[0] ? That way we could cope with
 classes that first call some private or protected classes an then ends
 up calling the stack walker.

I'm *very* strongly opposed to this. It makes auditing the code for correctness 
of VMStackWalker *much* harder and it adds no value.

 The next thing I would like to have is a method to get the calling
 method name. This would be good for logging. 

Yeah, we should probably add a method to get the calling method and maybe 
another one to get the entire stack trace (as a java.lang.reflect.Method[]).

 For the security part: Is it enough to check if the class loader of
 context[0] is the boot classloader?

You don't need to enforce that, the documentation is just intended to point out 
that this is a privileged call that is not available to untrusted code. The 
security model will be used to enforce that untrusted code cannot call code in 
the gnu.classpath.* package.

Regards,
Jeroen


___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


RE: Implementation details of VMStackWalker

2005-07-25 Thread Jeroen Frijters
Andrew Haley wrote:
 In gcj, we have a method called GetCallingClass(Class c).  It searches
 firstly for a method declared in class c, then for a method not
 declared in c.  We have found, after a certain amount of trouble, that
 this is the right way to do things; it means that an arbitrary number
 of stack frames can be between the direct caller of GetCallingClass
 and the user code, and it also means that you don't have to check for
 assignability, but for an exact match.

This is a dumb idea...

 For example, you may have
 
 class Foo
 {
 bar (Object O)
 {
return baz(O);
 }
 
 baz (Object O)
 {
   ... GetCallingClass (Foo) ...
 }
 }

class Frob extends Foo
{
  baz (Object o)
  {
super.baz(o);
  }
}

 ... and you will get class of the caller of bar, not Foo.class.

No, you'll get Frob.

Regards,
Jeroen


___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


RE: Implementation details of VMStackWalker

2005-07-25 Thread Jeroen Frijters
Andrew Haley wrote:
 However, as for overhead -- I don't believe it.  I doubt that not
 having this parameter saves anything much on any VM.

That's just your lack of imagination ;-) I was concerned with two
aspects wrt performance:
1) Class literals are inefficient -- this is no longer an issue to me,
since I now use ecj (in 1.5 mode) to compile GNU Classpath
2) I have to do multiple (partial) stack walks to retrieve multiple
frames

Remember, IKVM is built on top of another VM, so I have to use another
stack walking abstraction to walk the stack. Having to fetch multiple
frames is obviously more expensive than having to fetch a single frame.

*BUT* I have much stronger objections against this model:
1) Like Archie points out, there is no need for this additional
functionality (at least not in the common case)
2) It is very easy to introduce subtle bugs (like the one with the
override I showed)
3) It is tempting to do (either on purpose or accidentally):
  class Foo
  {
somemethod()
{
   // we know we're always being called by Bar and we need its
caller
   GetCaller(Bar.class);
}
  }
4) It makes advanced optimizations by the JIT harder (the current
interface is very easy for the JIT to special case)

Regards,
Jeroen


___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


RE: Implementation details of VMStackWalker

2005-07-25 Thread Andrew Haley
Jeroen Frijters writes:
  Andrew Haley wrote:
   In gcj, we have a method called GetCallingClass(Class c).  It searches
   firstly for a method declared in class c, then for a method not
   declared in c.  We have found, after a certain amount of trouble, that
   this is the right way to do things; it means that an arbitrary number
   of stack frames can be between the direct caller of GetCallingClass
   and the user code, and it also means that you don't have to check for
   assignability, but for an exact match.
  
  This is a dumb idea...
  
   For example, you may have
   
   class Foo
   {
   bar (Object O)
   {
  return baz(O);
   }
   
   baz (Object O)
   {
 ... GetCallingClass (Foo) ...
   }
   }
  
  class Frob extends Foo
  {
baz (Object o)
{
  super.baz(o);
}
  }
  
   ... and you will get class of the caller of bar, not Foo.class.
  
  No, you'll get Frob.

Which is the Right Answer: the caller of baz *is* Frob.

Andrew.


___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


RE: Implementation details of VMStackWalker

2005-07-25 Thread Andrew Haley
Jeroen Frijters writes:
  Andrew Haley wrote:
   However, as for overhead -- I don't believe it.  I doubt that not
   having this parameter saves anything much on any VM.
  
  That's just your lack of imagination ;-) I was concerned with two
  aspects wrt performance:
  1) Class literals are inefficient -- this is no longer an issue to me,
  since I now use ecj (in 1.5 mode) to compile GNU Classpath
  2) I have to do multiple (partial) stack walks to retrieve multiple
  frames
  
  Remember, IKVM is built on top of another VM, so I have to use another
  stack walking abstraction to walk the stack. Having to fetch multiple
  frames is obviously more expensive than having to fetch a single frame.

As I said:

  IMO trying to unify low-level stack walker code is unnecessary and
  leads to too many compromises; it's a merge too far.

Andrew.


___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


RE: Implementation details of VMStackWalker

2005-07-25 Thread Andrew Haley
Jeroen Frijters writes:
  Andrew Haley wrote:
   Which is the Right Answer: the caller of baz *is* Frob.
  
  It *may* be the right answer, but how can you be certain that the
  coder of class Foo intended this?

Ah, well that is a totally different matter.

  IMNSHO, code like this is completely unauditable (remember, this
  isn't just a correctness issue, access to class loaders is a
  potential security issue).

Of course, yes.  But it's security issues that I'm concerned about
here: what we want to know is the first caller of Foo.method() that is
not Foo.

Andrew.


___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


RE: Implementation details of VMStackWalker

2005-07-25 Thread Jeroen Frijters
Andrew Haley wrote:
 Which is the Right Answer: the caller of baz *is* Frob.

It *may* be the right answer, but how can you be certain that the coder of 
class Foo intended this? IMNSHO, code like this is completely unauditable 
(remember, this isn't just a correctness issue, access to class loaders is a 
potential security issue).

Regards,
Jeroen


___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


RE: Implementation details of VMStackWalker

2005-07-25 Thread Jeroen Frijters
Ingo Prötel wrote:
 I really like the idea of having a stack walker so I would like to use
 it wherever possible. For example in java.util.logging.Logger. If a
 class calls 'logger.warning(some warning)' this call will 
 be forwarded through a couple of calls until it will call
 'Logger.log(Level level, String message, Throwable thrown)' there the
 stack will be inspected to get the calling class and calling method.

I think that a (potentially) more optimized version of the stack walker API can 
be made for this. I have no problem with that since accuracy in the logging 
isn't that critical (getting it wrong will be annoying but won't create a 
security hole).

 Does this mean the VM must check for every call if it is a call into 
 the gnu.classpath.* package and if so check the classloader 
 of the calling class?

No, this is enforced by the system class loader. Untrusted code cannot even 
load classes from the gnu.classpath package (this isn't yet fully implemented 
yet, but most of the infrastructure is in place).

Only if you replace the GNU Classpath system class loader do you need to worry 
about this.

Regards,
Jeroen


___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


RE: Implementation details of VMStackWalker

2005-07-25 Thread Jeroen Frijters
Andrew Haley wrote:
 Of course, yes.  But it's security issues that I'm concerned about
 here: what we want to know is the first caller of Foo.method() that is
 not Foo.

Not necessarily. Typically what's important is the supplier of the arguments to 
the method. In the subclassing scenario, the subclass may be the one providing 
the arguments (i.e. passing different values then it was passed), but it may 
also be passing along the original values. If the subclasser is trusted but the 
original caller isn't, you have a problem. Now granted, this is a coding error, 
but I think it is facilitated by this too flexible model of walking the stack.

BTW, I'm not ruling out the need for this more flexible way of getting the 
caller, I just want to make sure that this isn't the default and is used only 
very cautiously.

Regards,
Jeroen


___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


RE: Implementation details of VMStackWalker

2005-07-25 Thread Ingo Prötel
Am Montag, den 25.07.2005, 10:16 +0200 schrieb Jeroen Frijters:
 Ingo Prötel wrote:
  I just implemented VMStackWalker for our VM and have some questions.
  
  The reference implementation of 'getCallingClass()' and
  'getCallingClassLoader()' just look at the third entry in the class
  context. Would it not be better to return the first class that is not
  assignable to the class in context[0] ? That way we could cope with
  classes that first call some private or protected classes an then ends
  up calling the stack walker.
 
 I'm *very* strongly opposed to this. It makes auditing the code for 
 correctness of VMStackWalker *much* harder and it adds no value.
 
I really like the idea of having a stack walker so I would like to use
it wherever possible. For example in java.util.logging.Logger. If a
class calls 'logger.warning(some warning)' this call will be forwarded
through a couple of calls until it will call 'Logger.log(Level level,
String message, Throwable thrown)' there the stack will be inspected to
get the calling class and calling method. The current implementation of
VMStackWalker would require the generation of the full class context to
get the class that called 'Logger.warning(String)' since
'VMStackWalker.getCallingClass()' would always return 'Logger'. But
creating the whole class context is expensive. 

And yes our AOT compiler can simply inline the methods but not the
simple interpreter that we use to run dynamically loaded classes.

No I'm not sure if checking for assignability really is useful. But if
somebody wants to create their own Logger subclass it would just work as
I would expect when a call is forwarded to the Logger. But I admit I
have not yet checked what happens on a Sun VM in such a case.

  The next thing I would like to have is a method to get the calling
  method name. This would be good for logging. 
 
 Yeah, we should probably add a method to get the calling method and maybe 
 another one to get the entire stack trace (as a java.lang.reflect.Method[]).
 
  For the security part: Is it enough to check if the class loader of
  context[0] is the boot classloader?
 
 You don't need to enforce that, the documentation is just intended to point 
 out that this is a privileged call that is not available to untrusted code. 
 The security model will be used to enforce that untrusted code cannot call 
 code in the gnu.classpath.* package.

Does this mean the VM must check for every call if it is a call into 
the gnu.classpath.* package and if so check the classloader of the calling 
class?

Ingo
-- 
Ingo Prötel  [EMAIL PROTECTED]
aicas GmbHhttp://www.aicas.com
Haid-und-Neu-Str. 18phone   +49 721 663 968-32
76131 Karlsruhe fax +49 721 663 968-93
Germany  




___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


Re: Implementation details of VMStackWalker

2005-07-24 Thread Andrew Haley
Mark Wielaard writes:
  
  On Fri, 2005-07-22 at 18:01 +0100, Andrew Haley wrote:
   Ingo Prötel writes:
   
 I just implemented VMStackWalker for our VM and have some questions.
 
 The reference implementation of 'getCallingClass()' and
 'getCallingClassLoader()' just look at the third entry in the class
 context. Would it not be better to return the first class that is not
 assignable to the class in context[0] ? That way we could cope with
 classes that first call some private or protected classes an then ends
 up calling the stack walker. 
   
   In gcj, we have a method called GetCallingClass(Class c).  It searches
   firstly for a method declared in class c, then for a method not
   declared in c.  We have found, after a certain amount of trouble, that
   this is the right way to do things; it means that an arbitrary number
   of stack frames can be between the direct caller of GetCallingClass
   and the user code, and it also means that you don't have to check for
   assignability, but for an exact match.
  
  I proposed something similar a while ago:
  http://lists.gnu.org/archive/html/classpath-patches/2005-01/msg00138.html
  
  Jeroen did object strongly to this approach though since he felt that
  having (and constructing) an extra argument for this function was too
  much overhead for runtimes that didn't need it to begin with.

Well, that's up to the VM implementer.  IMO trying to unify low-level
stack walker code is unnecessary and leads to too many compromises;
it's a merge too far.

However, as for overhead -- I don't believe it.  I doubt that not
having this parameter saves anything much on any VM.

Andrew.


___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


Re: Implementation details of VMStackWalker

2005-07-23 Thread ingo . proetel

 The next thing I would like to have is a method to get the calling
 method name. This would be good for logging.

 You could create a Throwable() object and then get the StackFrame[].

That is what the logging API currently does. But that's terrribly
inefficient. We could add a new method to VMStackWalker that returns the
calling method name and in the reference implementation it will just do
the Throwable thing. Then every VM can decide if they want do add a native
method.

 Otherwise, you'd have to add a new native method.

 For the security part: Is it enough to check if the class loader of
 context[0] is the boot classloader?

 Not sure what the question is here.

In the VMStackWalker source it says only code loaded from the boot
classloader may call the methods of this class. But how should we make
sure? My question was if it is enough to check if the class in
getClassContext()[0] (i.e. the class that actually invoked a method on
VMstackWalker directly or through reflection) was loaded by the boot
classloader.


Ingo



___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


Re: Implementation details of VMStackWalker

2005-07-23 Thread Ingo Prötel
Am Freitag, den 22.07.2005, 23:59 +0200 schrieb Mark Wielaard:
 Hi,
 
 On Fri, 2005-07-22 at 18:01 +0100, Andrew Haley wrote:
  Ingo Prötel writes:
  
I just implemented VMStackWalker for our VM and have some questions.

The reference implementation of 'getCallingClass()' and
'getCallingClassLoader()' just look at the third entry in the class
context. Would it not be better to return the first class that is not
assignable to the class in context[0] ? That way we could cope with
classes that first call some private or protected classes an then ends
up calling the stack walker. 
  
  In gcj, we have a method called GetCallingClass(Class c).  It searches
  firstly for a method declared in class c, then for a method not
  declared in c.  We have found, after a certain amount of trouble, that
  this is the right way to do things; it means that an arbitrary number
  of stack frames can be between the direct caller of GetCallingClass
  and the user code, and it also means that you don't have to check for
  assignability, but for an exact match.
 
 I proposed something similar a while ago:
 http://lists.gnu.org/archive/html/classpath-patches/2005-01/msg00138.html
 
 Jeroen did object strongly to this approach though since he felt that
 having (and constructing) an extra argument for this function was too
 much overhead for runtimes that didn't need it to begin with.
  
 It might be good to review that thread and see how/if we can have some
 solution/compromise here since it seems that gcj actually needs this
 functionality. For the other runtimes we did find some workarounds back
 then (how fragile those are I don't actually know).
 
My proposal does not go that far and would not be as universal as adding
a parameter.

I would like to get the common case where private or protected methods
are called before the actual stack check happens. This changes the
current semantic but I feel that it would be more useful and less
fragile to changes of classes that want to use VMStackWalker. If there
is a real need for a method with the current semantic we could just add
a method to the interface for the new semantic.

ingo
-- 
Ingo Prötel  [EMAIL PROTECTED]
aicas GmbHhttp://www.aicas.com
Haid-und-Neu-Str. 18phone   +49 721 663 968-32
76131 Karlsruhe fax +49 721 663 968-93
Germany  




___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


Re: Implementation details of VMStackWalker

2005-07-23 Thread Archie Cobbs

[EMAIL PROTECTED] wrote:

The next thing I would like to have is a method to get the calling
method name. This would be good for logging.


You could create a Throwable() object and then get the StackFrame[].


That is what the logging API currently does. But that's terrribly
inefficient. We could add a new method to VMStackWalker that returns the
calling method name and in the reference implementation it will just do
the Throwable thing. Then every VM can decide if they want do add a native
method.


Sounds reasonable. We take this same approach with getCallingClass()
and getCallingClassLoader(), which can be implemented more efficiently
in native code, but are also have the fallback Java implementation.


For the security part: Is it enough to check if the class loader of
context[0] is the boot classloader?


Not sure what the question is here.


In the VMStackWalker source it says only code loaded from the boot
classloader may call the methods of this class. But how should we make
sure? My question was if it is enough to check if the class in
getClassContext()[0] (i.e. the class that actually invoked a method on
VMstackWalker directly or through reflection) was loaded by the boot
classloader.


I didn't write that comment so I'm not sure how (if at all) it
is enforced. Not sure what the real security problem is though,
even if we did allow random code to call these methods. Maybe
someone else can shed some light?

-Archie

__
Archie Cobbs  *CTO, Awarix*  http://www.awarix.com


___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


Re: Implementation details of VMStackWalker

2005-07-23 Thread Archie Cobbs

Ingo Prötel wrote:

I would like to get the common case where private or protected methods
are called before the actual stack check happens. This changes the
current semantic but I feel that it would be more useful and less
fragile to changes of classes that want to use VMStackWalker. If there
is a real need for a method with the current semantic we could just add
a method to the interface for the new semantic.


None of the existing code in Classpath that uses VMStackWalker calls
it through private or protected methods, so I'm not sure why you're
saying that having private and protected methods is a common case.

Anyway, the reason it's done with the current simple semantics is to make
these calls as efficient as possible. Although your semantics would make
things more robust in the future, you have to balance that against
a (probably small) loss of efficiency.

-Archie

__
Archie Cobbs  *CTO, Awarix*  http://www.awarix.com


___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


Implementation details of VMStackWalker

2005-07-22 Thread Ingo Prötel
Hi,

I just implemented VMStackWalker for our VM and have some questions.

The reference implementation of 'getCallingClass()' and
'getCallingClassLoader()' just look at the third entry in the class
context. Would it not be better to return the first class that is not
assignable to the class in context[0] ? That way we could cope with
classes that first call some private or protected classes an then ends
up calling the stack walker. 

The next thing I would like to have is a method to get the calling
method name. This would be good for logging. 

For the security part: Is it enough to check if the class loader of
context[0] is the boot classloader?

Thanks
Ingo
-- 
Ingo Prötel  [EMAIL PROTECTED]
aicas GmbHhttp://www.aicas.com
Haid-und-Neu-Str. 18phone   +49 721 663 968-32
76131 Karlsruhe fax +49 721 663 968-93
Germany  




___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


Re: Implementation details of VMStackWalker

2005-07-22 Thread Andrew Haley
Ingo Prötel writes:

  I just implemented VMStackWalker for our VM and have some questions.
  
  The reference implementation of 'getCallingClass()' and
  'getCallingClassLoader()' just look at the third entry in the class
  context. Would it not be better to return the first class that is not
  assignable to the class in context[0] ? That way we could cope with
  classes that first call some private or protected classes an then ends
  up calling the stack walker. 

In gcj, we have a method called GetCallingClass(Class c).  It searches
firstly for a method declared in class c, then for a method not
declared in c.  We have found, after a certain amount of trouble, that
this is the right way to do things; it means that an arbitrary number
of stack frames can be between the direct caller of GetCallingClass
and the user code, and it also means that you don't have to check for
assignability, but for an exact match.

For example, you may have

class Foo
{
bar (Object O)
{
   return baz(O);
}

baz (Object O)
{
  ... GetCallingClass (Foo) ...
}
}

... and you will get class of the caller of bar, not Foo.class.

We use it like this:

java::lang::ClassLoader *
java::io::ObjectInputStream::currentLoader ()
{
  jclass caller = _Jv_StackTrace::GetCallingClass (ObjectInputStream::class$);
  if (caller)
return caller-getClassLoaderInternal();
  return NULL;
}

Andrew.


___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


Re: Implementation details of VMStackWalker

2005-07-22 Thread Mark Wielaard
Hi,

On Fri, 2005-07-22 at 18:01 +0100, Andrew Haley wrote:
 Ingo Prötel writes:
 
   I just implemented VMStackWalker for our VM and have some questions.
   
   The reference implementation of 'getCallingClass()' and
   'getCallingClassLoader()' just look at the third entry in the class
   context. Would it not be better to return the first class that is not
   assignable to the class in context[0] ? That way we could cope with
   classes that first call some private or protected classes an then ends
   up calling the stack walker. 
 
 In gcj, we have a method called GetCallingClass(Class c).  It searches
 firstly for a method declared in class c, then for a method not
 declared in c.  We have found, after a certain amount of trouble, that
 this is the right way to do things; it means that an arbitrary number
 of stack frames can be between the direct caller of GetCallingClass
 and the user code, and it also means that you don't have to check for
 assignability, but for an exact match.

I proposed something similar a while ago:
http://lists.gnu.org/archive/html/classpath-patches/2005-01/msg00138.html

Jeroen did object strongly to this approach though since he felt that
having (and constructing) an extra argument for this function was too
much overhead for runtimes that didn't need it to begin with.
 
It might be good to review that thread and see how/if we can have some
solution/compromise here since it seems that gcj actually needs this
functionality. For the other runtimes we did find some workarounds back
then (how fragile those are I don't actually know).

Cheers,

Mark



signature.asc
Description: This is a digitally signed message part
___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath