Merge remote-tracking branch

'origin/GP-6345_ghidragon_string_offcut_labels--SQUASHED' (Closes #8882)
2026-05-26 14:50:46 +08:00 · 2026-02-02 05:15:50 -05:00
parent cd0956377b 20901efbe6
commit 365221d673
5 changed files with 131 additions and 78 deletions
@@ -4,9 +4,9 @@
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
- * 
+ *
 *      http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -19,10 +19,8 @@ import javax.help.UnsupportedOperationException;

 import ghidra.framework.options.ToolOptions;
 import ghidra.program.model.address.Address;
-import ghidra.program.model.data.DataType;
 import ghidra.program.model.listing.*;
 import ghidra.program.model.symbol.Symbol;
-import ghidra.program.model.symbol.SymbolUtilities;

 /**
 * A version of {@link BrowserCodeUnitFormat} that changes how labels are rendered in offcut 
@@ -44,15 +42,9 @@ public class LabelCodeUnitFormat extends BrowserCodeUnitFormat {
 		Symbol offsym = program.getSymbolTable().getPrimarySymbol(offcutAddress);
 		Address instructionAddress = instruction.getMinAddress();
 		long diff = offcutAddress.subtract(instructionAddress);
-		if (!offsym.isDynamic()) {
-			return getDefaultOffcutString(offsym, instruction, diff, true);
-		}
-
-		Symbol containingSymbol = program.getSymbolTable().getPrimarySymbol(instructionAddress);
-		if (containingSymbol != null) {
-			return containingSymbol.getName() + PLUS + diff;
-		}
-		return getDefaultOffcutString(offsym, instruction, diff, true);
+		boolean decorate = !offsym.isDynamic();
+		boolean simplify = true;
+		return getDefaultOffcutString(offsym, instruction, diff, decorate, simplify);
 	}

 	@Override
@@ -61,18 +53,9 @@ public class LabelCodeUnitFormat extends BrowserCodeUnitFormat {
 		Symbol offcutSymbol = program.getSymbolTable().getPrimarySymbol(offcutAddress);
 		Address dataAddress = data.getMinAddress();
 		int diff = (int) offcutAddress.subtract(dataAddress);
-		if (!offcutSymbol.isDynamic()) {
-			return getDefaultOffcutString(offcutSymbol, data, diff, true);
-		}
-
-		DataType dt = data.getBaseDataType();
-		String prefix = getPrefixForStringData(data, dataAddress, diff, dt);
-		if (prefix != null) {
-			String addressString = SymbolUtilities.getAddressString(dataAddress);
-			return addOffcutInformation(prefix, addressString, diff, true);
-		}
-
-		return getDefaultOffcutString(offcutSymbol, data, diff, true);
+		boolean simplify = !(offcutSymbol.isDynamic() && data.hasStringValue());
+		boolean decorate = true;
+		return getDefaultOffcutString(offcutSymbol, data, diff, decorate, simplify);
 	}

 }
@@ -1426,18 +1426,27 @@ public class CodeUnitFormat {
 		Symbol offcutSymbol = program.getSymbolTable().getPrimarySymbol(offcutAddress);
 		Address dataAddress = data.getMinAddress();
 		int diff = (int) offcutAddress.subtract(dataAddress);
-		if (!offcutSymbol.isDynamic()) {
-			return getDefaultOffcutString(offcutSymbol, data, diff, false);
-		}

-		DataType dt = data.getBaseDataType();
-		String prefix = getPrefixForStringData(data, dataAddress, diff, dt);
-		if (prefix != null) {
+		boolean isDynamicStringOffsetLabel = offcutSymbol.isDynamic() && data.hasStringValue();
+		if (isDynamicStringOffsetLabel && !options.showOffcutInfo) {
+			return trimOffset(offcutSymbol.getName());
+		}
+		if (isDynamicStringOffsetLabel && options.displayOptions.useAbbreviatedForm()) {
 			String addressString = SymbolUtilities.getAddressString(dataAddress);
+			String prefix = getPrefixForStringData(data, dataAddress, diff, data.getBaseDataType());
 			return addOffcutInformation(prefix, addressString, diff, options.showOffcutInfo);
 		}
+		boolean simplify = !isDynamicStringOffsetLabel;
+		boolean decorate = false;		// never decorate in operand field
+		return getDefaultOffcutString(offcutSymbol, data, diff, decorate, simplify);
+	}

-		return getDefaultOffcutString(offcutSymbol, data, diff, false);
+	private String trimOffset(String name) {
+		int lastIndex = name.lastIndexOf("_");
+		if (lastIndex > 0) {
+			return name.substring(0, lastIndex);
+		}
+		return name;
 	}

 	/**
@@ -1457,20 +1466,20 @@ public class CodeUnitFormat {
 		Symbol offsym = program.getSymbolTable().getPrimarySymbol(offcutAddress);
 		Address instructionAddress = instruction.getMinAddress();
 		long diff = offcutAddress.subtract(instructionAddress);
-		if (!offsym.isDynamic()) {
-			return getDefaultOffcutString(offsym, instruction, diff, false);
-		}
-
-		Symbol containingSymbol = program.getSymbolTable().getPrimarySymbol(instructionAddress);
-		if (containingSymbol != null) {
-			String displayName = containingSymbol.getName();
-			if (markupAddress != null) {
-				displayName = addNamespace(program, containingSymbol.getParentNamespace(),
-					displayName, markupAddress);
+		boolean decorate = false;		// we never decorate in the operand field
+		boolean simplify = true;		// we always simplify names of instruction labels
+		if (offsym.isDynamic()) {
+			Symbol containingSymbol = program.getSymbolTable().getPrimarySymbol(instructionAddress);
+			if (containingSymbol != null) {
+				String displayName = containingSymbol.getName();
+				if (markupAddress != null) {
+					displayName = addNamespace(program, containingSymbol.getParentNamespace(),
+						displayName, markupAddress);
+				}
+				return simplifyTemplate(displayName) + PLUS + SymbolUtilities.getDiffString(diff);
 			}
-			return simplifyTemplate(displayName) + PLUS + diff;
 		}
-		return getDefaultOffcutString(offsym, instruction, diff, false);
+		return getDefaultOffcutString(offsym, instruction, diff, decorate, simplify);
 	}

 	protected String addOffcutInformation(String prefix, String addressString, int diff,
@@ -1478,8 +1487,7 @@ public class CodeUnitFormat {
 		if (!decorate) {
 			return prefix;
 		}
-
-		return prefix + UNDERSCORE + addressString + PLUS + diff;
+		return prefix + UNDERSCORE + addressString + PLUS + SymbolUtilities.getDiffString(diff);
 	}

 	protected String getPrefixForStringData(Data data, Address dataAddress, int diff, DataType dt) {
@@ -1491,16 +1499,24 @@ public class CodeUnitFormat {
 	}

 	protected String getDefaultOffcutString(Symbol symbol, CodeUnit cu, long diff,
-			boolean decorate) {
-		String name = options.simplifyTemplate(symbol.getName());
+			boolean decorate, boolean simplify) {
+		String name = symbol.getName();
+		if (simplify) {
+			name = options.simplifyTemplate(name);
+		}
 		if (decorate) {
-			return name + ' ' + '(' + cu.getMinAddress() + PLUS + diff + ')';
+			String cuLocation = cu.getMinAddress().toString();
+			Symbol primarySymbol = cu.getPrimarySymbol();
+			if (primarySymbol != null && !primarySymbol.isDynamic()) {
+				cuLocation = primarySymbol.getName();
+			}
+			return name + ' ' + '(' + cuLocation + PLUS + SymbolUtilities.getDiffString(diff) + ')';
 		}
 		return name;
 	}

 	/**
-	 * Returns ShowBlockName setting
+	 * {@return ShowBlockName setting}
 	 */
 	public ShowBlockName getShowBlockName() {
 		return options.showBlockName;
@@ -15,8 +15,7 @@
 */
 package ghidra.app.util.viewer.field;

-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;

 import java.nio.charset.StandardCharsets;

@@ -131,7 +130,7 @@ public class LabelFieldFactoryTest extends AbstractGhidraHeadedIntegrationTest {

 		// expecting: u_artika and u_rtika
 		// the rest is the text of the real label below the offcut label
-		assertEquals("u_artika_01003002 u_rtika_01003004 u_Kartika", tf.getText());
+		assertEquals("u_Kartika+2 u_Kartika+4 u_Kartika", tf.getText());
 	}

 	@Test
@@ -151,7 +150,7 @@ public class LabelFieldFactoryTest extends AbstractGhidraHeadedIntegrationTest {

 		// expecting: u_artika__+1
 		// the rest is the text of the real label below the offcut label
-		assertEquals("u_artika_01003002 u_Kartika", tf.getText());
+		assertEquals("u_Kartika+2 u_Kartika", tf.getText());
 	}

 	@Test
@@ -192,6 +191,22 @@ public class LabelFieldFactoryTest extends AbstractGhidraHeadedIntegrationTest {
 		assertEquals("Bob (01002000+2)", tf.getText());
 	}

+	@Test
+	public void testOffcutLabelNonDynamicWithLabelAtMinAddress() {
+		createLabel("1002000", "Joe");
+		String offcutAddress = "1002002";
+		String name = "Bob";
+		createLabel(offcutAddress, name);
+
+		String minAddress = "1002000";
+		Address address = addr(minAddress);
+		assertTrue(cb.goToField(address, LabelFieldFactory.FIELD_NAME, 0, 1));
+		ListingTextField tf = (ListingTextField) cb.getCurrentField();
+
+		// the rest is the text of the real label below the offcut label
+		assertEquals("Bob (Joe+2) Joe", tf.getText());
+	}
+
 	@Test
 	public void testMultipleOffcutLabelsNonDynamic() {

@@ -235,7 +250,7 @@ public class LabelFieldFactoryTest extends AbstractGhidraHeadedIntegrationTest {
 		assertTrue(cb.goToField(address, LabelFieldFactory.FIELD_NAME, 0, 1));
 		ListingTextField tf = (ListingTextField) cb.getCurrentField();

-		assertEquals("u__0100310c", tf.getText());
+		assertEquals("u__01003100+0xc", tf.getText());
 	}

 	@Test
@@ -251,7 +266,7 @@ public class LabelFieldFactoryTest extends AbstractGhidraHeadedIntegrationTest {
 		assertTrue(cb.goToField(address, LabelFieldFactory.FIELD_NAME, 0, 1));
 		ListingTextField tf = (ListingTextField) cb.getCurrentField();

-		assertEquals("s_bcdef_01003201 s_abcdef_01003200", tf.getText());
+		assertEquals("s_bcdef_01003200+1 s_abcdef_01003200", tf.getText());
 	}

 	@Test
@@ -311,7 +326,7 @@ public class LabelFieldFactoryTest extends AbstractGhidraHeadedIntegrationTest {
 		assertTrue(cb.goToField(address, LabelFieldFactory.FIELD_NAME, 0, 1));
 		ListingTextField tf = (ListingTextField) cb.getCurrentField();

-		assertEquals("s_ijklmnopqrstuvwxyz_132c_0026", tf.getText());
+		assertEquals("s_ijklmnopqrstuvwxyz_132c_0004+0x22", tf.getText());
 	}

 	@Test
@@ -331,7 +346,7 @@ public class LabelFieldFactoryTest extends AbstractGhidraHeadedIntegrationTest {
 		assertTrue(cb.goToField(address, LabelFieldFactory.FIELD_NAME, 0, 1));
 		ListingTextField tf = (ListingTextField) cb.getCurrentField();

-		assertEquals("s__132c_0038", tf.getText());
+		assertEquals("s__132c_0004+0x34", tf.getText());
 	}

 	@Test
@@ -55,6 +55,7 @@ public class OperandFieldFactoryTest extends AbstractGhidraHeadedIntegrationTest
 	private CodeBrowserPlugin cb;
 	private Options fieldOptions;
 	private Program program;
+	private ProgramBuilder builder;

 	@Before
 	public void setUp() throws Exception {
@@ -73,7 +74,7 @@ public class OperandFieldFactoryTest extends AbstractGhidraHeadedIntegrationTest
 	}

 	private ProgramDB buildProgram() throws Exception {
-		ProgramBuilder builder = new ProgramBuilder("notepad", ProgramBuilder._X86, this);
+		builder = new ProgramBuilder("notepad", ProgramBuilder._X86, this);
 		builder.createMemory(".text", "0x1001000", 0x6600);

 		// testOffcutReferencesToFunction_Indirect
@@ -219,9 +220,8 @@ public class OperandFieldFactoryTest extends AbstractGhidraHeadedIntegrationTest
 	@Test
 	public void testOffcutStringDynamicLabelReference() throws Exception {

-		// Note: for dynamic labels, we don't render the string based upon its offcut index, but
-		//       rather we just show the entire string.  Alternatively, if there is a label there,
-		//       we show that.  The label will show the string at its offcut location.
+		// Note: If the reference string has no defined label at the start, we display a default
+		// label which shows the string as though it started at the offcut location.
 		Address operandAddr = addr("01001000");
 		assertOperandText(operandAddr, "s_defgh_01001234+3");

@@ -233,6 +233,25 @@ public class OperandFieldFactoryTest extends AbstractGhidraHeadedIntegrationTest
 		assertOperandText(operandAddr, "STR_01001234+3");
 	}

+	@Test
+	public void testOffcutStringDefinedLabelReference() throws Exception {
+
+		// Note: If the reference string has a defined label at the start, we display a default
+		// offcut label which shows the label at the start plus an offset amount.
+
+		builder.createLabel("0x1001234", "foo");
+		Address operandAddr = addr("01001000");
+
+		assertOperandText(operandAddr, "foo+3");
+
+		//
+		// change the option and make sure the rendering updates
+		//
+		setBooleanOption(OptionsBasedDataTypeDisplayOptions.DISPLAY_ABBREVIATED_DEFAULT_LABELS,
+			true);
+		assertOperandText(operandAddr, "STR_01001234+3");
+	}
+
 	@Test
 	public void testOffcutCharArrayDynamicLabelReference() throws Exception {

@@ -280,6 +299,24 @@ public class OperandFieldFactoryTest extends AbstractGhidraHeadedIntegrationTest
 		assertEquals("s_defgh", tf.getText());
 	}

+	@Test
+	public void testOffcutStringDynamicLabelReference_ShowOffcutInfoOptionOn() {
+		//
+		// Use options to hide the offcut info for a dynamic string reference.  By default the
+		// option is on (unless we change that).  This tests the 'off' condition.
+		//
+
+		ToolOptions options = tool.getOptions(GhidraOptions.CATEGORY_BROWSER_FIELDS);
+		options.setBoolean(
+			GhidraOptions.OPERAND_GROUP_TITLE + Options.DELIMITER + "Show Offcut Information",
+			true);
+
+		assertTrue(cb.goToField(addr("1001000"), OperandFieldFactory.FIELD_NAME, 0, 1));
+		ListingTextField tf = (ListingTextField) cb.getCurrentField();
+
+		assertEquals("s_defgh_01001234+3", tf.getText());
+	}
+
 	/**
 	 * Tests that offcut references into arrays are painted as offsets into the array and not
 	 * as simply an offset from the min address.
@@ -296,10 +333,9 @@ public class OperandFieldFactoryTest extends AbstractGhidraHeadedIntegrationTest
 		Address arrayAddr = addr("01001888");
 		Command<Program> cmd = new CreateArrayCmd(arrayAddr, 3, structure, 12);
 		applyCmd(program, cmd);
-
 		String arrayName = "ArrayOfStructures";
-		cmd = new AddLabelCmd(arrayAddr, arrayName, SourceType.USER_DEFINED);
-		applyCmd(program, cmd);
+
+		builder.createLabel("0x1001888", arrayName);

 		String operandAddressString = "1006440";
 		Address operandAddr = addr(operandAddressString);
@@ -4,9 +4,9 @@
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
- * 
+ *
 *      http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -452,7 +452,7 @@ public class SymbolUtilities {
 				}
 				long datOffset = address.subtract(data2.getMinAddress());
 				return (datOffset == 0 ? data2.getPathName()
-						: data2.getPathName() + PLUS + datOffset);
+						: data2.getPathName() + PLUS + getDiffString(datOffset));
 			}
 		}

@@ -482,19 +482,11 @@ public class SymbolUtilities {
 		String prefix = dataType.getDefaultOffcutLabelPrefix(data, data, data.getLength(),
 			DataTypeDisplayOptions.DEFAULT, offcutOffset);

-		//
-		// Strings take precedence
-		//
-		if (isString) {
-			// we draw strings with their real address instead of with an offset
-			return prefix + UNDERSCORE + getAddressString(address);
-		}
-
 		//
 		// If there is a label at the CodeUnit start, then we want to be based upon that, except
 		// in special cases, like String data
 		//
-		String offcutText = PLUS + Integer.toString(offcutOffset);
+		String offcutText = PLUS + getDiffString(offcutOffset);
 		Symbol symbol = data.getPrimarySymbol();
 		if (symbol != null && !symbol.isDynamic()) {
 			return symbol.getName() + offcutText;
@@ -541,7 +533,7 @@ public class SymbolUtilities {

 	private static String getDyanmicOffcutInstructionName(Instruction instruction,
 			Address codeUnitAddress, long diff) {
-		String offcutText = PLUS + Long.toString(diff);
+		String offcutText = PLUS + getDiffString(diff);

 		//
 		// If there is a label at the CodeUnit start, then we want to be based upon that, except
@@ -565,6 +557,17 @@ public class SymbolUtilities {
 		return segmentedAddress.normalize(codeUnitAddress.getSegment());
 	}

+	/**
+	 * Returns a string representing an address offset. If the offset is less than 10, it doesn't
+	 * add a prefix, otherwise the difference is shown in hex and includes the "0x" prefix.
+	 * @param diff the address difference
+	 * @return a string representing an offset from an address that is formated simply for small
+	 * values and in hex with a prefix for larger values.
+	 */
+	public static String getDiffString(long diff) {
+		return diff < 10 ? Long.toString(diff) : "0x" + Long.toString(diff, 16);
+	}
+
 	/**
 	 * Parse a dynamic name and return its address or null if unable to parse.
 	 * @param factory address factory