Bug 572880: Keep internal representation of opcode as a String

We receive a string from GDB and then display the same string to the
user. So instead of converting it internally into a Byte[] to then
convert it back into a String, keep it as a String.

This fixes a regression where some GDBs' output format was not as space
separated 8-bit-bytes, but as words or similar.

Change-Id: I4ea241ff9ea45165489604fee26a3593ec3f6756

debug/org.eclipse.cdt.debug.dap/src/org/eclipse/cdt/debug/dap/DapDisassemblyBackend.java

@@ -258,13 +258,8 @@ public class DapDisassemblyBackend extends AbstractDisassemblyBackend {
 					funcOffset = ""; //$NON-NLS-1$
 				}
 
-				Byte[] opcode = null;
-				if (instruction.getInstructionBytes() != null) {
-					opcode = DisassemblyUtils.decodeOpcode(instruction.getInstructionBytes());
-				}
-
 				p = fCallback.getDocument().insertDisassemblyLine(p, address, instrLength.intValue(), funcOffset,
-						opcode, instruction.getInstruction(), file, lineNumber);
+						instruction.getInstructionBytes(), instruction.getInstruction(), file, lineNumber);
 				if (p == null) {
 					break;
 				}

debug/org.eclipse.cdt.debug.ui/src/org/eclipse/cdt/debug/internal/ui/disassembly/dsf/DisassemblyPosition.java

@@ -21,7 +21,10 @@ import java.math.BigInteger;
 public class DisassemblyPosition extends AddressRangePosition {
 
 	public char[] fFunction;
-	public Byte[] fOpcode;
+	/**
+	 * String of opcodes as it will be displayed to users. Can be null which is handled the same as empty string.
+	 */
+	public String fRawOpcode;
 
 	/**
 	 * @param offset
@@ -29,12 +32,12 @@ public class DisassemblyPosition extends AddressRangePosition {
 	 * @param addressOffset
 	 * @param addressLength
 	 * @param functionOffset
-	 * @param opcode
+	 * @param rawOpcode String of opcodes as it will be displayed to users. Can be null which is handled the same as empty string.
 	 */
 	public DisassemblyPosition(int offset, int length, BigInteger addressOffset, BigInteger addressLength,
-			String functionOffset, Byte[] opcode) {
+			String functionOffset, String rawOpcode) {
 		super(offset, length, addressOffset, addressLength);
-		fOpcode = opcode;
+		fRawOpcode = rawOpcode;
 		fFunction = functionOffset.toCharArray();
 	}
 

debug/org.eclipse.cdt.debug.ui/src/org/eclipse/cdt/debug/internal/ui/disassembly/dsf/DisassemblyUtils.java

@@ -15,8 +15,6 @@
 package org.eclipse.cdt.debug.internal.ui.disassembly.dsf;
 
 import java.math.BigInteger;
-import java.util.ArrayList;
-import java.util.List;
 
 import org.eclipse.cdt.debug.ui.CDebugUIPlugin;
 import org.eclipse.core.runtime.Platform;
@@ -70,41 +68,4 @@ public class DisassemblyUtils {
 		}
 		return new BigInteger(string);
 	}
-
-	/**
-	 * Decode given string representation of a space separated hex encoded byte
-	 * array
-	 *
-	 * @param value
-	 *            space separated hexadecimal byte array
-	 * @return opcode bytes as <code>Byte</code> array, an empty array if the decoding failed
-	 */
-	public static Byte[] decodeOpcode(String value) {
-		List<Byte> opcodeBytesList = new ArrayList<>();
-		if (value == null || value.isBlank()) {
-			return new Byte[0];
-		}
-		// Removing space separation and parse as bytes
-		for (String opcodeStringValue : value.split("\\s+")) { //$NON-NLS-1$
-			if (opcodeStringValue.length() > 0) {
-				// Check that the opcode does not contain some invalid byte sequence
-				if (opcodeStringValue.length() > 2) {
-					return new Byte[0];
-				}
-				byte byteValue = 0;
-				char charAtIndexZero = opcodeStringValue.charAt(0);
-				char charAtIndexOne = opcodeStringValue.length() > 1 ? opcodeStringValue.charAt(1) : 0;
-
-				int digitZero = Character.digit(charAtIndexZero, 16);
-				int digitOne = Character.digit(charAtIndexOne, 16);
-				// Check if characters are valid hex numbers
-				if (digitZero == -1 || digitOne == -1) {
-					return new Byte[0];
-				}
-				byteValue = (byte) ((digitZero << 4) + digitOne);
-				opcodeBytesList.add(Byte.valueOf(byteValue));
-			}
-		}
-		return opcodeBytesList.toArray(new Byte[0]);
-	}
 }

