Skip to content

Commit 40c1e73

Browse files
binary1230binary1230
committed
labels: refactor local +/- label validation
- add extra check for excluding branches in the wrong direction (can happen in weird but valid edge cases, or, with incorrect disassembly)
1 parent a43760b commit 40c1e73

File tree

1 file changed

+80
-59
lines changed

1 file changed

+80
-59
lines changed

‎Diz.Cpu.65816/src/CPU65C816.cs‎

Lines changed: 80 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -368,71 +368,48 @@ private string FormatOperandAddress(TByteSource data, int offset, Cpu65C816Const
368368
if (intermediateAddress < 0)
369369
return "";
370370

371-
if (data is IReadOnlyLabels labelProvider)
372-
{
373-
// is there a label for this absolute address? if so, lets use that
374-
var labelName = labelProvider.Labels.GetLabelName(intermediateAddress);
375-
if (labelName != "")
376-
{
377-
// couple of special cases.
378-
// we won't return +/- temp labels if this isn't an opcode we want to use with branching.
379-
if (!RomUtil.IsValidPlusMinusLabel(labelName)) // "+". "-", "++", "--", etc
380-
return labelName; // not a local label, OK to use
381-
382-
// this is a +/- local label, so, do some extra checks...
383-
var opcode = data.GetRomByte(offset); // advance the offset to the next byte
384-
var opcodeIsBranch = opcode == 0x80 || // BRA
385-
opcode == 0x10 || opcode == 0x30 || opcode == 0x50 ||
386-
opcode == 0x70 || // BPL BMI BVC BVS
387-
opcode == 0x90 || opcode == 0xB0 || opcode == 0xD0 ||
388-
opcode == 0xF0; // BCC BCS BNE BEQ
389-
// NOT going to do this for any JUMPs like JMP, JML, and also not BRL
390-
391-
// only allow us to use the local label if we're a branch
392-
if (opcodeIsBranch)
393-
return labelName;
394-
}
371+
// is there a label for this absolute address? if so, we'll use that
372+
var labelName = GetValidatedLabelNameForOffset(data, offset);
373+
if (labelName != "")
374+
return labelName;
395375

396-
// otherwise...
397-
398-
// TODO: extract some of this label mirroring logic into its own function so other stuff can call it
399-
400-
// OPTIONAL:
401-
// super-specific variable substitution. this needs to be heavily generalized.
402-
// this MAY NOT COVER EVERY EDGE CASE. feel free to modify or disable it.
403-
// this is to try and get more labels in the output by creating a mathematical expression
404-
// for ASAR to use. only works if you have accurate 'D' register (direct page) set.
405-
// usually only useful after you've done a lot of tracelog capture.
406-
if (AttemptTouseDirectPageArithmeticInFinalOutput && mode
407-
is Cpu65C816Constants.AddressMode.DirectPage
408-
or Cpu65C816Constants.AddressMode.DirectPageXIndex
409-
or Cpu65C816Constants.AddressMode.DirectPageYIndex
410-
or Cpu65C816Constants.AddressMode.DirectPageIndirect
411-
or Cpu65C816Constants.AddressMode.DirectPageXIndexIndirect
412-
or Cpu65C816Constants.AddressMode.DirectPageIndirectYIndex
413-
or Cpu65C816Constants.AddressMode.DirectPageLongIndirect
414-
or Cpu65C816Constants.AddressMode.DirectPageLongIndirectYIndex)
376+
// otherwise, if no appropriate label found...
377+
378+
// OPTIONAL:
379+
// super-specific variable substitution. this needs to be heavily generalized.
380+
// this MAY NOT COVER EVERY EDGE CASE. feel free to modify or disable it.
381+
// this is to try and get more labels in the output by creating a mathematical expression
382+
// for ASAR to use. only works if you have accurate 'D' register (direct page) set.
383+
// usually only useful after you've done a lot of tracelog capture.
384+
if (AttemptTouseDirectPageArithmeticInFinalOutput && mode
385+
is Cpu65C816Constants.AddressMode.DirectPage
386+
or Cpu65C816Constants.AddressMode.DirectPageXIndex
387+
or Cpu65C816Constants.AddressMode.DirectPageYIndex
388+
or Cpu65C816Constants.AddressMode.DirectPageIndirect
389+
or Cpu65C816Constants.AddressMode.DirectPageXIndexIndirect
390+
or Cpu65C816Constants.AddressMode.DirectPageIndirectYIndex
391+
or Cpu65C816Constants.AddressMode.DirectPageLongIndirect
392+
or Cpu65C816Constants.AddressMode.DirectPageLongIndirectYIndex)
393+
{
394+
var dp = data.GetDirectPage(offset);
395+
if (dp != 0)
415396
{
416-
var dp = data.GetDirectPage(offset);
417-
if (dp != 0)
397+
var candidateLabelName = data.Labels.GetLabelName(dp + intermediateAddress);
398+
if (candidateLabelName != "")
418399
{
419-
labelName = labelProvider.Labels.GetLabelName(dp + intermediateAddress);
420-
if (labelName != "")
421-
{
422-
// direct page addressing. we can use an expression to use a variable name for this
423-
// TODO: we can also use asar directive .dbase to set the assumed DB register.
424-
return $"{labelName}-${dp:X}"; // IMPORTANT: no spaces on that minus sign.
425-
}
400+
// direct page addressing. we can use an expression to use a variable name for this
401+
// TODO: we can also use asar directive .dbase to set the assumed DB register.
402+
return $"{candidateLabelName}-${dp:X}"; // IMPORTANT: no spaces on that minus sign.
426403
}
427404
}
428-
429-
// OPTIONAL 2: try some crazy hackery to deal with mirroring on RAM labels.
430-
// (this is super-hacky, we need to do this better)
431-
// also, can the DP address above ALSO interact with this? (probably, right?) if so, we need to keep that in mind
432-
var (labelAddressFound, labelEntryFound) = SearchForMirroredLabel(data, intermediateAddress);
433-
if (labelEntryFound != null)
434-
return $"{labelEntryFound.Name}";
435405
}
406+
407+
// OPTIONAL 2: try some crazy hackery to deal with mirroring on RAM labels.
408+
// (this is super-hacky, we need to do this better)
409+
// also, can the DP address above ALSO interact with this? (probably, right?) if so, we need to keep that in mind
410+
var (labelAddressFound, labelEntryFound) = SearchForMirroredLabel(data, intermediateAddress);
411+
if (labelEntryFound != null)
412+
return $"{labelEntryFound.Name}";
436413

437414
var count = BytesToShow(mode);
438415
if (mode is Cpu65C816Constants.AddressMode.Relative8 or Cpu65C816Constants.AddressMode.Relative16)
@@ -448,6 +425,50 @@ or Cpu65C816Constants.AddressMode.DirectPageLongIndirect
448425
return Util.NumberToBaseString(intermediateAddress, Util.NumberBase.Hexadecimal, 2 * count, true);
449426
}
450427

428+
private static string GetValidatedLabelNameForOffset(TByteSource data, int srcOffset)
429+
{
430+
var destinationIa = data.GetIntermediateAddress(srcOffset);
431+
if (destinationIa < 0)
432+
return "";
433+
434+
var candidateLabelName = data.Labels.GetLabelName(destinationIa);
435+
if (candidateLabelName == "")
436+
return "";
437+
438+
// some special cases related to +/- local labels:
439+
440+
// is this a local label? like "+". "-", "++", "--", etc?
441+
if (!RomUtil.IsValidPlusMinusLabel(candidateLabelName))
442+
return candidateLabelName; // not local label, so we're good
443+
444+
// this IS a local +/- label, so let's do some additional validation..
445+
446+
var opcode = data.GetRomByte(srcOffset);
447+
var opcodeIsBranch =
448+
opcode == 0x80 || // BRA
449+
opcode == 0x10 || opcode == 0x30 || opcode == 0x50 || opcode == 0x70 || // BPL BMI BVC BVS
450+
opcode == 0x90 || opcode == 0xB0 || opcode == 0xD0 || opcode == 0xF0; // BCC BCS BNE BEQ
451+
// NOT going to do this for any JUMPs like JMP, JML, and also not BRL
452+
453+
// don't allow local +/- labels unless the opcode is a branch
454+
if (!opcodeIsBranch)
455+
return "";
456+
457+
// finally, if this IS a branch AND a +/- label,
458+
// make sure the branch is in the correct direction
459+
// (no other checks prevent this except right here).
460+
// DIZ doesn't treat local labels special, so it's up
461+
// to us to enforce this here:
462+
var srcSnesAddress = data.ConvertPCtoSnes(srcOffset);
463+
var branchDirectionIsForward = candidateLabelName[0] == '+';
464+
465+
var validBranchDirection =
466+
srcSnesAddress != destinationIa && // infinite loop (branch to self)
467+
branchDirectionIsForward == (srcSnesAddress < destinationIa); // trying to branch the wrong way
468+
469+
return validBranchDirection ? candidateLabelName : "";
470+
}
471+
451472
private (int labelAddress, IAnnotationLabel? labelEntry) SearchForMirroredLabel(TByteSource data, int snesAddress)
452473
{
453474
// WARNING: this is an EXTREMELY wasteful and very inefficient search. cache wram addresses in labels if needed for perf

0 commit comments

Comments
 (0)