From 68c324bbe8de08f34a3b61d4ea679488cc851f45 Mon Sep 17 00:00:00 2001 From: Andy McFadden Date: Sat, 16 Nov 2019 16:34:42 -0800 Subject: [PATCH] Label rework, part 4 Update the symbol lookup in EditInstructionOperand, EditDataOperand, and GotoBox to correctly deal with non-unique labels. This is a little awkward because we're doing lookups by name on a non-unique symbol, and must resolve the ambiguity. In the case of an instruction operand that refers to an address this is pretty straightforward. For partial bytes (LDA #>:foo) or data directives (.DD1 :foo) we have to take a guess. We can probably make a more informed guess than we currently are, e.g. the LDA case could find the label that minimizes the adjustment, but I don't want to sink a lot of time into this until I'm sure it'll be useful. Data operands with multiple regions are something of a challenge, but I'm not sure specifying a single symbol for multiple locations is important. The "goto" box just finds the match that's closest to the selection. Unlike "find", it always grabs the closest, not the next one forward. (Not sure if this is useful or confusing.) --- SourceGen/DisasmProject.cs | 37 +++++++++ SourceGen/MainController.cs | 8 +- SourceGen/ProjectFile.cs | 2 +- SourceGen/PseudoOp.cs | 6 +- .../Source/2023-non-unique-labels.S | 24 +++++- SourceGen/Symbol.cs | 15 ++-- SourceGen/WpfGui/EditDataOperand.xaml.cs | 49 +++++++++-- .../WpfGui/EditInstructionOperand.xaml.cs | 82 +++++++++++++++---- SourceGen/WpfGui/GotoBox.xaml.cs | 30 ++++++- 9 files changed, 212 insertions(+), 41 deletions(-) diff --git a/SourceGen/DisasmProject.cs b/SourceGen/DisasmProject.cs index 7c8fbdf..320af6b 100644 --- a/SourceGen/DisasmProject.cs +++ b/SourceGen/DisasmProject.cs @@ -2353,6 +2353,43 @@ namespace SourceGen { return -1; } + /// + /// Finds a user label by name, searching only non-unique local address labels. + /// + /// Label to search for. Must not have the uniquifier tag (if + /// it had one, you wouldn't be here). + /// If multiple labels are found, we want the one that is + /// closest to this offset. + /// The symbol found, or null if no match. + public Symbol FindBestNonUniqueLabel(string label, int targetOffset) { + Symbol bestSym = null; + int bestDelta = int.MaxValue; + + // Simple linear search. Right now we're only doing this in a few specific + // UI-driven situations (edit operand, goto label), so performance isn't crucial. + foreach (KeyValuePair kvp in UserLabels) { + Symbol sym = kvp.Value; + if (sym.SymbolType != Symbol.Type.NonUniqueLocalAddr) { + continue; + } + if (sym.LabelWithoutTag == label) { + // found a match; is it the best one? + int delta = Math.Abs(kvp.Key - targetOffset); + if (delta < bestDelta) { + //Debug.WriteLine("FindBest: " + sym.Label + "/" + delta + " better than " + + // (bestSym != null ? bestSym.Label : "-") + "/" + bestDelta); + bestSym = sym; + bestDelta = delta; + } else { + //Debug.WriteLine("FindBest: " + sym.Label + "/" + delta + " not better than " + + // (bestSym != null ? bestSym.Label : "-") + "/" + bestDelta); + } + } + } + + return bestSym; + } + /// /// For debugging purposes, get some information about the currently loaded /// extension scripts. diff --git a/SourceGen/MainController.cs b/SourceGen/MainController.cs index e2ae299..e5285c2 100644 --- a/SourceGen/MainController.cs +++ b/SourceGen/MainController.cs @@ -2402,7 +2402,13 @@ namespace SourceGen { } public void Goto() { - GotoBox dlg = new GotoBox(mMainWin, mProject, mOutputFormatter); + int index = mMainWin.CodeListView_GetFirstSelectedIndex(); + if (index < 0) { + index = mMainWin.CodeListView_GetTopIndex(); // nothing selected + } + int offset = CodeLineList[index].FileOffset; + + GotoBox dlg = new GotoBox(mMainWin, mProject, offset, mOutputFormatter); if (dlg.ShowDialog() == true) { GoToLocation(new NavStack.Location(dlg.TargetOffset, 0, false), GoToMode.JumpToCodeData, true); diff --git a/SourceGen/ProjectFile.cs b/SourceGen/ProjectFile.cs index 2d339ce..5a81b47 100644 --- a/SourceGen/ProjectFile.cs +++ b/SourceGen/ProjectFile.cs @@ -279,7 +279,7 @@ namespace SourceGen { public SerSymbol() { } public SerSymbol(Symbol sym) { - Label = sym.LabelForSerialization; // use bare label here + Label = sym.LabelWithoutTag; // use bare label here Value = sym.Value; Source = sym.SymbolSource.ToString(); Type = sym.SymbolType.ToString(); diff --git a/SourceGen/PseudoOp.cs b/SourceGen/PseudoOp.cs index 21c486e..5067268 100644 --- a/SourceGen/PseudoOp.cs +++ b/SourceGen/PseudoOp.cs @@ -720,7 +720,7 @@ namespace SourceGen { // if it's from the assembler. if ((flags & FormatNumericOpFlags.OmitLabelPrefixSuffix) == 0) { symLabel = Symbol.ConvertLabelForDisplay(symLabel, sym.LabelAnno, - sym.IsNonUnique, formatter); + true, formatter); } else { // TODO(xyzzy): remapper will handle this symLabel = Symbol.ConvertLabelForDisplay(symLabel, Symbol.LabelAnnotation.None, @@ -863,7 +863,7 @@ namespace SourceGen { } if ((flags & FormatNumericOpFlags.OmitLabelPrefixSuffix) == 0) { symLabel = Symbol.ConvertLabelForDisplay(symLabel, sym.LabelAnno, - sym.IsNonUnique, formatter); + true, formatter); } else { // TODO(xyzzy): remapper will handle this symLabel = Symbol.ConvertLabelForDisplay(symLabel, Symbol.LabelAnnotation.None, @@ -964,7 +964,7 @@ namespace SourceGen { } if ((flags & FormatNumericOpFlags.OmitLabelPrefixSuffix) == 0) { symLabel = Symbol.ConvertLabelForDisplay(symLabel, sym.LabelAnno, - sym.IsNonUnique, formatter); + true, formatter); } else { // TODO(xyzzy): remapper will handle this symLabel = Symbol.ConvertLabelForDisplay(symLabel, Symbol.LabelAnnotation.None, diff --git a/SourceGen/SGTestData/Source/2023-non-unique-labels.S b/SourceGen/SGTestData/Source/2023-non-unique-labels.S index 8525acc..d47fcca 100644 --- a/SourceGen/SGTestData/Source/2023-non-unique-labels.S +++ b/SourceGen/SGTestData/Source/2023-non-unique-labels.S @@ -30,7 +30,7 @@ global1 nop ;EDIT bne :loop2 dex bne :loop1 - jmp btarg + jmp btarg global2 nop ;EDIT @@ -53,16 +53,34 @@ global4 nop ;EDIT gloop dex ;EDIT: local, name "loop" global5 nop bne gloop - + nop ; Test symbolic references. - global6 nop :spin1 jsr :spin2 ;EDIT: local, name "spin1", operand ref to ":spin2" :spin2 jsr :spin1 ;EDIT: local, name "spin2", operand ref to ":spin1" nop :spin3 lda :spin3 ;EDIT: local, name "spin1", operand ref to ":spin1" + beq :spin3 ;EDIT: operand ref to ":spin1" + + lda #<:spin1 + ldx #<:spin2 + lda #>:spin1 + ldx #>:spin2 + bne :skip + + dw :spin1 ;EDIT: local, name "spin1" + dw :spin2 ;EDIT: local, name "spin1" (will be offset) + dw :spin3 ;EDIT: local, name "spin1" + + dfb <:spin1 ;EDIT: local, name "spin1" (may need to do as + dfb <:spin2 ;EDIT: local, name "spin1" unique names and then + dfb >:spin1 ;EDIT: local, name "spin1" rename afterward) + dfb >:spin2 ;EDIT: local, name "spin1" + +:skip nop ;EDIT: local + ; Semi-related: test labels that are nothing but underscores. global_ nop diff --git a/SourceGen/Symbol.cs b/SourceGen/Symbol.cs index b36a786..ea057c6 100644 --- a/SourceGen/Symbol.cs +++ b/SourceGen/Symbol.cs @@ -119,9 +119,9 @@ namespace SourceGen { /// - /// Label without the non-unique tag. + /// Label without the non-unique tag. Used for serialization. /// - public string LabelForSerialization { + public string LabelWithoutTag { get { if (SymbolType != Type.NonUniqueLocalAddr) { return Label; @@ -217,7 +217,7 @@ namespace SourceGen { Debug.Assert(uniqueTag >= 0 && uniqueTag < 0x01000000); // fit in 6 hex digits Debug.Assert(label.IndexOf(UNIQUE_TAG_CHAR) < 0); // already extended? - Value = value; // passed a bogus value earlier for assert + Value = value; // passed a bogus value to base ctor for assert // Add tag to label to make it unique. Label = label + UNIQUE_TAG_CHAR + uniqueTag.ToString("x6"); @@ -230,7 +230,7 @@ namespace SourceGen { /// Formatter object. /// Label suitable for display. public string GenerateDisplayLabel(Asm65.Formatter formatter) { - return ConvertLabelForDisplay(Label, LabelAnno, IsNonUnique, formatter); + return ConvertLabelForDisplay(Label, LabelAnno, true, formatter); } /// @@ -273,14 +273,13 @@ namespace SourceGen { bool showNonUnique, Asm65.Formatter formatter) { StringBuilder sb = new StringBuilder(label.Length + 2); - if (showNonUnique) { - sb.Append(formatter.NonUniqueLabelPrefix); - } - if (label.Length > NON_UNIQUE_LEN && label[label.Length - NON_UNIQUE_LEN] == UNIQUE_TAG_CHAR) { // showNonUnique may be false if generating assembly code (but by this // point the unique tag should be remapped away) + if (showNonUnique) { + sb.Append(formatter.NonUniqueLabelPrefix); + } sb.Append(label.Substring(0, label.Length - NON_UNIQUE_LEN)); } else { sb.Append(label); diff --git a/SourceGen/WpfGui/EditDataOperand.xaml.cs b/SourceGen/WpfGui/EditDataOperand.xaml.cs index 1487f8f..30ab354 100644 --- a/SourceGen/WpfGui/EditDataOperand.xaml.cs +++ b/SourceGen/WpfGui/EditDataOperand.xaml.cs @@ -64,6 +64,11 @@ namespace SourceGen.WpfGui { /// public FormatDescriptor mFirstFormatDescriptor; + /// + /// Project reference. + /// + private DisasmProject mProject; + /// /// Raw file data. /// @@ -129,6 +134,7 @@ namespace SourceGen.WpfGui { Owner = owner; DataContext = this; + mProject = project; mFileData = project.FileData; mSymbolTable = project.SymbolTable; mAddrMap = project.AddrMap; @@ -345,11 +351,14 @@ namespace SourceGen.WpfGui { bool isOk = true; if (radioSimpleDataSymbolic.IsChecked == true) { // Just check for correct format. References to non-existent labels are allowed. - isOk = Asm65.Label.ValidateLabel(symbolEntryTextBox.Text); + Symbol.TrimAndValidateLabel(symbolEntryTextBox.Text, + mFormatter.NonUniqueLabelPrefix, out isOk, out bool unused1, + out bool unused2, out bool unused3, out Symbol.LabelAnnotation unused4); - // Actually, let's discourage references to auto-labels. + // Actually, let's discourage references to auto-labels and variables. if (isOk && mSymbolTable.TryGetValue(symbolEntryTextBox.Text, out Symbol sym)) { - isOk = sym.SymbolSource != Symbol.Source.Auto; + isOk = sym.SymbolSource != Symbol.Source.Auto && + sym.SymbolSource != Symbol.Source.Variable; } } IsValid = isOk; @@ -819,7 +828,9 @@ namespace SourceGen.WpfGui { break; } Debug.Assert(dfd.HasSymbol); - symbolEntryTextBox.Text = dfd.SymbolRef.Label; + symbolEntryTextBox.Text = Symbol.ConvertLabelForDisplay( + dfd.SymbolRef.Label, Symbol.LabelAnnotation.None, + true, mFormatter); break; default: Debug.Assert(false); @@ -965,7 +976,35 @@ namespace SourceGen.WpfGui { part = WeakSymbolRef.Part.Low; } subType = FormatDescriptor.SubType.Symbol; - symbolRef = new WeakSymbolRef(symbolEntryTextBox.Text, part); + + string weakLabel = symbolEntryTextBox.Text; + // Deal with non-unique labels. If the label refers to an existing + // symbol, use its label, which will have the tag. If the label doesn't + // have a match, discard it -- we don't support weak refs to ambiguous + // non-unique symbols. + string trimLabel = Symbol.TrimAndValidateLabel(weakLabel, + mFormatter.NonUniqueLabelPrefix, out bool isValid, out bool unused1, + out bool unused2, out bool hasNonUniquePrefix, + out Symbol.LabelAnnotation unused3); + if (isValid && hasNonUniquePrefix) { + // We want to find the match that's closest to the thing we're + // referencing, but that's awkward when there's multiple ranges and + // multiple ways of interpreting the data. So as a simple measure + // we just grab the lowest offset in the first range. + IEnumerator oiter = mSelection.RangeListIterator; + oiter.MoveNext(); + TypedRangeSet.TypedRange rng = oiter.Current; + int matchOffset = rng.Low; + + Symbol osym = mProject.FindBestNonUniqueLabel(trimLabel, matchOffset); + if (osym != null) { + weakLabel = osym.Label; + } else { + Debug.WriteLine("Attempt to create ref to nonexistant non-unique sym"); + subType = FormatDescriptor.SubType.Hex; + } + } + symbolRef = new WeakSymbolRef(weakLabel, part); } else { Debug.Assert(false); } diff --git a/SourceGen/WpfGui/EditInstructionOperand.xaml.cs b/SourceGen/WpfGui/EditInstructionOperand.xaml.cs index 6e9a914..f950e5b 100644 --- a/SourceGen/WpfGui/EditInstructionOperand.xaml.cs +++ b/SourceGen/WpfGui/EditInstructionOperand.xaml.cs @@ -225,13 +225,30 @@ namespace SourceGen.WpfGui { /// Looks up the symbol in the symbol table. If not found there, it checks for a /// match against the existing or edited project symbol. /// - private bool LookupSymbol(string label, out Symbol sym) { - if (mProject.SymbolTable.TryGetValue(label, out sym)) { - return true; - } - if (mEditedProjectSymbol != null && label.Equals(mEditedProjectSymbol.Label)) { - sym = mEditedProjectSymbol; - return true; + private bool LookupSymbol(string label, bool isNonUnique, out Symbol sym) { + if (isNonUnique) { + // Only applies to labels, so no need to check mEditedProjectSymbol. We + // could check mEditedLabel, but there's no reason to add a symbolic reference + // on top of the numeric reference. + int targetOffset; + Anattrib attr = mProject.GetAnattrib(mOffset); + if (attr.OperandOffset >= 0) { + targetOffset = attr.OperandOffset; + } else { + targetOffset = mOffset; + } + sym = mProject.FindBestNonUniqueLabel(label, targetOffset); + if (sym != null) { + return true; + } + } else { + if (mProject.SymbolTable.TryGetValue(label, out sym)) { + return true; + } + if (mEditedProjectSymbol != null && label.Equals(mEditedProjectSymbol.Label)) { + sym = mEditedProjectSymbol; + return true; + } } return false; } @@ -253,10 +270,15 @@ namespace SourceGen.WpfGui { if (FormatSymbol) { IsPartPanelEnabled = mOpDef.IsExtendedImmediate; - if (!Asm65.Label.ValidateLabel(SymbolLabel)) { + string trimLabel = Symbol.TrimAndValidateLabel(SymbolLabel, + mFormatter.NonUniqueLabelPrefix, out bool isValid, out bool unused1, + out bool unused2, out bool hasNonUniquePrefix, + out Symbol.LabelAnnotation unused3); + + if (!isValid) { SymbolValueHex = SYMBOL_INVALID; IsValid = false; - } else if (LookupSymbol(SymbolLabel, out Symbol sym)) { + } else if (LookupSymbol(trimLabel, hasNonUniquePrefix, out Symbol sym)) { if (sym.SymbolSource == Symbol.Source.Auto) { // We try to block references to auto labels, but it's possible to get // around it because FormatDescriptors are weak references (replace auto @@ -278,7 +300,8 @@ namespace SourceGen.WpfGui { SymbolValueHex = mFormatter.FormatHexValue(sym.Value, 4); SymbolValueDecimal = mFormatter.FormatDecimalValue(sym.Value); } else { - // Valid but unknown symbol. This is fine -- symbols don't have to exist. + // Valid but unknown symbol. This is fine -- symbols don't have to exist -- + // but it's a little weird for non-unique symbols. SymbolValueHex = SYMBOL_UNKNOWN; } } else { @@ -365,7 +388,11 @@ namespace SourceGen.WpfGui { sb.Append(mFormatter.FormatCharacterValue(operandValue, enc)); break; case FormatDescriptor.SubType.Symbol: - if (LookupSymbol(dfd.SymbolRef.Label, out Symbol sym)) { + string trimLabel = Symbol.TrimAndValidateLabel(SymbolLabel, + mFormatter.NonUniqueLabelPrefix, out bool isValid, out bool unused1, + out bool unused2, out bool hasNonUniquePrefix, + out Symbol.LabelAnnotation unused3); + if (LookupSymbol(trimLabel, hasNonUniquePrefix, out Symbol sym)) { // Block move is a little weird. "MVN label1,label2" is supposed to use // the bank byte, while "MVN #const1,#const2" uses the entire symbol. // The easiest thing to do is require the user to specify the "bank" @@ -500,8 +527,10 @@ namespace SourceGen.WpfGui { // Set the radio button when the user starts typing. if (mLoadDone) { FormatSymbol = true; + // this calls UpdateControls; don't do it twice + } else { + UpdateControls(); } - UpdateControls(); } } private string mSymbolLabel; @@ -634,7 +663,8 @@ namespace SourceGen.WpfGui { Debug.Assert(false); break; } - SymbolLabel = dfd.SymbolRef.Label; + SymbolLabel = Symbol.ConvertLabelForDisplay(dfd.SymbolRef.Label, + Symbol.LabelAnnotation.None, true, mFormatter); break; case FormatDescriptor.SubType.None: default: @@ -685,8 +715,28 @@ namespace SourceGen.WpfGui { Debug.Assert(false); part = WeakSymbolRef.Part.Low; } + + string weakLabel = SymbolLabel; + + // Deal with non-unique labels. If the label refers to an existing + // symbol, use its label, which will have the tag. If the label doesn't + // have a match, discard it -- we don't support weak refs to ambiguous + // non-unique symbols. + string trimLabel = Symbol.TrimAndValidateLabel(SymbolLabel, + mFormatter.NonUniqueLabelPrefix, out bool isValid, out bool unused1, + out bool unused2, out bool hasNonUniquePrefix, + out Symbol.LabelAnnotation unused3); + if (isValid && hasNonUniquePrefix) { + if (LookupSymbol(trimLabel, hasNonUniquePrefix, out Symbol sym)) { + weakLabel = sym.Label; + } else { + Debug.WriteLine("Attempt to create ref to non-existant non-unique sym"); + return null; + } + } + return FormatDescriptor.Create(instructionLength, - new WeakSymbolRef(SymbolLabel, part), false); + new WeakSymbolRef(weakLabel, part), false); } FormatDescriptor.SubType subType; @@ -863,7 +913,7 @@ namespace SourceGen.WpfGui { } else { NarLabelOffsetText = CURRENT_LABEL; } - NarTargetLabel = sym.Label; + NarTargetLabel = sym.GenerateDisplayLabel(mFormatter); mEditedLabel = sym; CreateEditLabelText = EDIT_LABEL; } else { @@ -943,7 +993,7 @@ namespace SourceGen.WpfGui { } else { ShowNarCurrentLabel = true; CreateEditLabelText = EDIT_LABEL; - NarTargetLabel = mEditedLabel.Label; + NarTargetLabel = mEditedLabel.GenerateDisplayLabel(mFormatter); } // Sort of nice to just hit return twice after entering a label, so move the focus diff --git a/SourceGen/WpfGui/GotoBox.xaml.cs b/SourceGen/WpfGui/GotoBox.xaml.cs index 8647180..3e95c53 100644 --- a/SourceGen/WpfGui/GotoBox.xaml.cs +++ b/SourceGen/WpfGui/GotoBox.xaml.cs @@ -37,6 +37,11 @@ namespace SourceGen.WpfGui { /// private DisasmProject mProject; + /// + /// Initial offset, used for finding the best match among non-unique labels. + /// + private int mInitialOffset; + /// /// Reference to formatter. This determines how values are displayed. /// @@ -88,12 +93,13 @@ namespace SourceGen.WpfGui { } - public GotoBox(Window owner, DisasmProject proj, Formatter formatter) { + public GotoBox(Window owner, DisasmProject proj, int initialOffset, Formatter formatter) { InitializeComponent(); Owner = owner; DataContext = this; mProject = proj; + mInitialOffset = initialOffset; mFormatter = formatter; TargetOffset = -1; } @@ -138,7 +144,23 @@ namespace SourceGen.WpfGui { // Try it as a label. If they give the label a hex name (e.g. "A001") they // can prefix it with '$' to disambiguate the address. - int labelOffset = mProject.FindLabelOffsetByName(input); + int labelOffset = -1; + string trimLabel = Symbol.TrimAndValidateLabel(input, + mFormatter.NonUniqueLabelPrefix, out bool isValid, out bool unused1, + out bool unused2, out bool hasNonUniquePrefix, + out Symbol.LabelAnnotation unused3); + if (isValid) { + // Could be a label. See if there's a match. + if (hasNonUniquePrefix) { + Symbol sym = mProject.FindBestNonUniqueLabel(trimLabel, mInitialOffset); + if (sym != null) { + labelOffset = mProject.FindLabelOffsetByName(sym.Label); + } + } else { + labelOffset = mProject.FindLabelOffsetByName(trimLabel); + } + } + if (labelOffset >= 0) { TargetOffset = labelOffset; } else if (Address.ParseAddress(input, 1 << 24, out int addr)) { @@ -158,10 +180,10 @@ namespace SourceGen.WpfGui { if (TargetOffset >= 0) { offsetStr = mFormatter.FormatOffset24(TargetOffset); int addr = mProject.GetAnattrib(TargetOffset).Address; - addressStr = mFormatter.FormatAddress(addr, addr > 0xffff); + addressStr = "$" + mFormatter.FormatAddress(addr, addr > 0xffff); Symbol sym = mProject.GetAnattrib(TargetOffset).Symbol; if (sym != null) { - labelStr = sym.Label; + labelStr = sym.GenerateDisplayLabel(mFormatter); } }