Merge remote-tracking branch

'origin/GP-1533-dragonmacher-decompiler-highlights-exception' (Closes
#3664)

Ghidra/Features/Base/src/main/java/ghidra/app/util/AddEditDialog.java

@@ -522,6 +522,7 @@ public class AddEditDialog extends DialogComponentProvider {
 	 */
 	private JPanel create() {
 		labelNameChoices = new GhidraComboBox<>();
+		labelNameChoices.setName("label.name.choices");
 		GhidraComboBox<NamespaceWrapper> comboBox = new GhidraComboBox<>();
 		comboBox.setEnterKeyForwarding(true);
 		namespaceChoices = comboBox;

Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/component/ClangHighlightController.java

@@ -27,6 +27,7 @@ import docking.widgets.fieldpanel.field.Field;
 import docking.widgets.fieldpanel.support.FieldLocation;
 import ghidra.app.decompiler.*;
 import ghidra.program.model.listing.Function;
+import ghidra.program.model.pcode.HighFunction;
 import ghidra.program.model.pcode.PcodeOp;
 import ghidra.util.ColorUtils;
 import util.CollectionUtils;
@@ -46,7 +47,7 @@ import util.CollectionUtils;
  * 		matching braces)
  *  </LI>
  *  <LI>Secondary Highlights - triggered by the user to show all occurrences of a particular
- *  	variable; they will stay until they are manually cleared by a user action.  The user can \
+ *  	variable; they will stay until they are manually cleared by a user action.  The user can
  *  	apply multiple secondary highlights at the same time, with different colors for each
  *  	highlight.
  *   	<B>These highlights apply to the function in use when the highlight is created.  Thus,
@@ -144,8 +145,8 @@ public abstract class ClangHighlightController {
 		return getSecondaryHighlight(token) != null;
 	}
 
-	public boolean hasSecondaryHighlights() {
-		return !secondaryHighlighters.isEmpty();
+	public boolean hasSecondaryHighlights(Function function) {
+		return !secondaryHighlightersbyFunction.get(function).isEmpty();
 	}
 
 	public Color getSecondaryHighlight(ClangToken token) {
@@ -174,7 +175,7 @@ public abstract class ClangHighlightController {
 	 * @param function the function
 	 * @return the highlighters
 	 */
-	public Set<ClangDecompilerHighlighter> getSecondaryHighlightersByFunction(Function function) {
+	public Set<ClangDecompilerHighlighter> getSecondaryHighlighters(Function function) {
 		return new HashSet<>(secondaryHighlightersbyFunction.get(function));
 	}
 
@@ -285,12 +286,14 @@ public abstract class ClangHighlightController {
 	 * @param f the function
 	 */
 	public void removeSecondaryHighlights(Function f) {
+
 		List<ClangDecompilerHighlighter> highlighters = secondaryHighlightersbyFunction.get(f);
 		for (ClangDecompilerHighlighter highlighter : highlighters) {
 			TokenHighlights highlights = highlighterHighlights.get(highlighter);
 			Consumer<ClangToken> clearHighlight = token -> updateHighlightColor(token);
 			doClearHighlights(highlights, clearHighlight);
 		}
+		highlighters.clear();
 		notifyListeners();
 	}
 
@@ -320,9 +323,18 @@ public abstract class ClangHighlightController {
 	}
 
 	public void removeHighlighter(DecompilerHighlighter highlighter) {
+
 		removeHighlighterHighlights(highlighter);
-		secondaryHighlighters.remove(highlighter);
 		highlighterHighlights.remove(highlighter);
+		secondaryHighlighters.remove(highlighter);
+
+		Collection<List<ClangDecompilerHighlighter>> lists =
+			secondaryHighlightersbyFunction.values();
+		for (List<ClangDecompilerHighlighter> highlighters : lists) {
+			if (highlighters.remove(highlighter)) {
+				break;
+			}
+		}
 	}
 
 	/**
@@ -330,6 +342,7 @@ public abstract class ClangHighlightController {
 	 * @param highlighter the highlighter
 	 */
 	public void removeHighlighterHighlights(DecompilerHighlighter highlighter) {
+
 		TokenHighlights highlighterTokens = highlighterHighlights.get(highlighter);
 		if (highlighterTokens == null) {
 			return;
@@ -348,21 +361,19 @@ public abstract class ClangHighlightController {
 	 */
 	public void addSecondaryHighlighter(Function function, ClangDecompilerHighlighter highlighter) {
 
-		// note: this highlighter has likely already been added the the this class, but has not
+		// Note: this highlighter has likely already been added the the this class, but has not
 		//       yet been bound to the given function.
 		secondaryHighlightersbyFunction.get(function).add(highlighter);
 		secondaryHighlighters.add(highlighter);
 		highlighterHighlights.putIfAbsent(highlighter, new TokenHighlights());
 	}
 
-	/**
-	 * Adds the given highlighter, but does not create any highlights
-	 * @param highlighter the highlighter
-	 */
+	// Note: this is used for all highlight types, secondary and highlighter service highlighters
 	public void addHighlighter(ClangDecompilerHighlighter highlighter) {
 		highlighterHighlights.putIfAbsent(highlighter, new TokenHighlights());
 	}
 
+	// Note: this is used for all highlight types, secondary and highlighter service highlights
 	public void addHighlighterHighlights(ClangDecompilerHighlighter highlighter,
 			Supplier<? extends Collection<ClangToken>> tokens,
 			ColorProvider colorProvider) {
@@ -488,9 +499,17 @@ public abstract class ClangHighlightController {
 
 	private Color blendHighlighterColors(ClangToken token) {
 
+		Function function = getFunction(token);
+		if (function == null) {
+			return null; // not sure if this can happen
+		}
+
+		Set<ClangDecompilerHighlighter> global = getGlobalHighlighters();
+		Set<ClangDecompilerHighlighter> secondary = getSecondaryHighlighters(function);
+		Iterable<ClangDecompilerHighlighter> it = CollectionUtils.asIterable(global, secondary);
 		Color lastColor = null;
-		Collection<TokenHighlights> allHighlights = highlighterHighlights.values();
-		for (TokenHighlights highlights : allHighlights) {
+		for (ClangDecompilerHighlighter highlighter : it) {
+			TokenHighlights highlights = highlighterHighlights.get(highlighter);
 			HighlightToken hlToken = highlights.get(token);
 			if (hlToken == null) {
 				continue;
@@ -508,9 +527,21 @@ public abstract class ClangHighlightController {
 		return lastColor;
 	}
 
+	private Function getFunction(ClangToken t) {
+		ClangFunction cFunction = t.getClangFunction();
+		if (cFunction == null) {
+			return null;
+		}
+
+		HighFunction highFunction = cFunction.getHighFunction();
+		if (highFunction == null) {
+			return null;
+		}
+		return highFunction.getFunction();
+	}
+
 	/**
-	 * If input token is a parenthesis, highlight all
-	 * tokens between it and its match
+	 * If input token is a parenthesis, highlight all tokens between it and its match
 	 * @param tok potential parenthesis token
 	 * @param highlightColor the highlight color
 	 * @return a list of all tokens that were highlighted.

Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/component/DecompilerPanel.java

@@ -149,8 +149,8 @@ public class DecompilerPanel extends JPanel implements FieldMouseListener, Field
 		return highlightController.getSecondaryHighlightColors();
 	}
 
-	public boolean hasSecondaryHighlights() {
-		return highlightController.hasSecondaryHighlights();
+	public boolean hasSecondaryHighlights(Function function) {
+		return highlightController.hasSecondaryHighlights(function);
 	}
 
 	public boolean hasSecondaryHighlight(ClangToken token) {
@@ -166,14 +166,14 @@ public class DecompilerPanel extends JPanel implements FieldMouseListener, Field
 	}
 
 	private Set<ClangDecompilerHighlighter> getSecondaryHighlihgtersByFunction(Function function) {
-		return highlightController.getSecondaryHighlightersByFunction(function);
+		return highlightController.getSecondaryHighlighters(function);
 	}
 
 	/**
 	 * Removes all secondary highlights for the current function
+	 * @param function the function containing the secondary highlights
 	 */
-	public void removeSecondaryHighlights() {
-		Function function = controller.getFunction();
+	public void removeSecondaryHighlights(Function function) {
 		highlightController.removeSecondaryHighlights(function);
 	}
 
@@ -283,9 +283,14 @@ public class DecompilerPanel extends JPanel implements FieldMouseListener, Field
 	}
 
 	/**
-	 * This is function is used to alert the panel that a token was renamed.
-	 * If the token that is being renamed had a secondary highlight, we must re-apply the highlight
-	 * to the new token.
+	 * This function is used to alert the panel that a token was renamed. If the token being renamed
+	 * had a secondary highlight, we must re-apply the highlight to the new token.
+	 * 
+	 * <p>This is not needed for highlighter service highlights, since they get called again to
+	 * re-apply highlights.  It is up to that highlighter to determine if highlighting still applies
+	 * to the new token name.  Alternatively, for secondary highlights, we know the user chose the
+	 * highlight based upon name.  Thus, when the name changes, we need to take action to update
+	 * the secondary highlight.
 	 * 
 	 * @param token the token being renamed
 	 * @param newName the new name of the token
@@ -307,9 +312,9 @@ public class DecompilerPanel extends JPanel implements FieldMouseListener, Field
 
 	private void cloneGlobalHighlighters(DecompilerPanel sourcePanel) {
 
-		Set<ClangDecompilerHighlighter> allHighlighters =
+		Set<ClangDecompilerHighlighter> globalHighlighters =
 			sourcePanel.highlightController.getGlobalHighlighters();
-		for (ClangDecompilerHighlighter otherHighlighter : allHighlighters) {
+		for (ClangDecompilerHighlighter otherHighlighter : globalHighlighters) {
 
 			ClangDecompilerHighlighter newHighlighter = otherHighlighter.clone(this);
 			highlightersById.put(newHighlighter.getId(), newHighlighter);
@@ -411,18 +416,19 @@ public class DecompilerPanel extends JPanel implements FieldMouseListener, Field
 		currentSearchLocation = null;
 
 		reapplySecondaryHighlights();
-		reapplyHighlighterHighlights();
+		reapplyGlobalHighlights();
 	}
 
-	private void reapplyHighlighterHighlights() {
+	private void reapplyGlobalHighlights() {
 
 		Function function = decompileData.getFunction();
 		if (function == null) {
 			return;
 		}
 
-		Collection<ClangDecompilerHighlighter> values = highlightersById.values();
-		for (ClangDecompilerHighlighter highlighter : values) {
+		Set<ClangDecompilerHighlighter> globalHighlighters =
+			highlightController.getGlobalHighlighters();
+		for (ClangDecompilerHighlighter highlighter : globalHighlighters) {
 			highlighter.clearHighlights();
 			highlighter.applyHighlights();
 		}

Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/RemoveAllSecondaryHighlightsAction.java

@@ -20,6 +20,7 @@ import ghidra.app.decompiler.component.ClangHighlightController;
 import ghidra.app.decompiler.component.DecompilerPanel;
 import ghidra.app.plugin.core.decompile.DecompilerActionContext;
 import ghidra.app.util.HelpTopics;
+import ghidra.program.model.listing.Function;
 import ghidra.util.HelpLocation;
 
 /**
@@ -46,12 +47,14 @@ public class RemoveAllSecondaryHighlightsAction extends AbstractDecompilerAction
 		}
 
 		DecompilerPanel panel = context.getDecompilerPanel();
-		return panel.hasSecondaryHighlights();
+		Function function = context.getFunction();
+		return panel.hasSecondaryHighlights(function);
 	}
 
 	@Override
 	protected void decompilerActionPerformed(DecompilerActionContext context) {
 		DecompilerPanel panel = context.getDecompilerPanel();
-		panel.removeSecondaryHighlights();
+		Function function = context.getFunction();
+		panel.removeSecondaryHighlights(function);
 	}
 }

Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/RenameFunctionAction.java

@@ -16,6 +16,7 @@
 package ghidra.app.plugin.core.decompile.actions;
 
 import java.awt.event.KeyEvent;
+import java.util.Objects;
 
 import docking.action.KeyBindingData;
 import docking.action.MenuData;
@@ -23,12 +24,14 @@ import ghidra.app.decompiler.ClangFuncNameToken;
 import ghidra.app.decompiler.ClangToken;
 import ghidra.app.decompiler.component.DecompilerUtils;
 import ghidra.app.plugin.core.decompile.DecompilerActionContext;
+import ghidra.app.plugin.core.decompile.DecompilerProvider;
 import ghidra.app.util.AddEditDialog;
 import ghidra.app.util.HelpTopics;
 import ghidra.program.model.listing.Function;
 import ghidra.program.model.listing.Program;
 import ghidra.program.model.pcode.HighFunctionShellSymbol;
 import ghidra.program.model.pcode.HighSymbol;
+import ghidra.program.model.symbol.Symbol;
 import ghidra.util.HelpLocation;
 import ghidra.util.UndefinedFunction;
 
@@ -41,6 +44,7 @@ public class RenameFunctionAction extends AbstractDecompilerAction {
 		setPopupMenuData(new MenuData(new String[] { "Rename Function" }, "Decompile"));
 	}
 
+	@Override
 	protected Function getFunction(DecompilerActionContext context) {
 		Program program = context.getProgram();
 		ClangToken tokenAtCursor = context.getTokenAtCursor();
@@ -68,6 +72,18 @@ public class RenameFunctionAction extends AbstractDecompilerAction {
 		Program program = context.getProgram();
 		Function function = getFunction(context);
 		AddEditDialog dialog = new AddEditDialog("Edit Function Name", context.getTool());
-		dialog.editLabel(function.getSymbol(), program);
+		Symbol symbol = function.getSymbol();
+		String originalName = symbol.getName();
+		dialog.editLabel(symbol, program);
+
+		String currentName = symbol.getName();
+		if (Objects.equals(originalName, currentName)) {
+			return; // no change
+		}
+
+		DecompilerProvider provider = context.getComponentProvider();
+		ClangToken tokenAtCursor = context.getTokenAtCursor();
+		provider.tokenRenamed(tokenAtCursor, currentName);
+
 	}
 }

Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/decompiler/component/DecompilerClangTest.java

@@ -17,14 +17,14 @@ package ghidra.app.decompiler.component;
 
 import static org.junit.Assert.*;
 
-import java.awt.Color;
-import java.awt.Window;
+import java.awt.*;
 import java.awt.event.MouseEvent;
 import java.util.*;
+import java.util.List;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
-import javax.swing.JButton;
+import javax.swing.*;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -41,6 +41,7 @@ import ghidra.app.decompiler.DecompileOptions.NamespaceStrategy;
 import ghidra.app.plugin.core.decompile.AbstractDecompilerTest;
 import ghidra.app.plugin.core.decompile.DecompilerProvider;
 import ghidra.app.plugin.core.decompile.actions.*;
+import ghidra.app.util.AddEditDialog;
 import ghidra.framework.options.ToolOptions;
 import ghidra.framework.plugintool.util.OptionsService;
 import ghidra.program.model.listing.CodeUnit;
@@ -456,6 +457,79 @@ public class DecompilerClangTest extends AbstractDecompilerTest {
 		assertNoFieldsSecondaryHighlighted(text2);
 	}
 
+	@Test
+	public void testSecondaryHighlighting_ClearAll_DoesNotAffectOtherFunctions() {
+
+		/*
+		
+		 Decomp of '_call_structure_A':
+		 
+			1|
+			2| void _call_structure_A(A *a)
+			3|
+			4| {
+			5|  	_printf("call_structure_A: %s\n",a->name);
+			6|  	_printf("call_structure_A: %s\n",(a->b).name);
+			7|  	_printf("call_structure_A: %s\n",(a->b).c.name);
+			8|  	_printf("call_structure_A: %s\n",(a->b).c.d.name);
+			9|  	_printf("call_structure_A: %s\n",(a->b).c.d.e.name);
+		   10|  	_call_structure_B(&a->b);
+		   11|  	return;
+		   12|	}
+		   
+		   
+		Decomp of '_call_structure_B':
+			
+			1|
+			2| void _call_structure_B(B *b)
+			3|
+			4| {
+			5|   	_printf("call_structure_B: %s\n",b->name);
+			6|   	_call_structure_C(&b->c);
+			7|   	return;
+			8| }
+		
+		
+		
+		 */
+
+		decompile("100000d60"); // '_call_structure_A'
+
+		// 5:2 "_printf"
+		int line1 = 5;
+		int charPosition1 = 2;
+		setDecompilerLocation(line1, charPosition1);
+		ClangToken token1 = getToken();
+
+		Color color1 = highlight();
+		assertAllFieldsSecondaryHighlighted(token1, color1);
+
+		decompile("100000e10"); // '_call_structure_B'
+
+		// 5:2 "_printf"
+		int line2 = 5;
+		int charPosition2 = 2;
+		setDecompilerLocation(line2, charPosition2);
+		ClangToken token2 = getToken();
+
+		Color color2 = highlight();
+		assertAllFieldsSecondaryHighlighted(token2, color2);
+
+		decompile("100000d60"); // '_call_structure_A'
+
+		// 5:2 "_printf"
+		setDecompilerLocation(line1, charPosition1);
+		clearAllHighlights();
+
+		// token 1 cleared; token 2 still highlighted
+		assertNoFieldsSecondaryHighlighted(token1.getText());
+
+		decompile("100000e10"); // '_call_structure_B'
+		setDecompilerLocation(line2, charPosition2);
+		token2 = getToken();
+		assertAllFieldsSecondaryHighlighted(token2, color2);
+	}
+
 	@Test
 	public void testSecondaryHighlighting_RenameHighlightedVariable() {
 
@@ -499,6 +573,49 @@ public class DecompilerClangTest extends AbstractDecompilerTest {
 		assertAllFieldsSecondaryHighlighted(token, color);
 	}
 
+	@Test
+	public void testSecondaryHighlighting_RenameHighlightedFunction() {
+
+		/*
+		
+		 Decomp of '_call_structure_A':
+		 
+			1|
+			2| void _call_structure_A(A *a)
+			3|
+			4| {
+			5|  	_printf("call_structure_A: %s\n",a->name);
+			6|  	_printf("call_structure_A: %s\n",(a->b).name);
+			7|  	_printf("call_structure_A: %s\n",(a->b).c.name);
+			8|  	_printf("call_structure_A: %s\n",(a->b).c.d.name);
+			9|  	_printf("call_structure_A: %s\n",(a->b).c.d.e.name);
+		   10|  	_call_structure_B(&a->b);
+		   11|  	return;
+		   12|	}
+		
+		 */
+
+		decompile("100000d60"); // '_call_structure_A'
+
+		// 5:2 "_printf"
+		int line = 5;
+		int charPosition = 2;
+		setDecompilerLocation(line, charPosition);
+
+		ClangToken token = getToken();
+		String text = token.getText();
+		assertEquals("_printf", text);
+
+		Color color = highlight();
+
+		renameFunction("bob");
+
+		token = getToken();
+		text = token.getText();
+		assertEquals("bob", text);
+		assertAllFieldsSecondaryHighlighted(token, color);
+	}
+
 	@Test
 	public void testSecondaryHighlighting_HighlightColorGetsReused() {
 
@@ -938,6 +1055,56 @@ public class DecompilerClangTest extends AbstractDecompilerTest {
 		assertAllFieldsSecondaryHighlighted(token, color);
 	}
 
+	@Test
+	public void testSecondaryHighlighting_DoesNotApplyToOtherFunctions() {
+
+		/*
+		
+		 Decomp of '_call_structure_A':
+		 
+			1|
+			2| void _call_structure_A(A *a)
+			3|
+			4| {
+			5|  	_printf("call_structure_A: %s\n",a->name);
+			6|  	_printf("call_structure_A: %s\n",(a->b).name);
+			7|  	_printf("call_structure_A: %s\n",(a->b).c.name);
+			8|  	_printf("call_structure_A: %s\n",(a->b).c.d.name);
+			9|  	_printf("call_structure_A: %s\n",(a->b).c.d.e.name);
+		   10|  	_call_structure_B(&a->b);
+		   11|  	return;
+		   12|	}
+		
+		Decomp of '_call_structure_B':
+			
+			1|
+			2| void _call_structure_B(B *b)
+			3|
+			4| {
+			5|   	_printf("call_structure_B: %s\n",b->name);
+			6|   	_call_structure_C(&b->c);
+			7|   	return;
+			8| }
+		
+		
+		
+		 */
+
+		decompile("100000d60"); // '_call_structure_A'
+
+		// 5:2 "_printf"
+		int line = 5;
+		int charPosition = 2;
+		setDecompilerLocation(line, charPosition);
+		ClangToken token = getToken();
+
+		Color color = highlight();
+		assertAllFieldsSecondaryHighlighted(token, color);
+
+		decompile("100000e10"); // '_call_structure_B'
+		assertNoFieldsSecondaryHighlighted(token.getText());
+	}
+
 	@Test
 	public void testHighlightService() {
 
@@ -1788,6 +1955,22 @@ public class DecompilerClangTest extends AbstractDecompilerTest {
 		waitForDecompiler();
 	}
 
+	private void renameFunction(String newName) {
+		DockingActionIf action = getAction(decompiler, "Rename Function");
+		performAction(action, provider.getActionContext(null), false);
+
+		AddEditDialog dialog = waitForDialogComponent(AddEditDialog.class);
+		runSwing(() -> {
+			JComboBox<?> comboBox =
+				(JComboBox<?>) findComponentByName(dialog, "label.name.choices");
+			Component comp = comboBox.getEditor().getEditorComponent();
+			((JTextField) comp).setText(newName);
+		});
+
+		pressButtonByText(dialog, "OK");
+		waitForDecompiler();
+	}
+
 	private void clearAllHighlights() {
 
 		DockingActionIf highlightAction =
@@ -1903,7 +2086,8 @@ public class DecompilerClangTest extends AbstractDecompilerTest {
 		Color combinedColor = getCombinedHighlightColor(token);
 		ColorMatcher cm = new ColorMatcher(color, combinedColor);
 		Color actual = token.getHighlight();
-		assertTrue("Token is not highlighted: '" + token + "'" + "\n\texpected: " + cm +
+		String tokenString = token.toString() + " at line " + token.getLineParent().getLineNumber();
+		assertTrue("Token is not highlighted: '" + tokenString + "'" + "\n\texpected: " + cm +
 			"; found: " + toString(actual), cm.matches(actual));
 	}
 
@@ -2123,7 +2307,6 @@ public class DecompilerClangTest extends AbstractDecompilerTest {
 
 		ColorMatcher(Color... colors) {
 			// note: we allow null
-
 			for (Color c : colors) {
 				myColors.add(c);
 			}