Skip to content

Commit

Permalink
Add null check for Disassembly view source
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@LizzMre @jonahgraham
LizzMre authored and jonahgraham committed Feb 23, 2023
1 parent 40e3314 commit 48b9774
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 32 deletions.
2 changes: 1 addition & 1 deletion dsf/org.eclipse.cdt.dsf.ui/META-INF/MANIFEST.MF
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
35 changes: 30 additions & 5 deletions ...pse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/DisassemblyPart.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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 {
Expand All @@ -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);
Expand Down
56 changes: 30 additions & 26 deletions ...f.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/model/DisassemblyDocument.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
5 changes: 5 additions & 0 deletions ...dt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/model/SourceFileInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
Expand Down

0 comments on commit 48b9774

Please sign in to comment.