debug/org.eclipse.cdt.debug.ui/src/org/eclipse/cdt/debug/internal/ui/disassembly/dsf/IDisassemblyDocument.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2010, 2012 Wind River Systems, Inc. and others.
+ * Copyright (c) 2010, 2021 Wind River Systems, Inc. and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -20,8 +20,9 @@ import java.math.BigInteger;
 import org.eclipse.jface.text.BadLocationException;
 
 /**
- * DSF Disassembly view backends (CDI and DSF) need this limited access to the
+ * Disassembly view backends need this limited access to the
- * editor/view Document.
+ * editor/view Document. The known backends are DSF, TCF and Dap. Formerly
+ * the CDI backend used it before it was removed from the CDT soure tree.
  */
 public interface IDisassemblyDocument {
 
@@ -34,10 +35,20 @@ public interface IDisassemblyDocument {
 			String functionOffset, String instruction, String compilationPath, int lineNumber)
 			throws BadLocationException;
 
+	/**
+	 * This method. that takes opcode as a Byte[] exists solely for TCF integration.
+	 */
 	AddressRangePosition insertDisassemblyLine(AddressRangePosition p, BigInteger address, int length,
 			String functionOffset, Byte[] opcode, String instruction, String compilationPath, int lineNumber)
 			throws BadLocationException;
 
+	/**
+	 * @param rawOpcode String of opcodes as it will be displayed to users. Can be null which is handled the same as empty string.
+	 */
+	AddressRangePosition insertDisassemblyLine(AddressRangePosition p, BigInteger address, int length,
+			String functionOffset, String rawOpcode, String instruction, String compilationPath, int lineNumber)
+			throws BadLocationException;
+
 	AddressRangePosition getDisassemblyPosition(BigInteger address);
 
 	BigInteger getAddressOfLine(int line);

dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/DisassemblyBackendDsf.java

@@ -739,10 +739,10 @@ public class DisassemblyBackendDsf extends AbstractDisassemblyBackend implements
 					functionOffset = ""; //$NON-NLS-1$
 				}
 
-				Byte[] opcode = {};
+				String opcode = null;
 				// Get raw Opcodes if available
 				if (instruction instanceof IInstructionWithRawOpcode) {
-					opcode = DisassemblyUtils.decodeOpcode(((IInstructionWithRawOpcode) instruction).getRawOpcode());
+					opcode = ((IInstructionWithRawOpcode) instruction).getRawOpcode();
 				}
 
 				p = fCallback.getDocument().insertDisassemblyLine(p, address, instrLength.intValue(), functionOffset,
@@ -895,10 +895,9 @@ public class DisassemblyBackendDsf extends AbstractDisassemblyBackend implements
 						funcOffset = ""; //$NON-NLS-1$
 					}
 
