Re: [Mesa-dev] [PATCH] gallivm: abort when trying to use non-existing intrinsic
Am 21.12.18 um 15:30 schrieb Jose Fonseca: > On 21/12/2018 14:28, Roland Scheidegger wrote: >> Am 21.12.18 um 08:46 schrieb Jose Fonseca: >>> On 21/12/2018 01:42, srol...@vmware.com wrote: From: Roland Scheidegger 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. > > Not really: debug_printf only prints on debug build, but _debug_printf > (note the leading underscore) always print. Ahh right I failed to catch the difference here. Roland > > Perhaps it's not the smartest naming convention, but it should do the > expected. > > > Jose ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallivm: abort when trying to use non-existing intrinsic
On 21/12/2018 14:28, Roland Scheidegger wrote: Am 21.12.18 um 08:46 schrieb Jose Fonseca: On 21/12/2018 01:42, srol...@vmware.com wrote: From: Roland Scheidegger 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. Not really: debug_printf only prints on debug build, but _debug_printf (note the leading underscore) always print. Perhaps it's not the smartest naming convention, but it should do the expected. Jose ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallivm: abort when trying to use non-existing intrinsic
Am 21.12.18 um 08:46 schrieb Jose Fonseca: > On 21/12/2018 01:42, srol...@vmware.com wrote: >> From: Roland Scheidegger >> >> 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 > > Jose ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallivm: abort when trying to use non-existing intrinsic
On 21/12/2018 01:42, srol...@vmware.com wrote: From: Roland Scheidegger 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.) + 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. Otherwise Reviewed-by: Jose Fonseca Jose ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] gallivm: abort when trying to use non-existing intrinsic
From: Roland Scheidegger 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); + abort(); + } + if (!set_callsite_attrs) lp_add_func_attributes(function, attr_mask); -- 2.17.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev