From 091955b9c228df3eabb821d306a8cdf36ae29f37 Mon Sep 17 00:00:00 2001 From: Andy McFadden Date: Wed, 25 Dec 2019 17:15:37 -0800 Subject: [PATCH] Allow setting the start/end address for a block If you have a single line selected, Set Address adds a .ORG directive that changes the addresses of all following data, until the next .ORG directive is reached. Sometimes code will relocate part of itself, and it's useful to be able to set the address at the end of the block to what it would have been before the .ORG change. If you have multiple lines selected, we now add the second .ORG to the offset that follows the last selected line. Also, fixed a bug in the Symbol value updater that wasn't handling non-unique labels correctly. --- CommonUtil/AddressMap.cs | 26 ++++- SourceGen/DataAnalysis.cs | 4 + SourceGen/DisasmProject.cs | 3 +- SourceGen/MainController.cs | 139 ++++++++++++++++++------ SourceGen/RuntimeData/Help/editors.html | 13 ++- SourceGen/RuntimeData/Help/mainwin.html | 7 +- SourceGen/Symbol.cs | 15 +++ SourceGen/WpfGui/EditAddress.xaml | 44 ++++++-- SourceGen/WpfGui/EditAddress.xaml.cs | 122 +++++++++++++++------ 9 files changed, 283 insertions(+), 90 deletions(-) diff --git a/CommonUtil/AddressMap.cs b/CommonUtil/AddressMap.cs index ca39344..fb96db9 100644 --- a/CommonUtil/AddressMap.cs +++ b/CommonUtil/AddressMap.cs @@ -33,6 +33,8 @@ namespace CommonUtil { /// script extensions. /// public class AddressMap : IEnumerable { + public const int NO_ENTRY_ADDR = -1; // address value indicating no entry + /// /// Code starting at the specified offset will have the specified address. /// @@ -45,6 +47,10 @@ namespace CommonUtil { /// Entries are mutable, but must only be altered by AddressMap. Don't retain /// instances of this across other activity. /// + /// + /// TODO: make this immutable. That should allow us to eliminate the copy constructor, + /// since we won't need to make copies of things. + /// [Serializable] public class AddressMapEntry { public int Offset { get; set; } @@ -56,6 +62,13 @@ namespace CommonUtil { Addr = addr; Length = len; } + + // Copy constructor. + public AddressMapEntry(AddressMapEntry src) { + Offset = src.Offset; + Addr = src.Addr; + Length = src.Length; + } } /// @@ -84,11 +97,11 @@ namespace CommonUtil { /// /// List of AddressMapEntry. public AddressMap(List entries) { - // TODO(someday): validate list contents mTotalLength = entries[entries.Count - 1].Offset + entries[entries.Count - 1].Length; foreach (AddressMapEntry ent in entries) { - mAddrList.Add(ent); + mAddrList.Add(new AddressMapEntry(ent)); } + DebugValidate(); } /// @@ -98,7 +111,7 @@ namespace CommonUtil { public List GetEntryList() { List newList = new List(mAddrList.Count); foreach (AddressMapEntry ent in mAddrList) { - newList.Add(ent); + newList.Add(new AddressMapEntry(ent)); } return newList; } @@ -127,7 +140,8 @@ namespace CommonUtil { /// /// Returns the Address value of the address map entry associated with the specified - /// offset, or -1 if there is no address map entry there. The offset must match exactly. + /// offset, or NO_ENTRY_ADDR if there is no address map entry there. The offset must + /// match exactly. /// public int Get(int offset) { foreach (AddressMapEntry ad in mAddrList) { @@ -135,7 +149,7 @@ namespace CommonUtil { return ad.Addr; } } - return -1; + return NO_ENTRY_ADDR; } /// @@ -159,7 +173,7 @@ namespace CommonUtil { /// 24-bit address. public void Set(int offset, int addr) { Debug.Assert(offset >= 0); - if (addr == -1) { + if (addr == NO_ENTRY_ADDR) { if (offset != 0) { // ignore attempts to remove entry at offset zero Remove(offset); } diff --git a/SourceGen/DataAnalysis.cs b/SourceGen/DataAnalysis.cs index 3c72cc3..affec10 100644 --- a/SourceGen/DataAnalysis.cs +++ b/SourceGen/DataAnalysis.cs @@ -573,6 +573,10 @@ namespace SourceGen { offset++; // Check to see if the address has changed from the previous entry. + // TODO(BUG): this test is insufficient -- they might have a .ORG that + // doesn't change the address. It's currently harmless because the + // .ORG is a no-op and gets swallowed up by the asm generator, but it + // looks wrong and could break things. if (offset < mAnattribs.Length && mAnattribs[offset-1].Address + 1 != mAnattribs[offset].Address) { // Must be an ORG here. Scan previous region. diff --git a/SourceGen/DisasmProject.cs b/SourceGen/DisasmProject.cs index 240d99e..3787c56 100644 --- a/SourceGen/DisasmProject.cs +++ b/SourceGen/DisasmProject.cs @@ -1168,8 +1168,7 @@ namespace SourceGen { Symbol sym = kvp.Value; int expectedAddr = AddrMap.OffsetToAddress(offset); if (sym.Value != expectedAddr) { - Symbol newSym = new Symbol(sym.Label, expectedAddr, sym.SymbolSource, - sym.SymbolType, sym.LabelAnno); + Symbol newSym = sym.UpdateValue(expectedAddr); Debug.WriteLine("Updating label value: " + sym + " --> " + newSym); changes[offset] = newSym; sym = newSym; diff --git a/SourceGen/MainController.cs b/SourceGen/MainController.cs index 31799d8..a4f6629 100644 --- a/SourceGen/MainController.cs +++ b/SourceGen/MainController.cs @@ -1653,54 +1653,130 @@ namespace SourceGen { } public bool CanEditAddress() { - if (SelectionAnalysis.mNumItemsSelected != 1) { + // First line must be code, data, or an ORG directive. + int selIndex = mMainWin.CodeListView_GetFirstSelectedIndex(); + if (selIndex < 0) { return false; } - EntityCounts counts = SelectionAnalysis.mEntityCounts; - // Line must be code, data, or an ORG directive. - return (counts.mDataLines > 0 || counts.mCodeLines > 0) || - (SelectionAnalysis.mLineType == LineListGen.Line.Type.OrgDirective); + LineListGen.Line selLine = CodeLineList[selIndex]; + if (selLine.LineType != LineListGen.Line.Type.Code && + selLine.LineType != LineListGen.Line.Type.Data && + selLine.LineType != LineListGen.Line.Type.OrgDirective) { + return false; + } + + // If multiple lines are selected, there must not be an address change between them. + int lastIndex = mMainWin.CodeListView_GetLastSelectedIndex(); + int firstOffset = CodeLineList[selIndex].FileOffset; + int lastOffset = CodeLineList[lastIndex].FileOffset; + if (firstOffset == lastOffset) { + // Single-item selection, we're fine. + return true; + } + + int nextOffset = lastOffset + CodeLineList[lastIndex].OffsetSpan; + + foreach (AddressMap.AddressMapEntry ent in mProject.AddrMap) { + // It's okay to have an existing entry at firstOffset or nextOffset. + if (ent.Offset > firstOffset && ent.Offset < nextOffset) { + Debug.WriteLine("Found mid-selection AddressMap entry at +" + + ent.Offset.ToString("x6")); + return false; + } + } + + return true; } public void EditAddress() { int selIndex = mMainWin.CodeListView_GetFirstSelectedIndex(); - int offset = CodeLineList[selIndex].FileOffset; - Anattrib attr = mProject.GetAnattrib(offset); + int lastIndex = mMainWin.CodeListView_GetLastSelectedIndex(); + int firstOffset = CodeLineList[selIndex].FileOffset; + int lastOffset = CodeLineList[lastIndex].FileOffset; + int nextOffset = lastOffset + CodeLineList[lastIndex].OffsetSpan; + int nextAddr; - // Compute load address, i.e. where the byte would have been placed if the entire - // file were loaded at the address of the first address map entry. We assume - // offsets wrap at the bank boundary. - int firstAddr = mProject.AddrMap.OffsetToAddress(0); - int loadAddr = ((firstAddr + offset) & 0xffff) | (firstAddr & 0xff0000); - EditAddress dlg = new EditAddress(mMainWin, attr.Address, loadAddr, - mProject.CpuDef.MaxAddressValue, mOutputFormatter); + if (firstOffset == lastOffset || nextOffset == mProject.FileDataLength) { + // Single item (which may not be a single *line*) is selected, or the + // last selected item is the end of the file. + nextOffset = -1; + nextAddr = AddressMap.NO_ENTRY_ADDR; + } else { + // Compute "nextAddr". If there's an existing entry at nextOffset, we use + // that. If not, we use the "load address", which is determined by the very + // first address. + // + // I tried this by just removing the selected entry and seeing what the address + // would be without it, useful for relocations inside relocations. This worked + // poorly when relocations were chained, i.e. two consecutive blocks were + // relocated to different places. The end address of the second block gets + // set based on the first address of the first block, which doesn't seem useful. +#if false + nextAddr = mProject.AddrMap.Get(nextOffset); + if (nextAddr == AddressMap.NO_ENTRY_ADDR) { + AddressMap cloneMap = new AddressMap(mProject.AddrMap.GetEntryList()); + if (firstOffset != 0) { + cloneMap.Remove(firstOffset); + } + nextAddr = cloneMap.OffsetToAddress(nextOffset); + } +#else + int fileStartAddr = mProject.AddrMap.OffsetToAddress(0); + nextAddr = ((fileStartAddr + nextOffset) & 0xffff) | (fileStartAddr & 0xff0000); +#endif + } + + EditAddress dlg = new EditAddress(mMainWin, firstOffset, nextOffset, nextAddr, + mProject, mOutputFormatter); if (dlg.ShowDialog() != true) { return; } - if (offset == 0 && dlg.Address < 0) { + if (firstOffset == 0 && dlg.NewAddress < 0) { // Not allowed. The AddressMap will just put it back, which confuses // the undo operation. Debug.WriteLine("EditAddress: not allowed to remove address at offset +000000"); - } else if (true || attr.Address != dlg.Address) { - // NOTE: we used to prevent creation of an apparently redundant address change, - // but it's really helpful to put one on code that isn't moving before you - // start moving other stuff around. - Debug.WriteLine("EditAddress: changing addr at offset +" + offset.ToString("x6") + - " to " + dlg.Address); + return; + } - AddressMap addrMap = mProject.AddrMap; - // Get the previous address map entry for this exact offset, if one - // exists. This may be different from the value used as the default - // (attr.Address), which is the address assigned to the offset, in - // the case where no previous mapping existed. - int prevAddress = addrMap.Get(offset); - UndoableChange uc = UndoableChange.CreateAddressChange(offset, - prevAddress, dlg.Address); - ChangeSet cs = new ChangeSet(uc); + ChangeSet cs = new ChangeSet(1); + + if (mProject.AddrMap.Get(firstOffset) != dlg.NewAddress) { + // Added / removed / changed existing entry. + // + // We allow creation of an apparently redundant address override, because + // sometimes it's helpful to add one to "anchor" an area before relocating + // something that appears earlier in the file. + int prevAddress = mProject.AddrMap.Get(firstOffset); + UndoableChange uc = UndoableChange.CreateAddressChange(firstOffset, + prevAddress, dlg.NewAddress); + cs.Add(uc); + Debug.WriteLine("EditAddress: changing addr at offset +" + + firstOffset.ToString("x6") + " to $" + dlg.NewAddress.ToString("x4")); + } + + // We want to create an entry for the chunk that follows the selected area. + // We don't modify the trailing address if an entry already exists. + // (Note the "can edit" code prevented us from being called if there's an + // address map entry in the middle of the selected area.) + // + // If they're removing an existing entry, don't add a new entry at the end. + if (nextAddr >= 0 && dlg.NewAddress != AddressMap.NO_ENTRY_ADDR && + mProject.AddrMap.Get(nextOffset) == AddressMap.NO_ENTRY_ADDR) { + // We don't screen for redundant entries here. That should only happen if + // they select a range and then don't change the address. Maybe it's useful? + int prevAddress = mProject.AddrMap.Get(nextOffset); + UndoableChange uc = UndoableChange.CreateAddressChange(nextOffset, + prevAddress, nextAddr); + cs.Add(uc); + Debug.WriteLine("EditAddress: setting trailing addr at offset +" + + nextOffset.ToString("x6") + " to $" + nextAddr.ToString("x4")); + } + + if (cs.Count > 0) { ApplyUndoableChanges(cs); } else { - Debug.WriteLine("EditAddress: no change"); + Debug.WriteLine("EditAddress: no changes"); } } @@ -3110,7 +3186,6 @@ namespace SourceGen { int lastOffset = Math.Max(firstOffset, CodeLineList[lastIndex].FileOffset + CodeLineList[lastIndex].OffsetSpan - 1); mHexDumpDialog.ShowOffsetRange(firstOffset, lastOffset); - } } diff --git a/SourceGen/RuntimeData/Help/editors.html b/SourceGen/RuntimeData/Help/editors.html index ac39e2f..b6b7db9 100644 --- a/SourceGen/RuntimeData/Help/editors.html +++ b/SourceGen/RuntimeData/Help/editors.html @@ -17,6 +17,16 @@

Edit Address

This adds a target address directive (".ORG") to the current offset. If you leave the text field blank, the directive will be removed.

+

The text entry field is initialized to the address of the +first selected line. The "load address", i.e. the place where the +code or data will live when the file is first loaded into memory, +is shown for reference.

+

If multiple lines were selected, some additional information will be +shown, and an address directive will be added after the last selected +line. This directive will set the address to the "load address". +This is useful for "relocating" a block of code or data in the middle of +the file. You're not allowed to do this when the selected range of +lines spans another address directive.

Addresses are always interpreted as hexadecimal. You can prefix it with a '$', but that's not required. 24-bit addresses may be written with a bank separator, e.g. "12/3456" @@ -25,9 +35,6 @@ would resolve to address $123456.

There will always be an address directive at the start of the file. Attempts to remove it will be ignored.

-

If the byte at the current offset is not at the address where it was -initially loaded, the "load address" will be shown for reference.

-

Edit Status Flag Override

The state of the processor status flags are tracked for every diff --git a/SourceGen/RuntimeData/Help/mainwin.html b/SourceGen/RuntimeData/Help/mainwin.html index 9fbaf9d..f097a5a 100644 --- a/SourceGen/RuntimeData/Help/mainwin.html +++ b/SourceGen/RuntimeData/Help/mainwin.html @@ -161,8 +161,11 @@ the Actions menu item in the menu bar. The set of options that are enabled will depend on what you have selected in the main window.

  • Set Address. Sets the - target address at that offset. Enabled when a single instruction or - data line is selected.
    • + target address at that offset. When multiple lines are selected, + the target addresses at the start and end of the range is set. + Enabled when the first line selected is code, data, or an address + override, and the full selected range does not overlap with another + address override.
  • Override Status Flags. Changes the status flags at that offset. Enabled when a single instruction line is selected.
    • diff --git a/SourceGen/Symbol.cs b/SourceGen/Symbol.cs index 4f1a6f0..e6cbd92 100644 --- a/SourceGen/Symbol.cs +++ b/SourceGen/Symbol.cs @@ -228,6 +228,21 @@ namespace SourceGen { Label = label + UNIQUE_TAG_CHAR + uniqueTag.ToString("x6"); } + /// + /// Creates a new Symbol where everything is identical to the argument except the value. + /// + public Symbol UpdateValue(int newValue) { + Symbol newSym = new Symbol(); + newSym.Label = Label; + newSym.Value = newValue; + newSym.SymbolType = SymbolType; + newSym.SymbolSource = SymbolSource; + newSym.LabelAnno = LabelAnno; + // generated field, not dependent on Value + newSym.SourceTypeString = SourceTypeString; + return newSym; + } + /// /// Generates a displayable form of the label. This will have the non-unique label /// prefix and annotation suffix, and will have the non-unique tag removed. diff --git a/SourceGen/WpfGui/EditAddress.xaml b/SourceGen/WpfGui/EditAddress.xaml index 6b72d07..978981d 100644 --- a/SourceGen/WpfGui/EditAddress.xaml +++ b/SourceGen/WpfGui/EditAddress.xaml @@ -27,34 +27,56 @@ limitations under the License. ContentRendered="Window_ContentRendered"> - - + + + + + + + + - + + IsInactiveSelectionHighlightEnabled="True"> + - - - + + + + + + + + + + + + + + + - public partial class EditAddress : Window, INotifyPropertyChanged { /// - /// Address typed by user. Only valid after the dialog returns OK. Will be set to -1 - /// if the user is attempting to delete the address. + /// Address typed by user. Only valid after the dialog returns OK. Will be set to + /// AddressMap.NO_ENTRY_ADDR if the user is attempting to delete the address. /// - public int Address { get; private set; } + public int NewAddress { get; private set; } + + /// + /// Offset being edited. + /// + private int mFirstOffset; + + /// + /// Offset after the end of the selection, or -1 if only one line is selected. + /// + private int mNextOffset; + + /// + /// Address after the end of the selection, or -1 if only one line is selected. + /// + private int mNextAddress; /// /// Maximum allowed address value. @@ -48,10 +63,30 @@ namespace SourceGen.WpfGui { /// private Formatter mFormatter; + public string FirstOffsetStr { + get { return mFormatter.FormatOffset24(mFirstOffset); } + } + public string NextOffsetStr { + get { return mFormatter.FormatOffset24(mNextOffset); } + } + public string NextAddressStr { + get { return '$' + mFormatter.FormatAddress(mNextAddress, mNextAddress > 0xffff); } + } + public string BytesSelectedStr { + get { + int count = mNextOffset - mFirstOffset; + return count.ToString() + " (" + mFormatter.FormatHexValue(count, 2) + ")"; + } + } + /// - /// Bound two-way property. + /// Address input TextBox. /// - public string AddressText { get; set; } + public string AddressText { + get { return mAddressText; } + set { mAddressText = value; OnPropertyChanged(); UpdateControls(); } + } + private string mAddressText; /// /// Set to true when input is valid. Controls whether the OK button is enabled. @@ -62,11 +97,12 @@ namespace SourceGen.WpfGui { } private bool mIsValid; - public Visibility LoadAddressVis { - get { return mLoadAddressVis; } - set { mLoadAddressVis = value; OnPropertyChanged(); } + public Visibility NextAddressVis { + get { return mNextAddressVis; } + set { mNextAddressVis = value; OnPropertyChanged(); } } - public Visibility mLoadAddressVis = Visibility.Collapsed; + public Visibility mNextAddressVis = Visibility.Collapsed; + public string LoadAddressText { get { return mLoadAddressText; } set { mLoadAddressText = value; OnPropertyChanged(); } @@ -80,25 +116,45 @@ namespace SourceGen.WpfGui { } - public EditAddress(Window owner, int initialAddr, int loadAddr, int maxAddressValue, - Formatter formatter) { - // Set the property before initializing the window -- we don't have a property - // change notifier. - Address = -2; - mMaxAddressValue = maxAddressValue; - mBaseAddr = loadAddr; - mFormatter = formatter; - - AddressText = Asm65.Address.AddressToString(initialAddr, false); - - if (initialAddr != loadAddr) { - LoadAddressVis = Visibility.Visible; - LoadAddressText = mFormatter.FormatAddress(loadAddr, loadAddr > 0xffff); - } - + /// + /// Constructor. + /// + /// Parent window. + /// Offset at top of selection. + /// Offset past bottom of selection, or -1 if only one + /// line is selected. + /// Project reference. + /// Text formatter object. + public EditAddress(Window owner, int firstOffset, int nextOffset, int nextAddr, + DisasmProject project, Formatter formatter) { InitializeComponent(); Owner = owner; DataContext = this; + + mFirstOffset = firstOffset; + mNextOffset = nextOffset; + mNextAddress = nextAddr; + mFormatter = formatter; + mMaxAddressValue = project.CpuDef.MaxAddressValue; + + // Compute load address, i.e. where the byte would have been placed if the entire + // file were loaded at the address of the first address map entry. We assume + // offsets wrap at the bank boundary. + int fileStartAddr = project.AddrMap.OffsetToAddress(0); + mBaseAddr = ((fileStartAddr + firstOffset) & 0xffff) | (fileStartAddr & 0xff0000); + + int firstAddr = project.GetAnattrib(firstOffset).Address; + Debug.Assert(project.AddrMap.OffsetToAddress(firstOffset) == firstAddr); + + AddressText = Asm65.Address.AddressToString(firstAddr, false); + + LoadAddressText = '$' + mFormatter.FormatAddress(mBaseAddr, mBaseAddr > 0xffff); + + if (nextOffset >= 0) { + NextAddressVis = Visibility.Visible; + } + + NewAddress = -2; } private void Window_ContentRendered(object sender, EventArgs e) { @@ -108,10 +164,11 @@ namespace SourceGen.WpfGui { private void OkButton_Click(object sender, RoutedEventArgs e) { if (AddressText.Length == 0) { - Address = -1; + NewAddress = CommonUtil.AddressMap.NO_ENTRY_ADDR; } else { - Asm65.Address.ParseAddress(AddressText, mMaxAddressValue, out int addr); - Address = addr; + bool ok = Asm65.Address.ParseAddress(AddressText, mMaxAddressValue, out int addr); + Debug.Assert(ok); + NewAddress = addr; } DialogResult = true; } @@ -123,12 +180,9 @@ namespace SourceGen.WpfGui { /// Must have UpdateSourceTrigger=PropertyChanged set for this to work. The default /// for TextBox is LostFocus. /// - private void TextBox_TextChanged(object sender, TextChangedEventArgs e) { - if (IsLoaded) { - string text = AddressText; - IsValid = (text.Length == 0) || - Asm65.Address.ParseAddress(text, mMaxAddressValue, out int unused); - } + private void UpdateControls() { + IsValid = (AddressText.Length == 0) || + Asm65.Address.ParseAddress(AddressText, mMaxAddressValue, out int unused); } }