On 12/01/2019 8:47 am, Mandy Chung wrote:
On 1/11/19 2:38 PM, David Holmes wrote:
There seem to be a number of spec issues around this. Shouldn't
findStaticSetter say something about what happens when the field is
final? Same for findSetter? This issue seems to be much bigger than
just a simple bug fix.
I don't see any issue with the spec for findSetter and
findStaticSetter. findSetter returns a method handle equivalent to
`putField` bytecode and it throws IAE when access checking is performed
since it's final and not writeable. Similiarly for findStaticSetter.
Well that is what it does, but it doesn't explicitly mention final
fields and the @throws information for IAE:
IllegalAccessException - if access checking fails, or if the field
is static
doesn't convey anything about getting IAE for a final field. Perhaps it
is implicit in the "this behaves like bytecode" but it is very obscure.
A CSR request will need to be filed.
Of course, as this is a spec change.
I'm unclear what spec is actually to be changed here and in what way? If
these methods produce MH that obey bytecode semantics then the fact the
Field had setAccessible(true) called on it is not relevant - the JVMS
knows nothing about setAccessible. The only issue I see is the
implementation not throwing IAE when the Field represents a final field,
because it examines the "accessibility" state. (And that should apply
whether the field is static or not.)
David
-----
Mandy
David
-----
On 12/01/2019 5:07 am, Mandy Chung wrote:
This issue requires a spec change and it's a behavioral change.
Field::set on a static final field throws IAE. Although there are
few cases in the world that hack around it, I think the compatibility
risk is low in this change.
I added a suggested fix in the JBS issue.
Mandy
On 1/11/19 10:07 AM, Mandy Chung wrote:
Hi Adam,
This is indeed a bug. I suggest to do the check in the
checkAccess method closes to where it performs the instance final
field check. Please also add a test case.
Mandy
On 1/11/19 6:38 AM, Adam Farley8 wrote:
Hi All,
First; thanks for responding so quickly. :)
Second, it sounds like changing the contents of a final field is
acceptable under the spec, but *not* changing the contents of a static
final field.
The test case uploaded to the bug uses a static final field for the
test,
so I think we can agree that the ability to create a setter for a
static
final field is still a bug.
It sounds like the same fix could work if we check for "static"
when we
check for "final", with a modified message.
As Field.set (post-setAccessible) fails to set the value of a
static final
field, I think we can agree that unreflectSetter should fail to
create a
setter MethodHandle for this same field.
Best Regards
Adam Farley
IBM Runtimes
