NationalSecurityAgency / ghidra

Ghidra is a software reverse engineering (SRE) framework
https://www.nsa.gov/ghidra
Apache License 2.0
49.06k stars 5.65k forks source link

DecompileResults doesn't respect the `HASH:` storage class #6620

Open NancyAurum opened 3 weeks ago

NancyAurum commented 3 weeks ago

Describe the bug DecompileResults doesn't respect the HASH: storage class for the variable names resulted from the "Rename Variable" task, when HighSymbols has isUniqueStorage() . I.e. Rename Local Variable doesn't work in case of temporary compiler variables.

To Reproduce Steps to reproduce the behavior:

  1. Open the strong.exe from the eponymous 1993 DOS game;
  2. Go to the StartExit routine in the code from the C0L.OBJ;
  3. In the decompiler try renaming the variable holding the startup priority, which resulted from the decompiler merging CF from the two cmp's into a single variable;
  4. The rename task correctly maps storage from the unique space to the HASH: space, but the follow up decompilations ignore it.

Expected behavior Variable gets rename correctly and following decompilations of that function showing a helpful name instead of some bVar2.

Screenshots If applicable, add screenshots to help explain your problem.

Attachments If applicable, please attach any files that caused problems or log files generated by the software.

Environment (please complete the following information):

Additional context Temporary fix for people who just want it working:

  public class HighSymbol {
    public void setName(String n) { name = n; }
    ...

  public class DecompileResults {
  ...
  private void decodeStream(Decoder decoder) {
    ...
    decoder.closeElement(docel);
    renameUniqs();
  }

    public void renameUniqs() {
        //System.out.printf("Decompiled HighSymbols for %s:%n", function.getName());
        LocalSymbolMap lsm = getHighFunction().getLocalSymbolMap();
        java.util.HashMap<String,String> renameMap = new java.util.HashMap<String,String>();
        for (HighSymbol highSymbol : lsm.getNameToSymbolMap().values()) {
            VariableStorage storage = highSymbol.getStorage();
            if (!storage.isUniqueStorage()) continue; //skip non unique: variables

            //unique variables have to be converted to HASH: storage, so we can compare them
            //with the user named hashes.
            HighVariable tmpHigh = highSymbol.getHighVariable();
            if (!storage.isHashStorage() && tmpHigh != null && tmpHigh.requiresDynamicStorage()) {
                DynamicEntry entry = DynamicEntry.build(tmpHigh.getRepresentative());
                storage = entry.getStorage();
            }
            String hash = storage.getFirstVarnode().getAddress().toString(true);
            //System.out.printf("  * %s %s%n", highSymbol.getName(), hash);
            for (ghidra.program.model.listing.Variable var : getFunction().getLocalVariables()) {
                if (!var.isUniqueVariable()) continue;
                String hash2 = var.getFirstStorageVarnode().getAddress().toString(true);
                if (hash.equals(hash2)) {
                    //System.out.printf("    %s is %s%n", highSymbol.getName(), var.getName());
                    renameMap.put(highSymbol.getName(), var.getName());
                    highSymbol.setName(var.getName());
                }
            }
        }
        if (renameMap.size() == 0) return;

        List<ClangNode> alltoks = new ArrayList<>();
        docroot.flatten(alltoks);
        if (alltoks.isEmpty()) return;

        for (int i = 0; i < alltoks.size(); ++i) {
            ClangToken token = (ClangToken) alltoks.get(i);
            String tokenText = token.getText();
            //System.out.printf("  %d:'%s'%n", token.isVariableRef()?1:0, tokenText);
            String renamed = renameMap.get(tokenText);
            if (renamed != null) token.setText(renamed);
        }
    }