Bug 315835

Comment 1 Warren Paul 2010-06-04 16:19:05 EDT

Created attachment 171169 [details]
patch

Comment 2 Warren Paul 2010-06-04 16:19:35 EDT

It would be good to get this into RC4 if it's not too late.

Comment 3 John Cortell 2010-06-04 16:54:30 EDT

+1. Would need to take a closer look to understand if previousPos being null is an expected state or not, but avoiding the NPE is certainly something we need to get in for Helios since you're actually experiencing it.

Comment 4 Warren Paul 2010-06-04 17:15:46 EDT

I don't completely understand the logic in there, but org.eclipse.cdt.dsf.debug.internal.ui.disassembly.model.DisassemblyDocument.getModelPosition(int) returns null every time for me when the disassembly view is first shown, e.g. first debug session after starting the IDE, or if you close and reopen the disassembly view.

Comment 5 John Cortell 2010-06-04 17:26:06 EDT

(In reply to comment #4)
> I don't completely understand the logic in there, but
> org.eclipse.cdt.dsf.debug.internal.ui.disassembly.model.DisassemblyDocument.getModelPosition(int)
> returns null every time for me when the disassembly view is first shown, e.g.
> first debug session after starting the IDE, or if you close and reopen the
> disassembly view.

Yeah. Toni's the expert on that logic, so he's the best person to determine whether there's a deeper issue here or not. But avoiding the NPE is must-have short term fix.

Comment 6 Warren Paul 2010-06-04 19:27:29 EDT

Committed the patch to HEAD.

Comment 7 CDT Genie 2010-06-04 20:23:00 EDT

*** cdt cvs genie on behalf of wpaul ***
 Bug 315835 -  NPE in DisassemblyPart

[*] DisassemblyPart.java 1.31 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/DisassemblyPart.java?root=Tools_Project&r1=1.30&r2=1.31

Comment 8 Marc Khouzam 2010-06-04 21:02:45 EDT

I don't know this code so this may be a stupid comment.

Do you guys feel confident that the solution should be

  } else if (previousPos == null || !previousPos.fValid) {
    onTop = true;
  }

and not

  } else if (previousPos != null && !previousPos.fValid) {
    onTop = true;
  }

Comment 9 Anton Leherbauer 2010-06-07 03:02:09 EDT

It would be good to have the stack trace of the NPE.
Normally, DisassemblyDocument.getModelPosition(int) should not return null for a valid position (provided the model is consistent), so the position must be invalid (that's where the stack trace would come in handy).
I think the correct fix should be to simply return from the method without trying to reveal an invalid offset which might in turn trigger another exception from the viewer.