-					Byte[] opcode = {};
+					String opcode = null;
 					if (instruction instanceof IInstructionWithRawOpcode) {
-						opcode = DisassemblyUtils
+						opcode = ((IInstructionWithRawOpcode) instruction).getRawOpcode();
-								.decodeOpcode(((IInstructionWithRawOpcode) instruction).getRawOpcode());
 					}
 
 					p = fCallback.getDocument().insertDisassemblyLine(p, address, instrLength.intValue(), funcOffset,

dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/OpcodeRulerColumn.java

@@ -13,8 +13,6 @@
  *******************************************************************************/
 package org.eclipse.cdt.dsf.debug.internal.ui.disassembly;
 
-import java.util.StringJoiner;
-
 import org.eclipse.cdt.debug.internal.ui.disassembly.dsf.AddressRangePosition;
 import org.eclipse.cdt.debug.internal.ui.disassembly.dsf.DisassemblyPosition;
 import org.eclipse.cdt.dsf.debug.internal.ui.disassembly.model.DisassemblyDocument;
@@ -31,7 +29,7 @@ public class OpcodeRulerColumn extends DisassemblyRulerColumn {
 	public static final String ID = "org.eclipse.cdt.dsf.ui.disassemblyColumn.opcode"; //$NON-NLS-1$
 
 	/** Maximum width of column (in characters) */
-	/** 15 bytes plus separator */
+	/** 15 bytes plus separator for typical 8-bit instruction words */
 	private static final int MAXWIDTH = 44;
 
 	/**
@@ -56,9 +54,8 @@ public class OpcodeRulerColumn extends DisassemblyRulerColumn {
 				AddressRangePosition pos = doc.getDisassemblyPosition(offset);
 				if (pos instanceof DisassemblyPosition && pos.length > 0 && pos.offset == offset && pos.fValid) {
 					DisassemblyPosition disassPos = (DisassemblyPosition) pos;
-					if (disassPos.fOpcode != null) {
+					if (disassPos.fRawOpcode != null) {
-						// Format the output.
+						return disassPos.fRawOpcode;
-						return getOpcodeString(disassPos.fOpcode);
 					}
 				} else if (pos != null && !pos.fValid) {
 					return DOTS.substring(0, nChars);
@@ -70,18 +67,6 @@ public class OpcodeRulerColumn extends DisassemblyRulerColumn {
 		return ""; //$NON-NLS-1$
 	}
 
-	protected String getOpcodeString(Byte[] opcode) {
-		if (opcode.length == 0) {
-			return "??"; //$NON-NLS-1$
-		}
-		StringJoiner opcodeStringJoiner = new StringJoiner(" "); //$NON-NLS-1$
-		for (int i = 0; i < opcode.length; i++) {
-			opcodeStringJoiner.add(String.format("%02x", //$NON-NLS-1$
-					opcode[i].intValue() & 0xff));
-		}
-		return opcodeStringJoiner.toString();
-	}
-
 	@Override
 	protected int computeNumberOfCharacters() {
 		DisassemblyDocument doc = (DisassemblyDocument) getParentRuler().getTextViewer().getDocument();

dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/model/DisassemblyDocument.java

@@ -24,6 +24,7 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.StringJoiner;
 
 import org.eclipse.cdt.debug.internal.ui.disassembly.dsf.AddressRangePosition;
 import org.eclipse.cdt.debug.internal.ui.disassembly.dsf.DisassemblyPosition;
@@ -73,7 +74,7 @@ public class DisassemblyDocument extends REDDocument implements IDisassemblyDocu
 	private final Map<IStorage, SourceFileInfo> fFileInfoMap = new HashMap<>();
 
 	private int fMaxFunctionLength = 0;
-	/** Max opcode length is equal to the single bytes shown */
+	/** Max opcode length is the maximum string length of the opcode strings */
 	private int fMaxOpcodeLength = 0;
 
 	private boolean fShowAddresses = false;
@@ -156,19 +157,7 @@ public class DisassemblyDocument extends REDDocument implements IDisassemblyDocu
 	}
 
 	public int getMaxOpcodeLength() {
-		// Check if an error on opcode decode occurred, in
+		return fMaxOpcodeLength;
-		// this case we show ??
-		if (fMaxOpcodeLength == 0) {
-			return 2;
-		}
-		int retVal = fMaxOpcodeLength;
-		// We show the bytes in hex, therefore it needs to be multiplied by 2
-		retVal *= 2;
-		// add bytes separator length
-		if (retVal > 0) {
-			retVal += fMaxOpcodeLength - 1;
-		}
-		return retVal;
 	}
 
 	public int getAddressLength() {
@@ -704,8 +693,8 @@ public class DisassemblyDocument extends REDDocument implements IDisassemblyDocu
 			if (functionLength > fMaxFunctionLength) {
 				fMaxFunctionLength = functionLength;
 			}
-			if (disassPos.fOpcode != null) {
+			if (disassPos.fRawOpcode != null) {
-				int opcodeLength = disassPos.fOpcode.length;
+				int opcodeLength = disassPos.fRawOpcode.length();
 				if (opcodeLength > fMaxOpcodeLength) {
 					fMaxOpcodeLength = opcodeLength;
 				}
@@ -1008,22 +997,33 @@ public class DisassemblyDocument extends REDDocument implements IDisassemblyDocu
 		return pos;
 	}
 
-	/* (non-Javadoc)
-	 * @see org.eclipse.cdt.debug.internal.ui.disassembly.dsf.IDisassemblyDocument#insertDisassemblyLine(org.eclipse.cdt.debug.internal.ui.disassembly.dsf.AddressRangePosition, java.math.BigInteger, int, java.lang.String, java.lang.String, java.lang.String, int)
-	 */
 	@Override
 	public AddressRangePosition insertDisassemblyLine(AddressRangePosition pos, BigInteger address, int length,
 			String functionOffset, String instruction, String file, int lineNr) throws BadLocationException {
-		return insertDisassemblyLine(pos, address, length, functionOffset, null, instruction, file, lineNr);
+		return insertDisassemblyLine(pos, address, length, functionOffset, (String) null, instruction, file, lineNr);
 	}
 
-	/* (non-Javadoc)
-	 * @see org.eclipse.cdt.debug.internal.ui.disassembly.dsf.IDisassemblyDocument#insertDisassemblyLine(org.eclipse.cdt.debug.internal.ui.disassembly.dsf.AddressRangePosition, java.math.BigInteger, int, java.lang.String, java.lang.String, java.lang.String, int)
-	 */
 	@Override
 	public AddressRangePosition insertDisassemblyLine(AddressRangePosition pos, BigInteger address, int length,
 			String functionOffset, Byte[] opcode, String instruction, String file, int lineNr)
 			throws BadLocationException {
+
+		String opcodeString = "??"; //$NON-NLS-1$
+		if (opcode != null && opcode.length > 0) {
+			StringJoiner opcodeStringJoiner = new StringJoiner(" "); //$NON-NLS-1$
+			for (int i = 0; i < opcode.length; i++) {
+				opcodeStringJoiner.add(String.format("%02x", //$NON-NLS-1$
+						opcode[i].intValue() & 0xff));
+			}
+			opcodeString = opcodeStringJoiner.toString();
+		}
+		return insertDisassemblyLine(pos, address, length, functionOffset, opcodeString, instruction, file, lineNr);
+	}
+
+	@Override
+	public AddressRangePosition insertDisassemblyLine(AddressRangePosition pos, BigInteger address, int length,
+			String functionOffset, String rawOpcode, String instruction, String file, int lineNr)
+			throws BadLocationException {
 		assert isGuiThread();
 		String disassLine = null;
 		if (instruction == null || instruction.length() == 0) {
@@ -1034,14 +1034,15 @@ public class DisassemblyDocument extends REDDocument implements IDisassemblyDocu
 		AddressRangePosition disassPos;
 		if (lineNr < 0) {
 			disassPos = new DisassemblyPosition(0, disassLine.length(), address, BigInteger.valueOf(length),
-					functionOffset, opcode);
+					functionOffset, rawOpcode);
 		} else {
 			disassPos = new DisassemblyWithSourcePosition(0, disassLine.length(), address, BigInteger.valueOf(length),
-					functionOffset, opcode, file, lineNr);
+					functionOffset, rawOpcode, file, lineNr);
 		}
 		pos = insertAddressRange(pos, disassPos, disassLine, true);
 		addDisassemblyPosition(disassPos);
 		return pos;
+
 	}
 
 	/**

dsf/org.eclipse.cdt.dsf.ui/src/org/eclipse/cdt/dsf/debug/internal/ui/disassembly/model/DisassemblyWithSourcePosition.java

@@ -31,11 +31,11 @@ public class DisassemblyWithSourcePosition extends DisassemblyPosition {
 	 * @param addressOffset
 	 * @param addressLength
 	 * @param functionOffset
-	 * @param opcode
+	 * @param rawOpcode String of opcodes as it will be displayed to users. Can be null which is handled the same as empty string.
 	 */
 	public DisassemblyWithSourcePosition(int offset, int length, BigInteger addressOffset, BigInteger addressLength,
-			String functionOffset, Byte[] opcode, String file, int lineNr) {
+			String functionOffset, String rawOpcode, String file, int lineNr) {
-		super(offset, length, addressOffset, addressLength, functionOffset, opcode);
+		super(offset, length, addressOffset, addressLength, functionOffset, rawOpcode);
 		fFile = file;
 		fLine = lineNr;
 	}