| Message ID | 20170409164651.31151-2-u@pkh.me |
|---|---|
| State | New |
| Headers | show |
On Sun, Apr 09, 2017 at 06:46:51PM +0200, Clément Bœsch wrote: > Toolchains which target debugging are meaningless with stripping and > toolchains which target memory tracking are meaningless with memory > pollution. > --- > configure | 8 ++++++++ > 1 file changed, 8 insertions(+) memory_poisoning is not enabled by default in configure this change would only change the case where the user of configure enabled explicitly memory_poisoning IMHO this feels wrong [...]
On Mon, Apr 10, 2017 at 03:51:40AM +0200, Michael Niedermayer wrote: > On Sun, Apr 09, 2017 at 06:46:51PM +0200, Clément Bœsch wrote: > > Toolchains which target debugging are meaningless with stripping and > > toolchains which target memory tracking are meaningless with memory > > pollution. > > --- > > configure | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > memory_poisoning is not enabled by default in configure > but it is enabled by the fate configure, so if you create a FATE instance with valgrind, you may forget that memory-poisoning is enabled and make it useless for uninitialized memory without realizing. [...]
On Mon, 10 Apr 2017 07:54:41 +0200 Clément Bœsch <u@pkh.me> wrote: > On Mon, Apr 10, 2017 at 03:51:40AM +0200, Michael Niedermayer wrote: > > On Sun, Apr 09, 2017 at 06:46:51PM +0200, Clément Bœsch wrote: > > > Toolchains which target debugging are meaningless with stripping and > > > toolchains which target memory tracking are meaningless with memory > > > pollution. > > > --- > > > configure | 8 ++++++++ > > > 1 file changed, 8 insertions(+) > > > > memory_poisoning is not enabled by default in configure > > > > but it is enabled by the fate configure, so if you create a FATE instance > with valgrind, you may forget that memory-poisoning is enabled and make it > useless for uninitialized memory without realizing. I love it when such questionable debugging features interfere with real debugging tools.
On Mon, Apr 10, 2017 at 08:53:09AM +0200, wm4 wrote: > On Mon, 10 Apr 2017 07:54:41 +0200 > Clément Bœsch <u@pkh.me> wrote: > > > On Mon, Apr 10, 2017 at 03:51:40AM +0200, Michael Niedermayer wrote: > > > On Sun, Apr 09, 2017 at 06:46:51PM +0200, Clément Bœsch wrote: > > > > Toolchains which target debugging are meaningless with stripping and > > > > toolchains which target memory tracking are meaningless with memory > > > > pollution. > > > > --- > > > > configure | 8 ++++++++ > > > > 1 file changed, 8 insertions(+) > > > > > > memory_poisoning is not enabled by default in configure > > > > > > > but it is enabled by the fate configure, so if you create a FATE instance > > with valgrind, you may forget that memory-poisoning is enabled and make it > > useless for uninitialized memory without realizing. > > I love it when such questionable debugging features interfere with real > debugging tools. Well, it's a debugging "tool". This is another debate, but if people prefer that I drop the feature, it's fine with me. I introduced it a while ago believing it would help spotting simple cases of uninitialized memory. I don't know if it had any practical use cases yet.
On Mon, Apr 10, 2017 at 8:53 AM, wm4 <nfxjfg@googlemail.com> wrote: > On Mon, 10 Apr 2017 07:54:41 +0200 > Clément Bœsch <u@pkh.me> wrote: > >> On Mon, Apr 10, 2017 at 03:51:40AM +0200, Michael Niedermayer wrote: >> > On Sun, Apr 09, 2017 at 06:46:51PM +0200, Clément Bœsch wrote: >> > > Toolchains which target debugging are meaningless with stripping and >> > > toolchains which target memory tracking are meaningless with memory >> > > pollution. >> > > --- >> > > configure | 8 ++++++++ >> > > 1 file changed, 8 insertions(+) >> > >> > memory_poisoning is not enabled by default in configure >> > >> >> but it is enabled by the fate configure, so if you create a FATE instance >> with valgrind, you may forget that memory-poisoning is enabled and make it >> useless for uninitialized memory without realizing. > > I love it when such questionable debugging features interfere with real > debugging tools. Memory poisoning is quite useful, really, and its just a configure flag like any other, disabled by default. IMHO, if someone sets up a valgrind box, its his job to remember to turn this off. configure should not disable explicitly set configure flags. - Hendrik
On Mon, 10 Apr 2017 09:28:06 +0200 Clément Bœsch <u@pkh.me> wrote: > On Mon, Apr 10, 2017 at 08:53:09AM +0200, wm4 wrote: > > On Mon, 10 Apr 2017 07:54:41 +0200 > > Clément Bœsch <u@pkh.me> wrote: > > > > > On Mon, Apr 10, 2017 at 03:51:40AM +0200, Michael Niedermayer wrote: > > > > On Sun, Apr 09, 2017 at 06:46:51PM +0200, Clément Bœsch wrote: > > > > > Toolchains which target debugging are meaningless with stripping and > > > > > toolchains which target memory tracking are meaningless with memory > > > > > pollution. > > > > > --- > > > > > configure | 8 ++++++++ > > > > > 1 file changed, 8 insertions(+) > > > > > > > > memory_poisoning is not enabled by default in configure > > > > > > > > > > but it is enabled by the fate configure, so if you create a FATE instance > > > with valgrind, you may forget that memory-poisoning is enabled and make it > > > useless for uninitialized memory without realizing. > > > > I love it when such questionable debugging features interfere with real > > debugging tools. > > Well, it's a debugging "tool". This is another debate, but if people > prefer that I drop the feature, it's fine with me. I introduced it a while > ago believing it would help spotting simple cases of uninitialized memory. > I don't know if it had any practical use cases yet. > I'd argue against the feature, because the same can be achieved with LD_PRELOADING something that replaces malloc.
diff --git a/configure b/configure index 539a126656..f27ede94d9 100755 --- a/configure +++ b/configure @@ -3591,11 +3591,14 @@ case "$toolchain" in cc_default="${toolchain%-asan}" add_cflags -fsanitize=address add_ldflags -fsanitize=address + disable stripping ;; *-msan) cc_default="${toolchain%-msan}" add_cflags -fsanitize=memory -fsanitize-memory-track-origins add_ldflags -fsanitize=memory + disable stripping + disable memory_poisoning ;; *-tsan) cc_default="${toolchain%-tsan}" @@ -3607,11 +3610,13 @@ case "$toolchain" in add_ldflags -fPIC ;; esac + disable stripping ;; *-usan) cc_default="${toolchain%-usan}" add_cflags -fsanitize=undefined add_ldflags -fsanitize=undefined + disable stripping ;; valgrind-*) target_exec_default="valgrind" @@ -3624,6 +3629,8 @@ case "$toolchain" in target_exec_args="--error-exitcode=1 --malloc-fill=0x2a --track-origins=yes --leak-check=full --gen-suppressions=all --suppressions=$source_path/tests/fate-valgrind.supp" ;; esac + disable stripping + disable memory_poisoning ;; msvc) # Check whether the current MSVC version needs the C99 converter. @@ -3664,6 +3671,7 @@ case "$toolchain" in gcov) add_cflags -fprofile-arcs -ftest-coverage add_ldflags -fprofile-arcs -ftest-coverage + disable stripping ;; llvm-cov) add_cflags -fprofile-arcs -ftest-coverage