From 25790f6fc128f0785a3568019727c0512adc079c Mon Sep 17 00:00:00 2001 From: Joel Brobecker Date: Mon, 20 Jan 2014 16:18:58 +0100 Subject: [PATCH] Remove assert in procfs.c::procfs_make_note_section (x86-solaris) On x86-solaris, the gcore command sometimes triggers the following internal error: (gdb) gcore /[...]/procfs.c:5523: internal-error: procfs_make_note_section: Assertion `thread_args.note_data != note_data' failed. The problem is extremely elusive, for reasons that will become clearer as I explain what is going on. The program used to produce this issue was really simple: | void break_me (void) { } | | int | main (void) | { | break_me (); | return 0; | } The procfs_make_note_section builds a buffer incrementally with the contents of the core's notes section. The interesting bits are: char *note_data = NULL; [...] note_data = (char *) elfcore_write_prpsinfo (obfd, note_data, note_size, fname, psargs); This is the first call to bfd's elfcore which initializes note_data. After that, we have a few more calls, which keep updating notes_data and note_size, but our interest lies in the following part of the function: thread_args.note_data = note_data; [...] proc_iterate_over_threads (pi, procfs_corefile_thread_callback, &thread_args); /* There should be always at least one thread. */ gdb_assert (thread_args.note_data != note_data); The comment implies that the assert is to verify that our loop iterated over at least one thread. The check is relying on the fact that the notes_data returned by the elfcore module changes at each iteration, via (in procfs_corefile_thread_callback): args->note_data = procfs_do_thread_registers (args->obfd, ptid, args->note_data, args->note_size, args->stop_signal); (which calls elfcore_write_lwpstatus). But, while it happens most of the time, thanks to a call to realloc in elfcore_write_note (the function that actually appends the data at the end of the notes buffer),... buf = (char *) realloc (buf, *bufsiz + newspace); ... this is by no means guarantied. In fact, under the right circumstances, the buffer was grown twice without changing addresses. Unfortunately, the circumstances are very sensitive, thus making this bug very elusive. This patch fixes the problem by simply removing the assert. This means we're losing the assertion that there is at least one thread, but I think that's OK. If we still want to keep the assertion, we have the option of either checking the buffer size, or else adding a boolean flag in the context structure that we'd set to true as soon as we have a thread. gdb/ChangeLog: * procfs.c (procfs_make_note_section): Remove assertion and associated comment. --- gdb/ChangeLog | 5 +++++ gdb/procfs.c | 3 --- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 504858d6dc3..3f47aff0d8c 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2014-01-27 Joel Brobecker + + * procfs.c (procfs_make_note_section): Remove assertion and + associated comment. + 2014-01-24 Yao Qi * remote.c (remote_read_bytes): Change type of len to ULONGEST. diff --git a/gdb/procfs.c b/gdb/procfs.c index 0ed788e5247..2383366ab35 100644 --- a/gdb/procfs.c +++ b/gdb/procfs.c @@ -5518,9 +5518,6 @@ procfs_make_note_section (bfd *obfd, int *note_size) thread_args.stop_signal = stop_signal; proc_iterate_over_threads (pi, procfs_corefile_thread_callback, &thread_args); - - /* There should be always at least one thread. */ - gdb_assert (thread_args.note_data != note_data); note_data = thread_args.note_data; auxv_len = target_read_alloc (¤t_target, TARGET_OBJECT_AUXV, -- 2.30.2