6502/6502bench
1
0
Fork 0
mirror of https://github.com/fadden/6502bench.git synced 2024-11-29 10:50:28 +00:00
Code Issues Projects Releases Wiki Activity

Correctly handle a label update edge case

Browse Source 
Test case:
 1. create a label FOO
   (can be referenced or unreferenced)
 2. add a platform symbol file that also defines FOO
   (the platform symbol will be masked by the user label)
 3. rename FOO to BAR
   (platform symbol should appear)
 4. hit "undo"
   (platform symbol should disappear)
 5. delete label FOO
   (platform symbol should appear)
 6. hit "undo"
   (platform symbol should disappear)

This will fail to update the display list properly, and/or crash
when we try to add FOO to a symbol table that already has a
symbol with that label.

The problem is the optimization that tries to avoid running the
data analysis pass if we're just renaming a user label.  We need to
check to see if the rename overlaps with project/platform symbols,
because we need to update the active def symbol set in that case.

To avoid the crash, we just need to use table[key]=value syntax
instead of table.Add(key,value).
This commit is contained in:
Andy McFadden 2019-10-13 15:53:46 -07:00
parent 76efbcfcbe
commit 3702448780
2 changed files with 59 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
SourceGen/DisasmProject.cs
View File

@ -799,7 +799,7 @@ namespace SourceGen {
//reanalysisTimer.DumpTimes("DisasmProject timers:", debugLog);
debugLog.LogI("Analysis complete");
Problems.DebugDump();
//Problems.DebugDump();
}
/// <summary>
@ -1009,6 +1009,28 @@ namespace SourceGen {
}
}
/// <summary>
/// Returns true if a symbol with a matching label appears in the project symbols
/// or any of the platform lists. This operates on the raw lists, not SymbolTable.
/// </summary>
/// <remarks>
/// Useful for determining whether a label masks a project or platform symbol.
/// </remarks>
private bool IsInProjectOrPlatformList(Symbol sym) {
if (sym == null) {
return false;
}
foreach (PlatformSymbols ps in PlatformSyms) {
if (ps.ContainsKey(sym.Label)) {
return true;
}
}
if (ProjectProps.ProjectSyms.ContainsKey(sym.Label)) {
return true;
}
return false;
}
/// <summary>
/// Merges symbols from UserLabels into SymbolTable. Existing entries with matching
/// labels will be replaced.
@ -1881,12 +1903,16 @@ namespace SourceGen {
// auto-generated label, we'll have a duplicate label unless we
// do a full code+data reanalysis. If we're okay with reanalyzing
// on user-label renames, we can allow such conflicts.
//
// We might be changing it to match an existing platform symbol
// though. (Ex: create label FOO, add .sym65 with symbol FOO,
// edit FOO to BAR, then hit undo.)
if (oldValue != null) {
SymbolTable.Remove((Symbol)oldValue);
}
UserLabels[offset] = (Symbol)newValue;
//SymbolTable[((Symbol)newValue).Label] = (Symbol)newValue;
SymbolTable.Add((Symbol)newValue);
SymbolTable[((Symbol)newValue).Label] = (Symbol)newValue;
//SymbolTable.Add((Symbol)newValue);
Debug.Assert(oldSym != null || uc.ReanalysisRequired ==
UndoableChange.ReanalysisScope.DataOnly);
}
@ -1908,18 +1934,33 @@ namespace SourceGen {
RefactorLabel(offset, ((Symbol)oldValue).Label);
}
affectedOffsets.Add(offset);
// Compute the affected offsets. We have one special case to
// consider: if we renamed a label, and the old or new name is
// in project or platform symbols, we need to restore that
// symbol to the symbol table. The most reliable way to do that
// is to switch us to a data re-analysis.
//
// The UI doesn't let you directly edit a label to overwrite a
// symbol, but see FOO/BAR example above.
if (IsInProjectOrPlatformList((Symbol)oldValue) ||
IsInProjectOrPlatformList((Symbol)newValue)) {
Debug.WriteLine("Label change masked/unmasked " +
"project/platform symbol");
needReanalysis |= UndoableChange.ReanalysisScope.DataOnly;
} else {
affectedOffsets.Add(offset);
// Use the cross-reference table to identify the offsets that
// we need to update.
if (mXrefs.TryGetValue(offset, out XrefSet xrefs)) {
foreach (XrefSet.Xref xr in xrefs) {
// This isn't quite right -- in theory we should be adding
// all offsets that are part of the instruction, so that
// affectedOffsets can hold a contiguous range instead of
// a collection of opcode offsets. In practice, for a
// label change, it shouldn't matter.
affectedOffsets.Add(xr.Offset);
// Use the cross-reference table to identify the offsets that
// we need to update.
if (mXrefs.TryGetValue(offset, out XrefSet xrefs)) {
foreach (XrefSet.Xref xr in xrefs) {
// This isn't quite right -- in theory we should be
// adding all offsets that are part of the instruction,
// so that affectedOffsets can hold a contiguous range
// instead of a collection of opcode offsets. In
// practice, for a label change, it shouldn't matter.
affectedOffsets.Add(xr.Offset);
}
}
}
} else {

4
SourceGen/PlatformSymbols.cs
View File

@ -68,6 +68,10 @@ namespace SourceGen {
return mSymbols.Values.GetEnumerator();
}
public bool ContainsKey(string label) {
return mSymbols.ContainsKey(label);
}
/// <summary>
/// Loads platform symbols.
/// </summary>