1
0
mirror of https://github.com/fadden/6502bench.git synced 2025-04-21 16:39:06 +00:00

Handle variable labels that are duplicates of non-variables

After thrashing around a bit, I had to choose between making the
uniquifier more complicated, or making de-duplication a separate
step.  Since I don't really expect duplicates to be a thing, I went
with the latter.

Updated the regression test.
This commit is contained in:
Andy McFadden 2019-08-31 21:54:20 -07:00
parent 963b351a52
commit c698048001
10 changed files with 106 additions and 54 deletions

View File

@ -39,8 +39,8 @@ namespace SourceGen.AsmGen {
bool doAddCycles = gen.Settings.GetBool(AppSettings.SRCGEN_SHOW_CYCLE_COUNTS, false);
LocalVariableLookup lvLookup = new LocalVariableLookup(proj.LvTables,
gen.Quirks.HasRedefinableSymbols ? null : proj.SymbolTable, proj);
LocalVariableLookup lvLookup = new LocalVariableLookup(proj.LvTables, proj,
!gen.Quirks.HasRedefinableSymbols);
GenerateHeader(gen, sw);

View File

@ -911,7 +911,7 @@ namespace SourceGen {
/// for the benefit of future uniqueness checks.
/// </summary>
private void GenerateVariableRefs() {
LocalVariableLookup lvLookup = new LocalVariableLookup(LvTables, null, this);
LocalVariableLookup lvLookup = new LocalVariableLookup(LvTables, this, false);
for (int offset = 0; offset < FileData.Length; ) {
// Was a table defined at this offset?
@ -926,8 +926,8 @@ namespace SourceGen {
//
// NOTE: if you try to run the main app with uniqification enabled,
// this will cause the various uniquified forms of local variables
// to end up in the main symbol table. This can clashes with user
// labels that would not occur otherwise.
// to end up in the main symbol table. This can cause clashes with
// user labels that would not occur otherwise.
SymbolTable[defSym.Label] = defSym;
} else if (!sym.IsVariable) {
// Somehow we have a variable and a non-variable with the same
@ -935,11 +935,13 @@ namespace SourceGen {
// this must be a clash with a user label. This could cause
// assembly source gen to fail later on. It's possible to do this
// by "hiding" a table and then adding a user label, so we can't just
// fix it at project load time. The full fix is to permanently
// rename the dup in the LvTable and reset the LvLookup here, but I
// hate trashing user data.
// fix it at project load time.
//
// This is now handled by the LvLookup code, which renames the
// duplicate label, so we shouldn't get here.
Debug.WriteLine("Found non-variable with var name in symbol table: "
+ sym);
Debug.Assert(false);
}
}
}
@ -1099,7 +1101,7 @@ namespace SourceGen {
}
}
LocalVariableLookup lvLookup = new LocalVariableLookup(LvTables, null, this);
LocalVariableLookup lvLookup = new LocalVariableLookup(LvTables, this, false);
// Walk through the Anattrib array, adding xref entries to things referenced
// by the entity at the current offset.

View File

@ -442,7 +442,7 @@ namespace SourceGen {
mFormattedLineCache = new FormattedOperandCache();
mShowCycleCounts = AppSettings.Global.GetBool(AppSettings.SRCGEN_SHOW_CYCLE_COUNTS,
false);
mLvLookup = new LocalVariableLookup(mProject.LvTables, null, mProject);
mLvLookup = new LocalVariableLookup(mProject.LvTables, mProject, false);
mDisplayList.ListGen = this;
}
@ -1334,6 +1334,11 @@ namespace SourceGen {
return FormattedParts.CreateLongComment("BAD INDEX +" + offset.ToString("x6") +
" sub=" + subLineIndex);
} else {
// We use the entry directly from the LvTable, without LvLookup processing.
// This is good, because the entries in the display list match what the editor
// shows, but not quite right, because labels that are duplicates of
// non-variables (which ideally wouldn't happen) won't show their de-duplicated
// form. I think this is fine.
DefSymbol defSym = lvt[subLineIndex];
// Use an operand length of 1 so things are shown as concisely as possible.
string addrStr = PseudoOp.FormatNumericOperand(mFormatter, mProject.SymbolTable,

View File

@ -29,13 +29,25 @@ namespace SourceGen {
private SortedList<int, LocalVariableTable> mLvTables;
/// <summary>
/// Table of symbols, used to ensure that all symbols are globally unique. Only used
/// when generating code for an assembler that doesn't support redefinable variables.
/// Table of symbols, used to ensure that symbols are globally unique.
/// </summary>
private SymbolTable mSymbolTable;
/// <summary>
/// Reference to project, so we can query the Anattrib array to identify "hidden" tables.
/// </summary>
private DisasmProject mProject;
/// <summary>
/// Set to true if we want all variables to be globally unique (because the assembler
/// can't redefine them).
/// </summary>
private bool mDoUniquify;
/// <summary>
/// Label uniquification helper.
///
/// The BaseLabel does not change, but Label is updated by MakeUnique.
/// </summary>
private class UniqueLabel {
public string BaseLabel { get; private set; }
@ -64,16 +76,22 @@ namespace SourceGen {
do {
Counter++;
testLabel = BaseLabel + "_" + Counter;
} while (symbolTable.TryGetValue(testLabel, out Symbol unused1));
} while (symbolTable.TryGetValue(testLabel, out Symbol sym));
Label = testLabel;
}
}
private Dictionary<string, UniqueLabel> mUniqueLabels;
/// <summary>
/// Reference to project, so we can query the Anattrib array to identify "hidden" tables.
/// Duplicate label re-map. This is applied before uniquification.
/// </summary>
private DisasmProject mProject;
/// <remarks>
/// It's hard to do this as part of uniquification because the remapped base name ends
/// up in the symbol table, and the uniqifier isn't able to tell that the entry in the
/// symbol table is itself. The logic is simpler if we just rename the label before
/// the uniquifier ever sees it.
/// </remarks>
private Dictionary<string, string> mDupRemap;
/// <summary>
/// Most recently processed offset.
@ -90,6 +108,7 @@ namespace SourceGen {
/// </summary>
private LocalVariableTable mCurrentTable;
// Next point of interest.
private int mNextLvtIndex;
private int mNextLvtOffset;
@ -99,17 +118,19 @@ namespace SourceGen {
/// </summary>
/// <param name="lvTables">List of tables from the DisasmProject.</param>
/// <param name="symbolTable">Full SymbolTable from the DisasmProject. Used to
/// generate globally unique symbol names. Pass null if uniqueness is not a
/// requirement.</param>
/// generate globally unique symbol names.</param>
/// <param name="project">Project reference.</param>
/// <param name="uniquify">Set to true if variable names cannot be redefined.</param>
public LocalVariableLookup(SortedList<int, LocalVariableTable> lvTables,
SymbolTable symbolTable, DisasmProject project) {
DisasmProject project, bool uniquify) {
mLvTables = lvTables;
mSymbolTable = symbolTable;
mSymbolTable = project.SymbolTable;
mProject = project;
mDoUniquify = uniquify;
mCurrentTable = new LocalVariableTable();
if (mSymbolTable != null) {
mDupRemap = new Dictionary<string, string>();
if (uniquify) {
mUniqueLabels = new Dictionary<string, UniqueLabel>();
}
Reset();
@ -120,6 +141,7 @@ namespace SourceGen {
mRecentSymbols = null;
mCurrentTable.Clear();
mUniqueLabels?.Clear();
mDupRemap.Clear();
if (mLvTables.Count == 0) {
mNextLvtIndex = -1;
mNextLvtOffset = mProject.FileDataLength;
@ -153,10 +175,17 @@ namespace SourceGen {
public DefSymbol GetSymbol(int offset, WeakSymbolRef symRef) {
AdvanceToOffset(offset);
// The symRef uses the non-uniqified symbol, so we need to get the unique value at
// the current offset.
// The symRef uses the non-uniquified symbol, so we need to get the unique value at
// the current offset. We may need to do this even when variables can be
// redefined, because we might have a variable that's a duplicate of a user label
// or project symbol.
string label = symRef.Label;
if (mDupRemap.TryGetValue(symRef.Label, out string remap)) {
label = remap;
}
//Debug.WriteLine("GetSymbol " + symRef.Label + " -> " + label);
if (mUniqueLabels != null && mUniqueLabels.TryGetValue(label, out UniqueLabel ulab)) {
//Debug.WriteLine(" Unique var " + symRef.Label + " -> " + ulab.Label);
label = ulab.Label;
}
DefSymbol defSym = mCurrentTable.GetByLabel(label);
@ -230,9 +259,23 @@ namespace SourceGen {
// discards entries that clash by name or value.
for (int i = 0; i < lvt.Count; i++) {
DefSymbol defSym = lvt[i];
if (mSymbolTable != null) {
// Look for non-variable symbols with the same label. Ordinarily the
// editor prevents this from happening, but there are ways to trick
// the system (e.g. add a symbol while the LvTable is hidden). We
// deal with it here.
if (mSymbolTable.TryGetNonVariableValue(defSym.Label, out Symbol unused)) {
Debug.WriteLine("Detected duplicate non-var label " + defSym.Label +
" at +" + mNextLvtOffset.ToString("x6"));
string newLabel = DeDupLabel(defSym.Label);
mDupRemap[defSym.Label] = newLabel;
defSym = new DefSymbol(defSym, newLabel);
}
if (mDoUniquify) {
if (mUniqueLabels.TryGetValue(defSym.Label, out UniqueLabel ulab)) {
// We've seen this label before; generate a unique version.
// We've seen this label before; generate a unique version by
// increasing the appended number.
ulab.MakeUnique(mSymbolTable);
defSym = new DefSymbol(defSym, ulab.Label);
} else {
@ -257,5 +300,18 @@ namespace SourceGen {
}
}
}
/// <summary>
/// Generates a unique label for the duplicate remap table.
/// </summary>
private string DeDupLabel(string baseLabel) {
string testLabel;
int counter = 0;
do {
counter++;
testLabel = baseLabel + "_DUP" + counter; // make it ugly and obvious
} while (mSymbolTable.TryGetNonVariableValue(testLabel, out Symbol unused));
return testLabel;
}
}
}

View File

@ -581,13 +581,6 @@ namespace SourceGen {
case FormatDescriptor.SubType.Symbol:
if (lvLookup != null && dfd.SymbolRef.IsVariable) {
Debug.Assert(operandLen == 1); // only doing 8-bit stuff
//Symbol.Type symType;
//if (dfd.SymbolRef.VarType == WeakSymbolRef.LocalVariableType.DpAddr) {
// symType = Symbol.Type.ExternalAddr;
//} else {
// symType = Symbol.Type.Constant;
//}
//DefSymbol defSym = lvLookup.GetSymbol(offset, operandValue, symType);
DefSymbol defSym = lvLookup.GetSymbol(offset, dfd.SymbolRef);
if (defSym != null) {
StringBuilder sb = new StringBuilder();

View File

@ -34,7 +34,7 @@
},
"LongComments":{
"-2147483647":{
"Text":"Would need to be edited to test duplicate labels. Those currently cause assembly to fail (see the X_* vars).","BoxMode":false,"MaxWidth":80,"BackgroundColor":0},
"Text":"Edited to have duplicate labels (PROJ_ZERO, DPCODE).","BoxMode":false,"MaxWidth":80,"BackgroundColor":0},
"81":{
"Text":"Test name redefinition. This is mostly of interest for assemblers without redefinable variables, but also of interest to the cross-reference window.","BoxMode":false,"MaxWidth":80,"BackgroundColor":0}},
"Notes":{
@ -74,11 +74,11 @@
"Variables":[{
"DataDescriptor":{
"Length":1,"Format":"NumericLE","SubFormat":"Hex","SymbolRef":null},
"Comment":"clash with project symbol","Label":"X_PROJ_ZERO","Value":16,"Source":"Variable","Type":"ExternalAddr"},
"Comment":"clash with project symbol","Label":"PROJ_ZERO","Value":16,"Source":"Variable","Type":"ExternalAddr"},
{
"DataDescriptor":{
"Length":1,"Format":"NumericLE","SubFormat":"Hex","SymbolRef":null},
"Comment":"clash with user label","Label":"X_DPCODE","Value":128,"Source":"Variable","Type":"ExternalAddr"}],"ClearPrevious":false},
"Comment":"clash with user label","Label":"DPCODE","Value":128,"Source":"Variable","Type":"ExternalAddr"}],"ClearPrevious":false},
"34":{
"Variables":[],"ClearPrevious":false},
"44":{

View File

@ -1,5 +1,4 @@
;Would need to be edited to test duplicate labels. Those currently cause
;assembly to fail (see the X_* vars).
;Edited to have duplicate labels (PROJ_ZERO, DPCODE).
.cpu "65816"
PROJ_ZERO = $00 ;project addr
PROJ_ONE = $01 ;project addr
@ -26,14 +25,14 @@ CONST_ZERO_VAR .var $f0
sta $f1,s
eor 0
ora 240,s
X_PROJ_ZERO .var $10 ;clash with project symbol
X_DPCODE .var $80 ;clash with user label
PROJ_ZERO_DUP1 .var $10 ;clash with project symbol
DPCODE_DUP1 .var $80 ;clash with user label
lda VAR_ZERO
lda VAR_ZERO+1
lda VAR_TWO
lda VAR_THREE
lda $04
lda X_PROJ_ZERO
lda PROJ_ZERO_DUP1
lda $11
lda DPCODE
ldx PROJ_ZERO

View File

@ -1,5 +1,4 @@
;Would need to be edited to test duplicate labels. Those currently cause
;assembly to fail (see the X_* vars).
;Edited to have duplicate labels (PROJ_ZERO, DPCODE).
PROJ_ZERO equ $00 ;project addr
PROJ_ONE equ $01 ;project addr
CONST_ZERO equ $f0 ;project const
@ -23,14 +22,14 @@ CONST_ZERO equ $f0 ;project const
sta $f1,S
eor 0
ora 240,S
]X_PROJ_ZERO equ $10 ;clash with project symbol
]X_DPCODE equ $80 ;clash with user label
]PROJ_ZERO_DUP1 equ $10 ;clash with project symbol
]DPCODE_DUP1 equ $80 ;clash with user label
lda ]VAR_ZERO
lda ]VAR_ZERO+1
lda ]VAR_TWO
lda ]VAR_THREE
lda $04
lda ]X_PROJ_ZERO
lda ]PROJ_ZERO_DUP1
lda $11
lda DPCODE
ldx PROJ_ZERO

View File

@ -1,5 +1,4 @@
;Would need to be edited to test duplicate labels. Those currently cause
;assembly to fail (see the X_* vars).
;Edited to have duplicate labels (PROJ_ZERO, DPCODE).
!cpu 65816
PROJ_ZERO = $00 ;project addr
PROJ_ONE = $01 ;project addr
@ -26,14 +25,14 @@ CONST_ZERO_VAR = $f0
sta $f1,S
eor 0
ora 240,S
X_PROJ_ZERO = $10 ;clash with project symbol
X_DPCODE = $80 ;clash with user label
PROJ_ZERO_DUP1 = $10 ;clash with project symbol
DPCODE_DUP1 = $80 ;clash with user label
lda VAR_ZERO
lda VAR_ZERO+1
lda VAR_TWO
lda VAR_THREE
lda $04
lda X_PROJ_ZERO
lda PROJ_ZERO_DUP1
lda $11
lda+1 DPCODE
ldx PROJ_ZERO

View File

@ -1,5 +1,4 @@
;Would need to be edited to test duplicate labels. Those currently cause
;assembly to fail (see the X_* vars).
;Edited to have duplicate labels (PROJ_ZERO, DPCODE).
.setcpu "65816"
PROJ_ZERO = $00 ;project addr
PROJ_ONE = $01 ;project addr
@ -27,14 +26,14 @@ CONST_ZERO_VAR = $f0
sta $f1,S
eor 0
ora 240,S
X_PROJ_ZERO = $10 ;clash with project symbol
X_DPCODE = $80 ;clash with user label
PROJ_ZERO_DUP1 = $10 ;clash with project symbol
DPCODE_DUP1 = $80 ;clash with user label
lda VAR_ZERO
lda VAR_ZERO+1
lda VAR_TWO
lda VAR_THREE
lda $04
lda X_PROJ_ZERO
lda PROJ_ZERO_DUP1
lda $11
lda z:DPCODE
ldx PROJ_ZERO