Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

It's impossible to @ModifyConstant a null value #531

Open
Earthcomputer opened this issue Nov 6, 2021 · 1 comment
Open

It's impossible to @ModifyConstant a null value #531

Earthcomputer opened this issue Nov 6, 2021 · 1 comment

Comments

@Earthcomputer
Copy link

It's impossible to modify a null constant (that is, an aconst_null instruction) with @ModifyConstant. This seems unintentional as @Constant(nullValue = true) is a thing.
Although I'm a little unsure exactly as to what signature I'm supposed to use for null, this is what I tried:

@ModifyConstant(method = "aMethodContainingAConstNull", constant = @Constant(nullValue = true))
private Object modifyNull(Object _null) { return _null; }

This fails the injection check.
Through some debugging, I found that ModifyConstant seems to expect the return type and parameter for modifying a null value to be void, which is impossible for the parameter and useless for the return value.

@Mumfrey
Copy link
Member

Mumfrey commented Nov 8, 2021

I actually remember noticing this a while ago but I forgot to add it to trello, yes you are correct. At the time I did correct the corresponding bug with BeforeConstant but I forgot to fix the injector, though I do vaguely remember looking at it.

It is not as simple as just consuming and returning Object because the type on the stack has to match the place it's used, eg. if it's assigned to a field then the handler needs to either return the field type, or the injector has to add a CHECKCAST after the handler. Failing to do this results in a VerifyError when the JVM reads the class. It should in theory be possible to have the handler consume/return any superclass or interface of the actual type provided the CHECKCAST is added, though it might make sense to require @Coerce for anything other than the specific type.

Either way, I'll look into this, it shouldn't be too difficult to handle this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants