[Mesa-dev] [PATCH] gallivm: abort when trying to use non-existing intrinsic

[Roland Scheidegger]

Am 21.12.18 um 08:46 schrieb Jose Fonseca:
> On 21/12/2018 01:42, sroland at vmware.com wrote:
>> From: Roland Scheidegger <sroland at vmware.com>
>>
>> Whenever llvm removes an intrinsic (we're using), we're hitting segfaults
>> due to llvm doing calls to address 0 in the jitted code instead.
>> However, Jose figured out we can actually detect this with
>> LLVMGetIntrinsicID(), so use this to abort, so we don't have to wonder
>> what got broken. (Of course, someone still needs to fix the code to
>> no longer use this intrinsic.)
>> ---
>>   src/gallium/auxiliary/gallivm/lp_bld_intr.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_intr.c
>> b/src/gallium/auxiliary/gallivm/lp_bld_intr.c
>> index 74ed16f33f0..c9df136b103 100644
>> --- a/src/gallium/auxiliary/gallivm/lp_bld_intr.c
>> +++ b/src/gallium/auxiliary/gallivm/lp_bld_intr.c
>> @@ -241,6 +241,16 @@ lp_build_intrinsic(LLVMBuilderRef builder,
>>           function = lp_declare_intrinsic(module, name, ret_type,
>> arg_types, num_args);
>>   +      /*
>> +       * If llvm removes an intrinsic we use, we'll hit this abort
>> (rather
>> +       * than a call to address zero in the jited code).
>> +       */
>> +      if (LLVMGetIntrinsicID(function) == 0) {
>> +         printf("llvm (version 0x%x) found no intrinsic for %s, going
>> to crash...\n",
>> +                HAVE_LLVM, name);
> 
> Better to use _debug_printf() so it's redirected to stderr (or
> OutpuDebug on Windows.)
Alright, though this will drop the output on non-debug builds.

> 
>> +         abort();
>> +      }
>> +
>>         if (!set_callsite_attrs)
>>            lp_add_func_attributes(function, attr_mask);
>>  
> 
> I think it's worth auditing we don't use lp_build_intrinsic() helper for
> LLVM functions we built ourselves.  I took a look, and didn't found any.
It never occurred to me you could use it for ordinary functions. But I
suppose you're right in theory someone could use it (although of course
the function body would have to be defined elsewhere).

Roland



> Otherwise
> 
> Reviewed-by: Jose Fonseca <jfonseca at vmware.com>
> 
> Jose