Community
Participate
Working Groups
Created attachment 272144 [details] project with example sources Follow up on https://github.com/spotbugs/spotbugs/issues/516. I found that ecj does not generate LVT entries for unused local variables, or generates them with wrong pc ranges. The pattern is common for all *blocks*: if there is only one unused variable defined, LVT entry will be NOT generated for it. If there are more "unused" variables defined, the *last one* will have NO LVT entry, and all other entries will have PC ranges off by one. This makes bytecode analysis tools like SpotBugs unhappy, because they can't resolve variable names. Such "unused" variables are typical after (bad) merges, so it is important to find and report such places ASAP. I've attached the example project where the missing / wrong LVT entries can be easily observed in Eclipse with the help of Bytecode Outline view (https://marketplace.eclipse.org/content/bytecode-outline). Bytecode Outline can resolve "ASTORE 1" to "ASTORE 1: variable" and also shows LVT content with the variable scopes. I can see that this is broken at least since Eclipse 3.8.2. See also bug 494225 which is basically the same for lambdas (3.8.2 does not know about lambdas :-)).
Stephan, if you could give me a hint where I can look for, I could try to provide a patch and tests for all this.
*** Bug 494225 has been marked as a duplicate of this bug. ***
(In reply to Andrey Loskutov from comment #1) > Stephan, if you could give me a hint where I can look for, I could try to > provide a patch and tests for all this. That'd be awesome. I believe most of the magic can be found in BlockScope.computeLocalVariablePositions(int, int, CodeStream) See that the non-javadoc comment of that method states "ignoring unused local variables", but OTOH there is compilerOptions().preserveAllLocalVariables. HTH
Forgot to mention: we should also add a test using lambdas to make sure bug 494225 is covered as well.
I have two variants of a patch, but I fear the current state is by design. https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.13 : local_variable_table[] Each entry in the local_variable_table array indicates a range of code array offsets within which a local variable has a value. It also indicates the index into the local variable array of the current frame at which that local variable can be found. Each entry must contain the following five items: start_pc, length The given local variable must have a value at indices into the code array in the interval [start_pc, start_pc + length), that is, between start_pc inclusive and start_pc + length exclusive. The value of start_pc must be a valid index into the code array of this Code attribute and must be the index of the opcode of an instruction. The value of start_pc + length must either be a valid index into the code array of this Code attribute and be the index of the opcode of an instruction, or it must be the first index beyond the end of that code array. There are two problems in the current code (which seem to be correct according to the spec), and I don't know if we can fix them. The local variable initial pc is given *after* first store into the variable. This explains that we see "wrong" ranges for all variables (not from the first pc where the variable value is being loaded: at this time the variable is not initialized yet, so according to the spec, we can't use that pc). The second problem is that class file does not write LVT entries where the pc init and pc end of the variable are equal. This is the case for *last* unused variables in any block, because the pc at block end is same as the last pc from the store into the unused variable. Why this is for blocks and not for simple method bodies? Because in the last case the last pc is the return statement (+1). The spec itself allows the start pc + lenght to go beyond the actual code array, so I can imagine that we could "fix" missing LVT entries by simply increasing the lenght from zero to one while writing the LVT, but I fear we can't fix the "off by one" pc values for all locals.
New Gerrit change created: https://git.eclipse.org/r/115033
New Gerrit change created: https://git.eclipse.org/r/115034
(In reply to Eclipse Genie from comment #6) > New Gerrit change created: https://git.eclipse.org/r/115033 (In reply to Eclipse Genie from comment #7) > New Gerrit change created: https://git.eclipse.org/r/115034 Both patches breaks class verification :-( Any ideas are welcome.
Since I see no way to resolve this without breaking the spec, closing as won't fix.
