From fa04c98dac3f21cedb59f355fbb0a18e45257eed Mon Sep 17 00:00:00 2001 From: Andy McFadden Date: Wed, 27 Oct 2021 14:18:52 -0700 Subject: [PATCH] Correct StdInline behavior for overlapping addresses The implementation was mapping labels to addresses, then formatting inline data at the matching address. This may be incorrect when there are multiple sections of the file mapped to the same address. The correct approach is to record the offsets of the matching labels, and then do an address-to-offset translation for each JSR. Also, show a note in the Info window when a JSR has been marked no-continue by an extension script. Also, updated Daily Tips. --- SourceGen/DailyTips.cs | 8 +- SourceGen/MainController.cs | 3 + SourceGen/RuntimeData/Common/StdInline.cs | 68 ++++++++++++---- SourceGen/RuntimeData/Tips/daily-tips.json | 5 +- SourceGen/SGTestData/20270-std-inline | Bin 210 -> 251 bytes SourceGen/SGTestData/20270-std-inline.dis65 | 77 +++++++++++++++++- .../Expected/20270-std-inline_64tass.S | 52 +++++++++++- .../Expected/20270-std-inline_acme.S | 52 +++++++++++- .../Expected/20270-std-inline_cc65.S | 47 ++++++++++- .../Expected/20270-std-inline_merlin32.S | 47 ++++++++++- .../SGTestData/Source/20270-std-inline.S | 56 +++++++++++++ 11 files changed, 374 insertions(+), 41 deletions(-) diff --git a/SourceGen/DailyTips.cs b/SourceGen/DailyTips.cs index bd45b5c..72c4f67 100644 --- a/SourceGen/DailyTips.cs +++ b/SourceGen/DailyTips.cs @@ -106,11 +106,11 @@ namespace SourceGen { public int DailyNumber { get { // We show a different tip every day by taking the day-of-year value and - // modding it by the number of tips we have. Doesn't do the right thing - // at the end of year transition, but everybody is off partying anyway. + // modding it by the number of tips we have. if (mTips.Count > 0) { - int doy = DateTime.Now.DayOfYear; - return doy % mTips.Count; + DateTime now = DateTime.Now; + int dayIndex = now.Year * 365 + now.DayOfYear; + return dayIndex % mTips.Count; } else { return 0; } diff --git a/SourceGen/MainController.cs b/SourceGen/MainController.cs index c8c723b..f5e6b37 100644 --- a/SourceGen/MainController.cs +++ b/SourceGen/MainController.cs @@ -4320,6 +4320,9 @@ namespace SourceGen { //sb.Append("DEBUG: opAddr=" + attr.OperandAddress.ToString("x4") + // " opOff=" + attr.OperandOffset.ToString("x4") + "\r\n"); + if (attr.NoContinueScript) { + sb.AppendLine("\"No-continue\" flag set by script"); + } if (attr.HasAnalyzerTag) { sb.Append("\u2022 Analyzer Tags: "); for (int i = 0; i < line.OffsetSpan; i++) { diff --git a/SourceGen/RuntimeData/Common/StdInline.cs b/SourceGen/RuntimeData/Common/StdInline.cs index bc39fcb..b81e364 100644 --- a/SourceGen/RuntimeData/Common/StdInline.cs +++ b/SourceGen/RuntimeData/Common/StdInline.cs @@ -16,6 +16,7 @@ using System; using System.Collections.Generic; + using PluginCommon; namespace RuntimeData.Common { @@ -37,9 +38,19 @@ namespace RuntimeData.Common { /// ASCII functions work for standard and high ASCII, auto-detecting the encoding based on /// the first character. /// + /// + /// As an optimization, we use a lookup table keyed by address, and another keyed by offset. + /// For a project that doesn't have overlapping address spaces this wouldn't be necessary, + /// and we could just map the address (JSR operand) to the inline data type. Since this + /// code is meant be a general-purpose, we need to use the offset, but that requires a lookup + /// in the address translation table, which we would prefer to avoid doing for every JSR in + /// the project. So we do a quick check on the address first, and only do the offset + /// translation if it looks like a possible match. + /// public class StdInline : MarshalByRefObject, IPlugin, IPlugin_SymbolList, IPlugin_InlineJsr { private IApplication mAppRef; private byte[] mFileData; + private AddressTranslate mAddrTrans; private class NameMap { public string Prefix { get; private set; } @@ -60,8 +71,11 @@ namespace RuntimeData.Common { new NameMap("InWA_", InlineKind.InWA), }; - // Map of addresses (not offsets) in project to inline data handled by code there. - private Dictionary mInlineLabels = new Dictionary(); + // Map of JSR offsets in project to inline data type expected to follow. + private Dictionary mInlineOffsets = new Dictionary(); + + // List of "interesting" addresses. Used as an optimization. + private Dictionary mInlineAddrs = new Dictionary(); // IPlugin public string Identifier { @@ -69,9 +83,10 @@ namespace RuntimeData.Common { } // IPlugin - public void Prepare(IApplication appRef, byte[] fileData, AddressTranslate unused) { + public void Prepare(IApplication appRef, byte[] fileData, AddressTranslate addrTrans) { mAppRef = appRef; mFileData = fileData; + mAddrTrans = addrTrans; mAppRef.DebugLog("StdInline(id=" + AppDomain.CurrentDomain.Id + "): prepare()"); } @@ -80,31 +95,34 @@ namespace RuntimeData.Common { public void Unprepare() { mAppRef = null; mFileData = null; + mAddrTrans = null; } // IPlugin_SymbolList public void UpdateSymbolList(List plSyms) { - mInlineLabels.Clear(); + mInlineOffsets.Clear(); + mInlineAddrs.Clear(); - // Find matching symbols. Save the symbol's value (its address) and the type. - // We want an exact match on L1STR_NAME, and prefix matches on the other two. + // Find matching symbols. foreach (PlSymbol sym in plSyms) { - // We might want to ignore user labels in non-addressable regions, which all - // show up with NON_ADDR as their address. In practice it doesn't matter. + if (sym.Value == AddressTranslate.NON_ADDR) { + // The non-addressable target won't be returned by the address-to-offset + // lookup, so this doesn't change the behavior. But there's no value in + // having NON_ADDR in the lookup table, so strip it out now. + //mAppRef.DebugLog("Ignoring non-addr label '" + sym.Label + "'"); + continue; + } foreach (NameMap map in sMap) { if (sym.Label.StartsWith(map.Prefix)) { - // Multiple offsets could have the same address. Map the first. - if (!mInlineLabels.ContainsKey(sym.Value)) { - mInlineLabels.Add(sym.Value, map.Kind); - } else { - mAppRef.DebugLog("Ignoring duplicate address " + - sym.Value.ToString("x4")); - } + // Offsets will be unique. + mInlineOffsets.Add(sym.Offset, map.Kind); + // Symbol values (addresses) may not be unique. + mInlineAddrs[sym.Value] = sym.Value; break; } } } - mAppRef.DebugLog("Found matches for " + mInlineLabels.Count + " labels"); + mAppRef.DebugLog("Found matches for " + mInlineOffsets.Count + " labels"); } // IPlugin_SymbolList @@ -124,12 +142,26 @@ namespace RuntimeData.Common { public void CheckJsr(int offset, int operand, out bool noContinue) { noContinue = false; - InlineKind kind; - if (!mInlineLabels.TryGetValue(operand, out kind)) { + // Do a quick test on the address. + int unused; + if (!mInlineAddrs.TryGetValue(operand, out unused)) { // JSR destination address not recognized. return; } + // Address matched. Translate the target address to the actual offset. This is + // important when multiple offsets have the same address. + int targetOffset = mAddrTrans.AddressToOffset(offset, operand); + if (targetOffset < 0) { + mAppRef.DebugLog("Failed to map address $" + operand.ToString("x4") + " to offset"); + return; + } + InlineKind kind; + if (!mInlineOffsets.TryGetValue(targetOffset, out kind)) { + // Actual call target doesn't have a matching label. + return; + } + offset += 3; // move past JSR switch (kind) { diff --git a/SourceGen/RuntimeData/Tips/daily-tips.json b/SourceGen/RuntimeData/Tips/daily-tips.json index e025013..69510e6 100644 --- a/SourceGen/RuntimeData/Tips/daily-tips.json +++ b/SourceGen/RuntimeData/Tips/daily-tips.json @@ -34,7 +34,10 @@ "Image" : "note-sample.png" }, { - "Text" : "You're not limited to global labels. You can create non-unique local labels, like \"@LOOP\", and define multiple labels for zero-page addresses in local variable tables." + "Text" : "You're not limited to global labels. You can create non-unique local labels, like \"@LOOP\", and define multiple labels for zero-page addresses in Local Variable Tables." + }, + { + "Text" : "You can copy and paste lines from the disassembly listing as text simply by selecting them and hitting Ctrl+C. This can be handy for bug reports and online forum postings. The set of columns copied can be chosen in the application settings." }, { "Text" : "2D bitmap images and 3D wireframe meshes can be converted to images that are displayed inline. This can make it much easier to figure out what a piece of code is drawing." diff --git a/SourceGen/SGTestData/20270-std-inline b/SourceGen/SGTestData/20270-std-inline index a151d1cf3a9eb73b1abd35ebd8fe972260ab0866..60f0f612035f343370161dbb82c2c7f5398832db 100644 GIT binary patch delta 83 zcmcb__?vOUdLIS_2EkW83?CX^DFB&52@qNsLhCRnBs2mg3=$d@I6f$det4Cjz#{O! cBtKsvCqFO!l>)OsN`7jwLSBA}LTX+L0Fh!GNdN!< delta 15 Xcmey(c!_bsde&_M3Wo(I-jxFYH8loV diff --git a/SourceGen/SGTestData/20270-std-inline.dis65 b/SourceGen/SGTestData/20270-std-inline.dis65 index df1d859..f4faf28 100644 --- a/SourceGen/SGTestData/20270-std-inline.dis65 +++ b/SourceGen/SGTestData/20270-std-inline.dis65 @@ -1,8 +1,8 @@ ### 6502bench SourceGen dis65 v1.0 ### { "_ContentVersion":5, -"FileDataLength":210, -"FileDataCrc32":-1608872177, +"FileDataLength":251, +"FileDataCrc32":-1864354559, "ProjectProps":{ "CpuName":"6502", "IncludeUndocumentedInstr":false, @@ -28,10 +28,55 @@ "Addr":4096, "Length":-1024, "PreLabel":"", +"IsRelative":false}, + +{ +"Offset":184, +"Addr":8192, +"Length":-1024, +"PreLabel":"", +"IsRelative":false}, + +{ +"Offset":192, +"Addr":8192, +"Length":-1024, +"PreLabel":"", +"IsRelative":false}, + +{ +"Offset":200, +"Addr":8192, +"Length":-1024, +"PreLabel":"", +"IsRelative":false}, + +{ +"Offset":209, +"Addr":-1025, +"Length":-1024, +"PreLabel":"", +"IsRelative":false}, + +{ +"Offset":215, +"Addr":61440, +"Length":-1024, +"PreLabel":"", "IsRelative":false}], "TypeHints":[{ "Low":0, "High":0, +"Hint":"Code"}, + +{ +"Low":192, +"High":192, +"Hint":"Code"}, + +{ +"Low":200, +"High":200, "Hint":"Code"}], "StatusFlagOverrides":{ }, @@ -93,6 +138,34 @@ "Value":4105, "Source":"User", "Type":"GlobalAddr", +"LabelAnno":"None"}, + +"184":{ +"Label":"InW_test1", +"Value":8192, +"Source":"User", +"Type":"GlobalAddr", +"LabelAnno":"None"}, + +"192":{ +"Label":"InW_test2", +"Value":8192, +"Source":"User", +"Type":"GlobalAddr", +"LabelAnno":"None"}, + +"200":{ +"Label":"not_inline", +"Value":8192, +"Source":"User", +"Type":"GlobalAddr", +"LabelAnno":"None"}, + +"209":{ +"Label":"InW_na_test", +"Value":-1025, +"Source":"User", +"Type":"GlobalAddr", "LabelAnno":"None"}}, "OperandFormats":{ diff --git a/SourceGen/SGTestData/Expected/20270-std-inline_64tass.S b/SourceGen/SGTestData/Expected/20270-std-inline_64tass.S index 565b853..b7f86b4 100644 --- a/SourceGen/SGTestData/Expected/20270-std-inline_64tass.S +++ b/SourceGen/SGTestData/Expected/20270-std-inline_64tass.S @@ -50,16 +50,60 @@ L1040 nop .byte $00 _L10AD nop - jsr _L10B6 - jsr _L10C3 + jsr InW_test1 + .word $1100 + nop + jmp LF000 + + .byte $80 + + .logical $2000 +InW_test1 nop + jsr InW_test1 + .word $1200 + rts + + .byte $80 + .here + + .logical $2000 +InW_test2 nop + jsr InW_test2 + .word $1300 + rts + + .byte $80 + .here + + .logical $2000 +not_inline nop + jsr not_inline + bit not_inline + rts + + .byte $81 + .here + .logical $0000 +InW_na_test .byte $ea + .byte $20 + .byte $00 + .byte $30 + .byte $60 + .byte $81 + .here + + .logical $f000 +LF000 jsr _LF008 + jsr _LF015 nop rts -_L10B6 jsr InA1_test +_LF008 jsr InA1_test .byte $ff .enc "sg_ascii" .text "too long" .byte $ea -_L10C3 jsr InAZ_test +_LF015 jsr InAZ_test .text "does not end" + .here diff --git a/SourceGen/SGTestData/Expected/20270-std-inline_acme.S b/SourceGen/SGTestData/Expected/20270-std-inline_acme.S index 164742e..ded3ffd 100644 --- a/SourceGen/SGTestData/Expected/20270-std-inline_acme.S +++ b/SourceGen/SGTestData/Expected/20270-std-inline_acme.S @@ -42,15 +42,59 @@ L1040 nop !byte $00 @L10AD nop - jsr @L10B6 - jsr @L10C3 + jsr InW_test1 + !word $1100 + nop + jmp LF000 + + !byte $80 + + !pseudopc $2000 { +InW_test1 nop + jsr InW_test1 + !word $1200 + rts + + !byte $80 + } + + !pseudopc $2000 { +InW_test2 nop + jsr InW_test2 + !word $1300 + rts + + !byte $80 + } + + !pseudopc $2000 { +not_inline nop + jsr not_inline + bit not_inline + rts + + !byte $81 + } + !pseudopc $0000 { +InW_na_test !byte $ea + !byte $20 + !byte $00 + !byte $30 + !byte $60 + !byte $81 + } + + !pseudopc $f000 { +LF000 jsr @LF008 + jsr @LF015 nop rts -@L10B6 jsr InA1_test +@LF008 jsr InA1_test !byte $ff !text "too long" !byte $ea -@L10C3 jsr InAZ_test +@LF015 jsr InAZ_test !text "does not end" + } diff --git a/SourceGen/SGTestData/Expected/20270-std-inline_cc65.S b/SourceGen/SGTestData/Expected/20270-std-inline_cc65.S index d0859fb..e0ad475 100644 --- a/SourceGen/SGTestData/Expected/20270-std-inline_cc65.S +++ b/SourceGen/SGTestData/Expected/20270-std-inline_cc65.S @@ -42,15 +42,54 @@ L1040: nop .byte $00 @L10AD: nop - jsr @L10B6 - jsr @L10C3 + jsr InW_test1 + .word $1100 + nop + jmp LF000 + + .byte $80 + + .org $2000 +InW_test1: nop + jsr InW_test1 + .word $1200 + rts + + .byte $80 + + .org $2000 +InW_test2: nop + jsr InW_test2 + .word $1300 + rts + + .byte $80 + + .org $2000 +not_inline: nop + jsr not_inline + bit not_inline + rts + + .byte $81 + .org $0000 +InW_na_test: .byte $ea + .byte $20 + .byte $00 + .byte $30 + .byte $60 + .byte $81 + + .org $f000 +LF000: jsr @LF008 + jsr @LF015 nop rts -@L10B6: jsr InA1_test +@LF008: jsr InA1_test .byte $ff .byte "too long" .byte $ea -@L10C3: jsr InAZ_test +@LF015: jsr InAZ_test .byte "does not end" diff --git a/SourceGen/SGTestData/Expected/20270-std-inline_merlin32.S b/SourceGen/SGTestData/Expected/20270-std-inline_merlin32.S index b8bd7b0..9de7742 100644 --- a/SourceGen/SGTestData/Expected/20270-std-inline_merlin32.S +++ b/SourceGen/SGTestData/Expected/20270-std-inline_merlin32.S @@ -41,15 +41,54 @@ L1040 nop dfb $00 :L10AD nop - jsr :L10B6 - jsr :L10C3 + jsr InW_test1 + dw $1100 + nop + jmp LF000 + + dfb $80 + + org $2000 +InW_test1 nop + jsr InW_test1 + dw $1200 + rts + + dfb $80 + + org $2000 +InW_test2 nop + jsr InW_test2 + dw $1300 + rts + + dfb $80 + + org $2000 +not_inline nop + jsr not_inline + bit not_inline + rts + + dfb $81 + org $0000 +InW_na_test dfb $ea + dfb $20 + dfb $00 + dfb $30 + dfb $60 + dfb $81 + + org $f000 +LF000 jsr :LF008 + jsr :LF015 nop rts -:L10B6 jsr InA1_test +:LF008 jsr InA1_test dfb $ff asc 'too long' dfb $ea -:L10C3 jsr InAZ_test +:LF015 jsr InAZ_test asc 'does not end' diff --git a/SourceGen/SGTestData/Source/20270-std-inline.S b/SourceGen/SGTestData/Source/20270-std-inline.S index de44f5b..ea6f0d1 100644 --- a/SourceGen/SGTestData/Source/20270-std-inline.S +++ b/SourceGen/SGTestData/Source/20270-std-inline.S @@ -56,7 +56,62 @@ calls nop brk cont nop + +; Test having multiple address spaces with the same target address. +; Two of the spaces have a matching symbol, one doesn't. If we +; match strictly by address we'll get it wrong. + jsr f_W_2k1 + !word $1100 + + nop + jmp end_stuff + + !byte $80 + + !pseudopc $2000 { ;EDIT: add address space, set label InW_ +f_W_2k1 nop + jsr f_W_2k1 + !word $1200 + rts + } + + !byte $80 + + !pseudopc $2000 { ;EDIT: add address space, set label InW_ +f_W_2k2 nop + jsr f_W_2k2 + !word $1300 + rts + } + + !byte $80 + + !pseudopc $2000 { ;EDIT: add address space, no label +notspec nop + jsr notspec + bit notspec + rts + } + + !byte $81 + +; Test having a label in a non-addressable area. The formatter should +; ignore it, since such areas can't have code in them. Note we can't +; actually call it, since that would require referencing a label in a +; non-addressable region, so we're really just using this as a way to +; exercise the setup code in the script. + !pseudopc $3000 { ;EDIT: add NA address space, set label InW_ +f_W_na nop + jsr f_W_na + rts + } + + !byte $81 + + + !pseudopc $f000 { ; end-of-file error cases +end_stuff jsr end_err1 jsr end_err2 @@ -70,3 +125,4 @@ end_err1 end_err2 jsr f_AZ !text "does not end" ;must be last + }