From 56942bbc50d95ffdd7ccf2606cdcaefcbc50d280 Mon Sep 17 00:00:00 2001 From: emteere <47253321+emteere@users.noreply.github.com> Date: Tue, 19 Oct 2021 20:45:29 +0000 Subject: [PATCH 1/3] GP-1417_emteere initial stab at lazy DataDB size --- .../program/database/code/CodeUnitDB.java | 11 +++-- .../ghidra/program/database/code/DataDB.java | 46 ++++++++++++++----- 2 files changed, 40 insertions(+), 17 deletions(-) diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/CodeUnitDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/CodeUnitDB.java index b04b3de2d9..0bbf30633d 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/CodeUnitDB.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/CodeUnitDB.java @@ -284,7 +284,7 @@ abstract class CodeUnitDB extends DatabaseObject implements CodeUnit, ProcessorC public Address getMaxAddress() { refreshIfNeeded(); if (endAddr == null) { - endAddr = length == 0 ? address : address.add(length - 1); + endAddr = getLength() == 0 ? address : address.add(getLength() - 1); } return endAddr; } @@ -669,9 +669,10 @@ abstract class CodeUnitDB extends DatabaseObject implements CodeUnit, ProcessorC try { checkIsValid(); populateByteArray(); - byte[] b = new byte[length]; - if (bytes.length < length) { - if (program.getMemory().getBytes(address, b) != length) { + int len = getLength(); + byte[] b = new byte[len]; + if (bytes.length < len) { + if (program.getMemory().getBytes(address, b) != len) { throw new MemoryAccessException("Couldn't get all bytes for CodeUnit"); } } @@ -829,7 +830,7 @@ abstract class CodeUnitDB extends DatabaseObject implements CodeUnit, ProcessorC } protected int getPreferredCacheLength() { - return length; + return getLength(); } private void validateOpIndex(int opIndex) { diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/DataDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/DataDB.java index ed11306b27..142d65e675 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/DataDB.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/DataDB.java @@ -71,10 +71,7 @@ class DataDB extends CodeUnitDB implements Data { baseDataType = getBaseDataType(dataType); defaultSettings = dataType.getDefaultSettings(); - computeLength(); - if (length < 0) { - Msg.error(this, " bad bad"); - } + length = -1; // lazy compute } protected static DataType getBaseDataType(DataType dataType) { @@ -121,10 +118,24 @@ class DataDB extends CodeUnitDB implements Data { dataType = dt; baseDataType = getBaseDataType(dataType); defaultSettings = dataType.getDefaultSettings(); - computeLength(); + length = -1; // set to compute lazily later return false; } + @Override + public int getLength() { + if (length == -1) { + lock.acquire(); + try { + computeLength(); + } + finally { + lock.release(); + } + } + return length; + } + private void computeLength() { length = dataType.getLength(); if (length < 1) { @@ -165,6 +176,17 @@ class DataDB extends CodeUnitDB implements Data { } } + bytes = null; + + // if this is not a component where the size could change and + // the length restricted by the following instruction/data item, assume + // the createData method stopped fixed code units that won't fit from being added + if (!(baseDataType instanceof Composite || baseDataType instanceof ArrayDataType)) { + return; + } + + // This is potentially expensive! So only do if necessary + // see if the datatype length is restricted by a following codeunit Address nextAddr = codeMgr.getDefinedAddressAfter(address); if ((nextAddr != null) && nextAddr.compareTo(endAddress) <= 0) { length = (int) nextAddr.subtract(address); @@ -269,7 +291,7 @@ class DataDB extends CodeUnitDB implements Data { lock.acquire(); try { checkIsValid(); - return dataType.getRepresentation(this, this, length); + return dataType.getRepresentation(this, this, getLength()); } finally { lock.release(); @@ -501,7 +523,7 @@ class DataDB extends CodeUnitDB implements Data { lock.acquire(); try { checkIsValid(); - if (offset < 0 || offset > length) { + if (offset < 0 || offset > getLength()) { return null; } @@ -538,7 +560,7 @@ class DataDB extends CodeUnitDB implements Data { lock.acquire(); try { checkIsValid(); - if (offset < 0 || offset >= length) { + if (offset < 0 || offset >= getLength()) { return null; } if (baseDataType instanceof Array) { @@ -662,7 +684,7 @@ class DataDB extends CodeUnitDB implements Data { lock.acquire(); try { checkIsValid(); - if (length < dataType.getLength()) { + if (getLength() < dataType.getLength()) { return -1; } if (baseDataType instanceof Composite) { @@ -715,7 +737,7 @@ class DataDB extends CodeUnitDB implements Data { lock.acquire(); try { checkIsValid(); - if (offset < 0 || offset >= length) { + if (offset < 0 || offset >= getLength()) { return null; } Data dc = getComponentContaining(offset); @@ -744,7 +766,7 @@ class DataDB extends CodeUnitDB implements Data { lock.acquire(); try { checkIsValid(); - return baseDataType.getValue(this, this, length); + return baseDataType.getValue(this, this, getLength()); } finally { lock.release(); @@ -773,7 +795,7 @@ class DataDB extends CodeUnitDB implements Data { if (options == null) { options = DataTypeDisplayOptions.DEFAULT; } - return dataType.getDefaultLabelPrefix(this, this, length, options); + return dataType.getDefaultLabelPrefix(this, this, getLength(), options); } @Override From 661b641a42ef50e7518bfd9afcf71b524565f494 Mon Sep 17 00:00:00 2001 From: ghidravore Date: Fri, 5 Nov 2021 13:53:55 -0400 Subject: [PATCH 2/3] added small improvement --- .../src/main/java/ghidra/program/database/code/DataDB.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/DataDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/DataDB.java index 142d65e675..959b44323b 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/DataDB.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/DataDB.java @@ -150,6 +150,11 @@ class DataDB extends CodeUnitDB implements Data { } } + // no need to do all that follow on checking when length == 1 + if (length == 1) { + return; + } + // FIXME Trying to get Data to display for External. if (address.isExternalAddress()) { // FIXME return; // FIXME From 28676b4ec0328b621692722f3a2b2c4c0be80294 Mon Sep 17 00:00:00 2001 From: emteere <47253321+emteere@users.noreply.github.com> Date: Tue, 9 Nov 2021 21:26:39 +0000 Subject: [PATCH 3/3] GP-1417_emteere code review changes --- .../program/database/code/CodeManager.java | 54 +++++++++++-------- .../ghidra/program/database/code/DataDB.java | 29 +++++----- 2 files changed, 48 insertions(+), 35 deletions(-) diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/CodeManager.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/CodeManager.java index 8b5e7ff45e..3087be53eb 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/CodeManager.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/CodeManager.java @@ -2825,31 +2825,37 @@ public class CodeManager implements ErrorHandler, ManagerDB { } Address getDefinedAddressAfter(Address address) { - DBRecord dataRec = null; - DBRecord instRec = null; + lock.acquire(); try { - dataRec = dataAdapter.getRecordAfter(address); - instRec = instAdapter.getRecordAfter(address); + DBRecord dataRec = null; + DBRecord instRec = null; + try { + dataRec = dataAdapter.getRecordAfter(address); + instRec = instAdapter.getRecordAfter(address); + } + catch (IOException e) { + program.dbError(e); + return null; + } + if (dataRec == null && instRec == null) { + return null; + } + if (dataRec == null) { + return addrMap.decodeAddress(instRec.getKey()); + } + if (instRec == null) { + return addrMap.decodeAddress(dataRec.getKey()); + } + Address dataAddr = addrMap.decodeAddress(dataRec.getKey()); + Address instAddr = addrMap.decodeAddress(instRec.getKey()); + if (dataAddr.compareTo(instAddr) < 0) { + return dataAddr; + } + return instAddr; } - catch (IOException e) { - program.dbError(e); - return null; + finally { + lock.release(); } - if (dataRec == null && instRec == null) { - return null; - } - if (dataRec == null) { - return addrMap.decodeAddress(instRec.getKey()); - } - if (instRec == null) { - return addrMap.decodeAddress(dataRec.getKey()); - } - Address dataAddr = addrMap.decodeAddress(dataRec.getKey()); - Address instAddr = addrMap.decodeAddress(instRec.getKey()); - if (dataAddr.compareTo(instAddr) < 0) { - return dataAddr; - } - return instAddr; } /////////////////////////////////////////////////////////////////// @@ -3117,12 +3123,16 @@ public class CodeManager implements ErrorHandler, ManagerDB { } int getLength(Address addr) { + lock.acquire(); try { return lengthMgr.getInt(addr); } catch (NoValueException e) { return -1; } + finally { + lock.release(); + } } private InstructionDB getInstructionDB(long addr) throws IOException { diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/DataDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/DataDB.java index 959b44323b..6c28bc0a40 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/DataDB.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/DataDB.java @@ -119,25 +119,26 @@ class DataDB extends CodeUnitDB implements Data { baseDataType = getBaseDataType(dataType); defaultSettings = dataType.getDefaultSettings(); length = -1; // set to compute lazily later + bytes = null; return false; } @Override public int getLength() { if (length == -1) { - lock.acquire(); - try { - computeLength(); - } - finally { - lock.release(); - } + computeLength(); } return length; } private void computeLength() { length = dataType.getLength(); + + // undefined will never change their size + if (dataType instanceof Undefined) { + return; + } + if (length < 1) { length = codeMgr.getLength(address); } @@ -181,14 +182,17 @@ class DataDB extends CodeUnitDB implements Data { } } - bytes = null; - // if this is not a component where the size could change and // the length restricted by the following instruction/data item, assume // the createData method stopped fixed code units that won't fit from being added - if (!(baseDataType instanceof Composite || baseDataType instanceof ArrayDataType)) { - return; - } + // + // TODO: If the data organization for a program changes, for example a long was 32-bits + // and is changed to 64-bits, that could cause an issue. + // If the data organization changing could be detected, this could be done. + // + // if (!(baseDataType instanceof Composite || baseDataType instanceof ArrayDataType)) { + // return; + // } // This is potentially expensive! So only do if necessary // see if the datatype length is restricted by a following codeunit @@ -196,7 +200,6 @@ class DataDB extends CodeUnitDB implements Data { if ((nextAddr != null) && nextAddr.compareTo(endAddress) <= 0) { length = (int) nextAddr.subtract(address); } - bytes = null; } @Override