This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2] symlookup: improves symbol lookup when a file is specified.
- From: Simon Marchi <simon dot marchi at ericsson dot com>
- To: Walfred Tedeschi <walfred dot tedeschi at intel dot com>, <palves at redhat dot com>, <simon dot marchi at polymtl dot ca>
- Cc: <gdb-patches at sourceware dot org>
- Date: Mon, 16 Oct 2017 13:06:18 -0400
- Subject: Re: [PATCH v2] symlookup: improves symbol lookup when a file is specified.
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=none (sender IP is ) smtp.mailfrom=simon dot marchi at ericsson dot com;
- References: <1507727728-30540-1-git-send-email-walfred.tedeschi@intel.com>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
On 2017-10-11 09:15 AM, Walfred Tedeschi wrote:
> The provided patch adds a scope lookup with higher priority in the case a
> a file is provided for the evaluation.
>
> Usual symbol lookup from GDB follows linker logic. This is what is
> desired most of the time. In the usage case presented a full qualified scope
> is provided provided, so the lookup has to follow this priority. Failing in
"provided provided"
> finding the symbol at this scope usual path is followed.
>
> As use and test case it is presented two shared objects having a global
> variable with the same name but comming from different source files.
> Without the patch evaluating those variables providing the file scope
> returns the wrong value; It returns the value seen in the first loaded
> shared object. Using the patch the value defined in the file scope is
> the one returned.
>
> One of the already existing test cases in print-file-var.exp starts to
> fail after aplying this patch. In fact the value that the linker
> sees is different then the one debugger can read from the shared object.
> In this sense it looks like to me that the test has to be changed.
> The fail is in the call:
> print 'print-file-var-lib2.c'::this_version_id == v2
>
> During load of the libraries linker resolves the symbol as the first one
> loaded and independent of the scope the symbol will have the same value,
> as defined in the first library loaded.
> However, when defining the scope the value should represent the value
> in that context. Diferent evaluations of the same symbols might also better
> spot the issue in users code.
>
> - I haven't changed the test because I wanted to hear the community
> thought on the subject.
I can't really comment on how right the fix is, but I can see that the current
behavior is clearly wrong:
(gdb) p 'print-file-var-dlopen-lib1.c'::this_version_id
$1 = 104
(gdb) p 'print-file-var-dlopen-lib2.c'::this_version_id
$2 = 104
And that your patch fixes it:
(gdb) p 'print-file-var-dlopen-lib1.c'::this_version_id
$1 = 104
(gdb) p 'print-file-var-dlopen-lib2.c'::this_version_id
$2 = 203
> 2017-07-13 Walfred Tedeschi <walfred.tedeschi@intel.com>
>
> gdb/ChangeLog:
>
> * symtab.c (lookup_global_symbol): Add new lookup to ensure
> priority on given block.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.base/print-file-var-dlopen-main.c: New file.
> * gdb.base/print-file-var-dlopen.exp: New test based on
> print-file-var.exp.
> * gdb.base/print-file-var-dlopen-lib1.c: New file.
> * gdb.base/print-file-var-dlopen-lib2.c: New file.
>
>
> ---
> gdb/symtab.c | 4 +
> .../gdb.base/print-file-var-dlopen-lib1.c | 25 +++++
> .../gdb.base/print-file-var-dlopen-lib2.c | 25 +++++
> .../gdb.base/print-file-var-dlopen-main.c | 61 +++++++++++
> gdb/testsuite/gdb.base/print-file-var-dlopen.exp | 113 +++++++++++++++++++++
> 5 files changed, 228 insertions(+)
> create mode 100644 gdb/testsuite/gdb.base/print-file-var-dlopen-lib1.c
> create mode 100644 gdb/testsuite/gdb.base/print-file-var-dlopen-lib2.c
> create mode 100644 gdb/testsuite/gdb.base/print-file-var-dlopen-main.c
> create mode 100644 gdb/testsuite/gdb.base/print-file-var-dlopen.exp
>
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 16a6b2e..a2c307f 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -2589,6 +2589,10 @@ lookup_global_symbol (const char *name,
> if (objfile != NULL)
> result = solib_global_lookup (objfile, name, domain);
>
> + /* We still need to look on the global scope of current object file. */
> + if (result.symbol == NULL && objfile != NULL)
> + result = lookup_symbol_in_objfile (objfile, GLOBAL_BLOCK, name, domain);
> +
> /* If that didn't work go a global search (of global blocks, heh). */
> if (result.symbol == NULL)
> {
> diff --git a/gdb/testsuite/gdb.base/print-file-var-dlopen-lib1.c b/gdb/testsuite/gdb.base/print-file-var-dlopen-lib1.c
> new file mode 100644
> index 0000000..09ec947
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/print-file-var-dlopen-lib1.c
> @@ -0,0 +1,25 @@
> +/* This testcase is part of GDB, the GNU debugger.
> + Copyright 2012-2017 Free Software Foundation, Inc.
Are the years right here (the 2012)?
> +
> + 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/>. */
> +
> +int this_version_id = 104;
> +
> +int
> +get_version (void)
> +{
> + static int test;
> + test = this_version_id;
> + return test;
> +}
> diff --git a/gdb/testsuite/gdb.base/print-file-var-dlopen-lib2.c b/gdb/testsuite/gdb.base/print-file-var-dlopen-lib2.c
> new file mode 100644
> index 0000000..b097cd2
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/print-file-var-dlopen-lib2.c
> @@ -0,0 +1,25 @@
> +/* This testcase is part of GDB, the GNU debugger.
> + Copyright 2012-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/>. */
> +
> +int this_version_id = 203;
> +
> +int
> +get_version (void)
> +{
> + static int test;
> + test = this_version_id;
> + return test;
> +}
> diff --git a/gdb/testsuite/gdb.base/print-file-var-dlopen-main.c b/gdb/testsuite/gdb.base/print-file-var-dlopen-main.c
> new file mode 100644
> index 0000000..954a64e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/print-file-var-dlopen-main.c
> @@ -0,0 +1,61 @@
> +/* This testcase is part of GDB, the GNU debugger.
> + Copyright 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 <dlfcn.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +int
> +dummy (void)
> +{
> + return 1;
> +}
> +
> +int
> +main (void)
> +{
> + int (*get_version1) (void);
> + int (*get_version2) (void);
> + int v1, v2;
> +
> + void *lib1 = dlopen ("print-file-var-dlopen-lib1.so", RTLD_LAZY);
> + void *lib2 = dlopen ("print-file-var-dlopen-lib2.so", RTLD_LAZY);
> +
> + if (lib1 == NULL || lib2 == NULL)
> + return 1;
> +
> + *(int **) (&get_version1) = dlsym (lib1, "get_version");
> + *(int **) (&get_version2) = dlsym (lib2, "get_version");
> +
> + if (get_version1 != NULL
> + && get_version2 != NULL)
> + {
> + v1 = get_version1();
> + v2 = get_version2();
> + }
> +
> +
> + if (v1 != 104 || v2 != 203)
> + return 1;
> +
> + dummy (); /* STOP */
> +
> + dlclose (lib1);
> + dlclose (lib2);
> +
> + return 0;
> +}
> +
> diff --git a/gdb/testsuite/gdb.base/print-file-var-dlopen.exp b/gdb/testsuite/gdb.base/print-file-var-dlopen.exp
> new file mode 100644
> index 0000000..1c331ba
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/print-file-var-dlopen.exp
> @@ -0,0 +1,113 @@
> +# Copyright 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/>. */
> +
> +if {[skip_shlib_tests]} {
> + return 0
> +}
> +
> +set executable print-file-var-dlopen-main
> +
> +set lib1 "print-file-var-dlopen-lib1"
> +set lib2 "print-file-var-dlopen-lib2"
> +set libsrc1 $srcdir/$subdir/${lib1}.c
> +set libsrc2 $srcdir/$subdir/${lib2}.c
> +set libobj1 [standard_output_file ${lib1}.so]
> +set libobj2 [standard_output_file ${lib2}.so]
> +set lib_dlopen1 [shlib_target_file ${libobj1}]
> +set lib_dlopen2 [shlib_target_file ${libobj2}]
> +
> +set srcfile $srcdir/$subdir/$executable.c
> +set binfile [standard_output_file $executable]
> +set shlibdir [standard_output_file {}]
> +
> +set lib_opts debug
> +set exec_opts [list debug shlib_load additional_flags=-DSHLIB_NAME=\"${lib_dlopen1}\" additional_flags=-DSHLIB_NAME2=\"${lib_dlopen2}\"]
> +
> +if { [gdb_compile_shlib $libsrc1 $libobj1 $lib_opts] != ""
> + || [gdb_compile_shlib $libsrc2 $libobj2 $lib_opts] != ""
> + || [gdb_compile $srcfile $binfile executable $exec_opts] != ""} {
> + untested "failed to compile"
> + return -1
> +}
> +
> +if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib1}.c \
> + ${libobj1} \
> + ${lib_opts} ] != "" } {
> + return -1
> +}
> +if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib2}.c \
> + ${libobj2} \
> + ${lib_opts} ] != "" } {
> + return -1
> +}
> +if { [gdb_compile "${srcdir}/${subdir}/${executable}.c" \
> + [standard_output_file ${executable}] \
> + executable \
> + [list debug shlib=-ldl]]
> + != ""} {
> + return -1
> +}
> +
> +
> +clean_restart $executable
> +
> +if ![runto_main] {
> + untested "could not run to main"
> + return -1
> +}
> +
> +# Try printing "this_version_num" qualified with the name of the file
> +# where the variables are defined. There are two global variables
> +# with that name, and some systems such as GNU/Linux merge them
> +# into one single entity, while some other systems such as Windows
> +# keep them separate.
I don't really understand what you mean by "GNU/Linux merge them into one
single entity". There are still two distinct variables, no?
> +gdb_test "continue" \
> + "Breakpoint \[0-9\]+, main \\(\\) at.*" \
> + "continue to STOP marker"
You could use gdb_continue_to_breakpoint.
Simon