Incorrect disassembly of LINK instruction

[ryanfaescotland]

Hey folks,

It appears to me that the active disassembler is generating the wrong LINK instruction.

Disassembling the opcode 4E 56 FF F4 is coming out as:

Code: Select all

 	LINK.w	A6, -(A7)

Where as my belief is that it should be:

Code: Select all

	LINK	A6, #$FFF4

or (in the format EaSY68K likes)

Code: Select all

	LINK	A6, #-($0C)

Anyone else experienced this or got an explanation behind it?

If you'd like to recreate then offset 000016D2 of 'Toejam & Earl (REV 00) (U) [!].bin' generates it.

EDIT - Looks to me like Exodus treats the LINK instruction as a 16 bit instruction and not 32, so is possibly ignoring the FFF4.

[TascoDLX]

Here's the explanation...

LINK does the following:
- push the source register onto the stack
- copy the stack pointer to the source register
- fetch an immediate word
- add it to the stack pointer

Exodus describes the instruction using 3 parameters: source, target, and offset. Target is not specified by the opcode -- it's implied to be -(SP) as it is the *push* target. When executed, it goes like this:
1) push: source -> target
2) copy: SP -> source
3) fetch: offset
4) add: offset -> SP

The instruction is being disassembled as 'source,target' instead of 'source,offset'. There you go.

P.S., In case someone cares to fix this in source: Exodus/Devices/M68000/Link.h, line 25.

[ryanfaescotland]

Hey thanks for the quick and detailed response TascoDLX.

I'll go nip into the code just now and see if I can resolve it.

My dev environment is playing funny buggers just now but I'll let you know how I get on.

EDIT - Sure enough, changing

Code: Select all

return Disassembly(GetOpcodeName() + L"." + DisassembleSize(size), source.Disassemble(labelSettings) + L", " + target.Disassemble(labelSettings));

to

Code: Select all

return Disassembly(GetOpcodeName() + L"." + DisassembleSize(size), source.Disassemble(labelSettings) + L", " + offset.Disassemble(labelSettings));

does the job.

(Although it still leaves in the size in, which if you want that removed as well remove the ' + L"." + DisassembleSize(size)' part as well)

[ryanfaescotland]

Hate to do this, but would someone mind writing me a little adjustment to the code?

I use Easy68K to reassemble the code and as mentioned it really doesn't like the LINK lines Exodus generates (even with the offset correction). What it likes is the negative value of 10000 minus the offset (So if the offset was FFF4, Easy68K likes (10000-FFF4 = 0C) * -1.

My C++ is pretty much non existent and I'm really struggling to adjust the code to do this. Would one of you be kind enough to give me the line I can apply locally to my copy?

[ryanfaescotland]

Managed to get it going with a little bit of help from the folks at RHDN.

I'll put the revised code below but sadly it breaks the active disassembly functionality, the output comes out with lots of missing labels and it seems all the BRANCH statements are broken. Rather than try fix it I'll likely revert it back to the corrected version below and then put in my own post-processor to convert all the LINKs to the format I need.

Anyway, should anyone ever want to replace:

Code: Select all

	LINK	A6, #$FFF4

with

Code: Select all

	LINK	A6, #-$0C

Then this (hacky) code in LINK.h is for you:

Code: Select all

	virtual Disassembly M68000Disassemble(const M68000::LabelSubstitutionSettings& labelSettings) const
	{
		std::wstring offsetTrimmed = offset.Disassemble(labelSettings).substr(2);
		unsigned long res = std::stoul(L"10000", nullptr, 16) - std::stoul(offsetTrimmed, nullptr, 16);
		std::wstringstream stream;
		stream << L"#-$" << std::setfill(L'0') << std::setw(2) << std::hex << std::uppercase << res;
		return Disassembly(GetOpcodeName(), source.Disassemble(labelSettings) + L", " + stream.str());
		//return Disassembly(GetOpcodeName() + L"." + DisassembleSize(size), source.Disassemble(labelSettings) + L", " + offset.Disassemble(labelSettings));
	}

But then it does produce code like this else where:

Code: Select all

	BRA.b	*+$FE	;Predicted (Offset array entry)
	TST.l	$00A10008
	BNE.b	*+$6
	TST.w	$00A1000C
	BNE.b	*+$7C
	LEA	*+$290, A5

[TascoDLX]

Yeah, you don't want to be messing with formatting there. Probably going to break things.

Actually, the existing code already has a mechanism for treating immediate data as signed, but it's a little hacky itself. Basically, all immediate data is automatically signed-extended and always disassembles as unsigned. However, if you set bool EffectiveAddress::dataSignExtended, you can get it to disassemble as signed data.

Apparently, this alternate behavior is only implemented for MOVEQ. That instruction has the immediate data embedded in the opcode, so it uses a different overload of EffectiveAddress::BuildImmediateData() that accepts a parameter to set the sign extend flag. You can modify the other EffectiveAddress::BuildImmediateData() to accept the same, and it should work just as well.

Although, I'm not quite sure why you're modifying the data in order to fit the EASy68K syntax. It seems that LINK A6, #-($0B) would displace the stack pointer by an odd number(!), which would cause a very nasty problem. Exodus should handle the conversion correctly in the manner described above, giving the correct output for EASy68K: LINK A6, #-$0C.

[ryanfaescotland]

Oops Some dodgy maths on my part, FFF4 should end up as -0C, not -0B. Have adjusted it throughout the previous posts to correct it. Away to retry the code with this fix in place as well, just on the off chance it fixes things with the other instructions too! (EDIT - It doesn't) Although I admit your way sounds like the correct way to do it, thanks for the pointers, so after giving the above a try I went back and done it.

Sure enough, it works just as you predicted.

I think that is everything covered! The assembler does seem to baulk at the outputted code but I think it's issue now is limitations in the assembler itself related to the size of files it can process. I'm going to split it up into smaller files and see if that helps.

[Nemesis]

Thanks for posting about this, and thanks TascoDLX for the great investigation! I've pushed a fix for this issue under EX-323.

[ryanfaescotland]

Thanks Nemesis, and you're welcome.

(Edit - just noticed my name mentioned in the release notes of the current version, fame at last!)