544959 - OP_bkpt seen in the wild

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect, P2)

Description

[Edwin Smith]

OP_bkpt (0x01) was thought to have never been used, and removed, but is actually present on the web in at least one case.  We need to restore it and support it, and maybe deprecate it in newer ABC versions.

Comment 1

[Lars T Hansen]

Foo :-)

Do we know the tool that produced this code?

Comment 2

[Edwin Smith]

judging from style, i'd say it's from ASC.  if the debugger mutates abc to add OP_bkpt instructions, then it might not be ASC's fault.

Comment 3

[Edwin Smith]

The thing is, we should be getting a VerifyError about an invalid opcode, but instead we're getting an assert.  But we get lucky: with the assert compiled out, we do what we should do: skip ahead one byte to the next opcode.  If we support OP_bkpt, we should skip over it without the assert.  If we stop supporting it for real, we should throw the VerifyError.

Comment 4

[Edwin Smith]

OP_0xF2 (used to be bkptline) and 0xF3 (timestamp) are also in a similar situation.  their size is not -1, yet they are not handled by the Verifier and go to the default case and assert.

Comment 5

[Dan Smith]

These 3 opcodes will not be supported in Argo and should be fully neutered.  The debug builds should not generate asserts() and the  opcodes should be converted to NOPs.  Please include test cases for each opcode to show it is ignored.

Comment 6

[Krzysztof Palacz]

Attachment: Uniform handling of bkpt, bkptline and timestamp: recognize but otherwise ignore

I have testcases (I updated the tools to make them), but will have to figure out if and how to integrate with the acceptance suite.
Formatting note: some of the files use tabs, my additions use spaces, it may look weird, I suggest that we reformat later in a reformat-only patch.

Comment 7

[Krzysztof Palacz]

Attachment: adds test cases

Comment 8

[Lars T Hansen]

I'm not sure where the test cases are here.

Comment 9

[Krzysztof Palacz]

Attachment: adds test cases (for real this time).

Comment 10

[Rob Winchell]

tr-argo changeset 3789:c9a0d821de24