From 431ad94d95966ef73fbac38a33bd94691e910afc Mon Sep 17 00:00:00 2001 From: Andy McFadden Date: Mon, 2 Sep 2019 15:57:59 -0700 Subject: [PATCH] Make "smart" PLP handling optional We try to be clever with PHP/PLP, but sometimes we get it wrong. If we get it wrong a lot, we want to turn it off. Now we can. --- SourceGen/CodeAnalysis.cs | 31 ++++++++++++------- SourceGen/DisasmProject.cs | 4 +-- SourceGen/ProjectFile.cs | 9 ++++++ SourceGen/ProjectProperties.cs | 4 +++ SourceGen/RuntimeData/Help/analysis.html | 11 ++++--- SourceGen/RuntimeData/Help/settings.html | 14 ++++++--- SourceGen/WpfGui/EditProjectProperties.xaml | 6 ++-- .../WpfGui/EditProjectProperties.xaml.cs | 8 +++++ 8 files changed, 64 insertions(+), 23 deletions(-) diff --git a/SourceGen/CodeAnalysis.cs b/SourceGen/CodeAnalysis.cs index 99630a7..64b41bb 100644 --- a/SourceGen/CodeAnalysis.cs +++ b/SourceGen/CodeAnalysis.cs @@ -146,6 +146,11 @@ namespace SourceGen { /// private StatusFlags mEntryFlags; + /// + /// User-configurable analysis parameters. + /// + private ProjectProperties.AnalysisParameters mAnalysisParameters; + /// /// Debug trace log. /// @@ -168,7 +173,8 @@ namespace SourceGen { /// Object that receives debug log messages. public CodeAnalysis(byte[] data, CpuDef cpuDef, Anattrib[] anattribs, AddressMap addrMap, TypeHint[] hints, StatusFlags[] statusFlagOverrides, - StatusFlags entryFlags, ScriptManager scriptMan, DebugLog debugLog) { + StatusFlags entryFlags, ProjectProperties.AnalysisParameters parms, + ScriptManager scriptMan, DebugLog debugLog) { mFileData = data; mCpuDef = cpuDef; mAnattribs = anattribs; @@ -177,6 +183,7 @@ namespace SourceGen { mStatusFlagOverrides = statusFlagOverrides; mEntryFlags = entryFlags; mScriptManager = scriptMan; + mAnalysisParameters = parms; mDebugLog = debugLog; mScriptSupport = new ScriptSupport(this); @@ -789,16 +796,18 @@ namespace SourceGen { backOffsetLimit = 0; } StatusFlags flags = StatusFlags.AllIndeterminate; - for (int offset = plpOffset - 1; offset >= backOffsetLimit; offset--) { - Anattrib attr = mAnattribs[offset]; - if (!attr.IsInstructionStart || !attr.IsVisited) { - continue; - } - OpDef op = mCpuDef.GetOpDef(mFileData[offset]); - if (op == OpDef.OpPHP_StackPush) { - LogI(plpOffset, "Found visited PHP at +" + offset.ToString("x6")); - flags = mAnattribs[offset].StatusFlags; - break; + if (mAnalysisParameters.SmartPlpHandling) { + for (int offset = plpOffset - 1; offset >= backOffsetLimit; offset--) { + Anattrib attr = mAnattribs[offset]; + if (!attr.IsInstructionStart || !attr.IsVisited) { + continue; + } + OpDef op = mCpuDef.GetOpDef(mFileData[offset]); + if (op == OpDef.OpPHP_StackPush) { + LogI(plpOffset, "Found visited PHP at +" + offset.ToString("x6")); + flags = mAnattribs[offset].StatusFlags; + break; + } } } diff --git a/SourceGen/DisasmProject.cs b/SourceGen/DisasmProject.cs index f22e715..3e97253 100644 --- a/SourceGen/DisasmProject.cs +++ b/SourceGen/DisasmProject.cs @@ -618,8 +618,8 @@ namespace SourceGen { reanalysisTimer.StartTask("CodeAnalysis.Analyze"); CodeAnalysis ca = new CodeAnalysis(mFileData, CpuDef, mAnattribs, AddrMap, - TypeHints, StatusFlagOverrides, ProjectProps.EntryFlags, mScriptManager, - debugLog); + TypeHints, StatusFlagOverrides, ProjectProps.EntryFlags, + ProjectProps.AnalysisParams, mScriptManager, debugLog); ca.Analyze(); reanalysisTimer.EndTask("CodeAnalysis.Analyze"); diff --git a/SourceGen/ProjectFile.cs b/SourceGen/ProjectFile.cs index 99ba153..6230f2c 100644 --- a/SourceGen/ProjectFile.cs +++ b/SourceGen/ProjectFile.cs @@ -219,6 +219,7 @@ namespace SourceGen { public string DefaultTextScanMode { get; set; } public int MinCharsForString { get; set; } public bool SeekNearbyTargets { get; set; } + public bool SmartPlpHandling { get; set; } public SerAnalysisParameters() { } public SerAnalysisParameters(ProjectProperties.AnalysisParameters src) { @@ -226,6 +227,7 @@ namespace SourceGen { DefaultTextScanMode = src.DefaultTextScanMode.ToString(); MinCharsForString = src.MinCharsForString; SeekNearbyTargets = src.SeekNearbyTargets; + SmartPlpHandling = src.SmartPlpHandling; } } public class SerAddressMap { @@ -511,6 +513,13 @@ namespace SourceGen { spf.ProjectProps.AnalysisParams.MinCharsForString; proj.ProjectProps.AnalysisParams.SeekNearbyTargets = spf.ProjectProps.AnalysisParams.SeekNearbyTargets; + if (spf._ContentVersion < 2) { + // This was made optional in v1.3. Default it to true for older projects. + proj.ProjectProps.AnalysisParams.SmartPlpHandling = true; + } else { + proj.ProjectProps.AnalysisParams.SmartPlpHandling = + spf.ProjectProps.AnalysisParams.SmartPlpHandling; + } // Deserialize ProjectProperties: external file identifiers. Debug.Assert(proj.ProjectProps.PlatformSymbolFileIdentifiers.Count == 0); diff --git a/SourceGen/ProjectProperties.cs b/SourceGen/ProjectProperties.cs index 763430a..2a1f976 100644 --- a/SourceGen/ProjectProperties.cs +++ b/SourceGen/ProjectProperties.cs @@ -51,18 +51,22 @@ namespace SourceGen { public TextScanMode DefaultTextScanMode { get; set; } public int MinCharsForString { get; set; } public bool SeekNearbyTargets { get; set; } + public bool SmartPlpHandling { get; set; } public AnalysisParameters() { + // Set default values. AnalyzeUncategorizedData = true; DefaultTextScanMode = TextScanMode.LowHighAscii; MinCharsForString = DataAnalysis.DEFAULT_MIN_STRING_LENGTH; SeekNearbyTargets = true; + SmartPlpHandling = true; } public AnalysisParameters(AnalysisParameters src) { AnalyzeUncategorizedData = src.AnalyzeUncategorizedData; DefaultTextScanMode = src.DefaultTextScanMode; MinCharsForString = src.MinCharsForString; SeekNearbyTargets = src.SeekNearbyTargets; + SmartPlpHandling = src.SmartPlpHandling; } } diff --git a/SourceGen/RuntimeData/Help/analysis.html b/SourceGen/RuntimeData/Help/analysis.html index 33f53c5..14c1369 100644 --- a/SourceGen/RuntimeData/Help/analysis.html +++ b/SourceGen/RuntimeData/Help/analysis.html @@ -244,15 +244,18 @@ status flag override to get correct disassembly.

The instruction that is most likely to cause problems is PLP, which pulls the processor status flags off of the stack. SourceGen doesn't try to track stack contents, so it can't know what values may -be pulled. In many cases the PLP appears not long after a PHP, so -SourceGen will scan backward through the file to find the nearest PHP, -and use the status flags from that. If no PHP can be found, then all +be pulled. In many cases the PLP appears not long after a +PHP, so SourceGen will scan backward through the file to +find the nearest PHP, and use the status flags from that. If +no PHP can be found, then all flags are set to "indeterminate". (The boot loader in the Apple II 5.25" floppy disk controller is an example where SourceGen gets it wrong. The code does CLC/PHP, followed a bit later by the PLP, but it's actually using the stack to pass the carry flag around. Flagging the carry bit as indeterminate with a status flag -override on the instruction following the PLP fixes things.)

+override on the instruction following the PLP fixes things.) The +"smart" behavior can be disabled in the project properties if it's coming +out wrong more often than right.

Some other things that the code analyzer can't recognize automatically: