search

btraceio / btrace

BTrace - a safe, dynamic tracing tool for the Java platform
5.82k stars 961 forks source link

@TargetMethodOrFiled gives user provided specification value instead of instrumented object's value when used with Kind.FIELD_GET or Kind.FIELD_SET #694

Closed yewton closed 1 month ago

yewton commented 3 months ago

Possible fix: https://github.com/yewton/btrace/pull/1/files

github-actions[bot] commented 1 month ago

Stale issue message

yewton commented 1 month ago

This is still the case.

jbachorik commented 1 month ago

Hi @yewton - do you mind opening a PR so the contribution is properly assigned to you once merged?

yewton commented 1 month ago

Hi @jbachorik

Thank you for your suggestion. I'm still wrapping my head around the instrumentor module to ensure I've covered all the necessary areas before creating a pull request. I'd appreciate it if you could point me in the right direction.

jbachorik commented 1 month ago

No problem, what exactly are you looking for?

yewton commented 1 month ago

I'm having trouble figuring out how to test the changes I made to Instrumentor . Could you provide some guidance on what specific test cases I should write to verify that this bug has been fixed? Are there any existing test cases that I could use as a reference?

jbachorik commented 1 month ago

Here is the test for static field get - https://github.com/btraceio/btrace/blob/4d6da4221a46537c5634d1e667449036b64ae4c2/btrace-instr/src/test/java/org/openjdk/btrace/instr/InstrumentorTest.java#L1867

There is similar test case for non-static get etc.

You would need to modify https://github.com/btraceio/btrace/blob/4d6da4221a46537c5634d1e667449036b64ae4c2/btrace-instr/src/test/btrace/onmethod/FieldGetBeforeStatic.java#L44 such that the field in the @Location definition is by regex as when it is an exact field name match there would be no difference with your patch (both, the target field name and the real field name are the same).

For integration tests, you would adjust/extend https://github.com/btraceio/btrace/blob/develop/integration-tests/src/test/java/tests/BTraceFunctionalTests.java

yewton commented 1 month ago

Thank you for your help. I'll give it a try.