From 23a4852838e3f283ba5426f7e540d13931d86d34 Mon Sep 17 00:00:00 2001
From: Andy McFadden <fadden@fadden.com>
Date: Mon, 15 Oct 2018 14:13:59 -0700
Subject: [PATCH] Fix goto-by-offset

Most of the decorative items associated with a file offset are
placed before the item in the display list, and given a span of
zero.  This yields the correct behavior in a binary search: an
exact match finds the decorative item (e.g. a blank line), while a
match partway into the instruction or multi-byte data item causes
the binary search to move on to the next line, where it's resolved.

The problem is that we were adding a blank line *after* instructions
in the no-continue case.  If the binary search found the blank line
before it found the instruction, it would guess "too high" rather
than "too low", and miss the actual instruction line.

We now set a flag and add the blank line as part of the following
item.  We do a little dance at the start to ensure that the blank
line doesn't disappear during a partial update.
---
 SourceGen/AppForms/ProjectView.cs |  3 +++
 SourceGen/DisplayList.cs          | 28 ++++++++++++++++++++++++++--
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/SourceGen/AppForms/ProjectView.cs b/SourceGen/AppForms/ProjectView.cs
index 8662074..93d07f6 100644
--- a/SourceGen/AppForms/ProjectView.cs
+++ b/SourceGen/AppForms/ProjectView.cs
@@ -4308,6 +4308,9 @@ namespace SourceGen.AppForms {
             if (line.OffsetSpan == 0) {
                 sb.AppendFormat(Properties.Resources.FMT_INFO_LINE_SUM_NON,
                     lineIndex, lineTypeStr);
+#if DEBUG
+                sb.Append(" [offset=+" + line.FileOffset.ToString("x6") + "]");
+#endif
                 if (!string.IsNullOrEmpty(extraStr)) {
                     sb.Append("\r\n\r\n");
                     sb.Append(extraStr);
diff --git a/SourceGen/DisplayList.cs b/SourceGen/DisplayList.cs
index ba0086f..d92bb14 100644
--- a/SourceGen/DisplayList.cs
+++ b/SourceGen/DisplayList.cs
@@ -875,6 +875,18 @@ namespace SourceGen {
                 }
             }
 
+            // Configure the initial value of addBlank.  The specific case we're handling is
+            // a no-continue instruction (e.g. JMP) followed by an instruction with a label.
+            // When we rename the label, we don't want the blank to disappear during the
+            // partial-list generation.
+            bool addBlank = false;
+            if (startOffset > 0) {
+                int baseOff = DataAnalysis.GetBaseOperandOffset(proj, startOffset - 1);
+                if (proj.GetAnattrib(baseOff).DoesNotContinue) {
+                    addBlank = true;
+                }
+            }
+
             int offset = startOffset;
             while (offset <= endOffset) {
                 Anattrib attr = proj.GetAnattrib(offset);
@@ -882,7 +894,11 @@ namespace SourceGen {
                         proj.GetAnattrib(offset - 1).IsData) {
                     // Transition from data to code.  (Don't add blank line for inline data.)
                     lines.Add(GenerateBlankLine(offset));
+                } else if (addBlank) {
+                    // Previous instruction wanted to be followed by a blank line.
+                    lines.Add(GenerateBlankLine(offset));
                 }
+                addBlank = false;
 
                 // Insert long comments and notes.  These may span multiple display lines,
                 // and require word-wrap, so it's easiest just to render them fully here.
@@ -953,8 +969,16 @@ namespace SourceGen {
                     // break in code, and before a data area.
                     // TODO(maybe): Might also want to do this if the next offset is data,
                     // to make things look nicer when code runs directly into data.
+                    //
+                    // We don't want to add it with the current line's offset.  If we do that,
+                    // the binary search will get confused, because blank lines have a span
+                    // of zero.  If the code is at offset 10 with length 3, and we search for
+                    // the byte at offset 11, then a blank line (with span=0) at offset 10 will
+                    // cause the binary search to assume that the target is farther down, when
+                    // it's actually one line up.  We deal with this by setting a flag and
+                    // generating the blank line on the next trip through the loop.
                     if (attr.DoesNotContinue) {
-                        lines.Add(GenerateBlankLine(offset));
+                        addBlank = true;
                     }
 
                     offset += len;
@@ -970,7 +994,7 @@ namespace SourceGen {
                 }
             }
 
-            // See if there were any address shifts in this section.  If so, add an ORG
+            // See if there were any address shifts in this section.  If so, insert an ORG
             // statement as the first entry for the offset.  We're expecting to have very
             // few AddressMap entries (usually just one), so it's more efficient to process
             // them here and walk through the sub-list than it is to ping the address map