Re: [PATCH] Avoid producing broken non-native core files

[Pedro Alves <palves at redhat dot com>]

On 10/15/2013 03:20 PM, Maciej W. Rozycki wrote:

> The cause of missing register information is elfcore_write_prstatus in BFD 
> that writes no data (and returns NULL) on non-native targets that have no 
> explicit support (bed->elf_backend_write_core_note is NULL), because 
> HAVE_PRSTATUS_T and HAVE_PRSTATUS32_T are both forcibly undefined for 
> non-native BFD configurations.

And if cross debugging, and bed->elf_backend_write_core_note is NULL for the
current target, but HAVE_PRSTATUS_T/HAVE_PRSTATUS32_T are defined (for the
native target), then gcore will generate bogus notes.  :-/
It probably will be a long time before bfd's core generation is
host-independent everywhere, unfortunately.  As future improvement, maybe
we should try _only_ bed->elf_backend_write_core_note, and skip the
HAVE_... bits, unless debugging with the native target.  Anyway,

>  Given that such core files produced are useless anyway I propose that for 
> targets where elfcore_write_prstatus is indeed used and returns NULL an 
> error message was printed and core file preparation aborted.  This is 
> implemented in linux_corefile_thread_callback where signal information is 
> also stored and currently overwrites any unsuccessful return status from 
> the register store worker function (linux_collect_thread_registers).  The 
> test framework is updated accordingly to handle the alternative error 
> message produced in that case.

> --- gdb-fsf-trunk-quilt.orig/gdb/linux-tdep.c	2013-10-14 22:44:49.868756722 +0100
> +++ gdb-fsf-trunk-quilt/gdb/linux-tdep.c	2013-10-14 22:46:21.887601484 +0100
> @@ -1211,7 +1211,9 @@ linux_corefile_thread_callback (struct t
>  				       args->stop_signal);
>        args->num_notes++;
>  
> -      if (siginfo_data != NULL)
> +      /* Don't return anything if we got no register information above,
> +         such a core file is useless.  */
> +      if (args->note_data != NULL && siginfo_data != NULL)

... I was surprised to find that it took me a bit to grok the flow of
this change.  I'd prefer the more explicit:

       args->note_data = args->collect (regcache, info->ptid, args->obfd,
 				       args->note_data, args->note_size,
 				       args->stop_signal);

 +      if (args->note_data == NULL)
 +	{
 +        /* Don't return anything if we got no register information above,
 +           such a core file is useless.  */
 +	   do_cleanups (old_chain);
 +	   return 1;
 +	}

       args->num_notes++;

       if (siginfo_data != NULL)
 	{
 	  args->note_data = elfcore_write_note (args->obfd,
 						args->note_data,
 						args->note_size,
 						"CORE", NT_SIGINFO,
 						siginfo_data, siginfo_size);
	  args->num_notes++;
  	}


This is OK with that change.

Thanks,
-- 
Pedro Alves