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

[Erik Groeneveld <erik@cq2.nl>]

(Sorry for the HTML post)

On 23 dec 2009, at 16:09, Bryce McKinlay <bmckinlay@gmail.com> wrote:
> On Wed, Dec 23, 2009 at 1:34 PM, Erik Groeneveld <erik@cq2.nl> wrote:
>
>> The following code demonstrates the problem.
>>
>> #include <gcj/cni.h>
>> #include <java/util/ArrayList.h>
>> #include <java/util/Iterator.h>
>> #include <java/lang/reflect/Method.h>
>> #include <stdio.h>
>>
>> int main(int argc, char* argv[]) {
>>    JvCreateJavaVM(NULL);
>>    JvAttachCurrentThread(NULL, NULL);
>>    java::util::ArrayList* l = new java::util::ArrayList();
>>    java::util::Iterator* i = l->iterator();
>>    java::lang::reflect::Method* m = i->getClass()->getDeclaredMethod(
>>            JvNewStringUTF("hasNext"), NULL);
>>    printf("calling invoke, it'll dump core in natMethod.cc line 194\n");
>>    m->invoke(i, NULL);
>>    return 0;
>> }
>>
>> $gcc problem.cpp -lgcj
>> $./a.out
>> calling invoke
>> Aborted (core dumped)
>>
>> $gdb -core core a.out
>> (gdb) where
>> #0  0x00002adf9252bed5 in raise () from /lib/libc.so.6
>> #1  0x00002adf9252d3f3 in abort () from /lib/libc.so.6
>> #2  0x00002adf90bdeed8 in _Jv_Throw (value=0x2adf932cd370) at
>> ../../../src/libjava/exception.cc:128
>> #3  0x00002adf90bd2a2a in _Jv_catch_segv (_p=<value optimized out>) at
>> ../../../src/libjava/prims.cc:184
>> #4  <signal handler called>
>> #5  0x00002adf90c217d3 in java::lang::reflect::Method::invoke
>> (this=0x2adf932d1c80, obj=0x2adf93ba6e40,
>>    args=0x0) at ../../../src/libjava/java/lang/reflect/natMethod.cc:194
>> #6  0x0000000000400a5c in main ()
>>
>> The top of the stack is from the NULL-pointer catching signal handler
>> we believe, so #5 and #6 are the relevant ones.
>>
>> The point is that we believe that the scenario in the C++ code is
>> valid, both from Java and from C++, and we do not see the reasons for
>> the additional check that has been added to the invoke() method.
>
> Method.invoke() is required to check accessibility according to the
> Java Language Specification, so this check is required.

You refer to this I assume:
http://java.sun.com/docs/books/jls/third_edition/html/names.html#6.6

"A member (class, interface, field, or method) of a reference (class,
interface, or array) type or a constructor of a class type is
accessible only if the type is accessible and the member or
constructor is declared to permit access:

    * If the member or constructor is declared public, then access is
permitted. All members of interfaces are implicitly public.
    * Otherwise, if the member or constructor is declared protected,
then access is permitted only when one of the following is true:
                + Access to the member or constructor occurs from
within the package containing the class in which the protected member
or constructor is declared.
                + Access is correct as described in §6.6.2.
    * Otherwise, if the member or constructor is declared private,
then access is permitted if and only if it occurs within the body of
the top level class (§7.6) that encloses the declaration of the member
or constructor.
    * Otherwise, we say there is default access, which is permitted
only when the access occurs from within the package in which the type
is declared."

It seems that the type is accessible (Iterator) and the method is
declared public. Then it is accessible without further conditions.

The patch in invoke falls back to the scenario under the last bullet,
which is not correct I believe.

> The only thing libgcj is doing wrong is failing to check for null
> "caller" (which can't happen in Java code). It should probably do this
> check and throw an IllegalAccessException, or maybe just permit the
> access.

Yes, it should check for NULL and grant access, otherwise it would be
too restrictive.

> A workaround is to just add a Method.setAccessible() call to your code
> to bypass access checks.

Thanks, I'll remember that from my delegation in Java project and I'll
give it a try.

Erik
>
> Bryce