From 49603ba4173e601f57552893d7ec38cadbbcf84f Mon Sep 17 00:00:00 2001
From: Andy McFadden
Date: Sun, 18 Oct 2020 13:22:24 -0700
Subject: [PATCH] Refine handling of C64 PRG header
A few tweaks:
- Test now requires an ORG on offset +000002, not just a correct
address.
- Suppress on-screen display of the initial ORG directive when
a PRG file is detected. Subtle, but helpful.
- In new project setup, fix initial address for PRG projects that
load at $0000.
- In new project setup, add a "load address" comment to the first line.
Also, fix some out-of-date documentation.
(issue #90)
---
SourceGen/AsmGen/AsmTass64.cs | 38 +++------------------
SourceGen/AsmGen/GenCommon.cs | 42 ++++++++++++++++++++++++
SourceGen/DisasmProject.cs | 18 +++++-----
SourceGen/LineListGen.cs | 7 +++-
SourceGen/Res/Strings.xaml | 1 +
SourceGen/Res/Strings.xaml.cs | 2 ++
SourceGen/RuntimeData/Help/analysis.html | 31 ++++++++---------
SourceGen/RuntimeData/Help/codegen.html | 4 ++-
8 files changed, 81 insertions(+), 62 deletions(-)
diff --git a/SourceGen/AsmGen/AsmTass64.cs b/SourceGen/AsmGen/AsmTass64.cs
index 65730b5..be54019 100644
--- a/SourceGen/AsmGen/AsmTass64.cs
+++ b/SourceGen/AsmGen/AsmTass64.cs
@@ -183,7 +183,7 @@ namespace SourceGen.AsmGen {
AssemblerInfo.Id.Tass64);
mColumnWidths = (int[])config.ColumnWidths.Clone();
- mHasPrgHeader = HasPrgHeader(project);
+ mHasPrgHeader = GenCommon.HasPrgHeader(project);
}
///
@@ -342,36 +342,6 @@ namespace SourceGen.AsmGen {
}
}
- private static bool HasPrgHeader(DisasmProject project) {
- if (project.FileDataLength < 3 || project.FileDataLength > 65536) {
- return false;
- }
- Anattrib attr0 = project.GetAnattrib(0);
- Anattrib attr1 = project.GetAnattrib(1);
- if (!(attr0.IsDataStart && attr1.IsData)) {
- Debug.WriteLine("PRG test: 0/1 not data");
- return false;
- }
- if (attr0.Length != 2) {
- Debug.WriteLine("PRG test: 0/1 not 16-bit value");
- return false;
- }
- if (attr0.Symbol != null || attr1.Symbol != null) {
- Debug.WriteLine("PRG test: 0/1 has label");
- return false;
- }
- // See if the address at offset 2 matches the value at 0/1.
- int value01 = project.FileData[0] | (project.FileData[1] << 8);
- int addr2 = project.AddrMap.OffsetToAddress(2);
- if (value01 != addr2) {
- Debug.WriteLine("PRG test: 0/1 value is " + value01.ToString("x4") +
- ", address at 2 is " + addr2);
- return false;
- }
-
- return true;
- }
-
// IGenerator
public string ModifyOpcode(int offset, OpDef op) {
if (op.IsUndocumented) {
@@ -642,10 +612,12 @@ namespace SourceGen.AsmGen {
// Any subsequent ORG changes are made to the program counter, and take the form
// of a pair of ops (.logical to open, .here to end). Omitting the .here
// causes an error.
+ Debug.Assert(offset >= StartOffset);
if (offset == StartOffset) {
// Set the "compile offset" to the initial address.
- OutputLine("*", "=", SourceFormatter.FormatHexValue(Project.AddrMap.Get(0), 4),
- string.Empty);
+ OutputLine("*", "=",
+ SourceFormatter.FormatHexValue(Project.AddrMap.Get(StartOffset), 4),
+ string.Empty);
} else {
if (mNeedHereOp) {
OutputLine(string.Empty, SourceFormatter.FormatPseudoOp(HERE_PSEUDO_OP),
diff --git a/SourceGen/AsmGen/GenCommon.cs b/SourceGen/AsmGen/GenCommon.cs
index d68b9e2..7249654 100644
--- a/SourceGen/AsmGen/GenCommon.cs
+++ b/SourceGen/AsmGen/GenCommon.cs
@@ -20,6 +20,7 @@ using System.Diagnostics;
using System.IO;
using Asm65;
+using CommonUtil;
namespace SourceGen.AsmGen {
public class GenCommon {
@@ -499,5 +500,46 @@ namespace SourceGen.AsmGen {
// "(" + alignToAddr.ToString("x4") + "): " + result);
return result;
}
+
+ ///
+ /// Determines whether the project appears to have a PRG header.
+ ///
+ /// Project to check.
+ /// True if we think we found a PRG header.
+ public static bool HasPrgHeader(DisasmProject project) {
+ if (project.FileDataLength < 3 || project.FileDataLength > 65536) {
+ //Debug.WriteLine("PRG test: incompatible file length");
+ return false;
+ }
+ Anattrib attr0 = project.GetAnattrib(0);
+ Anattrib attr1 = project.GetAnattrib(1);
+ if (!(attr0.IsDataStart && attr1.IsData)) {
+ //Debug.WriteLine("PRG test: +0/1 not data");
+ return false;
+ }
+ if (attr0.Length != 2) {
+ //Debug.WriteLine("PRG test: +0/1 not 16-bit value");
+ return false;
+ }
+ if (attr0.Symbol != null || attr1.Symbol != null) {
+ //Debug.WriteLine("PRG test: +0/1 has label");
+ return false;
+ }
+ // Confirm there's an address map entry at offset 2.
+ if (project.AddrMap.Get(2) == AddressMap.NO_ENTRY_ADDR) {
+ //Debug.WriteLine("PRG test: no ORG at +2");
+ return false;
+ }
+ // See if the address at offset 2 matches the value at 0/1.
+ int value01 = project.FileData[0] | (project.FileData[1] << 8);
+ int addr2 = project.AddrMap.OffsetToAddress(2);
+ if (value01 != addr2) {
+ //Debug.WriteLine("PRG test: +0/1 value is " + value01.ToString("x4") +
+ // ", address at +2 is " + addr2);
+ return false;
+ }
+
+ return true;
+ }
}
}
diff --git a/SourceGen/DisasmProject.cs b/SourceGen/DisasmProject.cs
index ebafcca..80e79c7 100644
--- a/SourceGen/DisasmProject.cs
+++ b/SourceGen/DisasmProject.cs
@@ -388,19 +388,19 @@ namespace SourceGen {
// Configure the load address.
if (SystemDefaults.GetFirstWordIsLoadAddr(sysDef) && mFileData.Length > 2) {
- // First two bytes are the load address, code starts at offset +000002. We
- // need to put the load address into the stream, but don't want it to get
- // picked up as an address for something else. So we set it to the same
- // address as the start of the file. The overlapping-address code should do
- // the right thing with it.
- //
- // Updated after adding the "load address" report to the address edit dialog.
- // We don't want the two-byte offset.
+ // First two bytes are the load address, with the actual file data starting
+ // at +000002. We need to assign an address to the bytes, but don't want them
+ // to be referenced as an address by something else. One approach is to use
+ // the load address, and rely on the multi-address code to keep them distinct,
+ // but that throws off the "load address" display in the set-address dialog.
+ // So we just give it an offset of (start - 2) and leave it to the user to
+ // update if necessary.
int loadAddr = RawData.GetWord(mFileData, 0, 2, false);
- AddrMap.Set(0, loadAddr - 2);
+ AddrMap.Set(0, loadAddr < 2 ? 0 : loadAddr - 2);
AddrMap.Set(2, loadAddr);
OperandFormats[0] = FormatDescriptor.Create(2, FormatDescriptor.Type.NumericLE,
FormatDescriptor.SubType.None);
+ Comments[0] = Res.Strings.LOAD_ADDRESS;
AnalyzerTags[0] = CodeAnalysis.AnalyzerTag.None;
AnalyzerTags[2] = CodeAnalysis.AnalyzerTag.Code;
} else {
diff --git a/SourceGen/LineListGen.cs b/SourceGen/LineListGen.cs
index 28a495b..0b3d93f 100644
--- a/SourceGen/LineListGen.cs
+++ b/SourceGen/LineListGen.cs
@@ -905,7 +905,7 @@ namespace SourceGen {
/// Complicated items, such as word-wrapped long comments, may be generated now
/// and saved off.
///
- /// This still needs a formatter arg even when no text is rendered because some
+ /// This still needs a formatter even when no text is rendered because some
/// options, like maximum per-line operand length, might affect how many lines
/// are generated.
///
@@ -1156,6 +1156,11 @@ namespace SourceGen {
if (ent.Offset < startOffset || ent.Offset > endOffset) {
continue;
}
+ if (ent.Offset == 0 && AsmGen.GenCommon.HasPrgHeader(mProject)) {
+ // Suppress the ORG at offset zero. We know there's another one
+ // at offset +000002, and that it matches the value at +0/1.
+ continue;
+ }
int index = FindLineByOffset(lines, ent.Offset);
if (index < 0) {
Debug.WriteLine("Couldn't find offset " + ent.Offset +
diff --git a/SourceGen/Res/Strings.xaml b/SourceGen/Res/Strings.xaml
index 5c511b3..9ba1846 100644
--- a/SourceGen/Res/Strings.xaml
+++ b/SourceGen/Res/Strings.xaml
@@ -121,6 +121,7 @@ limitations under the License.
Unable to format as word: each selected region must have an even number of bytes ({0} region(s) are selected).
• Clear variables
• Empty variable table
+ load address
Address bank overrun
Bank boundary crossed at offset {0}
Format ignored
diff --git a/SourceGen/Res/Strings.xaml.cs b/SourceGen/Res/Strings.xaml.cs
index 1f05dc8..49ab7d7 100644
--- a/SourceGen/Res/Strings.xaml.cs
+++ b/SourceGen/Res/Strings.xaml.cs
@@ -219,6 +219,8 @@ namespace SourceGen.Res {
(string)Application.Current.FindResource("str_InvalidFormatWordSelNon1");
public static string INVALID_FORMAT_WORD_SEL_UNEVEN_FMT =
(string)Application.Current.FindResource("str_InvalidFormatWordSelUnevenFmt");
+ public static string LOAD_ADDRESS =
+ (string)Application.Current.FindResource("str_LoadAddress");
public static string LOCAL_VARIABLE_TABLE_CLEAR =
(string)Application.Current.FindResource("str_LocalVariableTableClear");
public static string LOCAL_VARIABLE_TABLE_EMPTY =
diff --git a/SourceGen/RuntimeData/Help/analysis.html b/SourceGen/RuntimeData/Help/analysis.html
index 005ac35..0f85dce 100644
--- a/SourceGen/RuntimeData/Help/analysis.html
+++ b/SourceGen/RuntimeData/Help/analysis.html
@@ -32,7 +32,7 @@ does create the possibility that two different users might get different
results when opening the same project file with different versions of
SourceGen, but these effects are expected to be minor.
-The analyzer has the following steps (see the Analyze
+
The analyzer performs the following steps (see the Analyze
method in DisasmProject.cs
):
- Reset the symbol table.
@@ -249,24 +249,19 @@ performed, we assume a transition to emulation mode (E=1).
value of the flag, but we know what the value will be at both
addresses.
-Self-modifying code can render spoil any of these, possibly requiring a
+
Self-modifying code can spoil any of these, possibly requiring a
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
-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.) The
-"smart" behavior can be disabled in the project properties if it's coming
-out wrong more often than right.
+PHP
, so SourceGen can scan backward through the file to
+find the nearest PHP
, and use the status flags from that.
+In practice this doesn't work well, but the "smart" behavior can be
+enabled from the project properties if desired. Otherwise, a
+PLP
causes all flags to be set to "indeterminate", except
+for the M/X flags on the 65816 which are left unmodified.
Some other things that the code analyzer can't recognize automatically:
@@ -280,11 +275,11 @@ out wrong more often than right.
Sometimes the indirect jump targets are coming from a table of
addresses in the file. If so, these can be formatted as addresses,
and then the target locations tagged as code entry points.
-The 65816 adds an additional twist: 16-bit data access instructions
-use the data bank register ("B") to determine which bank to load from.
-SourceGen can't determine what the value is, so it currently assumes
-that it's equal to the program bank register ("K"). Handling this
-correctly will require improvements to the user interface.
+The 65816 adds an additional twist: some instructions combine their
+operands with the Data Bank Register ("B") to form a 24-bit address.
+SourceGen can't automatically determine what the register holds, so it
+assumes that it's equal to the program bank register ("K"), and provides
+a way to override the value.
diff --git a/SourceGen/RuntimeData/Help/codegen.html b/SourceGen/RuntimeData/Help/codegen.html
index 4bef231..6b7784b 100644
--- a/SourceGen/RuntimeData/Help/codegen.html
+++ b/SourceGen/RuntimeData/Help/codegen.html
@@ -121,11 +121,13 @@ an additional command-line option) to the assembler.
- the format at offset +000000 is a 16-bit numeric data item
(not executable code, not two 8-byte values, not the first part
of a 24-bit value, etc.)
+ - there is an ORG directive at +000002
- the 16-bit value at +000000 is equal to the address of the
byte at +000002
- there is no label at offset +000000 (explicit or auto-generated)
-If a file is being treated as PRG and you'd rather it wasn't, you
+
The definition is sufficiently narrow to avoid most false-positives.
+If a file is being treated as PRG and you'd rather it weren't, you
can add a label or reformat the bytes.