problem with class accessiblity check in invoke (natMethod.cc)

[Andrew Haley]

On 12/23/2009 10:38 AM, Erik Groeneveld wrote:

> We recently upgraded from libgcj7 to libgcj9. This broke our code. We
> found out that in natMethod.cc in invoke() there is an additional
> check for class accessibility. It starts out with:
> 
>   // Check accessibility, if required.
>   if (! this->isAccessible())
>     {
>       if (! (Modifier::isPublic (meth->accflags)))
> 	{
> 	  Class *caller = _Jv_StackTrace::GetCallingClass (&Method::class$);
> 	  if (! _Jv_CheckAccess(caller, declaringClass, meth->accflags))
> 	    throw new IllegalAccessException;
> 	}
> 
> and then continues here (this part is new):
> 
>       else
> 	// Method is public, check to see if class is accessible.
> 	{
> 	  jint flags = (declaringClass->accflags
> 			& (Modifier::PUBLIC
> 			   | Modifier::PROTECTED
> 			   | Modifier::PRIVATE));
> 	  if (flags == 0) // i.e. class is package private
> 	    {
> 	      Class *caller = _Jv_StackTrace::GetCallingClass (&Method::class$);
> 	      if (! _Jv_ClassNameSamePackage (caller->name,
> 					      declaringClass->name))
> 		throw new IllegalAccessException;
> 	    }
> 	}
>     }
> 
> The added code prevents legal calls to fail. For example, it is no
> longer possible to call invoke on the ´hasNext´ method on the iterator
> from AbstractList.  The method AbstractList.iterator() returns an
> anonymous innerclass object which according to the C code above is not
> accessible since it is package private.  There are two problems here:
> 
> 1. The systems segfaults on the caller->name because there is no calling class.

How can there be no calling class?

> 2. We believe class accessibility is not relevant here: there is no
> reason why a Method object with public access should not be invokable,
> or is there?
> 
> We found the following modification log, which is why we address
> Andrew Haley directly as well as the list:
> 
> 2007-04-16  Andrew Haley  <aph@redhat.com>
> 
>         * java/lang/reflect/natMethod.cc (Method::invoke): In invoke also
>         check that the method's declaring class is accessible.
> 
> Can you or anybody else explain why this was considered to be necessary?

Yes, but can you send a test case before we go any further?  Then at least
we'll all know what you're talking about.

Andrew.