Community
Participate
Working Groups
3.7 M7. Sometimes the debugger stops twice at the same breakpoint. I've encountered this from time to time and finally took time to find steps. Test Case: 1. paste the following code into the Package Explorer: public class BUG { public static void main(String[] args) { int[] i1= {1, 2, 3, 4}; int[] i2= {1, 2, 3, 4}; foo(1, 2, 3, 4); foo(1, 2, 3, 4); } static void foo(int i1, int i2, int i3, int i4) { } } 2. add breakpoints to lines 3, 5, 10 and 12. 3. start to Debug ==> stops at line 3 (good) 4. Resume (F8) ==> stops at line 5 (good) 5. Resume (F8) ==> stops at line 5 again (BAD!) 6. Resume (F8) ==> stops at line 10 (good) 7. Resume (F8) ==> stops at line 12 (good) 8. Resume (F8) ==> stops at line 12 again (BAD!) This can be very irritating when debugging complex code.
I have seen this too. I have compared the stack traces of the two hits and they are exactly the same. This is in a new file action handler, and I have verified that the action was executed only once, because other subsequent break points hit only once. It seems benign but did cost me a couple of hours since it took me down the wrong path. -thanks Ashwin.
This looks like a problem in JDT Core when generating the line info (though javap gives the same result). I get this: Line numbers: [pc: 0, line: 3] [pc: 20, line: 5] [pc: 25, line: 5] [pc: 29, line: 6] [pc: 33, line: 7] [pc: 37, line: 8] [pc: 39, line: 5] [pc: 40, line: 10] [pc: 47, line: 12] [pc: 48, line: 13] [pc: 49, line: 14] [pc: 50, line: 15] [pc: 51, line: 12] [pc: 54, line: 16] As you can see there are two code points for line 12, hence the debugger stops twice. So, why are there two code points? If it's correct to have two, how could JDT Debug know that it does not need to stop on the second one (pc 51)? Moving to JDT Core for comment.
During 3.1, the debugger was changed to stop at all possible locations for a given line, see bug 72431 for details.
Note that a breakoint on e.g. the following line: args.toString();args.toString(); only stops once (which is expected).
This is related to the last call to codeStream.recordPositionsFrom(..) at the end of MessageSend. This call will map any unmapped bytecodes generated since the last line number attribute to the first line of the method invocation. If we use the sourceEnd, then we would map the invoke... bytecode to the closing parenthesis of the method invocation. Replace line 264 of org.eclipse.jdt.internal.compiler.ast.MessageSend: codeStream.recordPositionsFrom(pc, (int)(this.nameSourcePosition >>> 32)); // highlight selector with: codeStream.recordPositionsFrom(pc, this.sourceEnd); Let me know if you think this improves the debugging.
This looks like a duplicate of bug 182218.
> Let me know if you think this improves the debugging. Yes, for the second problem from comment 0 (call to foo), but not for the first one (int[] i2=).
(In reply to comment #7) > > Let me know if you think this improves the debugging. > > Yes, for the second problem from comment 0 (call to foo), but not for the first > one (int[] i2=). This is normal that it doesn't change anything for the first one as this is a array initializer node. The same fix should be done for the LocalDeclaration node and the ArrayInitializer node. In general, we have the same pattern in all ast nodes. So we should review all usages to make sure where this needs to be changed.
Created attachment 217290 [details] Patch for Daniel's use case. Daniel, you can try this small patch that should fix your use case. This is not enough to cover all cases.
(In reply to comment #9) > Created attachment 217290 [details] [diff] > Patch for Daniel's use case. > > Daniel, you can try this small patch that should fix your use case. This is not > enough to cover all cases. Hi Olivier, nice to see you back :-). The patch fixes my use case, thanks!
(In reply to comment #10) > (In reply to comment #9) > > Created attachment 217290 [details] [diff] > > Patch for Daniel's use case. > > > > Daniel, you can try this small patch that should fix your use case. This is not > > enough to cover all cases. > > Hi Olivier, nice to see you back :-). He didn't leave fully to "be back" :) On behalf of all of us at JDT, we take this occasion to thank many many times for stream of patches, triaging efforts, reviews and consultations we have continued to see from your end Olivier ! Now, if only we could figure out how to similarly motivate the other committer emeriti from JDT :) Ayush, please follow up for 4.3 M1, TIA.
(In reply to comment #11) > On behalf of all of us at JDT, we take this occasion to thank many > many times for stream of patches, triaging efforts, reviews and > consultations we have continued to see from your end Olivier ! Indeed!
Thank you. When I get a few minutes and there is something simple to fix, I propose a patch. This gives the direction to investigate to fully fix this one :-).
(In reply to comment #10) > (In reply to comment #9) > > Created attachment 217290 [details] [diff] > > Patch for Daniel's use case. > > > > Daniel, you can try this small patch that should fix your use case. This is not > > enough to cover all cases. > > Hi Olivier, nice to see you back :-). > > The patch fixes my use case, thanks! I'm not sure if this is all we need here. In case of array initializer, the line 8 now corresponds to 2 pc's. So in the above code, putting a breakpoint on line 8 hits twice
(In reply to comment #14) > (In reply to comment #10) > > (In reply to comment #9) > > > Created attachment 217290 [details] [diff] > > > Patch for Daniel's use case. > > > > > > Daniel, you can try this small patch that should fix your use case. This is not > > > enough to cover all cases. > > > > Hi Olivier, nice to see you back :-). > > > > The patch fixes my use case, thanks! > > I'm not sure if this is all we need here. In case of array initializer, the > line 8 now corresponds to 2 pc's. So in the above code, putting a breakpoint on > line 8 hits twice This happens because the code in org.eclipse.jdt.internal.compiler.codegen.CodeStream.indexOfSameLineEntrySincePC(int, int) is bogus. Fixing it solves the problem. I'm checking for other cases.
Created attachment 217627 [details] proposed fix v1.0 + regression tests This patch fixes the case for array init in local declaration and assignment and message send. We no longer need to use source end for array init since the fix in CodeStream simply widens the existing entry instead of adding a new one. Olivier, before I check other cases, can you quickly see if this patch is going in the right direction? Thanks!
(In reply to comment #16) > Olivier, before I check other cases, can you quickly see if this patch is going > in the right direction? Thanks! As long as the line number attribute looks ok, this is fine for me.
Created attachment 217946 [details] proposed fix v1.1 + regression tests Here's a fix for all other relevant places in all the AST nodes. However, this alone does not fix all cases. Cases such as DebugAttributeTest#test014b() for example still fail with this patch, because the pc for both the message send used for the collection and the continue condition are still mapped to the same line number. Olivier, can you please take a look at the patch and suggest what should be done for this test and other new tests added in DebugAttributeTest that still fail with the patch? Thanks!
The patch goes in the right direction. It requires some cleanup though (all commented lines should be removed). I am looking at the failing test to see what could be done to fix it.
I don't expect this to make it into M1, pushing to M2.
My patch works fine, but after thinking about it, it is not that simple. The root cause of this is the bytecodes that are generated but that don't directly link to source code (like implicit this, ...). The easiest way to fix this thing would be to "forget" the line mapping for these bytecodes. Like this we would not try to map necessarily all bytecodes to a line. I'll try to provide another patch after my vacations.
Ran out of time for M2, retargetting at a coarse level to 4.3.
(In reply to comment #22) > Ran out of time for M2, retargetting at a coarse level to 4.3. I'd really like to see this fixed soon. Olivier, maybe you find some time?
Looks like we are running out of time for 4.3, unless Olivier can spare some time for this?
Too late for 4.3.
Sorry, we don't have the time to address this in Luna.
Sorry, this needs to be moved out of 4.5.
I hope there is no excuse this year :)
(In reply to T3rm1 from comment #28) > I hope there is no excuse this year :) Hmm... I really have no choice but to move out unless someone can spare time to work on this. Anyone?
Sorry, no hope for 4.6. Bulk moving to 4.7.
Maybe this year?
(In reply to T3rm1 from comment #31) > Maybe this year? I wish I had some positive news or you, but unfortunately not to be. Olivier who is the expert in this area is no longer with the team. And the team is currently busy with Java 9 work.
So, Java 9 is done. Time to work on this ticket.
(In reply to T3rm1 from comment #33) > So, Java 9 is done. Time to work on this ticket. Not really. We just shipped Java 10 support last week and now working on Java 11.
Bulk move out of 4.8 @Jay - retargetting to 4.9 - please modify the target as appropriate
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
