Community
Participate
Working Groups
Build ID: I20080617-2000 When debugging a plugin/bundle whose classfiles contain a source debug extension a la JSR 45 the debugger doesn't find the correct source files. Reason: the class PDESourceLookupQuery overrides the normal Java source lookup (to reflect OSGi-classloading) without considering the JSR-45 information. Proposed fix: Method PDESourceLookupQuery.run() should generally compute sourcePath like this: sourcePath = JavaDebugUtils.getSourceName(fElement); (i.e., independent of the type of fElement). From there, lookup using the classLoaderObject is just fine. One language for which this is relevant is ObjectTeams/Java, which requires some advanced translations between source and byte code. Any other language using JSR 45 would be affected, too. I'm not sure which ones are actually used for OSGi development, though. I tried the proposed fix as an aspect plugin (in OT/J) and it works as intended. If desired I could "port" this into the pde ui proper. (Currently don't have a workspace configured for that).
I was going to checkout the pde sources to provide a proper patch, but pde CVS seems to be unavailable (for anonymous pserver)? Old location has a recent readme.txt by Chris saying everything moved to /cvsroot/eclipse/pde but the new location has "no repository". ? where are you? ;-)
The repository is still /cvsroot/eclipse, but you now need to dig into the pde/ folder instead of the old org.eclipse.pde.* ones. You will even find a psf file there to be provision all the PDE projects in one click
Did you find it Stephan?
(In reply to comment #3) > Did you find it Stephan? Yes, thanks Benjamin & Chris. I should have known that cvs repositories are not nested, so /cvsroot/eclipse/pde must be _within_ /cvsroot/eclipse. I was just mislead by the readme.txt saying that pde has moved to a new *repository location*, where actually it only moved to a different directory. BTW: Also the web pages did not easily give an answer.
Created attachment 119682 [details] plain Java equivalent of our aspect OK, I (manually) translated our OT/J aspect into a plain Java patch. One gotcha: we'd have to kindly ask jdt.debug to add pde.ui to this list: Export-Package: ..., org.eclipse.jdt.internal.debug.core;x-friends:="org.eclipse.jdt.debug.ui,org.eclipse.jdt.launching" because that's where their nice JavaDebugUtils reside. Secondly, I could not test this variant, because I didn't feel like pulling in the complete Eclipse 3.5 source tree (I'm working on a 3.4.1). The OT/J equivalent, however, has been successfully in use for over four months now. The patch is programmed quite defensively: It still calls the old behavior in case a CoreException is thrown by the JavaDebugUtils. Next I will replace this fallback with logging an error and try this in daily work, because if the fallback isn't needed, the local implementation of generateSourceName() can be removed altogether. Still thinking how to implement a test case (which actually uses an SMAP). Do you have any tests for source lookup in pde.ui, where I could just add a new test case perhaps?
Ping :) No interest in adopting my patch to make the PDE JSR45-aware??
Darin?
Yes, we'll look at the patch (hopefully in M2, as noted by the milestone).
Are you OK with this change Darin?
Created attachment 147132 [details] my patch I prefer this patch that only uses API from JDT.
Applied to HEAD. Fixed.
Verified
in reply to comment 10: > I prefer this patch that only uses API from JDT. Thanks Darin for looking into this, but, I hate to say it, no, your patch is not a full solution. When using your patch I see sourcePaths becoming an array with two elements, corresponding to the two strata that are associated with the current class file. Unfortunately, just selecting the first element (sourcePaths[0]) does not help. The point here is that one use of JSR 45 is to map a single class file to multiple source files from which it has been compiled. Just by looking at the type you can never tell which source file should be used. Looking at how jdt.debug does it I learned that more reliable information has to come from the stack frame. In fact this just calls for a small modification to your patch. I'll create an alternate patch in a minute and attach it here for your kind consideration ;-)
Created attachment 147259 [details] alternate patch This patch against HEAD works fine - at least for ObjectTeams/Java. And it also avoids using non-API classes :)
Created attachment 147266 [details] alternative OK, I see what you're doing - retrieving the source for the default stratum. I'm curious if this patch is equivalent (i.e. retrieve the source for the type's default stratum). In addition, if a specific stratum has been set on the debug target, this patch retrieves source for that stratum. Would this be more flexible? (i.e. if you allow the user to change/select specific stratums?)
(In reply to comment #15) > Created an attachment (id=147266) [details] > alternative > > OK, I see what you're doing - retrieving the source for the default stratum. I wasn't even sure how the default stratum is involved in this computation. The call frame.getSourcePath() I copied from JavaDebugUtils. Stepping through the code I see this: - virtualMachine().getDefaultStratum() gives a null - ReferenceTypeImpl.getStratum(null) finds the ObjectTeams/Java stratum But that's only the start. JSR-45 defines LINE BASED mappings, so if ever possible, we must hold on to any line number information. I.e., if the source lookup is done for a stack frame, we MUST use the line number of that stack frame. This is the key difference between our patches. > I'm curious if this patch is equivalent (i.e. retrieve the source for the > type's default stratum). No, your alternative doesn't work for me. I couldn't see any difference with or without that patch. When passing "null" to get getSourcePaths() the type's default stratum will be used, which is "the right thing". > In addition, if a specific stratum has been set on the > debug target, this patch retrieves source for that stratum. Would this be more > flexible? (i.e. if you allow the user to change/select specific stratums?) Is that possible? How can a user change/select a specific stratum?? We need to use the type's default stratum for line-based lookup of files (and then lines). All this worked out-of-the box when debugging Java applications. My patch establishes the same behavior for PDE launches, too.
Thanks for the feedback Stephan. I don't have a JSR045 bundle, so I'm programming in the dark... do you have a suggested/simple test case I could use for this? I'm curious why this is not working as I expect... The Java debugger allows us to set the stratum to debug in. This allows users to see and step/debug in different sources, and switch betwee them. The Java debugger provides an action in the context menu to switch between strata, when more than one is available. In the interest of assuring myself we have the correct solution I will push this to M3. If you could help me with an example/test case I could use, that would be great.
(In reply to comment #17) > Thanks for the feedback Stephan. I don't have a JSR045 bundle, so I'm > programming in the dark... do you have a suggested/simple test case I could use > for this? I should give it a try. Since you probably don't want to include all of OT/Equinox in the test I'll do the following: - create a minimal OT/Equinox bundle that should work without any bytecode weaving - pack it with faked sources so the Java editors & compiler won't bail out on non-Java keywords. - write steps how to use this for manual testing Could you take over from there to create an automatic test case? > I'm curious why this is not working as I expect... The Java debugger allows us > to set the stratum to debug in. This allows users to see and step/debug in > different sources, and switch betwee them. The Java debugger provides an action > in the context menu to switch between strata, when more than one is available. Is this new in 3.6?? In the current OTDT (based on 3.5.1) I have plowed every milimeter of the UI and can't find such menu action. Where did you hide it? ;-) Secondly: my concern is not about switching strata but about selecting the correct sourcefile _within_ one stratum. So, an OT/J classfile has two strata: "Java" and "OTJ", the former will just map everything to itself, the second dispatches different bits of code to different source files. And its not the user selecting, but the current stack frame. I think a good analogy would be an "include" directive in a JSP page: it adds code to the current class whose sources are defined elsewhere. (I'm not a JSP expert, though). The stratum tells the debugger which method came from which source file, all within the same stratum ("JSP", I suppose). > In the interest of assuring myself we have the correct solution I will push > this to M3. If you could help me with an example/test case I could use, that > would be great. OK, M2 or M3 makes no big difference to me. I understand your discomfort when programming in the dark. Let's try to light a candle ;-)
(In reply to comment #18) > So, an OT/J classfile has two strata: "Java" and "OTJ", oops, I have to correct myself: we only record the OTJ stratum. So maybe that's why I don't see any menu option: there is no choice among multiple strata.
(In reply to comment #19) > > So, an OT/J classfile has two strata: "Java" and "OTJ", > oops, I have to correct myself: we only record the OTJ stratum. > So maybe that's why I don't see any menu option: there is no > choice among multiple strata. Correct, the "Show Source >" cascade menu only appears when there is > 1 choice. The code is in org.eclipse.jdt.internal.debug.ui.actions.ShowStratumAction.
Created attachment 147368 [details] faked OT/Equinox bundle useful for testing OK, here we go, the attachment is a minimal OT/Equinox bundle pretending to contain plain Java only. How to set up for testing: 1. Import the bundle: Import > Plug-in Development > Plug-ins and Fragments > Import as: binary project 2. Set breakpoint in fake-src/bug242461/SuperTeam.java:23 ("r.testMe();") 3. Launch Bug242461.launch in debug mode. Now every time the bundle is started (osgi console comes handy to repeat things) debugger stops at the breakpoint. First time suspending you need to add the "fake-src" folder to the source lookup. That was the plain part. Now for JSR-45: F5 > should jump to SuperTeam.java:13 ("hookMethod();") F5 > should jump to SubTeam.java:10 ("System.out.println("hookMethod");") F7 as desired to watch the same in reverse motion. Without proper patch, either F5-action will jump to the wrong file, since both will (quasi non-deterministically) use the same source file (the first in the array of sourcePaths). Explaining the man behind the curtain: The argument "r" in testTheRole will be an instance of a synthetic class SubTeam$__OT__Role, which combines the methods of SuperTeam.Role and SubTeam.Role (both classes are connected via implicit inheritance, where implicit inheritance is implemented by copying byte codes). The sources are "almost" original OT/J code, with these differences: - modifier "team" in SuperTeam and SubTeam - instantiating an abstract role ("new Role()") (the type is dynamically bound to the non-abstract class SubTeam.Role). Also contained: the minimal runtime library otre_min.jar, containing some pre-defined types. Please ask if you don't get it running or see different results.
For the curious: SubTeam$__OT__Role.class contains the following source debug extension: SMAP SubTeam$__OT__Role.class OTJ *S OTJ *F + 1 SuperTeam.java bug242461/SuperTeam.java + 2 SubTeam.java bug242461/SubTeam.java *L 11#1:15 13#1,2:16 1#2,14:1 65533#2,2:65533 *E The file sections (*F) of stratum OTJ contains two entries: + 1 SuperTeam.java bug242461/SuperTeam.java + 2 SubTeam.java bug242461/SubTeam.java The line numbers section (*L) refers to these files as "#1" or "#2", so while the bulk (lines 1-14) maps to #2 (SubTeam), byte-code lines 16-17 map to source lines 13-14 in #1 (SuperTeam), these are the codes of testMe() (this encoding is terrible to read, I know). Please ignore the special line number 65533 ;-) All this is just background information and shouldn't be relevant for working on this issue.
(In reply to comment #21) Sorry, one correction necessary: > First time suspending you need to > add the "fake-src" folder to the source lookup. This way "the bug doesn't work", because PDESourceLookupQuery will not find the source at all - findSourceElement() returns null, at what point everything will fall back to debug.core behavior, which produces the CORRECT result :-/ -> Failed to test PDE. Solution: add the source lookup to the bundle jar before starting to debug. In my initial 3.5-based setup I didn't see the bundle jar under Java Build Path, that's why I suggested differently.
I discovered the same behavior/problem with the test case (revering to Java source lookup, which works properly... :-) However, the difference here is that you have > 1 source file for a type. In JSPs, that is not the case (so in that case, asking a type for its source paths was equivalent to asking the frame). In this case, the line number plays an important role, and the stack frame always returns its souce path based on the location/line number (which can be different files). So, yes, I agree with your patch :-) Thanks.
Applied patch. Fixed in HEAD. Please verify, Curtis.
Marking fixed.
+1 verified.
(In reply to comment #24) > I discovered the same behavior/problem with the test case (revering to Java > source lookup, which works properly... :-) what do you do if your bug is broken ? ;-) > However, the difference here is that you have > 1 source file for a type. In > JSPs, that is not the case Didn't they have the option to merge multiple sources into one servlet? Never mind, it's ages since I looked at JSP. > So, yes, I agree with your patch :-) Thanks for the quick co-operation.
