Re: [PATCH v3] Add support for the readnever concept

[Pedro Alves <palves@redhat.com>]

On 11/29/2017 01:21 AM, Sergio Durigan Junior wrote:
> Changes from v2:
> 
> - Fixed a few nits pointed by Eli in the docs.
> 
> - Implemented Pedro's suggestion and moved the logic of readnever out
>   of the "*_build_psymtabs" and into the elf_symfile_read function.

See my comments here:
  https://sourceware.org/ml/gdb-patches/2017-11/msg00766.html

> While implementing the code for the 'symbol-file' command, I noticed a
> bug in 'symbol_file_command': GDB adds the symbol file before
> finishing parsing all the options, which means that the position of an
> option in the command impacts whether it will be considered or not.  I
> changed the code there in order to only add the symbol file after all
> options have been parsed.

Is this the same as:

 https://sourceware.org/ml/gdb-patches/2017-11/msg00656.html

?

Sounds like something that would be better split off to
a precursor patch.

> +++ b/gdb/testsuite/gdb.base/readnever.c
> @@ -0,0 +1,43 @@
> +/* Copyright 2016-2017 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <stdio.h>

This include isn't necessary.

> +
> +static void
> +fun_three (int a, char b, void *c)
> +{
> +  /* Do nothing.  */
> +}
> +
> +static void
> +fun_two (unsigned int p, const char *y)
> +{
> +  fun_three ((int) p, '1', (void *) y);
> +}
> +
> +static void
> +fun_one (int *x)
> +{
> +  fun_two (10, (const char *) x);
> +}
> +
> +int
> +main (void)
> +{
> +  int a = 10;
> +
> +  fun_one (&a);
> +  return 0;
> +}

> +# Test invalid combination of flags.
> +save_vars { GDBFLAGS } {
> +    append GDBFLAGS " --readnever --readnow"
> +    gdb_exit
> +    gdb_spawn_ignore_error
> +
> +    set test "test readnow and readnever at the same time"
> +    gdb_test_multiple "" $test {
> +	eof {
> +	    pass $test
> +	}
> +    }

I had suggested to match GDB's error output before the eof,
so I'm surprised to see only eof expected?

> +}
> +

>  # Default gdb_spawn procedure.
>  
> -proc default_gdb_spawn { } {
> +proc default_gdb_spawn { { ignore_error 0 } } {
>      global use_gdb_stub
>      global GDB
>      global INTERNAL_GDBFLAGS GDBFLAGS
> @@ -1610,11 +1610,12 @@ proc default_gdb_spawn { } {
>  	}
>      }
>      set res [remote_spawn host "$GDB $INTERNAL_GDBFLAGS $GDBFLAGS [host_info gdb_opts]"]
> -    if { $res < 0 || $res == "" } {
> -	perror "Spawning $GDB failed."
> -	return 1
> +    if { !$ignore_error } {
> +	if { $res < 0 || $res == "" } {
> +	    perror "Spawning $GDB failed."
> +	    return 1
> +	}
>      }

Hmm, I think I'm now confused on why this is needed in the first
place?  GDB-the-executable should be starting successfully, 
print some error output, and exit.  What error do you see
here, exactly?

> -
>      set gdb_spawn_id $res
>      return 0
>  }

Thanks,
Pedro Alves