From d62e3fa0c386c7e2aeb621f74e1603b4efd45f8b Mon Sep 17 00:00:00 2001 From: Lisa-Marie Saru Date: Wed, 22 Feb 2023 15:54:15 +0100 Subject: [PATCH] Add null check for Disassembly view source Disassembly view is expected to be populated with lines that are fetched from a given source file. There might be the case where instructions on what lines to append are wrong. This results in a null response that will propagate through the code leading to a NPE. The current commit is proofing the code from NPE by: - removing the source position of the lines that were not found within the given file - null checking the source before becoming a key element in the code flow - adding logging if expected lines are not found in the given file Resolves: #287 --- .../META-INF/MANIFEST.MF | 2 +- .../ui/disassembly/DisassemblyPart.java | 35 ++++++++++-- .../model/DisassemblyDocument.java | 56 ++++++++++--------- .../ui/disassembly/model/SourceFileInfo.java | 5 ++ 4 files changed, 66 insertions(+), 32 deletions(-) diff --git a/dsf/org.eclipse.cdt.dsf.ui/META-INF/MANIFEST.MF b/dsf/org.eclipse.cdt.dsf.ui/META-INF/MANIFEST.MF index 9ce234906be..2afc8bb5b19 100644 --- a/dsf/org.eclipse.cdt.dsf.ui/META-INF/MANIFEST.MF +++ b/dsf/org.eclipse.cdt.dsf.ui/META-INF/MANIFEST.MF @@ -3,7 +3,7 @@ Bundle-ManifestVersion: 2 Bundle-Name: %pluginName Bundle-Vendor: %providerName Bundle-SymbolicName: org.eclipse.cdt.dsf.ui;singleton:=true -Bundle-Version: 2.7.0.qualifier +Bundle-Version: 2.7.100.qualifier Bundle-Activator: org.eclipse.cdt.dsf.internal.ui.DsfUIPlugin Bundle-Localization: plugin Require-Bundle: org.eclipse.ui;bundle-version="3.5.0", diff --git a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/DisassemblyPart.java b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/DisassemblyPart.java index 73ab03b6f63..c01bff54e60 100644 --- a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/DisassemblyPart.java +++ b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/DisassemblyPart.java @@ -2778,7 +2778,12 @@ public void insertSource(AddressRangePosition _pos) { if (lineAddr == null) { fi.fLine2Addr[lineNr] = pos.fAddressOffset; String source = fi.getLines(lineNr, last); - fDocument.insertSource(pos, source, lineNr, true); + // Remove source position if the lines were not found + if (source == null) { + fDocument.removeSourcePosition(pos); + } else { + fDocument.insertSource(pos, source, lineNr, true); + } } else { final int comparison = lineAddr.compareTo(pos.fAddressOffset); if (comparison > 0) { @@ -2805,10 +2810,20 @@ public void insertSource(AddressRangePosition _pos) { } fi.fLine2Addr[lineNr] = pos.fAddressOffset; String source = fi.getLines(lineNr, last); - fDocument.insertSource(pos, source, lineNr, true); + // Remove source position if the lines were not found + if (source == null) { + fDocument.removeSourcePosition(pos); + } else { + fDocument.insertSource(pos, source, lineNr, true); + } } else if (comparison == 0) { String source = fi.getLines(lineNr, last); - fDocument.insertSource(pos, source, lineNr, true); + // Remove source position if the lines were not found + if (source == null) { + fDocument.removeSourcePosition(pos); + } else { + fDocument.insertSource(pos, source, lineNr, true); + } } else { // new source position is after old position try { @@ -2823,11 +2838,21 @@ public void insertSource(AddressRangePosition _pos) { fDocument.removeSourcePosition(pos); } else { String source = fi.getLines(lineNr, last); - fDocument.insertSource(pos, source, lineNr, true); + // Remove source position if the lines were not found + if (source == null) { + fDocument.removeSourcePosition(pos); + } else { + fDocument.insertSource(pos, source, lineNr, true); + } } } else { String source = fi.getLines(lineNr, last); - fDocument.insertSource(pos, source, lineNr, true); + // Remove source position if the lines were not found + if (source == null) { + fDocument.removeSourcePosition(pos); + } else { + fDocument.insertSource(pos, source, lineNr, true); + } } } catch (BadPositionCategoryException e) { internalError(e); diff --git a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/model/DisassemblyDocument.java b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/model/DisassemblyDocument.java index 157fa4666e4..2aad29302c5 100644 --- a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/model/DisassemblyDocument.java +++ b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/model/DisassemblyDocument.java @@ -1229,36 +1229,40 @@ public AddressRangePosition insertLabel(AddressRangePosition pos, BigInteger add public SourcePosition insertSource(SourcePosition pos, String source, int line, boolean endOfSource) { // System.out.println("insertSource at "+getAddressText(pos.fAddressOffset)); // System.out.println(source); - String sourceLines = source; - if (!source.isEmpty() && sourceLines.charAt(source.length() - 1) != '\n') { - sourceLines += "\n"; //$NON-NLS-1$ - } - try { - assert !pos.fValid; - int oldLength = pos.length; - pos.length = sourceLines.length(); - pos.fLine = line; - pos.fValid = true; - removeInvalidSourcePosition(pos); - replace(pos, oldLength, sourceLines); - if (!endOfSource) { - if (pos.length > 0) { - SourcePosition oldPos = getSourcePosition(pos.offset + pos.length); - if (oldPos == null || oldPos.fAddressOffset.compareTo(pos.fAddressOffset) != 0) { - pos = new SourcePosition(pos.offset + pos.length, 0, pos.fAddressOffset, pos.fFileInfo, line, - pos.fLast, false); - addSourcePosition(pos); - addModelPosition(pos); - addInvalidSourcePositions(pos); - } else { - //TLETODO need more checks for correct source pos - pos = oldPos; + // Check if source is not null, so it won't trigger NullPointerException later + if (source != null) { + String sourceLines = source; + if (!source.isEmpty() && sourceLines.charAt(source.length() - 1) != '\n') { + sourceLines += "\n"; //$NON-NLS-1$ + } + try { + assert !pos.fValid; + int oldLength = pos.length; + pos.length = sourceLines.length(); + pos.fLine = line; + pos.fValid = true; + removeInvalidSourcePosition(pos); + replace(pos, oldLength, sourceLines); + if (!endOfSource) { + if (pos.length > 0) { + SourcePosition oldPos = getSourcePosition(pos.offset + pos.length); + if (oldPos == null || oldPos.fAddressOffset.compareTo(pos.fAddressOffset) != 0) { + pos = new SourcePosition(pos.offset + pos.length, 0, pos.fAddressOffset, pos.fFileInfo, + line, pos.fLast, false); + addSourcePosition(pos); + addModelPosition(pos); + addInvalidSourcePositions(pos); + } else { + //TLETODO need more checks for correct source pos + pos = oldPos; + } } } + } catch (BadLocationException e) { + internalError(e); } - } catch (BadLocationException e) { - internalError(e); } + return pos; } diff --git a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/model/SourceFileInfo.java b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/model/SourceFileInfo.java index 67b544ec318..1756c67934a 100644 --- a/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/model/SourceFileInfo.java +++ b/dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/model/SourceFileInfo.java @@ -25,6 +25,7 @@ import org.eclipse.core.resources.IStorage; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IStatus; +import org.eclipse.core.runtime.Status; import org.eclipse.core.runtime.content.IContentType; import org.eclipse.core.runtime.jobs.Job; import org.eclipse.jface.text.BadLocationException; @@ -147,6 +148,10 @@ public String getLines(int first, int last) { } return fSource.get(startOffset, endOffset - startOffset); } catch (BadLocationException e) { + // Log error to indicate what is the cause of the issue + String warningMessage = "Line(s) " + Integer.toString(first) + "-" + Integer.toString(last) //$NON-NLS-1$ + + " cannot be found in file: " + fFileKey + ".\nSkipping lines!"; //$NON-NLS-1$ + DsfUIPlugin.log(Status.warning(warningMessage, e)); return null; } }