Merge branch 'GP-6161_BugFIxes' into Ghidra_12.0

Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/RangeMap.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.
@@ -32,27 +32,27 @@ import ghidra.util.map.IntValueMap;
  * that contains a value.
  */
 public class RangeMap {
-	
+
 	IntValueMap map;
 	int defaultValue;
-	
+
 	/**
 	 * Constructor for RangeMap with a default value of 0.
 	 */
 	public RangeMap() {
 		this(0);
 	}
-	
+
 	/**
-	 * Creates a new range map with spcified default value.
+	 * Creates a new range map with specified default value.
 	 * @param defaultValue the default value
 	 */
 	public RangeMap(int defaultValue) {
 		map = new IntValueMap("RangeMap");
 		this.defaultValue = defaultValue;
-		map.putInt(0, defaultValue);		
+		map.putInt(0, defaultValue);
 	}
-	
+
 	/**
 	 * Get the total number of ranges in map.
 	 * @return number of ranges
@@ -66,9 +66,9 @@ public class RangeMap {
 	 */
 	public void clear() {
 		map.removeRange(0, Long.MAX_VALUE);
-		map.putInt(0,defaultValue);
+		map.putInt(0, defaultValue);
 	}
-	
+
 	/**
 	 * Associates the given value with every index from start to end (inclusive)
 	 * Any previous associates are overwritten.
@@ -80,34 +80,33 @@ public class RangeMap {
 
 		// first fix up the end of the range, unless the end goes to the END
 		if (end != Long.MAX_VALUE) {
-			int origEndValue = getValue(end+1);
+			int origEndValue = getValue(end + 1);
 			if (origEndValue != value) {
-				map.putInt(end+1, origEndValue);
+				map.putInt(end + 1, origEndValue);
 			}
 			else {
-				map.remove(end+1);
+				map.remove(end + 1);
 			}
 		}
 
-		
 		// now remove any values stored from start to end
 		LongIterator it = map.getPropertyIterator(start);
-		while(it.hasNext()) {
+		while (it.hasNext()) {
 			long next = it.next();
-			if (next > end) break;
+			if (next > end)
+				break;
 			map.remove(next);
 		}
 
-
 		if (start == 0) {
-			map.putInt(0,value);
-		} 
+			map.putInt(0, value);
+		}
 		else {
 			int startValue = getValue(start);
 			if (startValue != value) {
 				map.putInt(start, value);
 			}
-		}			
+		}
 	}
 
 	/**
@@ -118,19 +117,19 @@ public class RangeMap {
 		try {
 			return map.getInt(index);
 		}
-		catch(NoValueException e) {
+		catch (NoValueException e) {
 			try {
-				index = map.getPreviousPropertyIndex(index);	
+				index = map.getPreviousPropertyIndex(index);
 				return map.getInt(index);
 			}
-			catch(NoSuchIndexException ex) {
+			catch (NoSuchIndexException ex) {
 			}
-			catch(NoValueException ex) {
+			catch (NoValueException ex) {
 			}
-		}		
+		}
 		return 0;
 	}
-	
+
 	/**
 	 * Returns the value range containing the given index. The value range indicates
 	 * the int value and the start and end index for the range.
@@ -139,7 +138,7 @@ public class RangeMap {
 	 */
 	public ValueRange getValueRange(long index) {
 		if (map.getSize() == 1) {
-			return new ValueRange(0,Long.MAX_VALUE,0);
+			return new ValueRange(0, Long.MAX_VALUE, defaultValue);
 		}
 		long start = 0;
 		if (map.hasProperty(index)) {
@@ -148,19 +147,28 @@ public class RangeMap {
 		else {
 			try {
 				start = map.getPreviousPropertyIndex(index);
-			}catch(NoSuchIndexException e){}
+			}
+			catch (NoSuchIndexException e) {
+				// Use minimum start if index not found: 0
+			}
 		}
 		long end = Long.MAX_VALUE;
 		try {
-			end = map.getNextPropertyIndex(start)-1;
-		}catch(NoSuchIndexException e){}
+			end = map.getNextPropertyIndex(start) - 1;
+		}
+		catch (NoSuchIndexException e) {
+			// Use maximum end: Long.MAX_VALUE
+		}
 		int value = 0;
 		try {
 			value = map.getInt(start);
-		} catch (NoValueException e1) {}
+		}
+		catch (NoValueException e1) {
+			// Use minimum value if (e.g., empty map): 0
+		}
 		return new ValueRange(start, end, value);
 	}
