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

[David Blaikie]

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?


>
> 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/abaf1bf6/attachment.html>