Re: FYI: Linker & Verifier fixes

[Bryce McKinlay <mckinlay at redhat dot com>]

Andrew Haley wrote:
> Bryce McKinlay writes:
>  > Hi Robert,
>  > 
>  > I believe there is still a bug here with the String being passed to 
>  > ffi_prep_closure:
>  > 
>  > +          // Initializes the cif and the closure. If that worked the 
>  > closure is
>  > +          // stored as a function pointer in the atable.
>  > +          if ( ffi_prep_cif(cif, FFI_DEFAULT_ABI, 1,
>  > +                            &ffi_type_void, arg_types) == FFI_OK
>  > +               && (ffi_prep_closure
>  > +                   (closure, cif,
>  > +                   _Jv_ThrowNoClassDefFoundErrorTrampoline,
>  > +                   (void *) _Jv_NewStringUtf8Const(sym.class_name))
>  > +                   == FFI_OK))
>  > 
>  > The String returned by _Jv_NewStringUtf8Const(sym.class_name) is 
>  > allocated by the GC, but there is no pointer to this string that is 
>  > visible to the GC, so it is at risk of being prematurely collected. (The 
>  > closure data itself will not be visible to the GC as it will not scan 
>  > malloc()'ed memory).
>  > 
>  > A simple solution would be to add the Strings as keys in a HashMap each 
>  > time a closure is created.
>
> Easier, surely, to:
>
>                (ffi_prep_closure
>                 (closure, cif,
>                 _Jv_ThrowNoClassDefFoundErrorTrampoline,
>                 sym.class_name)
>                 == FFI_OK))
>
> and do _Jv_NewStringUtf8Const() in the trampoline.
>   

Indeed. I was thinking the HashMap could also be used to avoid 
duplicating closures in cases where there is more than one reference to 
the same missing class, however, a better way to handle that would be to 
always create a JV_STATE_PHANTOM class for missing classes and storing 
the closure in there. This way, assuming the closure is GC allocated, 
everything would get collected automatically.

Bryce