-	
+
 	/**
 	 * Returns an iterator over all occupied ranges in the map.
 	 * @param index the index to start the iterator
@@ -177,6 +185,6 @@ public class RangeMap {
 	 * @return an iterator over all indexes where the value changes.
 	 */
 	public LongIterator getChangePointIterator(long start, long end) {
-		return map.getPropertyIterator(start, end);	
+		return map.getPropertyIterator(start, end);
 	}
 }

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/generic/LanguageFixupUtil.java

@@ -19,6 +19,7 @@ import java.util.*;
 
 import ghidra.framework.store.LockException;
 import ghidra.program.database.ProgramDB;
+import ghidra.program.database.data.ProgramDataTypeManager;
 import ghidra.program.database.symbol.SymbolManager;
 import ghidra.program.model.address.*;
 import ghidra.program.model.lang.AddressLabelInfo;
@@ -28,6 +29,7 @@ import ghidra.program.model.mem.MemoryBlockException;
 import ghidra.program.model.symbol.*;
 import ghidra.util.Msg;
 import ghidra.util.exception.*;
+import ghidra.util.task.TaskMonitor;
 
 /**
  * {@link LanguageFixupUtil} provides utility method intended for internal language upgrade
@@ -40,12 +42,16 @@ public class LanguageFixupUtil {
 	 * generally required.  Reconciling symbols is limited to those symbols contained within
 	 * processor defined memory blocks which are not within either the default code or data spaces.
 	 * @param programDB target program
+	 * @param monitor task monitor
+	 * @throws CancelledException if fixup task is cancelled
 	 */
