[llvm] r182542 - Missed removing one of the assert()'s from the LLVMCreateDisasmCPU() library

[Kevin Enderby]

On May 23, 2013, at 11:05 AM, David Blaikie <dblaikie at gmail.com> wrote:

> 
> 
> 
> On Thu, May 23, 2013 at 10:58 AM, Kevin Enderby <enderby at apple.com> wrote:
> Hi Eric,
> 
> What was happening is the library call was being handed a bad triple and should have simply returned it could not create a disassembler by returning the value 0 indicating failure.  The code calling this ABI correctly handled the return value of 0.  So I was testing my code in darwin's otool(1) that it was handled the error case correctly but it was crashing instead.
> 
> Without this code in the library:
> 
> > if (!TheTarget)
> >  return 0;
> 
> it was falling through and using the NULL value of TheTarget variable to be used and then crashing.
> 
> In this "normal error path" I don't want an assert in at any time with any configuration.
> 
> Is there a missing test case, then?

Yes, right now there are no test cases in the llvm tree that use this disassembler 'C' library API.  That is a TBD some day.

>  
> 
> Hope that explains what the fix was doing,
> Kev
> 
> On May 23, 2013, at 10:46 AM, Eric Christopher <echristo at gmail.com> wrote:
> 
> > Hi Kevin,
> >
> > I'm not sure I understand why removing the asserts is a good thing. I
> > can easily understand the:
> >
> > if (!TheTarget)
> >  return 0;
> >
> > part as that fixes a bug when used as a library, but you'll likely not
> > ship the library in asserts mode and the assert will help you find
> > bugs faster?
> >
> > -eric
> >
> >
> > On Wed, May 22, 2013 at 5:32 PM, Kevin Enderby <enderby at apple.com> wrote:
> >> Author: enderby
> >> Date: Wed May 22 19:32:34 2013
> >> New Revision: 182542
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=182542&view=rev
> >> Log:
> >> Missed removing one of the assert()'s from the LLVMCreateDisasmCPU() library
> >> API with my 176880 revision.  If a bad Triple is passed in it can also assert.
> >> In this case too it should just return 0 to indicate failure to create the
> >> disassembler.
> >>
> >> rdar://13955214
> >>
> >> Modified:
> >>    llvm/trunk/lib/MC/MCDisassembler/Disassembler.cpp
> >>
> >> Modified: llvm/trunk/lib/MC/MCDisassembler/Disassembler.cpp
> >> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCDisassembler/Disassembler.cpp?rev=182542&r1=182541&r2=182542&view=diff
> >> ==============================================================================
> >> --- llvm/trunk/lib/MC/MCDisassembler/Disassembler.cpp (original)
> >> +++ llvm/trunk/lib/MC/MCDisassembler/Disassembler.cpp Wed May 22 19:32:34 2013
> >> @@ -40,7 +40,8 @@ LLVMDisasmContextRef LLVMCreateDisasmCPU
> >>   // Get the target.
> >>   std::string Error;
> >>   const Target *TheTarget = TargetRegistry::lookupTarget(Triple, Error);
> >> -  assert(TheTarget && "Unable to create target!");
> >> +  if (!TheTarget)
> >> +    return 0;
> >>
> >>   const MCRegisterInfo *MRI = TheTarget->createMCRegInfo(Triple);
> >>   if (!MRI)
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130523/1154eb6c/attachment.html>