Community
Participate
Working Groups
This is a regression from earlier versions. Register values should be evaluated against the stack frame selected in the Debug view. Switching to another frame should force the re-evaluation of the entire view in the new context. This is a result of https://bugs.eclipse.org/bugs/show_bug.cgi?id=176627.
Created attachment 163787 [details] Fix. This patch replaces the default implementation provided by the platform. It contains the set of adapters required to integrate a CDI-backed debugger into the Registers view.
The proposed patch also fixes https://bugs.eclipse.org/bugs/show_bug.cgi?id=308052.
John, could you please review this patch. It's quite big and I'm not very comfortable submitting it without review. Please don't use GDB 7.0, there is a problem with it and you won't see registers at all.
Mikhail, with this patch, the Registers view is empty in a CDI session. Did you perhaps forget to include something in the patch file?
(In reply to comment #4) > Mikhail, with this patch, the Registers view is empty in a CDI session. Did you > perhaps forget to include something in the patch file? Is it working without my patch? Which version of gdb you are using? There is a problem with 7.0.
Jeesh. I overlooked (forgot, really) your comment # 3. I'll try again :-)
(In reply to comment #6) > Jeesh. I overlooked (forgot, really) your comment # 3. I'll try again :-) I was told that the problem was fixed in the GDB HEAD.
Hm. Well, I tried with MinGW 6.8 gdb, and now I see content in the Registers view, but no values. I backed out your changes and the values show up. I reapplied your patch and, again, no values for the registers.
(In reply to comment #8) > Hm. Well, I tried with MinGW 6.8 gdb, and now I see content in the Registers > view, but no values. I backed out your changes and the values show up. I > reapplied your patch and, again, no values for the registers. Let me try my patch again, maybe something is missing.
(In reply to comment #8) > Hm. Well, I tried with MinGW 6.8 gdb, and now I see content in the Registers > view, but no values. I backed out your changes and the values show up. I > reapplied your patch and, again, no values for the registers. I can see the problem now. If the view is open before the session started everything works fine. Otherwise, the values appear only after stepping or resuming. Will fix.
I didn't look at the cause of the problem but is it limited to CDI, and not affecting DSF?
(In reply to comment #11) > I didn't look at the cause of the problem but is it limited to CDI, and not > affecting DSF? DSF doesn't support this feature. I was thinking of submitting a request for it.
Created attachment 163821 [details] Updated patch.
John, please try the updated patch.
Correct version.
(In reply to comment #12) > (In reply to comment #11) > > I didn't look at the cause of the problem but is it limited to CDI, and not > > affecting DSF? > > DSF doesn't support this feature. I was thinking of submitting a request for > it. I could swear someone at Nokia opened one a while back.
Mikhail, here's my feedback. All minor stuff... - in CRegisterManager.setCurrentFrame, the call to getRegisterGroups() would be better situated within the if scope. Within that loop, I'd improve readability by making use of Java 5; that would even do away with the need for the 'groups' local variable. - shouldn't CRegisterManagerProxies query the context service rather than assume the Debug view is providing the context? - I think the name RegisterViewerInputProvider is misleading, especially looking at it from the plugin.xml perspective. A CDT debug element type can only have one such provider, and it is not viewer specific. Now, it so happens that right now we only need the input translation for the Registers view, but if next month we need it for another view, RegisterViewerInputProvider would need to handle that view too, and clearly that would be confusing. I think a better name would be CViewerInputProvider - The code in CDebugElementAdapterFactory that returns the RegisterViewerInputProvider could probably be consolidated a bit by having a single check that does three instanceof checks, following the pattern elsewhere in that method. - Seems to me the method CRegisterManagerModelProxy.refreshRoot is extraneous; the logic could just be located in update() - I think CRegisterManagerProxy.handleDebugEvents(DebugEvent[]) is subject to a race condition NPE. It should call getModelProxy() and store it in a local variable and use that local thereafter. - I think the comment in CRegisterManagerProxyMementoProvider.getElementName(Object, IPresentationContext) is probably misleading. That code will maintain a single expansion state for all of CDT. - I'd remove the commented out constants in RegistersViewColumnPresentation.
(In reply to comment #17) Thanks John, I owe you a drink, "White Russian" I guess. Will change the patch according your recomendations.
Created attachment 163850 [details] Patch updated according John's suggestions.
Applied to HEAD.
Mikhail, I just noticed that the Breakpoints view fails to show breakpoints in a CDI session as a result of this patch. If the Debug view selection is CDT debug element, then the breakpoints view goes empty.
(In reply to comment #21) > Mikhail, I just noticed that the Breakpoints view fails to show breakpoints in > a CDI session as a result of this patch. If the Debug view selection is CDT > debug element, then the breakpoints view goes empty. Hmm, interesting... I suspect the selection of adaptors is affected. I'll check it, thanks
Fixed. Replaced CViewerInputProvider by two input providers: for stack frames and for others (default). Each extends the corresponding platform input provider. Comment: it's getting more and more complicated to support the Standard Debug Model based implementations. Many of the platform classes we use are internal, the consequences of which are not clear. Minor changes in the platform classes can break major features in CDT.
Mikhail, I'm seeing an empty registers view for CDI.
(In reply to comment #24) > Mikhail, I'm seeing an empty registers view for CDI. It's not GDB 7.0, is it?
Ugh. I don't know why I have a mental block with that issue... :-( This is a serious problem, though. We're not going to ship Helios with it, are we? How difficult can it be to address? After all, DSF-GDB is able to get the register info from gdb 7.0.
(In reply to comment #26) > Ugh. I don't know why I have a mental block with that issue... :-( This is a > serious problem, though. We're not going to ship Helios with it, are we? How > difficult can it be to address? After all, DSF-GDB is able to get the register > info from gdb 7.0. I was told that it had been fixed in the HEAD of GDB. As far as I know DSF doesn't have an internal object model, it delegates all requests to the backend.That's why it it is not affected by this bug. We request the list of registers only once, at the begining. Theoretically, it's possible to try to get registers on each suspend. Not sure though that doing it to avoid this particular bug is a good idea.
(In reply to comment #27) > (In reply to comment #26) > > Ugh. I don't know why I have a mental block with that issue... :-( This is a > > serious problem, though. We're not going to ship Helios with it, are we? How > > difficult can it be to address? After all, DSF-GDB is able to get the register > > info from gdb 7.0. > I was told that it had been fixed in the HEAD of GDB. > As far as I know DSF doesn't have an internal object model, it delegates all > requests to the backend.That's why it it is not affected by this bug. > We request the list of registers only once, at the begining. Theoretically, > it's possible to try to get registers on each suspend. Not sure though that > doing it to avoid this particular bug is a good idea. How about only asking again if the first list of register was empty?
(In reply to comment #28) > How about only asking again if the first list of register was empty? I thought about that, just didn't want to spend time on it. OK, I'll reopen https://bugs.eclipse.org/bugs/show_bug.cgi?id=309212.
(In reply to comment #28) > How about only asking again if the first list of register was empty? Marc, I hope you're not trying to keep me busy :-)
(In reply to comment #30) > (In reply to comment #28) > > How about only asking again if the first list of register was empty? > Marc, I hope you're not trying to keep me busy :-) Not at all. In fact, I already thought of some bugs patches I'll need you review, so the least busy you are the better :-)
*** Bug 256917 has been marked as a duplicate of this bug. ***