[llvm-commits] Error-handling fixes for the x86 disassembler

[Sean Callanan]

Chris,

On Mar 31, 2010, at 5:57 PM, Chris Lattner wrote:
> Please don't check in #if 0'd code.

thanks, fixed.

>  The idiom that we use in other apis that can fail is something like this:
> 
> int dosomething_that_could_fail(..., std::string *Error) {
> 
> ...
>  // Oh crap an error happened!
>  if (Error) *Error = "whatever";
>  return -1;
> }
> 
> Then clients can handle this or not with code like:
> std::string ErrorInfo;
> if (dosomething_that_could_fail(..., &ErrorInfo) == -1)
>  print(ErrorInfo); // or whatever.

That's probably a reasonable approach, assuming we can get other code to sign on to doing this and returning -1 instead of calling llvm_unreachable and – in NDEBUG builds – misbehaving silently.

> However, I don't see how error messages from the disassembler are going to be very useful.  Errors like:
> 
> Corrupt table!  Unknown modrm_type
> Cannot have Mod = 0b11 and a SIB byte
> Expected a REG or R/M encoding in fixupReg
> No modifier but an operand expects one.
> 
> Are not going to mean anything to anyone.  I don't think there is such thing as a useful error message that can come out of a disassembler other than "unrecognized instruction".  Given this, I don't see the value in an error reporting api at all here.

I'll just have error() call fprintf(stderr) unless NDEBUG is defined.  New patch attached.

> -Chris

Sean

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100331/7a768545/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: x86-disassembler-assert.r3.diff
Type: application/octet-stream
Size: 26848 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100331/7a768545/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100331/7a768545/attachment-0001.html>