From 63d7a4870586183f21ad8682ed6daf144258fcbd Mon Sep 17 00:00:00 2001 From: Andy McFadden Date: Fri, 8 May 2020 17:41:26 -0700 Subject: [PATCH] Fix bug in inline JSR/JSL no-continue handling JSR/JSL calls with inline data have the option of reporting that they don't continue, which causes the code analyzer to treat them as JMPs instead. There was a bug that was causing the no-continue flag to be lost in certain circumstances. The code now explicitly records the plugin's response in an Anattrib flag. Test 2022-extension-scripts has been updated with a test case that exercises this situation. --- SourceGen/Anattrib.cs | 21 ++- SourceGen/CodeAnalysis.cs | 13 +- SourceGen/SGTestData/2022-extension-scripts | Bin 213 -> 544 bytes .../SGTestData/2022-extension-scripts-a.cs | 7 + .../SGTestData/2022-extension-scripts.dis65 | 134 +++++++++++++++--- .../Expected/2022-extension-scripts_64tass.S | 46 ++++-- .../2022-extension-scripts_Merlin32.S | 45 ++++-- .../Expected/2022-extension-scripts_acme.S | 46 ++++-- .../Expected/2022-extension-scripts_cc65.S | 48 +++++-- .../Expected/2022-extension-scripts_cc65.cfg | 6 +- .../Source/2022-extension-scripts.S | 44 +++++- 11 files changed, 339 insertions(+), 71 deletions(-) diff --git a/SourceGen/Anattrib.cs b/SourceGen/Anattrib.cs index f5abd8e..4471836 100644 --- a/SourceGen/Anattrib.cs +++ b/SourceGen/Anattrib.cs @@ -41,6 +41,7 @@ namespace SourceGen { ExternalBranch = 1 << 10, // this abs/rel branch lands outside input file NoContinue = 1 << 12, // execution does not continue to following instruction + NoContinueScript = 1 << 13, // no-continue flag set by extension script Visited = 1 << 16, // has the analyzer visited this byte? Changed = 1 << 17, // set/cleared as the analyzer works @@ -148,10 +149,7 @@ namespace SourceGen { } } } - public bool DoesNotContinue { - get { - return (mAttribFlags & AttribFlags.NoContinue) != 0; - } + public bool NoContinue { set { if (value) { mAttribFlags |= AttribFlags.NoContinue; @@ -160,6 +158,21 @@ namespace SourceGen { } } } + public bool NoContinueScript { + set { + if (value) { + mAttribFlags |= AttribFlags.NoContinueScript; + } else { + mAttribFlags &= ~AttribFlags.NoContinueScript; + } + } + } + public bool DoesNotContinue { + get { + return (mAttribFlags & AttribFlags.NoContinue) != 0 || + (mAttribFlags & AttribFlags.NoContinueScript) != 0; + } + } public bool DoesNotBranch { get { return (BranchTaken == OpDef.BranchTaken.Never); diff --git a/SourceGen/CodeAnalysis.cs b/SourceGen/CodeAnalysis.cs index 112c523..7833892 100644 --- a/SourceGen/CodeAnalysis.cs +++ b/SourceGen/CodeAnalysis.cs @@ -690,11 +690,11 @@ namespace SourceGen { } } - if (!doContinue) { - mAnattribs[offset].DoesNotContinue = true; + mAnattribs[offset].NoContinue = !doContinue; + if (mAnattribs[offset].DoesNotContinue) { + // If we just decided not to continue, or an extension script set a flag + // on a previous visit, stop scanning forward. break; - } else { - mAnattribs[offset].DoesNotContinue = false; } // Sanity check to avoid infinite loop. @@ -710,14 +710,15 @@ namespace SourceGen { break; } - // On first visit, check for JSR/JSL inline call. + // On first visit, check for JSR/JSL inline call. If it's "no-continue", + // set a flag and halt here. if (firstVisit) { // Currently ignoring OpDef.OpJSR_AbsIndexXInd if (op == OpDef.OpJSR_Abs || op == OpDef.OpJSR_AbsLong) { bool noContinue = CheckForInlineCall(op, offset, false); if (noContinue) { LogD(offset, "Script declared inline call no-continue"); - mAnattribs[offset].DoesNotContinue = true; + mAnattribs[offset].NoContinueScript = true; break; } } diff --git a/SourceGen/SGTestData/2022-extension-scripts b/SourceGen/SGTestData/2022-extension-scripts index f110a86fb257bb4fe58ead65cee10ee3e3146c3b..0d9c53e7d99e6252c55ac57da2a3fc497df64043 100644 GIT binary patch literal 544 zcmb36{m4L}SHQr~$k@cx%v_;Qz}(E##MsEtK%rkCuQVq|wKmOE1aLH&kL!U=(4%B4eb)V8H0&>}AGPM4G<#R+JAt6CY(AXt3t-NDqM|oP>F^7Xrhh2`k9S54eMxbH6K*PZ| zZ;rlv{^CnUMuNh20fq($u}eVVynuwlet`rF5Wz4ChzVFhb)v0Th>9 L3<*G))Vve`F(^=~ delta 114 zcmZ3$a+Og=;`bv1g>C@@LnC7oQ!{gg9szSRQxjt&Lj#50i83M5TLl#O1r<0185sKn z7?@TGyi#ZtP+7^)`u|nJ#9kS_RRRrb1sV>%d2{sT^A}$fdIge-^0QO(6c~$3iZb)k S84{Ej6d1V}5`Z$Pc_{!4ASLGj diff --git a/SourceGen/SGTestData/2022-extension-scripts-a.cs b/SourceGen/SGTestData/2022-extension-scripts-a.cs index 5d77b30..d7d0c5c 100644 --- a/SourceGen/SGTestData/2022-extension-scripts-a.cs +++ b/SourceGen/SGTestData/2022-extension-scripts-a.cs @@ -19,6 +19,8 @@ namespace RuntimeData.Test2022 { private int mInlineL2StringAddr; // jsl private int mInlineDciStringAddr; // jsl + private int mNoContAddr; // jsr + public string Identifier { get { return "Test 2022-extension-scripts A"; @@ -58,6 +60,9 @@ namespace RuntimeData.Test2022 { case "PrintInlineDciString": mInlineDciStringAddr = sym.Value; break; + case "NoCont": + mNoContAddr = sym.Value; + break; } } } @@ -101,6 +106,8 @@ namespace RuntimeData.Test2022 { } mAppRef.SetInlineDataFormat(offset + 3, nullOff - (offset + 3) + 1, DataType.StringNullTerm, DataSubType.Ascii, null); + } else if (operand == mNoContAddr) { + noContinue = true; } } diff --git a/SourceGen/SGTestData/2022-extension-scripts.dis65 b/SourceGen/SGTestData/2022-extension-scripts.dis65 index b98af41..0c28820 100644 --- a/SourceGen/SGTestData/2022-extension-scripts.dis65 +++ b/SourceGen/SGTestData/2022-extension-scripts.dis65 @@ -1,39 +1,133 @@ ### 6502bench SourceGen dis65 v1.0 ### { -"_ContentVersion":2,"FileDataLength":213,"FileDataCrc32":-798098677,"ProjectProps":{ -"CpuName":"65816","IncludeUndocumentedInstr":false,"TwoByteBrk":true,"EntryFlags":32702671,"AutoLabelStyle":"Simple","AnalysisParams":{ -"AnalyzeUncategorizedData":true,"DefaultTextScanMode":"LowHighAscii","MinCharsForString":4,"SeekNearbyTargets":true,"SmartPlpHandling":true}, -"PlatformSymbolFileIdentifiers":["PROJ:2022-extension-scripts.sym65"],"ExtensionScriptFileIdentifiers":["PROJ:2022-extension-scripts-a.cs","PROJ:2022-extension-scripts-b.cs"],"ProjectSyms":{ +"_ContentVersion":3, +"FileDataLength":544, +"FileDataCrc32":118019308, +"ProjectProps":{ +"CpuName":"65816", +"IncludeUndocumentedInstr":false, +"TwoByteBrk":true, +"EntryFlags":32702671, +"AutoLabelStyle":"Simple", +"AnalysisParams":{ +"AnalyzeUncategorizedData":true, +"DefaultTextScanMode":"LowHighAscii", +"MinCharsForString":4, +"SeekNearbyTargets":true, +"SmartPlpHandling":true}, + +"PlatformSymbolFileIdentifiers":["PROJ:2022-extension-scripts.sym65"], +"ExtensionScriptFileIdentifiers":["PROJ:2022-extension-scripts-a.cs", +"PROJ:2022-extension-scripts-b.cs"], +"ProjectSyms":{ "PrintInlineDciString":{ "DataDescriptor":{ -"Length":1,"Format":"NumericLE","SubFormat":"Hex","SymbolRef":null}, -"Comment":"","HasWidth":false,"Direction":"ReadWrite","MultiMask":null,"Label":"PrintInlineDciString","Value":77824,"Source":"Project","Type":"ExternalAddr"}}}, +"Length":1, +"Format":"NumericLE", +"SubFormat":"Hex", +"SymbolRef":null}, + +"Comment":"", +"HasWidth":false, +"Direction":"ReadWrite", +"MultiMask":null, +"Label":"PrintInlineDciString", +"Value":77824, +"Source":"Project", +"Type":"ExternalAddr", +"LabelAnno":"None"}}}, + "AddressMap":[{ -"Offset":0,"Addr":4096}, +"Offset":0, +"Addr":4096}, + { -"Offset":192,"Addr":4352}],"TypeHints":[{ -"Low":0,"High":0,"Hint":"Code"}],"StatusFlagOverrides":{ +"Offset":512, +"Addr":6144}, + +{ +"Offset":523, +"Addr":6208}], +"TypeHints":[{ +"Low":0, +"High":0, +"Hint":"Code"}], +"StatusFlagOverrides":{ }, + "Comments":{ -"181":"split across address change"}, +"183":"split across address change"}, + "LongComments":{ }, + "Notes":{ }, + "UserLabels":{ -"139":{ -"Label":"PrintInline8String","Value":4225,"Source":"User","Type":"LocalOrGlobalAddr"}, -"140":{ -"Label":"PrintInlineRev8String","Value":4226,"Source":"User","Type":"LocalOrGlobalAddr"}, "141":{ -"Label":"PrintInlineNullString","Value":4227,"Source":"User","Type":"LocalOrGlobalAddr"}, -"170":{ -"Label":"data02","Value":4256,"Source":"User","Type":"LocalOrGlobalAddr"}, -"173":{ -"Label":"data03","Value":4259,"Source":"User","Type":"LocalOrGlobalAddr"}, +"Label":"PrintInline8String", +"Value":4237, +"Source":"User", +"Type":"LocalOrGlobalAddr", +"LabelAnno":"None"}, + "142":{ -"Label":"data01","Value":4228,"Source":"User","Type":"LocalOrGlobalAddr"}}, +"Label":"PrintInlineRev8String", +"Value":4238, +"Source":"User", +"Type":"LocalOrGlobalAddr", +"LabelAnno":"None"}, + +"143":{ +"Label":"PrintInlineNullString", +"Value":4239, +"Source":"User", +"Type":"LocalOrGlobalAddr", +"LabelAnno":"None"}, + +"172":{ +"Label":"data02", +"Value":4268, +"Source":"User", +"Type":"LocalOrGlobalAddr", +"LabelAnno":"None"}, + +"175":{ +"Label":"data03", +"Value":4271, +"Source":"User", +"Type":"LocalOrGlobalAddr", +"LabelAnno":"None"}, + +"144":{ +"Label":"data01", +"Value":4240, +"Source":"User", +"Type":"LocalOrGlobalAddr", +"LabelAnno":"None"}, + +"196":{ +"Label":"Next1", +"Value":4292, +"Source":"User", +"Type":"GlobalAddr", +"LabelAnno":"None"}, + +"183":{ +"Label":"NoCont", +"Value":4279, +"Source":"User", +"Type":"GlobalAddr", +"LabelAnno":"None"}}, + "OperandFormats":{ }, + "LvTables":{ +}, + +"Visualizations":[], +"VisualizationAnimations":[], +"VisualizationSets":{ }} diff --git a/SourceGen/SGTestData/Expected/2022-extension-scripts_64tass.S b/SourceGen/SGTestData/Expected/2022-extension-scripts_64tass.S index 42f39bc..64086db 100644 --- a/SourceGen/SGTestData/Expected/2022-extension-scripts_64tass.S +++ b/SourceGen/SGTestData/Expected/2022-extension-scripts_64tass.S @@ -25,9 +25,9 @@ PrintInlineDciString = $013000 .text $14,$00,"string with length/2" jsl PrintInlineDciString .shift "DCI string" - jsr L10B5 - jsr L110F - jsr L1108 + jsr L1800 + jsr L184F + jsr L1848 brk #$01 .word data01 brk #$02 @@ -39,7 +39,7 @@ L1085 .byte $a9 .byte $00 sta $ff .byte $ea - rts + jmp Next1 PrintInline8String rts @@ -69,25 +69,53 @@ data02 .word _data03 .enc sg_hiascii _data03 .text "AllEight" -L10B5 jsr PrintInlineNullString ;split across address change - per $802d +NoCont pla ;split across address change + pla + rts + +L10BA jsr NoCont + + .byte $00 + .byte $80 + +L10BF jsr NoCont + + .byte $00 + .byte $80 + +Next1 jsr L10BA + jsr _L10CF + clc + jsr L10BF + rts + +_L10CF sec + jsr L10BF + rts + + .fill 300,$00 + + .logical $1800 +L1800 jsr PrintInlineNullString + per $8778 rtl .byte $65 .byte $6e .byte $20 .byte $01 - .logical $1100 + .here + .logical $1840 .enc sg_ascii .text "string" .byte $00 .byte $60 -L1108 jsl PrintInlineL2String +L1848 jsl PrintInlineL2String asl a brk #$60 -L110F jsr PrintInlineNullString +L184F jsr PrintInlineNullString adc $6e .byte $64 .here diff --git a/SourceGen/SGTestData/Expected/2022-extension-scripts_Merlin32.S b/SourceGen/SGTestData/Expected/2022-extension-scripts_Merlin32.S index 4059ba9..18d7651 100644 --- a/SourceGen/SGTestData/Expected/2022-extension-scripts_Merlin32.S +++ b/SourceGen/SGTestData/Expected/2022-extension-scripts_Merlin32.S @@ -18,9 +18,9 @@ PrintInlineDciString equ $013000 strl 'string with length/2' jsl PrintInlineDciString dci 'DCI string' - jsr L10B5 - jsr L110F - jsr L1108 + jsr L1800 + jsr L184F + jsr L1848 brk $01 dw data01 brk $02 @@ -32,7 +32,7 @@ L1085 dfb $a9 dfb $00 sta $ff dfb $ea - rts + jmp Next1 PrintInline8String rts @@ -61,23 +61,50 @@ data02 dw :data03 dfb $80 :data03 asc "AllEight" -L10B5 jsr PrintInlineNullString ;split across address change - per $802d +NoCont pla ;split across address change + pla + rts + +L10BA jsr NoCont + + dfb $00 + dfb $80 + +L10BF jsr NoCont + + dfb $00 + dfb $80 + +Next1 jsr L10BA + jsr :L10CF + clc + jsr L10BF + rts + +:L10CF sec + jsr L10BF + rts + + ds 300 + + org $1800 +L1800 jsr PrintInlineNullString + per $8778 rtl dfb $65 dfb $6e dfb $20 dfb $01 - org $1100 + org $1840 asc 'string' dfb $00 dfb $60 -L1108 jsl PrintInlineL2String +L1848 jsl PrintInlineL2String asl A brk $60 -L110F jsr PrintInlineNullString +L184F jsr PrintInlineNullString adc $6e dfb $64 diff --git a/SourceGen/SGTestData/Expected/2022-extension-scripts_acme.S b/SourceGen/SGTestData/Expected/2022-extension-scripts_acme.S index f105980..a34f758 100644 --- a/SourceGen/SGTestData/Expected/2022-extension-scripts_acme.S +++ b/SourceGen/SGTestData/Expected/2022-extension-scripts_acme.S @@ -21,9 +21,9 @@ PrintInlineDciString = $013000 !text $14,$00,"string with length/2" jsl PrintInlineDciString !text "DCI strin",$e7 - jsr L10B5 - jsr L110F - jsr L1108 + jsr L1800 + jsr L184F + jsr L1848 !byte $00,$01 !word data01 !byte $00,$02 @@ -35,7 +35,7 @@ L1085 !byte $a9 !byte $00 sta $ff !byte $ea - rts + jmp Next1 PrintInline8String rts @@ -66,24 +66,52 @@ data02 !word @data03 @data03 !text "AllEight" } -L10B5 jsr PrintInlineNullString ;split across address change - per $802d +NoCont pla ;split across address change + pla + rts + +L10BA jsr NoCont + + !byte $00 + !byte $80 + +L10BF jsr NoCont + + !byte $00 + !byte $80 + +Next1 jsr L10BA + jsr @L10CF + clc + jsr L10BF + rts + +@L10CF sec + jsr L10BF + rts + + !fill 300,$00 + + !pseudopc $1800 { +L1800 jsr PrintInlineNullString + per $8778 rtl !byte $65 !byte $6e !byte $20 !byte $01 - !pseudopc $1100 { + } ;!pseudopc + !pseudopc $1840 { !text "string" !byte $00 !byte $60 -L1108 jsl PrintInlineL2String +L1848 jsl PrintInlineL2String asl !byte $00,$60 -L110F jsr PrintInlineNullString +L184F jsr PrintInlineNullString adc $6e !byte $64 } ;!pseudopc diff --git a/SourceGen/SGTestData/Expected/2022-extension-scripts_cc65.S b/SourceGen/SGTestData/Expected/2022-extension-scripts_cc65.S index 3368a00..6f05ba4 100644 --- a/SourceGen/SGTestData/Expected/2022-extension-scripts_cc65.S +++ b/SourceGen/SGTestData/Expected/2022-extension-scripts_cc65.S @@ -22,9 +22,9 @@ PrintInlineDciString = $013000 .byte $14,$00,"string with length/2" jsl PrintInlineDciString .byte "DCI strin",$e7 - jsr L10B5 - jsr L110F - jsr L1108 + jsr L1800 + jsr L184F + jsr L1848 brk $01 .word data01 brk $02 @@ -36,7 +36,7 @@ L1085: .byte $a9 .byte $00 sta $ff .byte $ea - rts + jmp Next1 PrintInline8String: rts @@ -70,24 +70,52 @@ data02: .word @data03 .endmacro @data03: HiAscii "AllEight" -L10B5: jsr PrintInlineNullString ;split across address change - per $802d +NoCont: pla ;split across address change + pla + rts + +L10BA: jsr NoCont + + .byte $00 + .byte $80 + +L10BF: jsr NoCont + + .byte $00 + .byte $80 + +Next1: jsr L10BA + jsr @L10CF + clc + jsr L10BF + rts + +@L10CF: sec + jsr L10BF + rts + + .res 300,$00 + +; .segment "SEG001" + .org $1800 +L1800: jsr PrintInlineNullString + per $8778 rtl .byte $65 .byte $6e .byte $20 .byte $01 -; .segment "SEG001" - .org $1100 +; .segment "SEG002" + .org $1840 .byte "string" .byte $00 .byte $60 -L1108: jsl PrintInlineL2String +L1848: jsl PrintInlineL2String asl A brk $60 -L110F: jsr PrintInlineNullString +L184F: jsr PrintInlineNullString adc $6e .byte $64 diff --git a/SourceGen/SGTestData/Expected/2022-extension-scripts_cc65.cfg b/SourceGen/SGTestData/Expected/2022-extension-scripts_cc65.cfg index 242b98a..58efdda 100644 --- a/SourceGen/SGTestData/Expected/2022-extension-scripts_cc65.cfg +++ b/SourceGen/SGTestData/Expected/2022-extension-scripts_cc65.cfg @@ -1,13 +1,15 @@ # 6502bench SourceGen generated linker script for 2022-extension-scripts MEMORY { MAIN: file=%O, start=%S, size=65536; -# MEM000: file=%O, start=$1000, size=192; -# MEM001: file=%O, start=$1100, size=21; +# MEM000: file=%O, start=$1000, size=512; +# MEM001: file=%O, start=$1800, size=11; +# MEM002: file=%O, start=$1840, size=21; } SEGMENTS { CODE: load=MAIN, type=rw; # SEG000: load=MEM000, type=rw; # SEG001: load=MEM001, type=rw; +# SEG002: load=MEM002, type=rw; } FEATURES {} SYMBOLS {} diff --git a/SourceGen/SGTestData/Source/2022-extension-scripts.S b/SourceGen/SGTestData/Source/2022-extension-scripts.S index 34ca299..6d35b69 100644 --- a/SourceGen/SGTestData/Source/2022-extension-scripts.S +++ b/SourceGen/SGTestData/Source/2022-extension-scripts.S @@ -66,7 +66,7 @@ edge1 dfb $a9 ;2: LDA imm dfb $ff ;1: address $eaff nop ;2: - rts + jmp Next1 PrintInline8String rts ;EDIT: set label PrintInlineRev8String rts ;EDIT: set label @@ -93,10 +93,50 @@ data03 ;EDIT: set label asc "AllEight" +; +; The analyzer is trying to avoid calling the inline JSR check on code it +; has previously visited. There was a bug that caused us to lose the +; no-continue value if code was revisited. +; +; To ensure we revisit the function, we need to call it, then call +; something else that calls it, changing up the flags. +; + +NoCont pla + pla + rts + +TestNC1 jsr NoCont + dfb $00,$80 +TestNC2 jsr NoCont + dfb $00,$80 + +Next1 jsr TestNC1 + + jsr PushThing + clc + jsr TestNC2 + rts + +PushThing + sec + jsr TestNC2 + rts + + +; Failed call tests. Some of these need to be at the end of the file. +; We use an align directive to ensure their offsets don't change as we +; update stuff earlier in the file. + ds \ + ds 256 + + + org $1800 + ; check address split across string broken jsr PrintInlineNullString asc 'broken ',01 - org $1100 ;EDIT: split address + org $1840 ;EDIT: split address asc 'string',00 rts