Re: [RFA] Fix leak in linespec.c

Philippe Waroquiers
2019-01-09T03:26:15Z

On Tue, 2019-01-08 at 17:40 -0700, Tom Tromey wrote:
> > > > > > "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
>
> Simon> ... so you are adding the equivalent of the first cleanup.
>
> Oops,sorry about that.
>
> I hope eventually either we have ASAN and perhaps LSAN enabled in
> development builds; or that it becomes a lot simpler to run tests under
> valgrind... not trying to excuse my error, just trying to see how I
> could prevent it next time.
Those that do nothing do no errors, and you do a lot,
but you do not do a lot of errors :).
I started working on running the tests under valgrind after I pushed
a patch containing a 'jump using uninitialised data' error.

Effectively, activating tools as part of dev builds (or build bots) is
the way to go.

Regarding making it simpler to run tests under valgrind:
With the help of Simon, the problem of making a 'clean GDB quit' but
without big timeouts seems to be solved. I still need to do some refinement,
and then I could submit an RFAv2 for 'FORCE_LOCAL_GDB_QUIT_WAIT'.
(need to discuss if this should be activated by default, that was Pedro's
suggestion/hope).

Once this patch is in, it is relatively easy to run all tests under valgrind.
I however have one remaining problem: 2 tests are systematically blocking
under valgrind : gdb.multi/multi-term-settings.exp
and gdb.multi/multi-arch-exec.exp, and must be killed-9.
(a lingering multi-arch test even prevents the linux kernel to suspend,
so rather special state it seems).

Apart of these 2, the results for other tests are relatively comparable
between native and valgrind run, but not equal :(.
So, also some analysis needed on the results (some timeouts maybe ?).

But I hope it will soon be easy/easier to run all tests under valgrind,
including a helper script to scan the gdb.log files to extract the
errors, see below.

With regards to what valgrind detects :
There are still about 110 tests definitely leaking (initially, about 680 tests
were leaking).
However, I have one suppression entry for a (small) leak of some stdio_file in
ui::ui. This leak happens in many tests, I added a supp entry as I could not
yet solve it (without creating a dangling pointer problem).

For what concerns non leak errors, find below the list.
As far as I could quickly see, all 'Syscall param write(buf)' are
some uninitialised data in padding zone written in a core file
by the bfd lib. So probably not a real problem, and then need to see
if this is better fixed or if a supp entry is good enough.

There are a few other more worrying errors (such as jumps using uninitialised data,
invalid read, ...), but I had no time yet to work on these:
Â Â Â Â Â 32 gdb/testsuite/v7_outputs/gdb.base/debug-expr/gdb.log == Conditional jump or move depends on uninitialised value(s)
Â Â Â Â Â Â 5 gdb/testsuite/v7_outputs/gdb.base/debug-expr/gdb.log == Use of uninitialised value of size 8
Â Â Â Â Â 54 gdb/testsuite/v7_outputs/gdb.base/reread/gdb.log == Conditional jump or move depends on uninitialised value(s)
Â Â Â Â Â Â 1 gdb/testsuite/v7_outputs/gdb.cp/inherit/gdb.log == Invalid read of size 8
Â Â Â Â Â Â 1 gdb/testsuite/v7_outputs/gdb.cp/virtbase/gdb.log == Invalid read of size 2
Â Â Â Â Â Â 5 gdb/testsuite/v7_outputs/gdb.cp/virtbase/gdb.log == Invalid read of size 8
Â Â Â Â Â Â 2 gdb/testsuite/v7_outputs/gdb.dwarf2/dw2-icc-opaque/gdb.log == Conditional jump or move depends on uninitialised value(s)
Â Â Â Â Â 10 gdb/testsuite/v7_outputs/gdb.linespec/cpls-abi-tag/gdb.log == Conditional jump or move depends on uninitialised value(s)
Â Â Â Â Â Â 3 gdb/testsuite/v7_outputs/gdb.linespec/cpls-abi-tag/gdb.log == Use of uninitialised value of size 8
Â Â Â Â Â Â 1 gdb/testsuite/v7_outputs/gdb.python/lib-types/gdb.log == Use of uninitialised value of size 8
Â Â Â Â Â Â 1 gdb/testsuite/v7_outputs/gdb.python/py-pp-maint/gdb.log == Use of uninitialised value of size 8
Â Â Â Â Â Â 8 gdb/testsuite/v7_outputs/gdb.python/py-type/gdb.log == Use of uninitialised value of size 8

1 gdb/testsuite/v7_outputs/gdb.ada/task_switch_in_core/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.base/auxv/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.base/coredump-filter/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.base/debug-expr/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.base/gcore-buffer-overflow/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.base/gcore/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.base/gcore-relro/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.base/gcore-relro-pie/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.base/gcore-tls-pie/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.base/info-proc/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.base/print-symbol-loading/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.base/siginfo-obj/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.base/siginfo-thread/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.base/solib-search/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.base/vdso-warning/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.python/py-strfns/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.reverse/break-precsave/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.reverse/consecutive-precsave/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.reverse/finish-precsave/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.reverse/machinestate-precsave/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.reverse/sigall-precsave/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.reverse/solib-precsave/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.reverse/step-precsave/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.reverse/until-precsave/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.reverse/watch-precsave/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.threads/gcore-thread/gdb.log == Syscall param write(buf) points to uninitialised byte(s)
1 gdb/testsuite/v7_outputs/gdb.threads/tls-core/gdb.log == Syscall param write(buf) points to uninitialised byte(s)

1 gdb/testsuite/v7_outputs/gdb.gdb/unittest/gdb.log == Syscall param msync(start) points to unaddressable byte(s)

Philippe

>
> Simon> It would be nice to be able to free the suffix strings in linespec_state_destructor, the
> Simon> only problem is that we don't know the size of the canonical_names array at that point.
>
> linespec would definitely benefit from a couple more rounds of C++-ification.
> Some of the changes spill out into the rest of gdb though.
>
> Tom