From 03cc1b8468b380e21e73169ce2fc63d0f229e38b Mon Sep 17 00:00:00 2001 From: sad-dev <55233728+sad-dev@users.noreply.github.com> Date: Tue, 18 Jun 2024 16:28:01 +0800 Subject: [PATCH 1/7] Remove (useless?) cache of lastContext from ContextCache --- .../app/plugin/processors/sleigh/ContextCache.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/ContextCache.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/ContextCache.java index d437e8e40f..71d833cc4f 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/ContextCache.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/ContextCache.java @@ -26,9 +26,6 @@ public class ContextCache { private int context_size = 0; private Register contextBaseRegister = null; - private BigInteger lastContextValue; - private int[] lastContextWords; - public ContextCache() { } @@ -57,10 +54,7 @@ public class ContextCache { } } - private synchronized int[] getWords(BigInteger value) { - if (value.equals(lastContextValue)) { - return lastContextWords; - } + private int[] getWords(BigInteger value) { int[] words = new int[context_size]; byte[] bytes = value.toByteArray(); @@ -73,8 +67,6 @@ public class ContextCache { } words[i] = word; } - lastContextValue = value; - lastContextWords = words; return words; } From 789cbd9241b907dafa5d3862adfbcc3b25587923 Mon Sep 17 00:00:00 2001 From: sad-dev <55233728+sad-dev@users.noreply.github.com> Date: Tue, 18 Jun 2024 16:31:08 +0800 Subject: [PATCH 2/7] SleighLanguage: Use more performant ConcurrentHashMap --- .../processors/sleigh/SleighLanguage.java | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/SleighLanguage.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/SleighLanguage.java index 9d777e90b1..0e810c2b1f 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/SleighLanguage.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/SleighLanguage.java @@ -20,6 +20,7 @@ import static ghidra.pcode.utils.SlaFormat.*; import java.io.*; import java.math.BigInteger; import java.util.*; +import java.util.concurrent.ConcurrentHashMap; import java.util.Map.Entry; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -88,7 +89,7 @@ public class SleighLanguage implements Language { /** * Cached instruction prototypes */ - private LinkedHashMap instructProtoMap; + private ConcurrentHashMap instructProtoMap; private DecisionNode root = null; /** * table of AddressSpaces @@ -148,7 +149,7 @@ public class SleighLanguage implements Language { buildVolatileSymbolAddresses(); xrefRegisters(); - instructProtoMap = new LinkedHashMap<>(); + instructProtoMap = new ConcurrentHashMap<>(); initParallelHelper(); } @@ -378,16 +379,14 @@ public class SleighLanguage implements Language { newProto.cacheInfo(buf, context, true); } - synchronized (instructProtoMap) { - res = instructProtoMap.get(hashcode); - if (res == null) { // We have a prototype we have never seen - // before, build it fully - instructProtoMap.put(hashcode, newProto); - res = newProto; - } - if (inDelaySlot && res.hasDelaySlots()) { - throw new NestedDelaySlotException(); - } + res = instructProtoMap.get(hashcode); + if (res == null) { // We have a prototype we have never seen + // before, build it fully + instructProtoMap.put(hashcode, newProto); + res = newProto; + } + if (inDelaySlot && res.hasDelaySlots()) { + throw new NestedDelaySlotException(); } } catch (MemoryAccessException e) { From f5b8236976d24134c2d231fc9413e67ba3cb812c Mon Sep 17 00:00:00 2001 From: sad-dev <55233728+sad-dev@users.noreply.github.com> Date: Tue, 18 Jun 2024 16:39:45 +0800 Subject: [PATCH 3/7] HighFunction: store entryPoint/entryAddrSpace Unfortunately fetching entrypoint hits the database lock. As this is invariant for the HighFunction, cache these results instead. --- .../ghidra/program/model/pcode/HighFunction.java | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/HighFunction.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/HighFunction.java index ea569a49c6..1f0068159d 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/HighFunction.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/HighFunction.java @@ -50,6 +50,8 @@ public class HighFunction extends PcodeSyntaxTree { private GlobalSymbolMap globalSymbols; private List jumpTables; private List protoOverrides; + private Address entryPoint; + private AddressSpace entryAddrSpace; /** * @param function function associated with the higher level function abstraction. @@ -64,6 +66,8 @@ public class HighFunction extends PcodeSyntaxTree { this.language = language; this.compilerSpec = compilerSpec; AddressSpace stackSpace = function.getProgram().getAddressFactory().getStackSpace(); + entryPoint = function.getEntryPoint(); + entryAddrSpace = entryPoint.getAddressSpace(); localSymbols = new LocalSymbolMap(this, stackSpace); globalSymbols = new GlobalSymbolMap(this); proto = new FunctionPrototype(localSymbols, function); @@ -87,7 +91,7 @@ public class HighFunction extends PcodeSyntaxTree { if (func instanceof FunctionDB) { return func.getSymbol().getID(); } - return func.getProgram().getSymbolTable().getDynamicSymbolID(func.getEntryPoint()); + return func.getProgram().getSymbolTable().getDynamicSymbolID(entryPoint); } /** @@ -255,7 +259,7 @@ public class HighFunction extends PcodeSyntaxTree { } if (subel == ELEM_ADDR.id()) { Address addr = AddressXML.decode(decoder); - if (!func.getEntryPoint().equals(addr)) { + if (!entryPoint.equals(addr)) { throw new DecoderException("Mismatched address in function tag"); } } @@ -300,7 +304,7 @@ public class HighFunction extends PcodeSyntaxTree { private void decodeJumpTableList(Decoder decoder) throws DecoderException { int el = decoder.openElement(ELEM_JUMPTABLELIST); while (decoder.peekElement() != 0) { - JumpTable table = new JumpTable(func.getEntryPoint().getAddressSpace()); + JumpTable table = new JumpTable(entryAddrSpace); table.decode(decoder); if (!table.isEmpty()) { if (jumpTables == null) { @@ -318,10 +322,10 @@ public class HighFunction extends PcodeSyntaxTree { pcaddr = rep.getPCAddress(); if (pcaddr == Address.NO_ADDRESS) { try { - pcaddr = func.getEntryPoint().add(-1); + pcaddr = entryPoint.add(-1); } catch (AddressOutOfBoundsException e) { - pcaddr = func.getEntryPoint(); + pcaddr = entryPoint; } } } @@ -446,7 +450,7 @@ public class HighFunction extends PcodeSyntaxTree { encoder.writeBool(ATTRIB_NORETURN, true); } if (entryPoint == null) { - AddressXML.encode(encoder, func.getEntryPoint()); + AddressXML.encode(encoder, this.entryPoint); } else { AddressXML.encode(encoder, entryPoint); // Address is forced on XML From 7e5ffc2cf3fd8af6d6ce19d3fdbca05c1057edbe Mon Sep 17 00:00:00 2001 From: emteere <47253321+emteere@users.noreply.github.com> Date: Mon, 24 Jun 2024 12:23:02 -0400 Subject: [PATCH 4/7] GP-4712 certify changes --- .../java/ghidra/app/plugin/processors/sleigh/ContextCache.java | 1 - 1 file changed, 1 deletion(-) diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/ContextCache.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/ContextCache.java index 71d833cc4f..5b1e562b72 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/ContextCache.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/ContextCache.java @@ -1,6 +1,5 @@ /* ### * 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. From 19cf2fba3c4cac70a2bfad8c7702c57d6cad1e8f Mon Sep 17 00:00:00 2001 From: emteere <47253321+emteere@users.noreply.github.com> Date: Mon, 24 Jun 2024 16:20:10 -0400 Subject: [PATCH 5/7] GP-4712 refactoring, adding back in cache with no locking --- .../processors/sleigh/ContextCache.java | 19 ++++++++++++++++--- .../processors/sleigh/SleighLanguage.java | 9 ++++----- .../program/model/pcode/HighFunction.java | 4 +--- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/ContextCache.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/ContextCache.java index 5b1e562b72..639898eccb 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/ContextCache.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/ContextCache.java @@ -15,16 +15,19 @@ */ package ghidra.app.plugin.processors.sleigh; -import ghidra.program.model.address.Address; -import ghidra.program.model.lang.*; - import java.math.BigInteger; import java.util.Arrays; +import generic.stl.Pair; +import ghidra.program.model.address.Address; +import ghidra.program.model.lang.*; + public class ContextCache { private int context_size = 0; private Register contextBaseRegister = null; + Pair lastValue = null; + public ContextCache() { } @@ -55,7 +58,13 @@ public class ContextCache { private int[] getWords(BigInteger value) { + Pair lastValueTmp = lastValue; + if (lastValueTmp != null && value.equals(lastValueTmp.first)) { + return lastValueTmp.second; + } + int[] words = new int[context_size]; + byte[] bytes = value.toByteArray(); int byteIndexDiff = context_size * 4 - bytes.length; for (int i = 0; i < context_size; i++) { @@ -66,6 +75,10 @@ public class ContextCache { } words[i] = word; } + + lastValueTmp = new Pair(value, words); + lastValue = lastValueTmp; + return words; } diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/SleighLanguage.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/SleighLanguage.java index 0e810c2b1f..1688ba427b 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/SleighLanguage.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/SleighLanguage.java @@ -378,13 +378,12 @@ public class SleighLanguage implements Language { if (!instructProtoMap.containsKey(hashcode)) { newProto.cacheInfo(buf, context, true); } - - res = instructProtoMap.get(hashcode); - if (res == null) { // We have a prototype we have never seen - // before, build it fully - instructProtoMap.put(hashcode, newProto); + res = instructProtoMap.putIfAbsent(hashcode, newProto); + // if there was no previous value, assume newProto inserted + if (res == null) { res = newProto; } + if (inDelaySlot && res.hasDelaySlots()) { throw new NestedDelaySlotException(); } diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/HighFunction.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/HighFunction.java index 1f0068159d..ce492b49ad 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/HighFunction.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/HighFunction.java @@ -51,7 +51,6 @@ public class HighFunction extends PcodeSyntaxTree { private List jumpTables; private List protoOverrides; private Address entryPoint; - private AddressSpace entryAddrSpace; /** * @param function function associated with the higher level function abstraction. @@ -67,7 +66,6 @@ public class HighFunction extends PcodeSyntaxTree { this.compilerSpec = compilerSpec; AddressSpace stackSpace = function.getProgram().getAddressFactory().getStackSpace(); entryPoint = function.getEntryPoint(); - entryAddrSpace = entryPoint.getAddressSpace(); localSymbols = new LocalSymbolMap(this, stackSpace); globalSymbols = new GlobalSymbolMap(this); proto = new FunctionPrototype(localSymbols, function); @@ -304,7 +302,7 @@ public class HighFunction extends PcodeSyntaxTree { private void decodeJumpTableList(Decoder decoder) throws DecoderException { int el = decoder.openElement(ELEM_JUMPTABLELIST); while (decoder.peekElement() != 0) { - JumpTable table = new JumpTable(entryAddrSpace); + JumpTable table = new JumpTable(entryPoint.getAddressSpace()); table.decode(decoder); if (!table.isEmpty()) { if (jumpTables == null) { From 35cbc81d597d6daf74084433071ad1ddc0ea591c Mon Sep 17 00:00:00 2001 From: emteere <47253321+emteere@users.noreply.github.com> Date: Mon, 29 Jul 2024 18:07:18 -0400 Subject: [PATCH 6/7] GP-4712 Code review cachec lookup changes --- .../processors/sleigh/SleighLanguage.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/SleighLanguage.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/SleighLanguage.java index 1688ba427b..fbc1da4523 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/SleighLanguage.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/SleighLanguage.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. @@ -375,13 +375,15 @@ public class SleighLanguage implements Language { new SleighInstructionPrototype(this, buf, context, contextcache, inDelaySlot, null); Integer hashcode = newProto.hashCode(); - if (!instructProtoMap.containsKey(hashcode)) { - newProto.cacheInfo(buf, context, true); - } - res = instructProtoMap.putIfAbsent(hashcode, newProto); - // if there was no previous value, assume newProto inserted + // check proto map for already defined prototype + res = instructProtoMap.get(hashcode); if (res == null) { - res = newProto; + newProto.cacheInfo(buf, context, true); + res = instructProtoMap.putIfAbsent(hashcode, newProto); + // if there was no previous value, assume newProto inserted + if (res == null) { + res = newProto; + } } if (inDelaySlot && res.hasDelaySlots()) { From 4ff585d8dbed801b02daf78025132641bb6d08a9 Mon Sep 17 00:00:00 2001 From: emteere <47253321+emteere@users.noreply.github.com> Date: Wed, 31 Jul 2024 10:25:53 -0400 Subject: [PATCH 7/7] GP-4712 Simplified code putting prototype into map --- .../plugin/processors/sleigh/SleighLanguage.java | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/SleighLanguage.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/SleighLanguage.java index fbc1da4523..7dcde4a3a2 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/SleighLanguage.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/processors/sleigh/SleighLanguage.java @@ -375,16 +375,12 @@ public class SleighLanguage implements Language { new SleighInstructionPrototype(this, buf, context, contextcache, inDelaySlot, null); Integer hashcode = newProto.hashCode(); - // check proto map for already defined prototype - res = instructProtoMap.get(hashcode); - if (res == null) { - newProto.cacheInfo(buf, context, true); - res = instructProtoMap.putIfAbsent(hashcode, newProto); - // if there was no previous value, assume newProto inserted - if (res == null) { - res = newProto; - } - } + // get existing proto and use it + // if doesn't exist in map, cache info and store new proto + res = instructProtoMap.computeIfAbsent(hashcode, h -> { + newProto.cacheInfo(buf, context, true); + return newProto; + }); if (inDelaySlot && res.hasDelaySlots()) { throw new NestedDelaySlotException();