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

[Kevin Enderby]

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.

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