From 86ead987d503fc89c96ca8c7e1bf215254d94df9 Mon Sep 17 00:00:00 2001
From: Andy McFadden <fadden@fadden.com>
Date: Tue, 30 Jun 2020 11:31:43 -0700
Subject: [PATCH] Tweak OMF converter

Changed bank-start comments to notes, added a summary to the top-of-file
comment.

Also, fixed a bug where the app settings dialog wasn't identifying
display settings as a preset for 64tass and cc65.
---
 Asm65/Formatter.cs                           |  48 +++++--
 SourceGen/Res/Strings.xaml                   |   3 +-
 SourceGen/Res/Strings.xaml.cs                |   2 +
 SourceGen/Tools/Omf/Loader.cs                | 135 ++++++++++++-------
 SourceGen/Tools/Omf/WpfGui/OmfViewer.xaml.cs |   2 +-
 SourceGen/WpfGui/EditAppSettings.xaml.cs     |   1 +
 6 files changed, 130 insertions(+), 61 deletions(-)

diff --git a/Asm65/Formatter.cs b/Asm65/Formatter.cs
index 5ef0ef4..d676df2 100644
--- a/Asm65/Formatter.cs
+++ b/Asm65/Formatter.cs
@@ -111,6 +111,43 @@ namespace Asm65 {
                 }
                 return em;
             }
+
+            // TODO: FormatConfig should be a class with properties so we can avoid this nonsense
+            public void Normalize() {
+                if (mForceDirectOperandPrefix == null) {
+                    mForceDirectOperandPrefix = string.Empty;
+                }
+                if (mForceAbsOpcodeSuffix == null) {
+                    mForceAbsOpcodeSuffix = string.Empty;
+                }
+                if (mForceAbsOperandPrefix == null) {
+                    mForceAbsOperandPrefix = string.Empty;
+                }
+                if (mForceDirectOpcodeSuffix == null) {
+                    mForceDirectOpcodeSuffix = string.Empty;
+                }
+                if (mForceLongOpcodeSuffix == null) {
+                    mForceLongOpcodeSuffix = string.Empty;
+                }
+                if (mForceLongOperandPrefix == null) {
+                    mForceLongOperandPrefix = string.Empty;
+                }
+                if (mLocalVariableLabelPrefix == null) {
+                    mLocalVariableLabelPrefix = string.Empty;
+                }
+                if (mNonUniqueLabelPrefix == null) {
+                    mNonUniqueLabelPrefix = string.Empty;
+                }
+                if (mEndOfLineCommentDelimiter == null) {
+                    mEndOfLineCommentDelimiter = string.Empty;
+                }
+                if (mFullLineCommentDelimiterBase == null) {
+                    mFullLineCommentDelimiterBase = string.Empty;
+                }
+                if (mBoxLineCommentDelimiter == null) {
+                    mBoxLineCommentDelimiter = string.Empty;
+                }
+            }
         }
 
         #region Text Delimiters