-	public static void applyPSpecFixups(ProgramDB programDB) throws CancelledException {
+	public static void applyPSpecFixups(ProgramDB programDB, TaskMonitor monitor)
+			throws CancelledException {
 
 		try {
-
 			Language language = programDB.getLanguage();
+			ProgramDataTypeManager dtm = programDB.getDataTypeManager();
+
 			AddressSpace defaultSpace = language.getDefaultSpace();
 			AddressSpace defaultDataSpace = language.getDefaultDataSpace();
 
@@ -59,6 +65,7 @@ public class LanguageFixupUtil {
 			AddressSet processorDefinedSafeBlockSet = new AddressSet();
 
 			for (MemoryBlockDefinition defaultMemoryBlockDef : language.getDefaultMemoryBlocks()) {
+				monitor.checkCancelled();
 				try {
 					MemoryBlock block = defaultMemoryBlockDef.fixupBlock(programDB);
 					AddressRange blockRange = block.getAddressRange();
@@ -78,9 +85,10 @@ public class LanguageFixupUtil {
 				}
 			}
 
-			HashSet<Symbol> goodSymbols = new HashSet<>();
-
+			// Create default symbols within processorDefinedBlockSet if missing.
+			// The goodSymbols set is used to record all processor defined symbols to assist cleanup
 			SymbolManager symbolTable = programDB.getSymbolTable();
+			HashSet<Symbol> goodSymbols = new HashSet<>();
 			for (AddressLabelInfo labelInfo : language.getDefaultSymbols()) {
 
 				String name = labelInfo.getLabel();
@@ -94,14 +102,18 @@ public class LanguageFixupUtil {
 				// Check all symbols within processor-defined blocks
 				Symbol existingSymbol = null;
 				for (Symbol s : symbolTable.getGlobalSymbols(name)) {
+					monitor.checkCancelled();
 					if (s.getSymbolType() != SymbolType.LABEL) {
 						continue;
 					}
 					if (addr.equals(s.getAddress())) {
+						// Keep existing label which matches spec
 						existingSymbol = s;
 						goodSymbols.add(s);
 					}
-					else if (s.getSource() == SourceType.IMPORTED) {
+					else if (s.getSource() == SourceType.IMPORTED &&
+						processorDefinedBlockSet.contains(s.getAddress())) {
+						// Remove label from its old location 
 						s.delete();
 					}
 				}
@@ -117,23 +129,26 @@ public class LanguageFixupUtil {
 				}
 			}
 
-			// Remove all symbols with processor defined blocks which are no longer defined.
+			// Remove all symbols within processor defined blocks which are no longer defined.
 			// This is restricted to safe address spaces since loader may have imported other symbols
-			// which we do not want to delete.
+			// which we do not want to delete.  We collect symbols first to avoid concurent 
+			// modification concerns.
 			List<Symbol> deleteSet = new ArrayList<>(); // defered delete to avoid iterator resets
 			for (Symbol s : symbolTable.getSymbols(processorDefinedSafeBlockSet, SymbolType.LABEL,
 				true)) {
+				monitor.checkCancelled();
 				if (s.getSource() == SourceType.IMPORTED && !goodSymbols.contains(s)) {
 					deleteSet.add(s);
 				}
 			}
 			for (Symbol s : deleteSet) {
+				monitor.checkCancelled();
 				s.delete();
 			}
-
 		}
 		catch (UnsupportedOperationException e) {
 			// skip
 		}
 	}
+
 }

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/generic/MemoryBlockDefinition.java

@@ -33,10 +33,7 @@ import ghidra.util.xml.XmlUtilities;
 import ghidra.xml.XmlElement;
 
 /**
- * 
- *
- * TODO To change the template for this generated type comment go to
- * Window - Preferences - Java - Code Style - Code Templates
+ * {@link MemoryBlockDefinition} provides a default memory block specification.
  */
 public class MemoryBlockDefinition {
 
@@ -229,7 +226,7 @@ public class MemoryBlockDefinition {
 
 			if (bitMappedAddress != null) {
 				Address mappedAddr = parseAddress(bitMappedAddress, program, "bit-mapped address");
-				if (addr.equals(currentStartAddress) &&
+				if (addr.equals(currentStartAddress) && sourceInfo.getMappedRange().isPresent() &&
 					mappedAddr.equals(sourceInfo.getMappedRange().get().getMinAddress()) &&
 					length == block.getSize()) {
 					return block;
@@ -240,7 +237,7 @@ public class MemoryBlockDefinition {
 			else if (byteMappedAddress != null) {
 				Address mappedAddr =
 					parseAddress(byteMappedAddress, program, "byte-mapped address");
-				if (addr.equals(currentStartAddress) &&
+				if (addr.equals(currentStartAddress) && sourceInfo.getMappedRange().isPresent() &&
 					mappedAddr.equals(sourceInfo.getMappedRange().get().getMinAddress()) &&
 					length == block.getSize()) {
 					return block;

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/ProgramDB.java

@@ -2142,7 +2142,7 @@ public class ProgramDB extends DomainObjectAdapterDB implements Program, ChangeM
 				}
 
 				// apply pspec default markup as defined by translator and pspec
-				LanguageFixupUtil.applyPSpecFixups(this);
+				LanguageFixupUtil.applyPSpecFixups(this, monitor);
 
 				dataMap.put(LANGUAGE_ID, languageID.getIdAsString());
 				dataMap.put(COMPILER_SPEC_ID, compilerSpecID.getIdAsString());

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/util/ProcessorSymbolType.java

@@ -1,13 +1,12 @@
 /* ###
  * IP: GHIDRA
- * REVIEWED: YES
  *
  * 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.
@@ -30,6 +29,7 @@ public enum ProcessorSymbolType {
 		if (lowerCase.equals("code_ptr")) {
 			return CODE_PTR;
 		}
-		return null;
+		// NOTE: This should have been prevented by relax grammar spec
+		throw new IllegalArgumentException("unsupported symbol type: " + string);
 	}
 }