diff --git a/SMPStaticAnalyzer.cpp b/SMPStaticAnalyzer.cpp index 13313ce869017bd31072030887b115af58076c53..b123c54095d5f6ba02461df9436d1f952c117056 100644 --- a/SMPStaticAnalyzer.cpp +++ b/SMPStaticAnalyzer.cpp @@ -968,7 +968,7 @@ bool MDPatchUnconvertedBytes(ea_t CurrDisasmAddr) { return false; } PatchInstr.PrintOperands(); - op_t CallDest = PatchInstr.GetUse(0); + op_t CallDest = PatchInstr.GetUse(0).GetOp(); if ((o_near != CallDest.type) || (0 != CallDest.addr)) { msg("Cannot patch call unless it is call near ptr 0 at %x", CurrDisasmAddr); @@ -993,7 +993,7 @@ bool MDPatchUnconvertedBytes(ea_t CurrDisasmAddr) { return true; } // end of MDPatchUnconvertedBytes() -// Create lists of code addresses identified by IDA Pro (in IDAProLocs) +// Use the lists of code addresses identified by IDA Pro (in IDAProLocs) // and an external disassembler (in DisasmLocs). Compare the lists and // try to convert addresses to code that are found in DisasmLocs but // not in IDAProLocs. Emit warnings when IDAProLocs has a code address @@ -1080,6 +1080,7 @@ void FixCodeIdentification(void) { FixupRegion CurrRegion(ConvertRegion); CodeReanalyzeList.push_back(CurrRegion); bool AllConverted = true; + bool AllNops = true; do { flags_t InstrFlags = getFlags(CurrDisasmAddr); if (!isUnknown(InstrFlags)) { @@ -1090,6 +1091,8 @@ void FixCodeIdentification(void) { if (InstrLen > 0) { // Successfully converted to code SMPInstr NewInstr(CurrDisasmAddr); NewInstr.Analyze(); + if (!NewInstr.MDIsNop()) + AllNops = false; #if SMP_DEBUG_FIXUP_IDB #if 0 msg("FixCodeID success at %x: len: %d %s\n", CurrDisasmAddr, @@ -1117,6 +1120,21 @@ void FixCodeIdentification(void) { ++DisasmIndex; // skip cleanup loop } } while (CurrDisasmAddr < CurrAddr); + if (AllConverted && AllNops) { + // We want to convert the region back to unexplored bytes + // and take it off the work list. Regions that are all nops + // create data flow analysis problems sometimes. The region + // is often unreachable code and produces a basic block with + // no predecessors within a function. This often happens when + // an optimizing compiler uses nops as padding to align jump + // targets on cache line bounaries. With no fall through into + // the nops, they are unreachable and should be left as unknown. + msg("FixCodeID nops region from %x to %x\n", CurrRegion.GetStart(), + CurrRegion.GetEnd()); + do_unknown_range(CurrRegion.GetStart(), + CurrRegion.GetEnd() - CurrRegion.GetStart(), DOUNK_SIMPLE); + CodeReanalyzeList.pop_back(); + } } // end if (SkipArea) ... else ... } // end if (addr < CurrDisasmAddr) .. else if ... else ... } // end while (DisasmIndex <= DisasmLocs.size() @@ -1176,6 +1194,12 @@ int FixupNewCodeChunks(void) { int changes = 0; for (CurrRegion = CodeReanalyzeList.begin(); CurrRegion != CodeReanalyzeList.end(); ++CurrRegion) { bool AllConverted = true; + bool AllNops = true; + bool NoFixups = (0 == CurrRegion->FixupInstrs.size()); + if (NoFixups) { + CurrRegion->SetStart(BADADDR); // mark for removal + continue; // skip to next region + } list<ea_t>::iterator CurrInstr; for (CurrInstr = CurrRegion->FixupInstrs.begin(); CurrInstr != CurrRegion->FixupInstrs.end(); ++CurrInstr) { int InstrLen = ua_code(*CurrInstr); @@ -1185,7 +1209,10 @@ int FixupNewCodeChunks(void) { #if SMP_DEBUG_FIXUP_IDB msg("FixupNewCodeChunks success at %x: len: %d\n", *CurrInstr, InstrLen); #endif - *CurrInstr = BADADDR; // mark for removal + if (!NewInstr.MDIsNop()) { + AllNops = false; + *CurrInstr = BADADDR; // mark for removal + } } else { AllConverted = false; @@ -1194,13 +1221,22 @@ int FixupNewCodeChunks(void) { #endif } } // end for all instrs in CurrRegion - if (AllConverted) { + if (AllConverted && !AllNops) { #if SMP_DEBUG_FIXUP_IDB msg("FixupNewCodeChunks success for region from %x to %x\n", CurrRegion->GetStart(), CurrRegion->GetEnd()); #endif CurrRegion->SetStart(BADADDR); // mark for removal } + else if (AllConverted && AllNops) { +#if SMP_DEBUG_FIXUP_IDB + msg("FixupNewCodeChunks re-converting nops region from %x to %x\n", + CurrRegion->GetStart(), CurrRegion->GetEnd()); +#endif + do_unknown_range(CurrRegion->GetStart(), + CurrRegion->GetEnd() - CurrRegion->GetStart(), DOUNK_SIMPLE); + CurrRegion->SetStart(BADADDR); // mark for removal + } else { // Remove only the instructions that were fixed up. CurrInstr = CurrRegion->FixupInstrs.begin();