@@ -391,15 +428,8 @@ namespace Asm65 {
         /// </summary>
         public Formatter(FormatConfig config) {
             mFormatConfig = config;     // copy struct
-            if (mFormatConfig.mEndOfLineCommentDelimiter == null) {
-                mFormatConfig.mEndOfLineCommentDelimiter = string.Empty;
-            }
-            if (mFormatConfig.mFullLineCommentDelimiterBase == null) {
-                mFormatConfig.mFullLineCommentDelimiterBase = string.Empty;
-            }
-            if (mFormatConfig.mBoxLineCommentDelimiter == null) {
-                mFormatConfig.mBoxLineCommentDelimiter = string.Empty;
-            }
+
+            mFormatConfig.Normalize();
 
             if (string.IsNullOrEmpty(mFormatConfig.mNonUniqueLabelPrefix)) {
                 mFormatConfig.mNonUniqueLabelPrefix = "@";
diff --git a/SourceGen/Res/Strings.xaml b/SourceGen/Res/Strings.xaml
index 8b24a58..c133eca 100644
--- a/SourceGen/Res/Strings.xaml
+++ b/SourceGen/Res/Strings.xaml
@@ -130,7 +130,8 @@ limitations under the License.
     <system:String x:Key="str_MsgVisualizationIgnored">Visualization ignored</system:String>
     <system:String x:Key="str_NoFilesAvailable">no files available</system:String>
     <system:String x:Key="str_NoExportedSymbolsFound">No exported symbols found.</system:String>
-    <system:String x:Key="str_OmfSegCommentFmt" xml:space="preserve">&#x0d;Segment {0}: Kind={2}, SegName='{1}'&#x0d;&#x0d;</system:String>
+    <system:String x:Key="str_OmfSegCommentFmt">Segment {0:D2}: {3} Kind={1}, SegName='{2}'</system:String>
+    <system:String x:Key="str_OmfSegNoteFmt">Segment {0:D2}: {1} '{2}'</system:String>
     <system:String x:Key="str_OpenDataDoesntExist">The file doesn't exist.</system:String>
     <system:String x:Key="str_OpenDataEmpty">File is empty</system:String>
     <system:String x:Key="str_OpenDataFailCaption">Unable to load data file</system:String>
diff --git a/SourceGen/Res/Strings.xaml.cs b/SourceGen/Res/Strings.xaml.cs
index 8a76d76..37bd598 100644
--- a/SourceGen/Res/Strings.xaml.cs
+++ b/SourceGen/Res/Strings.xaml.cs
@@ -243,6 +243,8 @@ namespace SourceGen.Res {
             (string)Application.Current.FindResource("str_NoExportedSymbolsFound");
         public static string OMF_SEG_COMMENT_FMT =
             (string)Application.Current.FindResource("str_OmfSegCommentFmt");
+        public static string OMF_SEG_NOTE_FMT =
+            (string)Application.Current.FindResource("str_OmfSegNoteFmt");
         public static string OPEN_DATA_DOESNT_EXIST =
             (string)Application.Current.FindResource("str_OpenDataDoesntExist");
         public static string OPEN_DATA_EMPTY =
diff --git a/SourceGen/Tools/Omf/Loader.cs b/SourceGen/Tools/Omf/Loader.cs
index 35dda01..1029b7a 100644
--- a/SourceGen/Tools/Omf/Loader.cs
+++ b/SourceGen/Tools/Omf/Loader.cs
@@ -17,7 +17,9 @@ using System;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
+using System.Text;
 
+using Asm65;
 using CommonUtil;
 
 namespace SourceGen.Tools.Omf {
@@ -30,6 +32,7 @@ namespace SourceGen.Tools.Omf {
         private const string IIGS_SYSTEM_DEF = "Apple IIgs (GS/OS)";
 
         private OmfFile mOmfFile;
+        private Formatter mFormatter;
 
         private byte[] mLoadedData;
         private DisasmProject mNewProject;
@@ -50,10 +53,11 @@ namespace SourceGen.Tools.Omf {
         /// Constructor.
         /// </summary>
         /// <param name="omfFile">OMF file to load.</param>
-        public Loader(OmfFile omfFile) {
+        public Loader(OmfFile omfFile, Formatter formatter) {
             Debug.Assert(omfFile.OmfFileKind == OmfFile.FileKind.Load);
 
             mOmfFile = omfFile;
+            mFormatter = formatter;
         }
 
         /// <summary>
@@ -289,35 +293,32 @@ namespace SourceGen.Tools.Omf {
                 Debug.WriteLine("Failed to apply Apple IIgs system definition");
             }
 
-            // Add header comment.
-            string cmt = string.Format(Res.Strings.DEFAULT_HEADER_COMMENT_FMT, App.ProgramVersion);
-            proj.LongComments.Add(LineListGen.Line.HEADER_COMMENT_OFFSET,
-                new MultiLineComment(cmt));
-
             ChangeSet cs = new ChangeSet(mSegmentMap.Count * 2);
+            AddHeaderComment(proj, cs);
 
             // Load the segments, and add entries to the project.
             int bufOffset = 0;
             foreach (SegmentMapEntry ent in mSegmentMap) {
-                if (ent != null) {
-                    // Perform relocation.
-                    if (!RelocSegment(ent, data, bufOffset)) {
-                        return false;
-                    }
-
-                    // Add one or more address entries.  (Normally one, but data segments
-                    // can straddle multiple pages.)
-                    AddAddressEntries(proj, ent, bufOffset, cs);
-
-                    // Add a comment identifying the segment.
-                    string segCmt = string.Format(Res.Strings.OMF_SEG_COMMENT_FMT,
-                        ent.Segment.SegNum, ent.Segment.SegName, ent.Segment.Kind);
-                    UndoableChange uc = UndoableChange.CreateLongCommentChange(bufOffset, null,
-                        new MultiLineComment(segCmt));
-                    cs.Add(uc);
-
-                    bufOffset += ent.Segment.Length;
+                if (ent == null) {
+                    continue;
                 }
+                // Perform relocation.
+                if (!RelocSegment(ent, data, bufOffset)) {
+                    return false;
+                }
+
+                // Add one or more address entries.  (Normally one, but data segments
+                // can straddle multiple pages.)
+                AddAddressEntries(proj, ent, bufOffset, cs);
+
+                // Add a note identifying the segment.
+                string segCmt = string.Format(Res.Strings.OMF_SEG_NOTE_FMT,
+                    ent.Segment.SegNum, ent.Segment.Kind, ent.Segment.SegName);
+                UndoableChange uc = UndoableChange.CreateNoteChange(bufOffset, null,
+                    new MultiLineComment(segCmt));
+                cs.Add(uc);
+
+                bufOffset += ent.Segment.Length;
             }
 
             proj.PrepForNew(data, "new_proj");
@@ -328,44 +329,44 @@ namespace SourceGen.Tools.Omf {
             return true;
         }
 
-        private static void AddAddressEntries(DisasmProject proj, SegmentMapEntry ent,
-                int bufOffset, ChangeSet cs) {
-            int addr = ent.Address;
-            int segRem = ent.Segment.Length;
-
-            while (true) {
-                int origAddr = proj.AddrMap.Get(bufOffset);
-                UndoableChange uc = UndoableChange.CreateAddressChange(bufOffset,
-                    origAddr, addr);
-                cs.Add(uc);
-
-                // Compare amount of space in this bank to amount left in segment.
-                int bankRem = 0x00010000 - (addr & 0xffff);
-                if (bankRem > segRem) {
-                    // All done, bail.
-                    break;
+        private void AddHeaderComment(DisasmProject proj, ChangeSet cs) {
+            // Add header comment.
+            StringBuilder sb = new StringBuilder();
+            sb.AppendLine(string.Format(Res.Strings.DEFAULT_HEADER_COMMENT_FMT,
+                App.ProgramVersion));
+            sb.AppendLine();
+            foreach (SegmentMapEntry ent in mSegmentMap) {
+                if (ent == null) {
+                    continue;
                 }
-
-                // Advance to start of next bank.
-                addr += bankRem;
-                Debug.Assert((addr & 0x0000ffff) == 0);
-                bufOffset += bankRem;
-                segRem -= bankRem;
-                Debug.WriteLine("Adding additional ORG at " + addr);
+                string segCmt = string.Format(Res.Strings.OMF_SEG_COMMENT_FMT,
+                    ent.Segment.SegNum, ent.Segment.Kind, ent.Segment.SegName,
+                    mFormatter.FormatAddress(ent.Address, true));
+                sb.AppendLine(segCmt);
             }
+
+            UndoableChange uc = UndoableChange.CreateLongCommentChange(
+                LineListGen.Line.HEADER_COMMENT_OFFSET,
+                null, new MultiLineComment(sb.ToString()));
+            cs.Add(uc);
         }
 
+        /// <summary>
+        /// Edits the data file, changing values based on the relocation dictionary.
+        /// </summary>
         private bool RelocSegment(SegmentMapEntry ent, byte[] data, int bufOffset) {
-            //const int INVALID_RELOC = 0x00ffffff;
+            const int INVALID_ADDR = 0x00ffffff;
+
             byte[] srcData = ent.Segment.GetConstData();
             Array.Copy(srcData, 0, data, bufOffset, srcData.Length);
 
             foreach (OmfReloc omfRel in ent.Segment.Relocs) {
                 int relocAddr = omfRel.RelOffset;
                 if (omfRel.FileNum != -1 && omfRel.FileNum != 1) {
-                    // Some other file; not much we can do with this.
+                    // Some other file; not much we can do with this.  Drop in an obviously
+                    // invalid address and keep going.
                     Debug.WriteLine("Unable to process reloc with FileNum=" + omfRel.FileNum);
-                    return false;
+                    relocAddr = INVALID_ADDR;
                 } else if (omfRel.SegNum == -1) {
                     // Within this segment.
                     relocAddr += ent.Address;
@@ -373,6 +374,9 @@ namespace SourceGen.Tools.Omf {
                     // Find other segment.  This may fail if the file is damaged.
                     if (omfRel.SegNum < 0 || omfRel.SegNum >= mSegmentMap.Count ||
                             mSegmentMap[omfRel.SegNum] == null) {
+                        // Can't find the segment.  Unlike the file case, this was expected to
+                        // be something we could resolve with what we were given, so this is
+                        // a hard failure.
                         Debug.WriteLine("Reloc SegNum=" + omfRel.SegNum + " not in map");
                         return false;
                     } else {
@@ -413,5 +417,36 @@ namespace SourceGen.Tools.Omf {
 
             return true;
         }
+
+        /// <summary>
+        /// Adds one or more entries to the address map for the specified segment.
+        /// </summary>
+        private static void AddAddressEntries(DisasmProject proj, SegmentMapEntry ent,
+                int bufOffset, ChangeSet cs) {
+            int addr = ent.Address;
+            int segRem = ent.Segment.Length;
+
+            while (true) {
+                // Generate an ORG directive.
+                int origAddr = proj.AddrMap.Get(bufOffset);
+                UndoableChange uc = UndoableChange.CreateAddressChange(bufOffset,
+                    origAddr, addr);
+                cs.Add(uc);
+
+                // Compare amount of space in this bank to amount left in segment.
+                int bankRem = 0x00010000 - (addr & 0xffff);
+                if (bankRem > segRem) {
+                    // All done, bail.
+                    break;
+                }
+
+                // Advance to start of next bank.
+                addr += bankRem;
+                Debug.Assert((addr & 0x0000ffff) == 0);
+                bufOffset += bankRem;
+                segRem -= bankRem;
+                Debug.WriteLine("Adding additional ORG at " + addr);
+            }
+        }
     }
 }
diff --git a/SourceGen/Tools/Omf/WpfGui/OmfViewer.xaml.cs b/SourceGen/Tools/Omf/WpfGui/OmfViewer.xaml.cs
index cf79bdb..c9cd731 100644
--- a/SourceGen/Tools/Omf/WpfGui/OmfViewer.xaml.cs
+++ b/SourceGen/Tools/Omf/WpfGui/OmfViewer.xaml.cs
@@ -166,7 +166,7 @@ namespace SourceGen.Tools.Omf.WpfGui {
         }
 
         private void GenerateProject_Click(object sender, RoutedEventArgs e) {
-            Loader loader = new Loader(mOmfFile);
+            Loader loader = new Loader(mOmfFile, mFormatter);
             if (!loader.Prepare()) {
                 // Unexpected.  If there's a valid reason for this, we need to add details
                 // to the error message.
diff --git a/SourceGen/WpfGui/EditAppSettings.xaml.cs b/SourceGen/WpfGui/EditAppSettings.xaml.cs
index 3acd45e..500b893 100644
--- a/SourceGen/WpfGui/EditAppSettings.xaml.cs
+++ b/SourceGen/WpfGui/EditAppSettings.xaml.cs
@@ -984,6 +984,7 @@ namespace SourceGen.WpfGui {
 
                 gen.GetDefaultDisplayFormat(out PseudoOp.PseudoOpNames unused,
                     out Asm65.Formatter.FormatConfig formatConfig);
+                formatConfig.Normalize();
 
                 DisplayPresets[i + 2] = new DisplayFormatPreset((int)asmInfo.AssemblerId,
                     asmInfo.Name, formatConfig.mForceAbsOpcodeSuffix,