Temporary stack variable being stored as the wrong type #3224
Conversation
JohnDumbell
requested review from
cesaro,
cristina-david,
NathanJPhillips,
peterschrammel,
pkesseli,
smowton and
thk123
as code owners
JohnDumbell
force-pushed
the
jd/bugfix/temp_variable_type_wrong
branch
2 times, most recently
from
05180a8 to
7bbedf6
Compare
| CORE | ||
| Primary.class | ||
| --function stuff.array_issue.Primary.Run | ||
| ^VERIFICATION SUCCESSFUL|VERIFICATION FAILED$ |
There was a problem hiding this comment.
We really don't know which one to expect?
There was a problem hiding this comment.
We don't really mind, as long as it completes in some form the fix will still be in place (it just exceptioned half way through before).
There was a problem hiding this comment.
True, but generally we should know what our tests are doing and specify that. In this case I recommend putting an assert(false) in the Accept method and checking that we have VERIFICATION FAILED and also that we failed that particular assertion, thus establishing that the test explored the path where we make the call (cf. failing an earlier null-pointer check or something) as we intended.
| @@ -3241,7 +3241,8 @@ void java_bytecode_convert_methodt::save_stack_entries( | |||
| } | |||
| if(replace) | |||
| { | |||
| create_stack_tmp_var(tmp_var_prefix, tmp_var_type, block, stack_entry); | |||
| create_stack_tmp_var( | |||
There was a problem hiding this comment.
Style commit I just pushed got rid of the change for some reason - fixed now.
In certain situations the stack of tmp_var's had the wrong type associated with it, as each replacement entry was being created with the type of the expression it was built from. In this particular case a java array created and directly passed into a method was causing the 'this' temp variable to be stored as the type of the array itself, causing an assertion error further on.
The type of code causing this error:
this.objectInstance.Method(new int[]{0, 1, 2});
JohnDumbell
force-pushed
the
jd/bugfix/temp_variable_type_wrong
branch
from
7bbedf6 to
0bb165d
Compare
JohnDumbell
force-pushed
the
jd/bugfix/temp_variable_type_wrong
branch
from
0bb165d to
29d81de
Compare
There was a problem hiding this comment.
This PR failed Diffblue compatibility checks (cbmc commit: 0bb165d).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/88978662
Status will be re-evaluated on next push.
Please contact @peterschrammel, @thk123, or @allredj for support.
Common spurious failures:
- the cbmc commit has disappeared in the mean time (e.g. in a force-push)
- the author is not in the list of contributors (e.g. first-time contributors).
The incompatibility may have been introduced by an earlier PR. In that case merging this
PR should be avoided unless it fixes the current incompatibility.
JohnDumbell
force-pushed
the
jd/bugfix/temp_variable_type_wrong
branch
from
29d81de to
b9392c5
Compare
There was a problem hiding this comment.
Passed Diffblue compatibility checks (cbmc commit: b9392c5).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/88986777
In a very particular instance (shown in the unit test) the type of the temporary stack variable got changed from what it was originally to the type of whatever expression triggered the save_stack_entries call. This just makes sure the type of the stored variable doesn't change.