diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/viewer/field/LabelCodeUnitFormat.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/viewer/field/LabelCodeUnitFormat.java index 0ef6165b8c..5c2197ffc9 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/viewer/field/LabelCodeUnitFormat.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/viewer/field/LabelCodeUnitFormat.java @@ -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); } } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/program/model/listing/CodeUnitFormat.java b/Ghidra/Features/Base/src/main/java/ghidra/program/model/listing/CodeUnitFormat.java index b5bdf462df..f8e3b02dff 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/program/model/listing/CodeUnitFormat.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/program/model/listing/CodeUnitFormat.java @@ -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; diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/util/viewer/field/LabelFieldFactoryTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/util/viewer/field/LabelFieldFactoryTest.java index 243a068cf7..0d7536f1cd 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/util/viewer/field/LabelFieldFactoryTest.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/util/viewer/field/LabelFieldFactoryTest.java @@ -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 diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/util/viewer/field/OperandFieldFactoryTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/util/viewer/field/OperandFieldFactoryTest.java index f483d6cc46..349ba7eda3 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/util/viewer/field/OperandFieldFactoryTest.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/util/viewer/field/OperandFieldFactoryTest.java @@ -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 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); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/symbol/SymbolUtilities.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/symbol/SymbolUtilities.java index 9e6fdf3e38..76975364c8 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/symbol/SymbolUtilities.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/symbol/SymbolUtilities.java @@ -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