[llvm-commits] [PATCH] Enhance ARMDisassembler to report UNPREDICTABLE insns as soft errors

[James Molloy]

Hi,

 

As discussed on llvm-dev recently, this patch adds support for
"soft-failing" on disassembly of certain ARM instructions.

 

Certain instructions are marked as "UNPREDICTABLE" in the ARMARM - the
disassembly of these instructions may still have value however, especially
to a debugger or debugging code generated by another toolchain.

 

This patch promotes the return value of all Decode* operations in the ARM
disassembly to a ternary value (Success, Unpredictable, Fail) and touches a
lot of the error handling code to perform status demotion from Success to
Unpredictable, and returning early on Fail. It also touches the
FixedLenDecoderEmitter to facilitate the same changes in autogenerated code.

 

The major changelist is:

.         Add a new type "DecodeStatus", local to ARMDisassembler.cpp, and
add a macro CHECK(S, X) that will possibly demote a DecodeStatus S based on
a function X's return value, and exit early if the value becomes "Fail".

.         Modify the return value of Decode* from bool to "DecodeStatus",
modify "if(!X()) return false;" to "CHECK(S, X());", assuming a local
accumulator "DecodeStatus S = Success;".

.         Parameterise FixedLenDecoderEmitter to take as extra (optional)
arguments:

o   The return value of the functions to emit (default "bool")

o    "Guard" prefix and postfix to wrap any function call that may fail
(default "if (!" and ") return false;" respectively)

o   Value to return if everything went OK (default "true")

o   Value to return on failure (default "false")

o   Any extra local variables to add (default "")

.         These defaults maintain the current behaviour of
FixedLenDecoderEmitter. ARM/Thumb has been special cased in
DisassemblerEmitter.cpp.

 

The patch as is does not change the user-seen behaviour of the disassembler
at all. I'm still not sure the best way to expose this information to the
user.

 

AFAIK, Intel or most other architectures don't have the same concept as ARM
of a valid but unpredictable instruction. I'm therefore fearful of changing
MCDisassembler.h to cope with it. At the moment I have an alternate
getInstruction() function that also takes a bool& to which it writes if the
insn was predictable or not.

 

The problem with this obviously is that it requires the user to #include
<ARMDisassembler.h>, which is impossible in the general case as it is a
hidden header.

 

I can see three options, none of which being particularly elegant:

 

.         Make ARMDisassembler.h a public header so the user can
cast<ARMDisassembler> and access an alternate getInstruction() function.

.         Add a bool* parameter to getInstruction() for UNPREDICTABLE;
default is NULL and the other backends ignore it.

.         Add an "wasInsnUnpredictable()" function to MCDisassembler, which
returns false for all other backends and only the ARM backend deals with.
Nastily non-reentrant.

.         ??? An alternative.

 

What would you suggest?

 

Cheers,

 

James
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110816/c224f24a/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: unpredictable.patch
Type: application/octet-stream
Size: 119040 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110816/c224f24a/attachment.obj>