[PATCH v2 0/8] Handle copy relocations and add $_ada_exception

[PATCH v2 0/8] Handle copy relocations and add $_ada_exception

[Tom Tromey]

This is v2 of the patch series to add copy relocation support to gdb,
and to implement the $_ada_exception convenience variable.

v1 was here:

    https://sourceware.org/ml/gdb-patches/2019-08/msg00014.html

I believe this version addresses all the review comments.

I've merged some patches (in particular Pedro and Andrew both sent
test case improvements, and I squashed these) and reordered the series
a bit in an attempt to remove some intermediate regressions.
(However, a couple still remain, and I didn't want to go all-out and
squash most of the series.)

Let me know what you think.

Tom

[PATCH v2 6/8] Make print-file-var.exp test attribute visibility hidden, dlopen, and main symbol

[Tom Tromey]

From: Pedro Alves <palves@redhat.com>

Make gdb.base/print-file-var.exp test all combinations of:

  - attribute hidden in the this_version_id symbols or not
  - dlopen or not
  - this_version_id symbol in main file or not
  - C++

gdb/testsuite/ChangeLog
2019-08-27  Pedro Alves  <palves@redhat.com>
	    Andrew Burgess  <andrew.burgess@embecosm.com>

	* gdb.base/print-file-var-lib1.c: Include <stdio.h> and
	"print-file-var.h".
	(this_version_id) Use ATTRIBUTE_VISIBILITY.
	(get_version_1): Print this_version_id and its address.
	Add extern "C" wrappers around interface functions.
	* gdb.base/print-file-var-lib2.c: Include <stdio.h> and
	"print-file-var.h".
	(this_version_id) Use ATTRIBUTE_VISIBILITY.
	(get_version_2): Print this_version_id and its address.
	Add extern "C" wrappers around interface functions.
	* gdb.base/print-file-var-main.c: Include <dlfcn.h>, <assert.h>,
	<stddef.h> and "print-file-var.h".
	Add extern "C" wrappers around interface functions.
	[VERSION_ID_MAIN] (this_version_id): Define.
	(main): Define v0.  Use dlopen if SHLIB_NAME is defined.
	* gdb.base/print-file-var.h: Add some #defines to simplify setting
	up extern "C" blocks.
	* gdb.base/print-file-var.exp (test): New, factored out from top
	level.
	(top level): Test all combinations of attribute hidden or not,
	dlopen or not, and this_version_id symbol in main file or not.
	Compile tests as both C++ and C, make test names unique.
---
 gdb/testsuite/ChangeLog                      |  26 +++
 gdb/testsuite/gdb.base/print-file-var-lib1.c |  11 +-
 gdb/testsuite/gdb.base/print-file-var-lib2.c |  10 +-
 gdb/testsuite/gdb.base/print-file-var-main.c |  42 ++++-
 gdb/testsuite/gdb.base/print-file-var.exp    | 186 ++++++++++++-------
 gdb/testsuite/gdb.base/print-file-var.h      |  34 ++++
 6 files changed, 230 insertions(+), 79 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/print-file-var.h

diff --git a/gdb/testsuite/gdb.base/print-file-var-lib1.c b/gdb/testsuite/gdb.base/print-file-var-lib1.c
index b5f4fb90b39..d172c15bc7d 100644
--- a/gdb/testsuite/gdb.base/print-file-var-lib1.c
+++ b/gdb/testsuite/gdb.base/print-file-var-lib1.c
@@ -14,10 +14,19 @@
    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;
+#include <stdio.h>
+#include "print-file-var.h"
+
+ATTRIBUTE_VISIBILITY int this_version_id = 104;
+
+START_EXTERN_C
 
 int
 get_version_1 (void)
 {
+  printf ("get_version_1: &this_version_id=%p, this_version_id=%d\n", &this_version_id, this_version_id);
+
   return this_version_id;
 }
+
+END_EXTERN_C
diff --git a/gdb/testsuite/gdb.base/print-file-var-lib2.c b/gdb/testsuite/gdb.base/print-file-var-lib2.c
index 28bd1acb17f..b392aff9f3d 100644
--- a/gdb/testsuite/gdb.base/print-file-var-lib2.c
+++ b/gdb/testsuite/gdb.base/print-file-var-lib2.c
@@ -14,10 +14,18 @@
    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;
+#include <stdio.h>
+#include "print-file-var.h"
+
+ATTRIBUTE_VISIBILITY int this_version_id = 203;
+
+START_EXTERN_C
 
 int
 get_version_2 (void)
 {
+  printf ("get_version_2: &this_version_id=%p, this_version_id=%d\n", &this_version_id, this_version_id);
   return this_version_id;
 }
+
+END_EXTERN_C
diff --git a/gdb/testsuite/gdb.base/print-file-var-main.c b/gdb/testsuite/gdb.base/print-file-var-main.c
index ddc54f14d98..1472bd44883 100644
--- a/gdb/testsuite/gdb.base/print-file-var-main.c
+++ b/gdb/testsuite/gdb.base/print-file-var-main.c
@@ -14,21 +14,49 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#ifdef SHLIB_NAME
+# include <dlfcn.h>
+#endif
+
+#include <assert.h>
+#include <stddef.h>
+
+#include "print-file-var.h"
+
+START_EXTERN_C
+
 extern int get_version_1 (void);
 extern int get_version_2 (void);
 
+END_EXTERN_C
+
+#if VERSION_ID_MAIN
+ATTRIBUTE_VISIBILITY int this_version_id = 55;
+#endif
+
 int
 main (void)
 {
+#if VERSION_ID_MAIN
+  int vm = this_version_id;
+#endif
   int v1 = get_version_1 ();
-  int v2 = get_version_2 ();
+  int v2;
+
+#ifdef SHLIB_NAME
+  {
+    void *handle = dlopen (SHLIB_NAME, RTLD_LAZY);
+    int (*getver2) (void);
+
+    assert (handle != NULL);
 
-  if (v1 != 104)
-    return 1;
-  /* The value returned by get_version_2 depends on the target system.  */
-  if (v2 != 104 && v2 != 203)
-    return 2;
+    getver2 = (int (*)(void)) dlsym (handle, "get_version_2");
+
+    v2 = getver2 ();
+  }
+#else
+  v2 = get_version_2 ();
+#endif
 
   return 0; /* STOP */
 }
-
diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp
index 1f733fb4dee..1a065cf568b 100644
--- a/gdb/testsuite/gdb.base/print-file-var.exp
+++ b/gdb/testsuite/gdb.base/print-file-var.exp
@@ -17,76 +17,122 @@ if {[skip_shlib_tests]} {
     return -1
 }
 
-set executable print-file-var-main
-
-set lib1 "print-file-var-lib1"
-set lib2 "print-file-var-lib2"
-
-set libobj1 [standard_output_file ${lib1}.so]
-set libobj2 [standard_output_file ${lib2}.so]
-
-set lib_opts { debug additional_flags=-fPIC }
-
-if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib1}.c \
-                        ${libobj1} \
-                        ${lib_opts} ] != "" } {
-    return -1
+proc test {hidden dlopen version_id_main lang} {
+    global srcdir subdir
+
+    set main "print-file-var-main"
+
+    set suffix "-hidden$hidden-dlopen$dlopen-version_id_main$version_id_main"
+
+    set executable $main$suffix
+
+    set lib1 "print-file-var-lib1"
+    set lib2 "print-file-var-lib2"
+
+    set libobj1 [standard_output_file ${lib1}$suffix.so]
+    set libobj2 [standard_output_file ${lib2}$suffix.so]
+
+    set lib_opts { debug additional_flags=-fPIC $lang }
+    lappend lib_opts "additional_flags=-DHIDDEN=$hidden"
+
+    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
+    }
+
+    set main_opts [list debug shlib=${libobj1} $lang]
+
+    if {$dlopen} {
+	lappend main_opts "shlib_load" \
+	    "additional_flags=-DSHLIB_NAME=\"$libobj2\""
+    } else {
+	lappend main_opts "shlib=${libobj2}"
+    }
+
+    lappend main_opts "additional_flags=-DVERSION_ID_MAIN=$version_id_main"
+
+    if { [gdb_compile "${srcdir}/${subdir}/${main}.c" \
+	      [standard_output_file ${executable}] \
+	      executable \
+	      $main_opts]
+	 != ""} {
+	return -1
+    }
+
+    clean_restart $executable
+    gdb_load_shlib $libobj1
+    gdb_load_shlib $libobj2
+
+    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 three 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.  In the first situation, we have to verify
+    # that GDB does not randomly select the wrong instance, even when
+    # a specific filename is used to qualified the lookup.  And in the
+    # second case, we have to verify that GDB does select the instance
+    # defined in the given filename.
+    #
+    # To avoid adding target-specific code in this testcase, the program
+    # sets three local variables named 'vm', 'v1' and 'v2' with the value of
+    # our global variables.  This allows us to compare the value that
+    # GDB returns for each query against the actual value seen by
+    # the program itself.
+
+    # Get past the initialization of the v* variables.
+
+    set bp_location \
+	[gdb_get_line_number "STOP" "${main}.c"]
+    gdb_test "break $main.c:$bp_location" \
+	"Breakpoint \[0-9\]+ at 0x\[0-9a-fA-F\]+: .*" \
+	"breapoint at STOP marker"
+
+    gdb_test "continue" \
+	"Breakpoint \[0-9\]+, main \\(\\) at.*STOP.*" \
+	"continue to STOP marker"
+
+    # Now check the value of this_version_id in all of
+    # print-file-var-main.c, print-file-var-lib1.c and
+    # print-file-var-lib2.c.
+
+    # Compare the values of $sym1 and $sym2.
+    proc compare {sym1 sym2} {
+	with_test_prefix "sym1=$sym1,sym2=$sym2" {
+	    # Done this way instead of comparing the symbols with "print $sym1
+	    # == sym2" in GDB directly so that the values of the symbols end
+	    # up visible in the logs, for debug purposes.
+	    set vsym1 [get_integer_valueof $sym1 -1]
+	    set vsym2 [get_integer_valueof $sym2 -1]
+	    gdb_assert {$vsym1 == $vsym2} "$sym1 == $sym2"
+	}
+    }
+
+    if $version_id_main {
+	compare "'print-file-var-main.c'::this_version_id" "vm"
+	compare "this_version_id" "vm"
+    }
+
+    compare "'print-file-var-lib1.c'::this_version_id" "v1"
+    compare "'print-file-var-lib2.c'::this_version_id" "v2"
 }
-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=${libobj1} shlib=${libobj2}]]
-     != ""} {
-    return -1
-}
-
-clean_restart $executable
-gdb_load_shlib $libobj1
-gdb_load_shlib $libobj2
 
-if ![runto_main] {
-    untested "could not run to main"
-    return -1
+foreach_with_prefix lang { c c++ } {
+    foreach_with_prefix hidden {0 1} {
+	foreach_with_prefix dlopen {0 1} {
+	    foreach_with_prefix version_id_main {0 1} {
+		test $hidden $dlopen $version_id_main $lang
+	    }
+	}
+    }
 }
-
-# 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.  In the first situation, we have to verify
-# that GDB does not randomly select the wrong instance, even when
-# a specific filename is used to qualified the lookup.  And in the
-# second case, we have to verify that GDB does select the instance
-# defined in the given filename.
-#
-# To avoid adding target-specific code in this testcase, the program
-# sets two local variable named 'v1' and 'v2' with the value of
-# our global variables.  This allows us to compare the value that
-# GDB returns for each query against the actual value seen by
-# the program itself.
-
-# Get past the initialization of variables 'v1' and 'v2'.
-
-set bp_location \
-    [gdb_get_line_number "STOP" "${executable}.c"]
-gdb_test "break $executable.c:$bp_location" \
-         "Breakpoint \[0-9\]+ at 0x\[0-9a-fA-F\]+: .*" \
-         "breapoint past v1 & v2 initialization"
-
-gdb_test "continue" \
-         "Breakpoint \[0-9\]+, main \\(\\) at.*STOP.*" \
-         "continue to STOP marker"
-
-# Now check the value of this_version_id in both print-file-var-lib1.c
-# and print-file-var-lib2.c.
-
-gdb_test "print 'print-file-var-lib1.c'::this_version_id == v1" \
-         " = 1"
-
-gdb_test "print 'print-file-var-lib2.c'::this_version_id == v2" \
-         " = 1"
diff --git a/gdb/testsuite/gdb.base/print-file-var.h b/gdb/testsuite/gdb.base/print-file-var.h
new file mode 100644
index 00000000000..c44e4848b4a
--- /dev/null
+++ b/gdb/testsuite/gdb.base/print-file-var.h
@@ -0,0 +1,34 @@
+/* This testcase is part of GDB, the GNU debugger.
+   Copyright 2019 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/>.  */
+
+#ifndef PRINT_FILE_VAR_H
+#define PRINT_FILE_VAR_H
+
+#if HIDDEN
+# define ATTRIBUTE_VISIBILITY __attribute__((visibility ("hidden")))
+#else
+# define ATTRIBUTE_VISIBILITY
+#endif
+
+#ifdef __cplusplus
+# define START_EXTERN_C extern "C" {
+# define END_EXTERN_C }
+#else
+# define START_EXTERN_C
+# define END_EXTERN_C
+#endif
+
+#endif /* PRINT_FILE_VAR_H */
-- 
2.20.1

Re: [PATCH v2 6/8] Make print-file-var.exp test attribute visibility hidden, dlopen, and main symbol

[Andrew Burgess]

* Tom Tromey <tromey@adacore.com> [2019-09-20 13:20:15 -0600]:

> From: Pedro Alves <palves@redhat.com>
> 
> Make gdb.base/print-file-var.exp test all combinations of:
> 
>   - attribute hidden in the this_version_id symbols or not
>   - dlopen or not
>   - this_version_id symbol in main file or not
>   - C++
> 
> gdb/testsuite/ChangeLog
> 2019-08-27  Pedro Alves  <palves@redhat.com>
> 	    Andrew Burgess  <andrew.burgess@embecosm.com>
> 
> 	* gdb.base/print-file-var-lib1.c: Include <stdio.h> and
> 	"print-file-var.h".
> 	(this_version_id) Use ATTRIBUTE_VISIBILITY.
> 	(get_version_1): Print this_version_id and its address.
> 	Add extern "C" wrappers around interface functions.
> 	* gdb.base/print-file-var-lib2.c: Include <stdio.h> and
> 	"print-file-var.h".
> 	(this_version_id) Use ATTRIBUTE_VISIBILITY.
> 	(get_version_2): Print this_version_id and its address.
> 	Add extern "C" wrappers around interface functions.
> 	* gdb.base/print-file-var-main.c: Include <dlfcn.h>, <assert.h>,
> 	<stddef.h> and "print-file-var.h".
> 	Add extern "C" wrappers around interface functions.
> 	[VERSION_ID_MAIN] (this_version_id): Define.
> 	(main): Define v0.  Use dlopen if SHLIB_NAME is defined.
> 	* gdb.base/print-file-var.h: Add some #defines to simplify setting
> 	up extern "C" blocks.
> 	* gdb.base/print-file-var.exp (test): New, factored out from top
> 	level.
> 	(top level): Test all combinations of attribute hidden or not,
> 	dlopen or not, and this_version_id symbol in main file or not.
> 	Compile tests as both C++ and C, make test names unique.

The C++ build of this test should be optional, the following patch
applied on top of this one fixes this issue.

Thanks,
Andrew

---

diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp
index 1a065cf568b..9669f99202c 100644
--- a/gdb/testsuite/gdb.base/print-file-var.exp
+++ b/gdb/testsuite/gdb.base/print-file-var.exp
@@ -24,6 +24,16 @@ proc test {hidden dlopen version_id_main lang} {
 
     set suffix "-hidden$hidden-dlopen$dlopen-version_id_main$version_id_main"
 
+    # Normally we place different builds (C/C++) of a test into
+    # subdirectories within the test build directory, however, using
+    # gdb_load_shlib doesn't work well with this approach, so instead
+    # add a language specific suffix to the binary and library names.
+    if { $lang == "c" } {
+	set suffix "${suffix}_c"
+    } else {
+	set suffix "${suffix}_cpp"
+    }
+
     set executable $main$suffix
 
     set lib1 "print-file-var-lib1"
@@ -127,7 +137,14 @@ proc test {hidden dlopen version_id_main lang} {
     compare "'print-file-var-lib2.c'::this_version_id" "v2"
 }
 
-foreach_with_prefix lang { c c++ } {
+# Only test C++ if we are able.  Always use C.
+if { [skip_cplus_tests] || [get_compiler_info "c++"] } {
+    set lang_list {c}
+} else {
+    set lang_list {c c++}
+}
+
+foreach_with_prefix lang $lang_list {
     foreach_with_prefix hidden {0 1} {
 	foreach_with_prefix dlopen {0 1} {
 	    foreach_with_prefix version_id_main {0 1} {

Re: [PATCH v2 6/8] Make print-file-var.exp test attribute visibility hidden, dlopen, and main symbol

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> The C++ build of this test should be optional, the following patch
Andrew> applied on top of this one fixes this issue.

Thanks.  I've applied this.

Tom

[PATCH v2 2/8] Search global block from basic_lookup_symbol_nonlocal

[Tom Tromey]

From: Andrew Burgess <andrew.burgess@embecosm.com>

This changes lookup_global_symbol to look in the global block
of the passed-in block.  If no block was passed in, it reverts to the
previous behavior.

This change is needed to ensure that 'FILENAME'::NAME lookups work
properly.  As debugging Pedro's test case showed, this was not working
properly in the case where multiple identical names could be found
(the one situation where this feature is truly needed :-).

This also removes some old comments from basic_lookup_symbol_nonlocal
that no longer apply.

Note that the new test cases for this change will appear in a later
patch.  They are in gdb.base/print-file-var.exp.

gdb/ChangeLog
2019-08-27  Andrew Burgess  <andrew.burgess@embecosm.com>

	* symtab.c (lookup_global_symbol): Search global block.
---
 gdb/ChangeLog |  4 ++++
 gdb/symtab.c  | 41 +++++++++++++----------------------------
 2 files changed, 17 insertions(+), 28 deletions(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 35eab08cb37..0d29e243b9e 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -2414,34 +2414,6 @@ basic_lookup_symbol_nonlocal (const struct language_defn *langdef,
 {
   struct block_symbol result;
 
-  /* NOTE: carlton/2003-05-19: The comments below were written when
-     this (or what turned into this) was part of lookup_symbol_aux;
-     I'm much less worried about these questions now, since these
-     decisions have turned out well, but I leave these comments here
-     for posterity.  */
-
-  /* NOTE: carlton/2002-12-05: There is a question as to whether or
-     not it would be appropriate to search the current global block
-     here as well.  (That's what this code used to do before the
-     is_a_field_of_this check was moved up.)  On the one hand, it's
-     redundant with the lookup in all objfiles search that happens
-     next.  On the other hand, if decode_line_1 is passed an argument
-     like filename:var, then the user presumably wants 'var' to be
-     searched for in filename.  On the third hand, there shouldn't be
-     multiple global variables all of which are named 'var', and it's
-     not like decode_line_1 has ever restricted its search to only
-     global variables in a single filename.  All in all, only
-     searching the static block here seems best: it's correct and it's
-     cleanest.  */
-
-  /* NOTE: carlton/2002-12-05: There's also a possible performance
-     issue here: if you usually search for global symbols in the
-     current file, then it would be slightly better to search the
-     current global block before searching all the symtabs.  But there
-     are other factors that have a much greater effect on performance
-     than that one, so I don't think we should worry about that for
-     now.  */
-
   /* NOTE: dje/2014-10-26: The lookup in all objfiles search could skip
      the current objfile.  Searching the current objfile first is useful
      for both matching user expectations as well as performance.  */
@@ -2674,6 +2646,19 @@ lookup_global_symbol (const char *name,
 		      const struct block *block,
 		      const domain_enum domain)
 {
+  /* If a block was passed in, we want to search the corresponding
+     global block first.  This yields "more expected" behavior, and is
+     needed to support 'FILENAME'::VARIABLE lookups.  */
+  const struct block *global_block = block_global_block (block);
+  if (global_block != nullptr)
+    {
+      symbol *sym = lookup_symbol_in_block (name,
+					    symbol_name_match_type::FULL,
+					    global_block, domain);
+      if (sym != nullptr)
+	return { sym, global_block };
+    }
+
   struct objfile *objfile = lookup_objfile_from_block (block);
   return lookup_global_or_static_symbol (name, GLOBAL_BLOCK, objfile, domain);
 }
-- 
2.20.1

Re: [PATCH v2 2/8] Search global block from basic_lookup_symbol_nonlocal

[Christian Biesinger via gdb-patches]

On Sat, Sep 21, 2019 at 4:20 AM Tom Tromey <tromey@adacore.com> wrote:
> From: Andrew Burgess <andrew.burgess@embecosm.com>
>
> This changes lookup_global_symbol to look in the global block
> of the passed-in block.  If no block was passed in, it reverts to the
> previous behavior.
>
> This change is needed to ensure that 'FILENAME'::NAME lookups work
> properly.  As debugging Pedro's test case showed, this was not working
> properly in the case where multiple identical names could be found
> (the one situation where this feature is truly needed :-).

This further extends the situations where lookup_global_symbols checks
a local scope first (currently only objfile) but lookup_static_symbol
doesn't. Is that really correct? I would think that
lookup_static_symbol is even more likely to need that check, since
static symbols are probably (?) more likely to share the same name. Is
my interpretation wrong?

Christian

Re: [PATCH v2 2/8] Search global block from basic_lookup_symbol_nonlocal

[Andrew Burgess]

* Christian Biesinger <cbiesinger@google.com> [2019-09-21 13:31:30 +0900]:

> On Sat, Sep 21, 2019 at 4:20 AM Tom Tromey <tromey@adacore.com> wrote:
> > From: Andrew Burgess <andrew.burgess@embecosm.com>
> >
> > This changes lookup_global_symbol to look in the global block
> > of the passed-in block.  If no block was passed in, it reverts to the
> > previous behavior.
> >
> > This change is needed to ensure that 'FILENAME'::NAME lookups work
> > properly.  As debugging Pedro's test case showed, this was not working
> > properly in the case where multiple identical names could be found
> > (the one situation where this feature is truly needed :-).
> 
> This further extends the situations where lookup_global_symbols checks
> a local scope first (currently only objfile) but lookup_static_symbol
> doesn't.

[ Note, in the below I talk about "my patch", I really mean mine and
  Tom's as my suggested rework was based on his original patch. ]

I didn't fully understand this for two reasons, first, you say
"further extends", however, both lookup_global_symbol and
lookup_static_symbol pre-patch, both just call
lookup_global_or_static_symbol.  And I couldn't see any code in
lookup_global_or_static_symbol that was conditional on global or
static lookup.  So my question is where is the existing situation in
which lookup_global_symbol searches a local scope first?  I ask only
so I can try to understand your question and my change in relation to
the existing code.

Second, I wanted to better understand what you mean by "local scope".
My understanding of lookup_global_symbol is that it searches every
global block from every object file in some particular order.  The
change here is simply that it makes more sense to search the global
scope relating to the current compilation unit before searching other
global scopes.  I can fully understand that this can be referred to as
a "local scope", I just wanted to make sure we're all agreed on what's
happening here.

> Is that really correct?

I think it makes sense.  If we have multiple global symbols with the
same name, we should find the one corresponding to the current scope I
think.  That feels like what I'd expect to happen.  And especially as
when we do a FILE::VAR lookup we end up in lookup_global_symbol with
the block corresponding to FILE.  If we then ignore that block and
search all global scopes in the predefined order then we will always
find the same global symbol, which is certainly wrong.

> I would think that
> lookup_static_symbol is even more likely to need that check, since
> static symbols are probably (?) more likely to share the same name. Is
> my interpretation wrong?

No, I think you are very right....

But...

There are currently 5 uses of lookup_static_symbol that I could find,
2 of these are in:

  d-namespace.c:find_symbol_in_baseclass
  cp-namespace.c:cp_lookup_nested_symbol_1

These are interesting because almost immediately before calling
lookup_static_symbol we call lookup_symbol_in_static_block, which I
think is solving the same problem as this proposed patch.

Then we have a call to lookup_static_symbol in:

  symtab.c:lookup_symbol_aux

This calls lookup_static_symbol as the last fallback action after
calling the language specific hook la_lookup_symbol_nonlocal.  I've
only audited some languages, but for those I've looked at they all
call lookup_symbol_in_static_block as one of their early actions:

  symtab.c:basic_lookup_symbol_nonlocal		(for C, Pascal)
  cp-namespace.c:cp_lookup_bare_symbol		(for C++, Fortran)
  cp-namespace.c:cp_lookup_bare_symbol		(for Fortran)
  d-namespace.c:d_lookup_symbol			(for D)
  rust-lang.c:rust_lookup_symbol_nonlocal	(for Rust)

That leaves two uses of lookup_static_symbol, these are:

  python/py-symbol.c:gdbpy_lookup_static_symbol
  d-namespace.c:d_lookup_nested_symbol

In these cases there is no call to lookup_symbol_in_static_block. I
would need to investigate more to see if these are working as expected
or not.  I suspect the python use case might not always do what a user
expects, as it searches static symbols in a predefined order, if we
have multiple with the same name we would always find the same one
first, but we'd probably expect to find one from the current object
file before one from a different object file.  As for the D use case,
I don't know D, so don't feel qualified to judge.

I think my conclusion is that you're right, but, refactoring this code
to have lookup_static_symbol call lookup_symbol_in_static_block (or
equivalent in all cases) seems a pretty scary change.  I'd ideally
like to see that refactoring separated from this patch series.

My vote would be to merge this, and then, possibly we can look at
reworking symbol lookup inline with your suggestion.  What do you
think?

Thanks,
Andrew

Re: [PATCH v2 2/8] Search global block from basic_lookup_symbol_nonlocal

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> I think my conclusion is that you're right, but, refactoring this code
Andrew> to have lookup_static_symbol call lookup_symbol_in_static_block (or
Andrew> equivalent in all cases) seems a pretty scary change.  I'd ideally
Andrew> like to see that refactoring separated from this patch series.

I think it would be better on the whole to have much simpler symbol
lookup functions, and then put the complicated parts (like "this"
searching or iteration over static blocks) into the per-language parser
or evaluation code.  Maybe that's too big a job to contemplate though.

Andrew> My vote would be to merge this, and then, possibly we can look at
Andrew> reworking symbol lookup inline with your suggestion.  What do you
Andrew> think?

I am going to go ahead and do this now.

Tom

Re: [PATCH v2 2/8] Search global block from basic_lookup_symbol_nonlocal

[Christian Biesinger via gdb-patches]

I just realized I never responded to this. I just wanted to mention a
couple of things, below.

On Mon, Sep 23, 2019 at 9:16 AM Andrew Burgess
<andrew.burgess@embecosm.com> wrote:
>
> * Christian Biesinger <cbiesinger@google.com> [2019-09-21 13:31:30 +0900]:
>
> > On Sat, Sep 21, 2019 at 4:20 AM Tom Tromey <tromey@adacore.com> wrote:
> > > From: Andrew Burgess <andrew.burgess@embecosm.com>
> > >
> > > This changes lookup_global_symbol to look in the global block
> > > of the passed-in block.  If no block was passed in, it reverts to the
> > > previous behavior.
> > >
> > > This change is needed to ensure that 'FILENAME'::NAME lookups work
> > > properly.  As debugging Pedro's test case showed, this was not working
> > > properly in the case where multiple identical names could be found
> > > (the one situation where this feature is truly needed :-).
> >
> > This further extends the situations where lookup_global_symbols checks
> > a local scope first (currently only objfile) but lookup_static_symbol
> > doesn't.
>
> [ Note, in the below I talk about "my patch", I really mean mine and
>   Tom's as my suggested rework was based on his original patch. ]
>
> I didn't fully understand this for two reasons, first, you say
> "further extends", however, both lookup_global_symbol and
> lookup_static_symbol pre-patch, both just call
> lookup_global_or_static_symbol.  And I couldn't see any code in
> lookup_global_or_static_symbol that was conditional on global or
> static lookup.  So my question is where is the existing situation in
> which lookup_global_symbol searches a local scope first?  I ask only
> so I can try to understand your question and my change in relation to
> the existing code.

lookup_static_symbol always passes nullptr as the objfile to
lookup_global_or_static_symbol, unlike lookup_global_symbol, which is
the difference I meant.

> Second, I wanted to better understand what you mean by "local scope".
> My understanding of lookup_global_symbol is that it searches every
> global block from every object file in some particular order.  The
> change here is simply that it makes more sense to search the global
> scope relating to the current compilation unit before searching other
> global scopes.  I can fully understand that this can be referred to as
> a "local scope", I just wanted to make sure we're all agreed on what's
> happening here.

lookup_global_symbol takes an optional block argument; it starts the
search in the objfile associated with that block (or did, before your
patch).

The argument I was making is that it probably makes sense for both
global and static lookups to take a block (or global block) argument
for starting the search, and that this is more important for static
symbols because there are more likely to be name collisions there.

My apologies for expressing myself poorly.

> I think it makes sense.  If we have multiple global symbols with the
> same name, we should find the one corresponding to the current scope I
> think.  That feels like what I'd expect to happen.  And especially as
> when we do a FILE::VAR lookup we end up in lookup_global_symbol with
> the block corresponding to FILE.  If we then ignore that block and
> search all global scopes in the predefined order then we will always
> find the same global symbol, which is certainly wrong.
>
> > I would think that
> > lookup_static_symbol is even more likely to need that check, since
> > static symbols are probably (?) more likely to share the same name. Is
> > my interpretation wrong?
>
> No, I think you are very right....
>
> But...
>
> There are currently 5 uses of lookup_static_symbol that I could find,
> 2 of these are in:
>
>   d-namespace.c:find_symbol_in_baseclass
>   cp-namespace.c:cp_lookup_nested_symbol_1
>
> These are interesting because almost immediately before calling
> lookup_static_symbol we call lookup_symbol_in_static_block, which I
> think is solving the same problem as this proposed patch.
>
> Then we have a call to lookup_static_symbol in:
>
>   symtab.c:lookup_symbol_aux
>
> This calls lookup_static_symbol as the last fallback action after
> calling the language specific hook la_lookup_symbol_nonlocal.  I've
> only audited some languages, but for those I've looked at they all
> call lookup_symbol_in_static_block as one of their early actions:
>
>   symtab.c:basic_lookup_symbol_nonlocal         (for C, Pascal)
>   cp-namespace.c:cp_lookup_bare_symbol          (for C++, Fortran)
>   cp-namespace.c:cp_lookup_bare_symbol          (for Fortran)
>   d-namespace.c:d_lookup_symbol                 (for D)
>   rust-lang.c:rust_lookup_symbol_nonlocal       (for Rust)
>
> That leaves two uses of lookup_static_symbol, these are:
>
>   python/py-symbol.c:gdbpy_lookup_static_symbol
>   d-namespace.c:d_lookup_nested_symbol
>
> In these cases there is no call to lookup_symbol_in_static_block. I
> would need to investigate more to see if these are working as expected
> or not.  I suspect the python use case might not always do what a user
> expects, as it searches static symbols in a predefined order, if we
> have multiple with the same name we would always find the same one
> first, but we'd probably expect to find one from the current object
> file before one from a different object file.  As for the D use case,
> I don't know D, so don't feel qualified to judge.

The symbol lookup code is so confusing :(

For someone like me, new to GDB, it is difficult to understand this
code -- why does global lookup take a block but local doesn't.
Apparently the answer is that the callers do the static block lookup.
But.. why? Anyway that's the context for my questions.

Thanks for your work in this area!

Christian

[PATCH v2 3/8] Don't call decode_line_with_current_source from select_source_symtab

[Tom Tromey]

select_source_symtab currently calls decode_line_with_current_source.
However, this function iterates over all program spaces, and so it is
possible that it will return a "main" from some other program space.

This patch changes select_source_symtab to simply use the symbol it
already found in the current program space.

2019-08-21  Tom Tromey  <tromey@adacore.com>

	* source.c (select_source_symtab): Don't call
	decode_line_with_current_source.
---
 gdb/ChangeLog |  5 +++++
 gdb/source.c  | 11 ++++-------
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/gdb/source.c b/gdb/source.c
index 0171f2748b4..77eced31628 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -251,17 +251,14 @@ select_source_symtab (struct symtab *s)
 
   /* Make the default place to list be the function `main'
      if one exists.  */
-  if (lookup_symbol (main_name (), 0, VAR_DOMAIN, 0).symbol)
+  block_symbol bsym = lookup_symbol (main_name (), 0, VAR_DOMAIN, 0);
+  if (bsym.symbol != nullptr && SYMBOL_CLASS (bsym.symbol) == LOC_BLOCK)
     {
-      std::vector<symtab_and_line> sals
-	= decode_line_with_current_source (main_name (),
-					   DECODE_LINE_FUNFIRSTLINE);
-      const symtab_and_line &sal = sals[0];
+      symtab_and_line sal = find_function_start_sal (bsym.symbol, true);
       current_source_pspace = sal.pspace;
       current_source_symtab = sal.symtab;
       current_source_line = std::max (sal.line - (lines_to_list - 1), 1);
-      if (current_source_symtab)
-	return;
+      return;
     }
 
   /* Alright; find the last file in the symtab list (ignoring .h's
-- 
2.20.1

[PATCH v2 7/8] Back out earlier Ada exception change

[Tom Tromey]

commit 2ff0a9473 (Fix "catch exception" with dynamic linking) changed
how ada-lang.c creates expressions to determine if an exception
catchpoint should stop.

That patch is no longer needed now that copy relocations are handled
more directly.

2019-08-21  Tom Tromey  <tromey@adacore.com>

	* ada-lang.c (ada_lookup_simple_minsyms): Remove.
	(create_excep_cond_exprs): Simplify exception string computation.
	(ada_exception_catchpoint_cond_string): Likewise.
---
 gdb/ChangeLog  |   6 +++
 gdb/ada-lang.c | 107 ++++++++++++-------------------------------------
 2 files changed, 31 insertions(+), 82 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 2c8be4de857..e4702b96876 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -4845,36 +4845,6 @@ ada_lookup_simple_minsym (const char *name)
   return result;
 }
 
-/* Return all the bound minimal symbols matching NAME according to Ada
-   decoding rules.  Returns an empty vector if there is no such
-   minimal symbol.  Names prefixed with "standard__" are handled
-   specially: "standard__" is first stripped off, and only static and
-   global symbols are searched.  */
-
-static std::vector<struct bound_minimal_symbol>
-ada_lookup_simple_minsyms (const char *name)
-{
-  std::vector<struct bound_minimal_symbol> result;
-
-  symbol_name_match_type match_type = name_match_type_from_name (name);
-  lookup_name_info lookup_name (name, match_type);
-
-  symbol_name_matcher_ftype *match_name
-    = ada_get_symbol_name_matcher (lookup_name);
-
-  for (objfile *objfile : current_program_space->objfiles ())
-    {
-      for (minimal_symbol *msymbol : objfile->msymbols ())
-	{
-	  if (match_name (MSYMBOL_LINKAGE_NAME (msymbol), lookup_name, NULL)
-	      && MSYMBOL_TYPE (msymbol) != mst_solib_trampoline)
-	    result.push_back ({msymbol, objfile});
-	}
-    }
-
-  return result;
-}
-
 /* For all subprograms that statically enclose the subprogram of the
    selected frame, add symbols matching identifier NAME in DOMAIN
    and their blocks to the list of data in OBSTACKP, as for
@@ -12367,6 +12337,8 @@ static void
 create_excep_cond_exprs (struct ada_catchpoint *c,
                          enum ada_exception_catchpoint_kind ex)
 {
+  struct bp_location *bl;
+
   /* Nothing to do if there's no specific exception to catch.  */
   if (c->excep_string.empty ())
     return;
@@ -12375,45 +12347,28 @@ create_excep_cond_exprs (struct ada_catchpoint *c,
   if (c->loc == NULL)
     return;
 
-  /* We have to compute the expression once for each program space,
-     because the expression may hold the addresses of multiple symbols
-     in some cases.  */
-  std::multimap<program_space *, struct bp_location *> loc_map;
-  for (bp_location *bl = c->loc; bl != NULL; bl = bl->next)
-    loc_map.emplace (bl->pspace, bl);
-
-  scoped_restore_current_program_space save_pspace;
+  /* Compute the condition expression in text form, from the specific
+     expection we want to catch.  */
+  std::string cond_string
+    = ada_exception_catchpoint_cond_string (c->excep_string.c_str (), ex);
 
-  std::string cond_string;
-  program_space *last_ps = nullptr;
-  for (auto iter : loc_map)
+  /* Iterate over all the catchpoint's locations, and parse an
+     expression for each.  */
+  for (bl = c->loc; bl != NULL; bl = bl->next)
     {
       struct ada_catchpoint_location *ada_loc
-	= (struct ada_catchpoint_location *) iter.second;
-
-      if (ada_loc->pspace != last_ps)
-	{
-	  last_ps = ada_loc->pspace;
-	  set_current_program_space (last_ps);
-
-	  /* Compute the condition expression in text form, from the
-	     specific expection we want to catch.  */
-	  cond_string
-	    = ada_exception_catchpoint_cond_string (c->excep_string.c_str (),
-						    ex);
-	}
-
+	= (struct ada_catchpoint_location *) bl;
       expression_up exp;
 
-      if (!ada_loc->shlib_disabled)
+      if (!bl->shlib_disabled)
 	{
 	  const char *s;
 
 	  s = cond_string.c_str ();
 	  try
 	    {
-	      exp = parse_exp_1 (&s, ada_loc->address,
-				 block_for_pc (ada_loc->address),
+	      exp = parse_exp_1 (&s, bl->address,
+				 block_for_pc (bl->address),
 				 0);
 	    }
 	  catch (const gdb_exception_error &e)
@@ -13083,18 +13038,18 @@ ada_exception_catchpoint_cond_string (const char *excep_string,
                                       enum ada_exception_catchpoint_kind ex)
 {
   int i;
+  bool is_standard_exc = false;
   std::string result;
-  const char *name;
 
   if (ex == ada_catch_handlers)
     {
       /* For exception handlers catchpoints, the condition string does
          not use the same parameter as for the other exceptions.  */
-      name = ("long_integer (GNAT_GCC_exception_Access"
-	      "(gcc_exception).all.occurrence.id)");
+      result = ("long_integer (GNAT_GCC_exception_Access"
+		"(gcc_exception).all.occurrence.id)");
     }
   else
-    name = "long_integer (e)";
+    result = "long_integer (e)";
 
   /* The standard exceptions are a special case.  They are defined in
      runtime units that have been compiled without debugging info; if
@@ -13113,35 +13068,23 @@ ada_exception_catchpoint_cond_string (const char *excep_string,
      If an exception named contraint_error is defined in another package of
      the inferior program, then the only way to specify this exception as a
      breakpoint condition is to use its fully-qualified named:
-     e.g. my_package.constraint_error.
-
-     Furthermore, in some situations a standard exception's symbol may
-     be present in more than one objfile, because the compiler may
-     choose to emit copy relocations for them.  So, we have to compare
-     against all the possible addresses.  */
+     e.g. my_package.constraint_error.  */
 
-  /* Storage for a rewritten symbol name.  */
-  std::string std_name;
   for (i = 0; i < sizeof (standard_exc) / sizeof (char *); i++)
     {
       if (strcmp (standard_exc [i], excep_string) == 0)
 	{
-	  std_name = std::string ("standard.") + excep_string;
-	  excep_string = std_name.c_str ();
+	  is_standard_exc = true;
 	  break;
 	}
     }
 
-  excep_string = ada_encode (excep_string);
-  std::vector<struct bound_minimal_symbol> symbols
-    = ada_lookup_simple_minsyms (excep_string);
-  for (const bound_minimal_symbol &msym : symbols)
-    {
-      if (!result.empty ())
-	result += " or ";
-      string_appendf (result, "%s = %s", name,
-		      pulongest (BMSYMBOL_VALUE_ADDRESS (msym)));
-    }
+  result += " = ";
+
+  if (is_standard_exc)
+    string_appendf (result, "long_integer (&standard.%s)", excep_string);
+  else
+    string_appendf (result, "long_integer (&%s)", excep_string);
 
   return result;
 }
-- 
2.20.1

[PATCH v2 8/8] Add $_ada_exception convenience variable

[Tom Tromey]

This adds the $_ada_exception convenience variable.  It is set by the
Ada exception catchpoints, and holds the address of the exception
currently being thrown.  This is useful because it allows more
fine-grained filtering of exceptions than is possible using the
existing "catch" syntax.

This also simplifies Ada catchpoints somewhat; because the catchpoint
must now carry the "kind", it's possible to remove many helper
functions.

gdb/ChangeLog
2019-08-21  Tom Tromey  <tromey@adacore.com>

	* NEWS: Add $_ada_exception entry.
	* ada-lang.c (struct ada_catchpoint): Add constructor.
	<m_kind>: New member.
	(allocate_location_exception, re_set_exception): Remove
	"ex" parameter.
	(should_stop_exception): Compute $_ada_exception.
	(check_status_exception, print_it_exception)
	(print_one_exception, print_mention_exception): Remove
	"ex" parameter.
	(allocate_location_catch_exception, re_set_catch_exception)
	(check_status_exception, print_it_catch_exception)
	(print_one_catch_exception, print_mention_catch_exception)
	(print_recreate_catch_exception)
	(allocate_location_catch_exception_unhandled)
	(re_set_catch_exception_unhandled)
	(check_status_exception, print_it_catch_exception_unhandled)
	(print_one_catch_exception_unhandled)
	(print_mention_catch_exception_unhandled)
	(print_recreate_catch_exception_unhandled)
	(allocate_location_catch_assert, re_set_catch_assert)
	(check_status_assert, print_it_catch_assert)
	(print_one_catch_assert, print_mention_catch_assert)
	(print_recreate_catch_assert)
	(allocate_location_catch_handlers, re_set_catch_handlers)
	(check_status_handlers, print_it_catch_handlers)
	(print_one_catch_handlers, print_mention_catch_handlers)
	(print_recreate_catch_handlers): Remove.
	(create_ada_exception_catchpoint): Update.
	(initialize_ada_catchpoint_ops): Update.

gdb/doc/ChangeLog
2019-08-21  Tom Tromey  <tromey@adacore.com>

	* gdb.texinfo (Set Catchpoints, Convenience Vars): Document
	$_ada_exception.

gdb/testsuite/ChangeLog
2019-08-21  Tom Tromey  <tromey@adacore.com>

	* gdb.ada/catch_ex_std.exp: Add $_ada_exception test.
---
 gdb/ChangeLog                          |  32 +++
 gdb/NEWS                               |   3 +
 gdb/ada-lang.c                         | 307 +++++++------------------
 gdb/doc/ChangeLog                      |   5 +
 gdb/doc/gdb.texinfo                    |  20 +-
 gdb/testsuite/ChangeLog                |   4 +
 gdb/testsuite/gdb.ada/catch_ex_std.exp |   3 +
 7 files changed, 142 insertions(+), 232 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 1fefd814095..959ea27e749 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -34,6 +34,9 @@
 
 * GDB can now be compiled with Python 3 on Windows.
 
+* New convenience variable $_ada_exception holds the address of the
+  Ada exception being thrown.  This is set by Ada-related catchpoints.
+
 * Python API
 
   ** The gdb.Value type has a new method 'format_string' which returns a
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index e4702b96876..1f388eb4dba 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -12326,8 +12326,16 @@ public:
 
 struct ada_catchpoint : public breakpoint
 {
+  explicit ada_catchpoint (enum ada_exception_catchpoint_kind kind)
+    : m_kind (kind)
+  {
+  }
+
   /* The name of the specific exception the user specified.  */
   std::string excep_string;
+
+  /* What kind of catchpoint this is.  */
+  enum ada_exception_catchpoint_kind m_kind;
 };
 
 /* Parse the exception condition string in the context of each of the
@@ -12387,8 +12395,7 @@ create_excep_cond_exprs (struct ada_catchpoint *c,
    structure for all exception catchpoint kinds.  */
 
 static struct bp_location *
-allocate_location_exception (enum ada_exception_catchpoint_kind ex,
-			     struct breakpoint *self)
+allocate_location_exception (struct breakpoint *self)
 {
   return new ada_catchpoint_location (self);
 }
@@ -12397,7 +12404,7 @@ allocate_location_exception (enum ada_exception_catchpoint_kind ex,
    exception catchpoint kinds.  */
 
 static void
-re_set_exception (enum ada_exception_catchpoint_kind ex, struct breakpoint *b)
+re_set_exception (struct breakpoint *b)
 {
   struct ada_catchpoint *c = (struct ada_catchpoint *) b;
 
@@ -12407,7 +12414,7 @@ re_set_exception (enum ada_exception_catchpoint_kind ex, struct breakpoint *b)
 
   /* Reparse the exception conditional expressions.  One for each
      location.  */
-  create_excep_cond_exprs (c, ex);
+  create_excep_cond_exprs (c, c->m_kind);
 }
 
 /* Returns true if we should stop for this breakpoint hit.  If the
@@ -12422,6 +12429,30 @@ should_stop_exception (const struct bp_location *bl)
     = (const struct ada_catchpoint_location *) bl;
   int stop;
 
+  struct internalvar *var = lookup_internalvar ("_ada_exception");
+  if (c->m_kind == ada_catch_assert)
+    clear_internalvar (var);
+  else
+    {
+      try
+	{
+	  const char *expr;
+
+	  if (c->m_kind == ada_catch_handlers)
+	    expr = ("GNAT_GCC_exception_Access(gcc_exception)"
+		    ".all.occurrence.id");
+	  else
+	    expr = "e";
+
+	  struct value *exc = parse_and_eval (expr);
+	  set_internalvar (var, exc);
+	}
+      catch (const gdb_exception_error &ex)
+	{
+	  clear_internalvar (var);
+	}
+    }
+
   /* With no specific exception, should always stop.  */
   if (c->excep_string.empty ())
     return 1;
@@ -12455,7 +12486,7 @@ should_stop_exception (const struct bp_location *bl)
    for all exception catchpoint kinds.  */
 
 static void
-check_status_exception (enum ada_exception_catchpoint_kind ex, bpstat bs)
+check_status_exception (bpstat bs)
 {
   bs->stop = should_stop_exception (bs->bp_location_at);
 }
@@ -12464,7 +12495,7 @@ check_status_exception (enum ada_exception_catchpoint_kind ex, bpstat bs)
    for all exception catchpoint kinds.  */
 
 static enum print_stop_action
-print_it_exception (enum ada_exception_catchpoint_kind ex, bpstat bs)
+print_it_exception (bpstat bs)
 {
   struct ui_out *uiout = current_uiout;
   struct breakpoint *b = bs->breakpoint_at;
@@ -12490,13 +12521,14 @@ print_it_exception (enum ada_exception_catchpoint_kind ex, bpstat bs)
      ada_find_printable_frame).  */
   select_frame (get_current_frame ());
 
-  switch (ex)
+  struct ada_catchpoint *c = (struct ada_catchpoint *) b;
+  switch (c->m_kind)
     {
       case ada_catch_exception:
       case ada_catch_exception_unhandled:
       case ada_catch_handlers:
 	{
-	  const CORE_ADDR addr = ada_exception_name_addr (ex, b);
+	  const CORE_ADDR addr = ada_exception_name_addr (c->m_kind, b);
 	  char exception_name[256];
 
 	  if (addr != 0)
@@ -12520,7 +12552,7 @@ print_it_exception (enum ada_exception_catchpoint_kind ex, bpstat bs)
 	     it clearer to the user which kind of catchpoint just got
 	     hit.  We used ui_out_text to make sure that this extra
 	     info does not pollute the exception name in the MI case.  */
-	  if (ex == ada_catch_exception_unhandled)
+	  if (c->m_kind == ada_catch_exception_unhandled)
 	    uiout->text ("unhandled ");
 	  uiout->field_string ("exception-name", exception_name);
 	}
@@ -12553,8 +12585,7 @@ print_it_exception (enum ada_exception_catchpoint_kind ex, bpstat bs)
    for all exception catchpoint kinds.  */
 
 static void
-print_one_exception (enum ada_exception_catchpoint_kind ex,
-                     struct breakpoint *b, struct bp_location **last_loc)
+print_one_exception (struct breakpoint *b, struct bp_location **last_loc)
 { 
   struct ui_out *uiout = current_uiout;
   struct ada_catchpoint *c = (struct ada_catchpoint *) b;
@@ -12566,7 +12597,7 @@ print_one_exception (enum ada_exception_catchpoint_kind ex,
     uiout->field_skip ("addr");
 
   annotate_field (5);
-  switch (ex)
+  switch (c->m_kind)
     {
       case ada_catch_exception:
         if (!c->excep_string.empty ())
@@ -12610,8 +12641,7 @@ print_one_exception (enum ada_exception_catchpoint_kind ex,
    for all exception catchpoint kinds.  */
 
 static void
-print_mention_exception (enum ada_exception_catchpoint_kind ex,
-                         struct breakpoint *b)
+print_mention_exception (struct breakpoint *b)
 {
   struct ada_catchpoint *c = (struct ada_catchpoint *) b;
   struct ui_out *uiout = current_uiout;
@@ -12621,7 +12651,7 @@ print_mention_exception (enum ada_exception_catchpoint_kind ex,
   uiout->field_signed ("bkptno", b->number);
   uiout->text (": ");
 
-  switch (ex)
+  switch (c->m_kind)
     {
       case ada_catch_exception:
         if (!c->excep_string.empty ())
@@ -12664,12 +12694,11 @@ print_mention_exception (enum ada_exception_catchpoint_kind ex,
    for all exception catchpoint kinds.  */
 
 static void
-print_recreate_exception (enum ada_exception_catchpoint_kind ex,
-			  struct breakpoint *b, struct ui_file *fp)
+print_recreate_exception (struct breakpoint *b, struct ui_file *fp)
 {
   struct ada_catchpoint *c = (struct ada_catchpoint *) b;
 
-  switch (ex)
+  switch (c->m_kind)
     {
       case ada_catch_exception:
 	fprintf_filtered (fp, "catch exception");
@@ -12695,192 +12724,10 @@ print_recreate_exception (enum ada_exception_catchpoint_kind ex,
   print_recreate_thread (b, fp);
 }
 
-/* Virtual table for "catch exception" breakpoints.  */
-
-static struct bp_location *
-allocate_location_catch_exception (struct breakpoint *self)
-{
-  return allocate_location_exception (ada_catch_exception, self);
-}
-
-static void
-re_set_catch_exception (struct breakpoint *b)
-{
-  re_set_exception (ada_catch_exception, b);
-}
-
-static void
-check_status_catch_exception (bpstat bs)
-{
-  check_status_exception (ada_catch_exception, bs);
-}
-
-static enum print_stop_action
-print_it_catch_exception (bpstat bs)
-{
-  return print_it_exception (ada_catch_exception, bs);
-}
-
-static void
-print_one_catch_exception (struct breakpoint *b, struct bp_location **last_loc)
-{
-  print_one_exception (ada_catch_exception, b, last_loc);
-}
-
-static void
-print_mention_catch_exception (struct breakpoint *b)
-{
-  print_mention_exception (ada_catch_exception, b);
-}
-
-static void
-print_recreate_catch_exception (struct breakpoint *b, struct ui_file *fp)
-{
-  print_recreate_exception (ada_catch_exception, b, fp);
-}
-
+/* Virtual tables for various breakpoint types.  */
 static struct breakpoint_ops catch_exception_breakpoint_ops;
-
-/* Virtual table for "catch exception unhandled" breakpoints.  */
-
-static struct bp_location *
-allocate_location_catch_exception_unhandled (struct breakpoint *self)
-{
-  return allocate_location_exception (ada_catch_exception_unhandled, self);
-}
-
-static void
-re_set_catch_exception_unhandled (struct breakpoint *b)
-{
-  re_set_exception (ada_catch_exception_unhandled, b);
-}
-
-static void
-check_status_catch_exception_unhandled (bpstat bs)
-{
-  check_status_exception (ada_catch_exception_unhandled, bs);
-}
-
-static enum print_stop_action
-print_it_catch_exception_unhandled (bpstat bs)
-{
-  return print_it_exception (ada_catch_exception_unhandled, bs);
-}
-
-static void
-print_one_catch_exception_unhandled (struct breakpoint *b,
-				     struct bp_location **last_loc)
-{
-  print_one_exception (ada_catch_exception_unhandled, b, last_loc);
-}
-
-static void
-print_mention_catch_exception_unhandled (struct breakpoint *b)
-{
-  print_mention_exception (ada_catch_exception_unhandled, b);
-}
-
-static void
-print_recreate_catch_exception_unhandled (struct breakpoint *b,
-					  struct ui_file *fp)
-{
-  print_recreate_exception (ada_catch_exception_unhandled, b, fp);
-}
-
 static struct breakpoint_ops catch_exception_unhandled_breakpoint_ops;
-
-/* Virtual table for "catch assert" breakpoints.  */
-
-static struct bp_location *
-allocate_location_catch_assert (struct breakpoint *self)
-{
-  return allocate_location_exception (ada_catch_assert, self);
-}
-
-static void
-re_set_catch_assert (struct breakpoint *b)
-{
-  re_set_exception (ada_catch_assert, b);
-}
-
-static void
-check_status_catch_assert (bpstat bs)
-{
-  check_status_exception (ada_catch_assert, bs);
-}
-
-static enum print_stop_action
-print_it_catch_assert (bpstat bs)
-{
-  return print_it_exception (ada_catch_assert, bs);
-}
-
-static void
-print_one_catch_assert (struct breakpoint *b, struct bp_location **last_loc)
-{
-  print_one_exception (ada_catch_assert, b, last_loc);
-}
-
-static void
-print_mention_catch_assert (struct breakpoint *b)
-{
-  print_mention_exception (ada_catch_assert, b);
-}
-
-static void
-print_recreate_catch_assert (struct breakpoint *b, struct ui_file *fp)
-{
-  print_recreate_exception (ada_catch_assert, b, fp);
-}
-
 static struct breakpoint_ops catch_assert_breakpoint_ops;
-
-/* Virtual table for "catch handlers" breakpoints.  */
-
-static struct bp_location *
-allocate_location_catch_handlers (struct breakpoint *self)
-{
-  return allocate_location_exception (ada_catch_handlers, self);
-}
-
-static void
-re_set_catch_handlers (struct breakpoint *b)
-{
-  re_set_exception (ada_catch_handlers, b);
-}
-
-static void
-check_status_catch_handlers (bpstat bs)
-{
-  check_status_exception (ada_catch_handlers, bs);
-}
-
-static enum print_stop_action
-print_it_catch_handlers (bpstat bs)
-{
-  return print_it_exception (ada_catch_handlers, bs);
-}
-
-static void
-print_one_catch_handlers (struct breakpoint *b,
-			  struct bp_location **last_loc)
-{
-  print_one_exception (ada_catch_handlers, b, last_loc);
-}
-
-static void
-print_mention_catch_handlers (struct breakpoint *b)
-{
-  print_mention_exception (ada_catch_handlers, b);
-}
-
-static void
-print_recreate_catch_handlers (struct breakpoint *b,
-			       struct ui_file *fp)
-{
-  print_recreate_exception (ada_catch_handlers, b, fp);
-}
-
 static struct breakpoint_ops catch_handlers_breakpoint_ops;
 
 /* See ada-lang.h.  */
@@ -13154,7 +13001,7 @@ create_ada_exception_catchpoint (struct gdbarch *gdbarch,
   const struct breakpoint_ops *ops = NULL;
   struct symtab_and_line sal = ada_exception_sal (ex_kind, &addr_string, &ops);
 
-  std::unique_ptr<ada_catchpoint> c (new ada_catchpoint ());
+  std::unique_ptr<ada_catchpoint> c (new ada_catchpoint (ex_kind));
   init_ada_exception_breakpoint (c.get (), gdbarch, sal, addr_string.c_str (),
 				 ops, tempflag, disabled, from_tty);
   c->excep_string = excep_string;
@@ -14345,43 +14192,43 @@ initialize_ada_catchpoint_ops (void)
 
   ops = &catch_exception_breakpoint_ops;
   *ops = bkpt_breakpoint_ops;
-  ops->allocate_location = allocate_location_catch_exception;
-  ops->re_set = re_set_catch_exception;
-  ops->check_status = check_status_catch_exception;
-  ops->print_it = print_it_catch_exception;
-  ops->print_one = print_one_catch_exception;
-  ops->print_mention = print_mention_catch_exception;
-  ops->print_recreate = print_recreate_catch_exception;
+  ops->allocate_location = allocate_location_exception;
+  ops->re_set = re_set_exception;
+  ops->check_status = check_status_exception;
+  ops->print_it = print_it_exception;
+  ops->print_one = print_one_exception;
+  ops->print_mention = print_mention_exception;
+  ops->print_recreate = print_recreate_exception;
 
   ops = &catch_exception_unhandled_breakpoint_ops;
   *ops = bkpt_breakpoint_ops;
-  ops->allocate_location = allocate_location_catch_exception_unhandled;
-  ops->re_set = re_set_catch_exception_unhandled;
-  ops->check_status = check_status_catch_exception_unhandled;
-  ops->print_it = print_it_catch_exception_unhandled;
-  ops->print_one = print_one_catch_exception_unhandled;
-  ops->print_mention = print_mention_catch_exception_unhandled;
-  ops->print_recreate = print_recreate_catch_exception_unhandled;
+  ops->allocate_location = allocate_location_exception;
+  ops->re_set = re_set_exception;
+  ops->check_status = check_status_exception;
+  ops->print_it = print_it_exception;
+  ops->print_one = print_one_exception;
+  ops->print_mention = print_mention_exception;
+  ops->print_recreate = print_recreate_exception;
 
   ops = &catch_assert_breakpoint_ops;
   *ops = bkpt_breakpoint_ops;
-  ops->allocate_location = allocate_location_catch_assert;
-  ops->re_set = re_set_catch_assert;
-  ops->check_status = check_status_catch_assert;
-  ops->print_it = print_it_catch_assert;
-  ops->print_one = print_one_catch_assert;
-  ops->print_mention = print_mention_catch_assert;
-  ops->print_recreate = print_recreate_catch_assert;
+  ops->allocate_location = allocate_location_exception;
+  ops->re_set = re_set_exception;
+  ops->check_status = check_status_exception;
+  ops->print_it = print_it_exception;
+  ops->print_one = print_one_exception;
+  ops->print_mention = print_mention_exception;
+  ops->print_recreate = print_recreate_exception;
 
   ops = &catch_handlers_breakpoint_ops;
   *ops = bkpt_breakpoint_ops;
-  ops->allocate_location = allocate_location_catch_handlers;
-  ops->re_set = re_set_catch_handlers;
-  ops->check_status = check_status_catch_handlers;
-  ops->print_it = print_it_catch_handlers;
-  ops->print_one = print_one_catch_handlers;
-  ops->print_mention = print_mention_catch_handlers;
-  ops->print_recreate = print_recreate_catch_handlers;
+  ops->allocate_location = allocate_location_exception;
+  ops->re_set = re_set_exception;
+  ops->check_status = check_status_exception;
+  ops->print_it = print_it_exception;
+  ops->print_one = print_one_exception;
+  ops->print_mention = print_mention_exception;
+  ops->print_recreate = print_recreate_exception;
 }
 
 /* This module's 'new_objfile' observer.  */
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a129ea0aa1b..c3ed62002f8 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -4788,9 +4788,16 @@ called @code{Constraint_Error} is defined in package @code{Pck}, then
 the command to use to catch such exceptions is @kbd{catch exception
 Pck.Constraint_Error}.
 
+@vindex $_ada_exception@r{, convenience variable}
+The convenience variable @code{$_ada_exception} holds the address of
+the exception being thrown.  This can be useful when setting a
+condition for such a catchpoint.
+
 @item exception unhandled
 @kindex catch exception unhandled
-An exception that was raised but is not handled by the program.
+An exception that was raised but is not handled by the program.  The
+convenience variable @code{$_ada_exception} is set as for @code{catch
+exception}.
 
 @item handlers @r{[}@var{name}@r{]}
 @kindex catch handlers
@@ -4812,9 +4819,13 @@ user-defined one.  For instance, assuming an exception called
 command to use to catch such exceptions handling is
 @kbd{catch handlers Pck.Constraint_Error}.
 
+The convenience variable @code{$_ada_exception} is set as for
+@code{catch exception}.
+
 @item assert
 @kindex catch assert
-A failed Ada assertion.
+A failed Ada assertion.  Note that the convenience variable
+@code{$_ada_exception} is @emph{not} set by this catchpoint.
 
 @item exec
 @kindex catch exec
@@ -11823,6 +11834,11 @@ The program has exited
 The variable @code{$_exception} is set to the exception object being
 thrown at an exception-related catchpoint.  @xref{Set Catchpoints}.
 
+@item $_ada_exception
+The variable @code{$_ada_exception} is set to the address of the
+exception being caught or thrown at an Ada exception-related
+catchpoint.  @xref{Set Catchpoints}.
+
 @item $_probe_argc
 @itemx $_probe_arg0@dots{}$_probe_arg11
 Arguments to a static probe.  @xref{Static Probe Points}.
diff --git a/gdb/testsuite/gdb.ada/catch_ex_std.exp b/gdb/testsuite/gdb.ada/catch_ex_std.exp
index 63714a8aa81..839d0bb092f 100644
--- a/gdb/testsuite/gdb.ada/catch_ex_std.exp
+++ b/gdb/testsuite/gdb.ada/catch_ex_std.exp
@@ -101,3 +101,6 @@ gdb_test "catch exception some_kind_of_error" \
 gdb_test "cont" \
     "Catchpoint \[0-9\]+, .* at .*foo\.adb:\[0-9\]+.*" \
     "caught the exception"
+
+gdb_test "print \$_ada_exception = some_package.some_kind_of_error'Address" \
+    " = true"
-- 
2.20.1

[PATCH v2 1/8] Change SYMBOL_VALUE_ADDRESS to be an rvalue

[Tom Tromey]

This changes SYMBOL_VALUE_ADDRESS to be an rvalue.  The symbol readers
generally assign using this, so this also introduces
SET_SYMBOL_VALUE_ADDRESS and updates the readers.  Making this change
is useful in a subsequent patch, which redefined SYMBOL_VALUE_ADDRESS.

gdb/ChangeLog
2019-08-21  Tom Tromey  <tromey@adacore.com>

	* coffread.c (process_coff_symbol): Update.
	* dwarf2read.c (var_decode_location, new_symbol): Update.
	* mdebugread.c (parse_symbol): Update.
	* objfiles.c (relocate_one_symbol): Update.
	* stabsread.c (define_symbol, fix_common_block)
	(scan_file_globals): Update.
	* symtab.h (SYMBOL_VALUE_ADDRESS): Expand to an rvalue.
	(SET_SYMBOL_VALUE_ADDRESS): New macro.
	* xcoffread.c (process_xcoff_symbol): Update.
---
 gdb/ChangeLog    | 12 ++++++++++++
 gdb/coffread.c   | 14 ++++++++------
 gdb/dwarf2read.c | 19 ++++++++++++-------
 gdb/mdebugread.c |  6 +++---
 gdb/objfiles.c   |  4 +++-
 gdb/stabsread.c  | 22 +++++++++++++---------
 gdb/symtab.h     |  4 +++-
 gdb/xcoffread.c  |  6 ++++--
 8 files changed, 58 insertions(+), 29 deletions(-)

diff --git a/gdb/coffread.c b/gdb/coffread.c
index c44b69069e4..869a32b2c19 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -1609,9 +1609,10 @@ process_coff_symbol (struct coff_symbol *cs,
 	case C_THUMBEXTFUNC:
 	case C_EXT:
 	  SYMBOL_ACLASS_INDEX (sym) = LOC_STATIC;
-	  SYMBOL_VALUE_ADDRESS (sym) = (CORE_ADDR) cs->c_value;
-	  SYMBOL_VALUE_ADDRESS (sym) += ANOFFSET (objfile->section_offsets,
-						  SECT_OFF_TEXT (objfile));
+	  SET_SYMBOL_VALUE_ADDRESS (sym,
+				    (CORE_ADDR) cs->c_value
+				    + ANOFFSET (objfile->section_offsets,
+						SECT_OFF_TEXT (objfile)));
 	  add_symbol_to_list (sym, get_global_symbols ());
 	  break;
 
@@ -1619,9 +1620,10 @@ process_coff_symbol (struct coff_symbol *cs,
 	case C_THUMBSTATFUNC:
 	case C_STAT:
 	  SYMBOL_ACLASS_INDEX (sym) = LOC_STATIC;
-	  SYMBOL_VALUE_ADDRESS (sym) = (CORE_ADDR) cs->c_value;
-	  SYMBOL_VALUE_ADDRESS (sym) += ANOFFSET (objfile->section_offsets,
-						  SECT_OFF_TEXT (objfile));
+	  SET_SYMBOL_VALUE_ADDRESS (sym,
+				    (CORE_ADDR) cs->c_value
+				    + ANOFFSET (objfile->section_offsets,
+						SECT_OFF_TEXT (objfile)));
 	  if (within_function)
 	    {
 	      /* Static symbol of local scope.  */
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 1052501c351..4cc558fce2a 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -21484,15 +21484,20 @@ var_decode_location (struct attribute *attr, struct symbol *sym,
       unsigned int dummy;
 
       if (DW_BLOCK (attr)->data[0] == DW_OP_addr)
-	SYMBOL_VALUE_ADDRESS (sym) =
-	  read_address (objfile->obfd, DW_BLOCK (attr)->data + 1, cu, &dummy);
+	SET_SYMBOL_VALUE_ADDRESS (sym,
+				  read_address (objfile->obfd,
+						DW_BLOCK (attr)->data + 1,
+						cu, &dummy));
       else
-	SYMBOL_VALUE_ADDRESS (sym) =
-	  read_addr_index_from_leb128 (cu, DW_BLOCK (attr)->data + 1, &dummy);
+	SET_SYMBOL_VALUE_ADDRESS
+	  (sym, read_addr_index_from_leb128 (cu, DW_BLOCK (attr)->data + 1,
+					     &dummy));
       SYMBOL_ACLASS_INDEX (sym) = LOC_STATIC;
       fixup_symbol_section (sym, objfile);
-      SYMBOL_VALUE_ADDRESS (sym) += ANOFFSET (objfile->section_offsets,
-					      SYMBOL_SECTION (sym));
+      SET_SYMBOL_VALUE_ADDRESS (sym,
+				SYMBOL_VALUE_ADDRESS (sym)
+				+ ANOFFSET (objfile->section_offsets,
+					    SYMBOL_SECTION (sym)));
       return;
     }
 
@@ -21606,7 +21611,7 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 
 	      addr = attr_value_as_address (attr);
 	      addr = gdbarch_adjust_dwarf2_addr (gdbarch, addr + baseaddr);
-	      SYMBOL_VALUE_ADDRESS (sym) = addr;
+	      SET_SYMBOL_VALUE_ADDRESS (sym, addr);
 	    }
 	  SYMBOL_TYPE (sym) = objfile_type (objfile)->builtin_core_addr;
 	  SYMBOL_DOMAIN (sym) = LABEL_DOMAIN;
diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c
index 0956fbdb675..eed714636dc 100644
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -632,7 +632,7 @@ parse_symbol (SYMR *sh, union aux_ext *ax, char *ext_sh, int bigend,
       b = BLOCKVECTOR_BLOCK (SYMTAB_BLOCKVECTOR (top_stack->cur_st),
 			     GLOBAL_BLOCK);
       s = new_symbol (name);
-      SYMBOL_VALUE_ADDRESS (s) = (CORE_ADDR) sh->value;
+      SET_SYMBOL_VALUE_ADDRESS (s, (CORE_ADDR) sh->value);
       add_data_symbol (sh, ax, bigend, s, LOC_STATIC, b, objfile, name);
       break;
 
@@ -649,7 +649,7 @@ parse_symbol (SYMR *sh, union aux_ext *ax, char *ext_sh, int bigend,
 	  global_sym_chain[bucket] = s;
 	}
       else
-	SYMBOL_VALUE_ADDRESS (s) = (CORE_ADDR) sh->value;
+	SET_SYMBOL_VALUE_ADDRESS (s, (CORE_ADDR) sh->value);
       add_data_symbol (sh, ax, bigend, s, LOC_STATIC, b, objfile, name);
       break;
 
@@ -706,7 +706,7 @@ parse_symbol (SYMR *sh, union aux_ext *ax, char *ext_sh, int bigend,
       s = new_symbol (name);
       SYMBOL_DOMAIN (s) = VAR_DOMAIN;	/* So that it can be used */
       SYMBOL_ACLASS_INDEX (s) = LOC_LABEL;	/* but not misused.  */
-      SYMBOL_VALUE_ADDRESS (s) = (CORE_ADDR) sh->value;
+      SET_SYMBOL_VALUE_ADDRESS (s, (CORE_ADDR) sh->value);
       SYMBOL_TYPE (s) = objfile_type (objfile)->builtin_int;
       add_symbol (s, top_stack->cur_st, top_stack->cur_block);
       break;
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index 90c4650bd47..f9e7d20bab5 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -707,7 +707,9 @@ relocate_one_symbol (struct symbol *sym, struct objfile *objfile,
        || SYMBOL_CLASS (sym) == LOC_STATIC)
       && SYMBOL_SECTION (sym) >= 0)
     {
-      SYMBOL_VALUE_ADDRESS (sym) += ANOFFSET (delta, SYMBOL_SECTION (sym));
+      SET_SYMBOL_VALUE_ADDRESS (sym,
+				SYMBOL_VALUE_ADDRESS (sym)
+				+ ANOFFSET (delta, SYMBOL_SECTION (sym)));
     }
 }
 
diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index 48c88ded553..da455da3658 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -942,7 +942,7 @@ define_symbol (CORE_ADDR valu, const char *string, int desc, int type,
       SYMBOL_TYPE (sym) = read_type (&p, objfile);
       SYMBOL_ACLASS_INDEX (sym) = LOC_LABEL;
       SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
-      SYMBOL_VALUE_ADDRESS (sym) = valu;
+      SET_SYMBOL_VALUE_ADDRESS (sym, valu);
       add_symbol_to_list (sym, get_local_symbols ());
       break;
 
@@ -1188,7 +1188,7 @@ define_symbol (CORE_ADDR valu, const char *string, int desc, int type,
       /* Static symbol at top level of file.  */
       SYMBOL_TYPE (sym) = read_type (&p, objfile);
       SYMBOL_ACLASS_INDEX (sym) = LOC_STATIC;
-      SYMBOL_VALUE_ADDRESS (sym) = valu;
+      SET_SYMBOL_VALUE_ADDRESS (sym, valu);
       if (gdbarch_static_transform_name_p (gdbarch)
 	  && gdbarch_static_transform_name (gdbarch,
 					    SYMBOL_LINKAGE_NAME (sym))
@@ -1204,7 +1204,8 @@ define_symbol (CORE_ADDR valu, const char *string, int desc, int type,
 		(gdbarch, SYMBOL_LINKAGE_NAME (sym));
 
 	      SYMBOL_SET_LINKAGE_NAME (sym, new_name);
-	      SYMBOL_VALUE_ADDRESS (sym) = BMSYMBOL_VALUE_ADDRESS (msym);
+	      SET_SYMBOL_VALUE_ADDRESS (sym,
+					BMSYMBOL_VALUE_ADDRESS (msym));
 	    }
 	}
       SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
@@ -1380,7 +1381,7 @@ define_symbol (CORE_ADDR valu, const char *string, int desc, int type,
       /* Static symbol of local scope.  */
       SYMBOL_TYPE (sym) = read_type (&p, objfile);
       SYMBOL_ACLASS_INDEX (sym) = LOC_STATIC;
-      SYMBOL_VALUE_ADDRESS (sym) = valu;
+      SET_SYMBOL_VALUE_ADDRESS (sym, valu);
       if (gdbarch_static_transform_name_p (gdbarch)
 	  && gdbarch_static_transform_name (gdbarch,
 					    SYMBOL_LINKAGE_NAME (sym))
@@ -1396,7 +1397,7 @@ define_symbol (CORE_ADDR valu, const char *string, int desc, int type,
 		(gdbarch, SYMBOL_LINKAGE_NAME (sym));
 
 	      SYMBOL_SET_LINKAGE_NAME (sym, new_name);
-	      SYMBOL_VALUE_ADDRESS (sym) = BMSYMBOL_VALUE_ADDRESS (msym);
+	      SET_SYMBOL_VALUE_ADDRESS (sym, BMSYMBOL_VALUE_ADDRESS (msym));
 	    }
 	}
       SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
@@ -4363,7 +4364,9 @@ fix_common_block (struct symbol *sym, CORE_ADDR valu)
       int j;
 
       for (j = next->nsyms - 1; j >= 0; j--)
-	SYMBOL_VALUE_ADDRESS (next->symbol[j]) += valu;
+	SET_SYMBOL_VALUE_ADDRESS (next->symbol[j],
+				  SYMBOL_VALUE_ADDRESS (next->symbol[j])
+				  + valu);
     }
 }
 \f
@@ -4641,8 +4644,9 @@ scan_file_globals (struct objfile *objfile)
 			}
 		      else
 			{
-			  SYMBOL_VALUE_ADDRESS (sym)
-			    = MSYMBOL_VALUE_ADDRESS (resolve_objfile, msymbol);
+			  SET_SYMBOL_VALUE_ADDRESS
+			    (sym, MSYMBOL_VALUE_ADDRESS (resolve_objfile,
+							 msymbol));
 			}
 		      SYMBOL_SECTION (sym) = MSYMBOL_SECTION (msymbol);
 		    }
@@ -4680,7 +4684,7 @@ scan_file_globals (struct objfile *objfile)
 
 	  /* Change the symbol address from the misleading chain value
 	     to address zero.  */
-	  SYMBOL_VALUE_ADDRESS (prev) = 0;
+	  SET_SYMBOL_VALUE_ADDRESS (prev, 0);
 
 	  /* Complain about unresolved common block symbols.  */
 	  if (SYMBOL_CLASS (prev) == LOC_STATIC)
diff --git a/gdb/symtab.h b/gdb/symtab.h
index d0465987749..2d5667b620c 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -463,7 +463,9 @@ extern CORE_ADDR symbol_overlayed_address (CORE_ADDR, struct obj_section *);
    field only, instead of the SYMBOL parameter.  */
 
 #define SYMBOL_VALUE(symbol)		(symbol)->ginfo.value.ivalue
-#define SYMBOL_VALUE_ADDRESS(symbol)	(symbol)->ginfo.value.address
+#define SYMBOL_VALUE_ADDRESS(symbol)	((symbol)->ginfo.value.address + 0)
+#define SET_SYMBOL_VALUE_ADDRESS(symbol, new_value)	\
+  ((symbol)->ginfo.value.address = (new_value))
 #define SYMBOL_VALUE_BYTES(symbol)	(symbol)->ginfo.value.bytes
 #define SYMBOL_VALUE_COMMON_BLOCK(symbol) (symbol)->ginfo.value.common_block
 #define SYMBOL_BLOCK_VALUE(symbol)	(symbol)->ginfo.value.block
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 2fa27066539..aec19237214 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -1576,7 +1576,7 @@ process_xcoff_symbol (struct coff_symbol *cs, struct objfile *objfile)
   initialize_objfile_symbol (sym);
 
   /* default assumptions */
-  SYMBOL_VALUE_ADDRESS (sym) = cs->c_value + off;
+  SET_SYMBOL_VALUE_ADDRESS (sym, cs->c_value + off);
   SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
   SYMBOL_SECTION (sym) = secnum_to_section (cs->c_secnum, objfile);
 
@@ -1674,7 +1674,9 @@ process_xcoff_symbol (struct coff_symbol *cs, struct objfile *objfile)
 			       cs->c_name, 0, 0, objfile);
 	  if (sym != NULL)
 	    {
-	      SYMBOL_VALUE_ADDRESS (sym) += static_block_base;
+	      SET_SYMBOL_VALUE_ADDRESS (sym,
+					SYMBOL_VALUE_ADDRESS (sym)
+					+ static_block_base);
 	      SYMBOL_SECTION (sym) = static_block_section;
 	    }
 	  return sym;
-- 
2.20.1

[PATCH v2 4/8] Make current_source_* per-program-space

[Tom Tromey]

This changes current_source_symtab and current_source_line to be
per-program-space.  This ensures that switching inferiors will
preserve the current "list" location for that inferior, and also
ensures that the default expression evaluation context always comes
with the current inferior.

No test case, because the latter problem crops up with an existing
gdb.multi test case once this entire series has been applied.

2019-08-21  Tom Tromey  <tromey@adacore.com>

	* source.c (struct current_source_location): New.
	(current_source_key): New global.
	(current_source_symtab, current_source_line)
	(current_source_pspace): Remove.
	(get_source_location): New function.
	(get_current_source_symtab_and_line)
	(set_default_source_symtab_and_line)
	(set_current_source_symtab_and_line)
	(clear_current_source_symtab_and_line, select_source_symtab)
	(info_source_command, print_source_lines_base)
	(info_line_command, search_command_helper, _initialize_source):
	Update.
---
 gdb/ChangeLog |  15 ++++++
 gdb/source.c  | 124 +++++++++++++++++++++++++++++++-------------------
 2 files changed, 91 insertions(+), 48 deletions(-)

diff --git a/gdb/source.c b/gdb/source.c
index 77eced31628..7bee2a4420b 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -66,15 +66,20 @@ struct substitute_path_rule
 
 static struct substitute_path_rule *substitute_path_rules = NULL;
 
-/* Symtab of default file for listing lines of.  */
+/* An instance of this is attached to each program space.  */
 
-static struct symtab *current_source_symtab;
+struct current_source_location
+{
+  /* Symtab of default file for listing lines of.  */
+
+  struct symtab *symtab = nullptr;
 
-/* Default next line to list.  */
+  /* Default next line to list.  */
 
-static int current_source_line;
+  int line = 0;
+};
 
-static struct program_space *current_source_pspace;
+static program_space_key<current_source_location> current_source_key;
 
 /* Default number of lines to print with commands like "list".
    This is based on guessing how many long (i.e. more than chars_per_line
@@ -162,6 +167,19 @@ get_lines_to_list (void)
   return lines_to_list;
 }
 
+/* A helper to return the current source location object for PSPACE,
+   creating it if it does not exist.  */
+
+static current_source_location *
+get_source_location (program_space *pspace)
+{
+  current_source_location *loc
+    = current_source_key.get (pspace);
+  if (loc == nullptr)
+    loc = current_source_key.emplace (pspace);
+  return loc;
+}
+
 /* Return the current source file for listing and next line to list.
    NOTE: The returned sal pc and end fields are not valid.  */
    
@@ -169,10 +187,11 @@ struct symtab_and_line
 get_current_source_symtab_and_line (void)
 {
   symtab_and_line cursal;
+  current_source_location *loc = get_source_location (current_program_space);
 
-  cursal.pspace = current_source_pspace;
-  cursal.symtab = current_source_symtab;
-  cursal.line = current_source_line;
+  cursal.pspace = current_program_space;
+  cursal.symtab = loc->symtab;
+  cursal.line = loc->line;
   cursal.pc = 0;
   cursal.end = 0;
   
@@ -194,7 +213,8 @@ set_default_source_symtab_and_line (void)
     error (_("No symbol table is loaded.  Use the \"file\" command."));
 
   /* Pull in a current source symtab if necessary.  */
-  if (current_source_symtab == 0)
+  current_source_location *loc = get_source_location (current_program_space);
+  if (loc->symtab == nullptr)
     select_source_symtab (0);
 }
 
@@ -208,15 +228,16 @@ set_current_source_symtab_and_line (const symtab_and_line &sal)
 {
   symtab_and_line cursal;
 
-  cursal.pspace = current_source_pspace;
-  cursal.symtab = current_source_symtab;
-  cursal.line = current_source_line;
+  current_source_location *loc = get_source_location (sal.pspace);
+
+  cursal.pspace = sal.pspace;
+  cursal.symtab = loc->symtab;
+  cursal.line = loc->line;
   cursal.pc = 0;
   cursal.end = 0;
 
-  current_source_pspace = sal.pspace;
-  current_source_symtab = sal.symtab;
-  current_source_line = sal.line;
+  loc->symtab = sal.symtab;
+  loc->line = sal.line;
 
   /* Force the next "list" to center around the current line.  */
   clear_lines_listed_range ();
@@ -229,8 +250,9 @@ set_current_source_symtab_and_line (const symtab_and_line &sal)
 void
 clear_current_source_symtab_and_line (void)
 {
-  current_source_symtab = 0;
-  current_source_line = 0;
+  current_source_location *loc = get_source_location (current_program_space);
+  loc->symtab = nullptr;
+  loc->line = 0;
 }
 
 /* See source.h.  */
@@ -240,13 +262,15 @@ select_source_symtab (struct symtab *s)
 {
   if (s)
     {
-      current_source_symtab = s;
-      current_source_line = 1;
-      current_source_pspace = SYMTAB_PSPACE (s);
+      current_source_location *loc
+	= get_source_location (SYMTAB_PSPACE (s));
+      loc->symtab = s;
+      loc->line = 1;
       return;
     }
 
-  if (current_source_symtab)
+  current_source_location *loc = get_source_location (current_program_space);
+  if (loc->symtab != nullptr)
     return;
 
   /* Make the default place to list be the function `main'
@@ -255,16 +279,15 @@ select_source_symtab (struct symtab *s)
   if (bsym.symbol != nullptr && SYMBOL_CLASS (bsym.symbol) == LOC_BLOCK)
     {
       symtab_and_line sal = find_function_start_sal (bsym.symbol, true);
-      current_source_pspace = sal.pspace;
-      current_source_symtab = sal.symtab;
-      current_source_line = std::max (sal.line - (lines_to_list - 1), 1);
+      loc->symtab = sal.symtab;
+      loc->line = std::max (sal.line - (lines_to_list - 1), 1);
       return;
     }
 
   /* Alright; find the last file in the symtab list (ignoring .h's
      and namespace symtabs).  */
 
-  current_source_line = 1;
+  loc->line = 1;
 
   for (objfile *ofp : current_program_space->objfiles ())
     {
@@ -277,15 +300,12 @@ select_source_symtab (struct symtab *s)
 
 	      if (!(len > 2 && (strcmp (&name[len - 2], ".h") == 0
 				|| strcmp (name, "<<C++-namespaces>>") == 0)))
-		{
-		  current_source_pspace = current_program_space;
-		  current_source_symtab = symtab;
-		}
+		loc->symtab = symtab;
 	    }
 	}
     }
 
-  if (current_source_symtab)
+  if (loc->symtab != nullptr)
     return;
 
   for (objfile *objfile : current_program_space->objfiles ())
@@ -293,9 +313,9 @@ select_source_symtab (struct symtab *s)
       if (objfile->sf)
 	s = objfile->sf->qf->find_last_source_symtab (objfile);
       if (s)
-	current_source_symtab = s;
+	loc->symtab = s;
     }
-  if (current_source_symtab)
+  if (loc->symtab != nullptr)
     return;
 
   error (_("Can't find a default source file"));
@@ -619,7 +639,9 @@ add_path (const char *dirname, char **which_path, int parse_separators)
 static void
 info_source_command (const char *ignore, int from_tty)
 {
-  struct symtab *s = current_source_symtab;
+  current_source_location *loc
+    = get_source_location (current_program_space);
+  struct symtab *s = loc->symtab;
   struct compunit_symtab *cust;
 
   if (!s)
@@ -1178,8 +1200,11 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
   struct ui_out *uiout = current_uiout;
 
   /* Regardless of whether we can open the file, set current_source_symtab.  */
-  current_source_symtab = s;
-  current_source_line = line;
+  current_source_location *loc
+    = get_source_location (current_program_space);
+
+  loc->symtab = s;
+  loc->line = line;
   first_line_listed = line;
 
   /* If printing of source lines is disabled, just print file and line
@@ -1273,13 +1298,13 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
     {
       char buf[20];
 
-      last_line_listed = current_source_line;
+      last_line_listed = loc->line;
       if (flags & PRINT_SOURCE_LINES_FILENAME)
         {
           uiout->text (symtab_to_filename_for_display (s));
           uiout->text (":");
         }
-      xsnprintf (buf, sizeof (buf), "%d\t", current_source_line++);
+      xsnprintf (buf, sizeof (buf), "%d\t", loc->line++);
       uiout->text (buf);
 
       while (*iter != '\0')
@@ -1371,12 +1396,14 @@ info_line_command (const char *arg, int from_tty)
 
   if (arg == 0)
     {
-      curr_sal.symtab = current_source_symtab;
+      current_source_location *loc
+	= get_source_location (current_program_space);
+      curr_sal.symtab = loc->symtab;
       curr_sal.pspace = current_program_space;
       if (last_line_listed != 0)
 	curr_sal.line = last_line_listed;
       else
-	curr_sal.line = current_source_line;
+	curr_sal.line = loc->line;
 
       sals = curr_sal;
     }
@@ -1478,12 +1505,14 @@ search_command_helper (const char *regex, int from_tty, bool forward)
   if (msg)
     error (("%s"), msg);
 
-  if (current_source_symtab == 0)
+  current_source_location *loc
+    = get_source_location (current_program_space);
+  if (loc->symtab == nullptr)
     select_source_symtab (0);
 
-  scoped_fd desc (open_source_file (current_source_symtab));
+  scoped_fd desc (open_source_file (loc->symtab));
   if (desc.get () < 0)
-    perror_with_name (symtab_to_filename_for_display (current_source_symtab));
+    perror_with_name (symtab_to_filename_for_display (loc->symtab));
 
   int line = (forward
 	      ? last_line_listed + 1
@@ -1491,12 +1520,12 @@ search_command_helper (const char *regex, int from_tty, bool forward)
 
   const std::vector<off_t> *offsets;
   if (line < 1
-      || !g_source_cache.get_line_charpos (current_source_symtab, &offsets)
+      || !g_source_cache.get_line_charpos (loc->symtab, &offsets)
       || line > offsets->size ())
     error (_("Expression not found"));
 
   if (lseek (desc.get (), (*offsets)[line - 1], 0) < 0)
-    perror_with_name (symtab_to_filename_for_display (current_source_symtab));
+    perror_with_name (symtab_to_filename_for_display (loc->symtab));
 
   gdb_file_up stream = desc.to_file (FDOPEN_MODE);
   clearerr (stream.get ());
@@ -1531,9 +1560,9 @@ search_command_helper (const char *regex, int from_tty, bool forward)
       if (re_exec (buf.data ()) > 0)
 	{
 	  /* Match!  */
-	  print_source_lines (current_source_symtab, line, line + 1, 0);
+	  print_source_lines (loc->symtab, line, line + 1, 0);
 	  set_internalvar_integer (lookup_internalvar ("_"), line);
-	  current_source_line = std::max (line - lines_to_list / 2, 1);
+	  loc->line = std::max (line - lines_to_list / 2, 1);
 	  return;
 	}
 
@@ -1547,7 +1576,7 @@ search_command_helper (const char *regex, int from_tty, bool forward)
 	  if (fseek (stream.get (), (*offsets)[line - 1], 0) < 0)
 	    {
 	      const char *filename
-		= symtab_to_filename_for_display (current_source_symtab);
+		= symtab_to_filename_for_display (loc->symtab);
 	      perror_with_name (filename);
 	    }
 	}
@@ -1811,7 +1840,6 @@ _initialize_source (void)
 {
   struct cmd_list_element *c;
 
-  current_source_symtab = 0;
   init_source_path ();
 
   /* The intention is to use POSIX Basic Regular Expressions.
-- 
2.20.1

[PATCH v2 5/8] Handle copy relocations

[Tom Tromey]

In ELF, if a data symbol is defined in a shared library and used by
the main program, it will be subject to a "copy relocation".  In this
scenario, the main program has a copy of the symbol in question, and a
relocation that tells ld.so to copy the data from the shared library.
Then the symbol in the main program is used to satisfy all references.

This patch changes gdb to handle this scenario.  Data symbols coming
from ELF shared libraries get a special flag that indicates that the
symbol's address may be subject to copy relocation.

I looked briefly into handling copy relocations by looking at the
actual relocations in the main program, but this seemed difficult to
do with BFD.

Note that no caching is done here.  Perhaps this could be changed if
need be; I wanted to avoid possible problems with either objfile
lifetimes and changes, or conflicts with the long-term (vapor-ware)
objfile splitting project.

2019-08-21  Tom Tromey  <tromey@adacore.com>

	* symmisc.c (dump_msymbols): Don't use MSYMBOL_VALUE_ADDRESS.
	* ada-lang.c (lesseq_defined_than): Handle
	LOC_STATIC.
	* dwarf2read.c (dwarf2_per_objfile): Add can_copy
	parameter.
	(dwarf2_has_info): Likewise.
	(new_symbol): Set maybe_copied on symbol when
	appropriate.
	* dwarf2read.h (dwarf2_per_objfile): Add can_copy
	parameter.
	<can_copy>: New member.
	* elfread.c (record_minimal_symbol): Set maybe_copied
	on symbol when appropriate.
	(elf_symfile_read): Update call to dwarf2_has_info.
	* minsyms.c (lookup_minimal_symbol_linkage): New
	function.
	* minsyms.h (lookup_minimal_symbol_linkage): Declare.
	* symtab.c (get_symbol_address, get_msymbol_address):
	New functions.
	* symtab.h (get_symbol_address, get_msymbol_address):
	Declare.
	(SYMBOL_VALUE_ADDRESS, MSYMBOL_VALUE_ADDRESS): Handle
	maybe_copied.
	(struct symbol, struct minimal_symbol) <maybe_copied>:
	New member.
---
 gdb/ChangeLog    | 28 ++++++++++++++++++++++++++++
 gdb/ada-lang.c   |  9 +++++++++
 gdb/dwarf2read.c | 44 ++++++++++++++++++++++++++------------------
 gdb/dwarf2read.h | 10 ++++++++--
 gdb/elfread.c    | 16 +++++++++++-----
 gdb/minsyms.c    | 23 +++++++++++++++++++++++
 gdb/minsyms.h    | 11 +++++++++++
 gdb/symfile.h    |  3 ++-
 gdb/symmisc.c    | 10 +++++++---
 gdb/symtab.c     | 44 ++++++++++++++++++++++++++++++++++++++++++++
 gdb/symtab.h     | 42 +++++++++++++++++++++++++++++++++++++++---
 11 files changed, 208 insertions(+), 32 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index c34733e754f..2c8be4de857 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -4733,6 +4733,15 @@ lesseq_defined_than (struct symbol *sym0, struct symbol *sym1)
     case LOC_CONST:
       return SYMBOL_VALUE (sym0) == SYMBOL_VALUE (sym1)
         && equiv_types (SYMBOL_TYPE (sym0), SYMBOL_TYPE (sym1));
+
+    case LOC_STATIC:
+      {
+        const char *name0 = SYMBOL_LINKAGE_NAME (sym0);
+        const char *name1 = SYMBOL_LINKAGE_NAME (sym1);
+        return (strcmp (name0, name1) == 0
+                && SYMBOL_VALUE_ADDRESS (sym0) == SYMBOL_VALUE_ADDRESS (sym1));
+      }
+
     default:
       return 0;
     }
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 4cc558fce2a..bbe4aaee573 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -2138,8 +2138,10 @@ attr_value_as_address (struct attribute *attr)
 /* See declaration.  */
 
 dwarf2_per_objfile::dwarf2_per_objfile (struct objfile *objfile_,
-					const dwarf2_debug_sections *names)
-  : objfile (objfile_)
+					const dwarf2_debug_sections *names,
+					bool can_copy_)
+  : objfile (objfile_),
+    can_copy (can_copy_)
 {
   if (names == NULL)
     names = &dwarf2_elf_names;
@@ -2214,11 +2216,14 @@ private:
 /* Try to locate the sections we need for DWARF 2 debugging
    information and return true if we have enough to do something.
    NAMES points to the dwarf2 section names, or is NULL if the standard
-   ELF names are used.  */
+   ELF names are used.  CAN_COPY is true for formats where symbol
+   interposition is possible and so symbol values must follow copy
+   relocation rules.  */
 
 int
 dwarf2_has_info (struct objfile *objfile,
-                 const struct dwarf2_debug_sections *names)
+                 const struct dwarf2_debug_sections *names,
+		 bool can_copy)
 {
   if (objfile->flags & OBJF_READNEVER)
     return 0;
@@ -2228,7 +2233,8 @@ dwarf2_has_info (struct objfile *objfile,
 
   if (dwarf2_per_objfile == NULL)
     dwarf2_per_objfile = dwarf2_objfile_data_key.emplace (objfile, objfile,
-							  names);
+							  names,
+							  can_copy);
 
   return (!dwarf2_per_objfile->info.is_virtual
 	  && dwarf2_per_objfile->info.s.section != NULL
@@ -21704,19 +21710,21 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 		}
 	      else if (attr2 && (DW_UNSND (attr2) != 0))
 		{
-		  /* Workaround gfortran PR debug/40040 - it uses
-		     DW_AT_location for variables in -fPIC libraries which may
-		     get overriden by other libraries/executable and get
-		     a different address.  Resolve it by the minimal symbol
-		     which may come from inferior's executable using copy
-		     relocation.  Make this workaround only for gfortran as for
-		     other compilers GDB cannot guess the minimal symbol
-		     Fortran mangling kind.  */
-		  if (cu->language == language_fortran && die->parent
-		      && die->parent->tag == DW_TAG_module
-		      && cu->producer
-		      && startswith (cu->producer, "GNU Fortran"))
-		    SYMBOL_ACLASS_INDEX (sym) = LOC_UNRESOLVED;
+		  if (SYMBOL_CLASS (sym) == LOC_STATIC
+		      && (objfile->flags & OBJF_MAINLINE) == 0
+		      && dwarf2_per_objfile->can_copy)
+		    {
+		      /* A global static variable might be subject to
+			 copy relocation.  We first check for a local
+			 minsym, though, because maybe the symbol was
+			 marked hidden, in which case this would not
+			 apply.  */
+		      bound_minimal_symbol found
+			= (lookup_minimal_symbol_linkage
+			   (SYMBOL_LINKAGE_NAME (sym), objfile));
+		      if (found.minsym != nullptr)
+			sym->maybe_copied = 1;
+		    }
 
 		  /* A variable with DW_AT_external is never static,
 		     but it may be block-scoped.  */
diff --git a/gdb/dwarf2read.h b/gdb/dwarf2read.h
index d5a02990d41..82d56f09b0e 100644
--- a/gdb/dwarf2read.h
+++ b/gdb/dwarf2read.h
@@ -106,9 +106,12 @@ struct dwarf2_per_objfile
 {
   /* Construct a dwarf2_per_objfile for OBJFILE.  NAMES points to the
      dwarf2 section names, or is NULL if the standard ELF names are
-     used.  */
+     used.  CAN_COPY is true for formats where symbol
+     interposition is possible and so symbol values must follow copy
+     relocation rules.  */
   dwarf2_per_objfile (struct objfile *objfile,
-		      const dwarf2_debug_sections *names);
+		      const dwarf2_debug_sections *names,
+		      bool can_copy);
 
   ~dwarf2_per_objfile ();
 
@@ -208,6 +211,9 @@ public:
      original data was compressed using 'dwz -m'.  */
   std::unique_ptr<struct dwz_file> dwz_file;
 
+  /* Whether copy relocations are supported by this object format.  */
+  bool can_copy;
+
   /* A flag indicating whether this objfile has a section loaded at a
      VMA of 0.  */
   bool has_section_at_zero = false;
diff --git a/gdb/elfread.c b/gdb/elfread.c
index 901710f2833..e4da6912803 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -203,10 +203,16 @@ record_minimal_symbol (minimal_symbol_reader &reader,
       || ms_type == mst_text_gnu_ifunc)
     address = gdbarch_addr_bits_remove (gdbarch, address);
 
-  return reader.record_full (name, name_len, copy_name, address,
-			     ms_type,
-			     gdb_bfd_section_index (objfile->obfd,
-						    bfd_section));
+  struct minimal_symbol *result
+    = reader.record_full (name, name_len, copy_name, address,
+			  ms_type,
+			  gdb_bfd_section_index (objfile->obfd,
+						 bfd_section));
+  if ((objfile->flags & OBJF_MAINLINE) == 0
+      && (ms_type == mst_data || ms_type == mst_bss))
+    result->maybe_copied = 1;
+
+  return result;
 }
 
 /* Read the symbol table of an ELF file.
@@ -1239,7 +1245,7 @@ elf_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
 				bfd_section_size (str_sect));
     }
 
-  if (dwarf2_has_info (objfile, NULL))
+  if (dwarf2_has_info (objfile, NULL, true))
     {
       dw_index_kind index_kind;
 
diff --git a/gdb/minsyms.c b/gdb/minsyms.c
index 0f734ebc761..56dd38af9ce 100644
--- a/gdb/minsyms.c
+++ b/gdb/minsyms.c
@@ -519,6 +519,29 @@ iterate_over_minimal_symbols
 
 /* See minsyms.h.  */
 
+bound_minimal_symbol
+lookup_minimal_symbol_linkage (const char *name, struct objfile *objf)
+{
+  unsigned int hash = msymbol_hash (name) % MINIMAL_SYMBOL_HASH_SIZE;
+
+  for (objfile *objfile : objf->separate_debug_objfiles ())
+    {
+      for (minimal_symbol *msymbol = objfile->per_bfd->msymbol_hash[hash];
+	   msymbol != NULL;
+	   msymbol = msymbol->hash_next)
+	{
+	  if (strcmp (MSYMBOL_LINKAGE_NAME (msymbol), name) == 0
+	      && (MSYMBOL_TYPE (msymbol) == mst_data
+		  || MSYMBOL_TYPE (msymbol) == mst_bss))
+	    return {msymbol, objfile};
+	}
+    }
+
+  return {};
+}
+
+/* See minsyms.h.  */
+
 struct bound_minimal_symbol
 lookup_minimal_symbol_text (const char *name, struct objfile *objf)
 {
diff --git a/gdb/minsyms.h b/gdb/minsyms.h
index bb43165620d..e57a52c381c 100644
--- a/gdb/minsyms.h
+++ b/gdb/minsyms.h
@@ -204,6 +204,17 @@ struct bound_minimal_symbol lookup_bound_minimal_symbol (const char *);
 struct bound_minimal_symbol lookup_minimal_symbol_text (const char *,
 							struct objfile *);
 
+/* Look through the minimal symbols in OBJF (and its separate debug
+   objfiles) for a global (not file-local) minsym whose linkage name
+   is NAME.  This is somewhat similar to lookup_minimal_symbol_text,
+   only data symbols (not text symbols) are considered, and a non-NULL
+   objfile is not accepted.  Returns a bound minimal symbol that
+   matches, or an "empty" bound minimal symbol otherwise.  */
+
+extern struct bound_minimal_symbol lookup_minimal_symbol_linkage
+  (const char *name, struct objfile *objf)
+  ATTRIBUTE_NONNULL (1) ATTRIBUTE_NONNULL (2);
+
 /* Look through all the current minimal symbol tables and find the
    first minimal symbol that matches NAME and PC.  If OBJF is non-NULL,
    limit the search to that objfile.  Returns a pointer to the minimal
diff --git a/gdb/symfile.h b/gdb/symfile.h
index 642a5e223a3..cb5bed9d85c 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -585,7 +585,8 @@ struct dwarf2_debug_sections {
 };
 
 extern int dwarf2_has_info (struct objfile *,
-                            const struct dwarf2_debug_sections *);
+                            const struct dwarf2_debug_sections *,
+			    bool = false);
 
 /* Dwarf2 sections that can be accessed by dwarf2_get_section_info.  */
 enum dwarf2_section_enum {
diff --git a/gdb/symmisc.c b/gdb/symmisc.c
index 4699fd09206..c91ad5e5f45 100644
--- a/gdb/symmisc.c
+++ b/gdb/symmisc.c
@@ -236,9 +236,13 @@ dump_msymbols (struct objfile *objfile, struct ui_file *outfile)
 	  break;
 	}
       fprintf_filtered (outfile, "[%2d] %c ", index, ms_type);
-      fputs_filtered (paddress (gdbarch, MSYMBOL_VALUE_ADDRESS (objfile,
-								msymbol)),
-		      outfile);
+
+      /* Use the relocated address as shown in the symbol here -- do
+	 not try to respect copy relocations.  */
+      CORE_ADDR addr = (msymbol->value.address
+			+ ANOFFSET (objfile->section_offsets,
+				    msymbol->section));
+      fputs_filtered (paddress (gdbarch, addr), outfile);
       fprintf_filtered (outfile, " %s", MSYMBOL_LINKAGE_NAME (msymbol));
       if (section)
 	{
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 0d29e243b9e..e5eec1e8d45 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -6265,6 +6265,50 @@ symbol_set_symtab (struct symbol *symbol, struct symtab *symtab)
   symbol->owner.symtab = symtab;
 }
 
+/* See symtab.h.  */
+
+CORE_ADDR
+get_symbol_address (const struct symbol *sym)
+{
+  gdb_assert (sym->maybe_copied);
+  gdb_assert (SYMBOL_CLASS (sym) == LOC_STATIC);
+
+  const char *linkage_name = SYMBOL_LINKAGE_NAME (sym);
+
+  for (objfile *objfile : current_program_space->objfiles ())
+    {
+      bound_minimal_symbol minsym
+	= lookup_minimal_symbol_linkage (linkage_name, objfile);
+      if (minsym.minsym != nullptr)
+	return BMSYMBOL_VALUE_ADDRESS (minsym);
+    }
+  return sym->ginfo.value.address;
+}
+
+/* See symtab.h.  */
+
+CORE_ADDR
+get_msymbol_address (struct objfile *objf, const struct minimal_symbol *minsym)
+{
+  gdb_assert (minsym->maybe_copied);
+  gdb_assert ((objf->flags & OBJF_MAINLINE) == 0);
+
+  const char *linkage_name = MSYMBOL_LINKAGE_NAME (minsym);
+
+  for (objfile *objfile : current_program_space->objfiles ())
+    {
+      if ((objfile->flags & OBJF_MAINLINE) != 0)
+	{
+	  bound_minimal_symbol found
+	    = lookup_minimal_symbol_linkage (linkage_name, objfile);
+	  if (found.minsym != nullptr)
+	    return BMSYMBOL_VALUE_ADDRESS (found);
+	}
+    }
+  return (minsym->value.address
+	  + ANOFFSET (objf->section_offsets, minsym->section));
+}
+
 \f
 
 void
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 2d5667b620c..1d9c503e189 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -454,6 +454,14 @@ extern const char *symbol_get_demangled_name
 
 extern CORE_ADDR symbol_overlayed_address (CORE_ADDR, struct obj_section *);
 
+/* Return the address of SYM.  The MAYBE_COPIED flag must be set on
+   SYM.  If SYM appears in the main program's minimal symbols, then
+   that minsym's address is returned; otherwise, SYM's address is
+   returned.  This should generally only be used via the
+   SYMBOL_VALUE_ADDRESS macro.  */
+
+extern CORE_ADDR get_symbol_address (const struct symbol *sym);
+
 /* Note that all the following SYMBOL_* macros are used with the
    SYMBOL argument being either a partial symbol or
    a full symbol.  Both types have a ginfo field.  In particular
@@ -463,7 +471,9 @@ extern CORE_ADDR symbol_overlayed_address (CORE_ADDR, struct obj_section *);
    field only, instead of the SYMBOL parameter.  */
 
 #define SYMBOL_VALUE(symbol)		(symbol)->ginfo.value.ivalue
-#define SYMBOL_VALUE_ADDRESS(symbol)	((symbol)->ginfo.value.address + 0)
+#define SYMBOL_VALUE_ADDRESS(symbol)			      \
+  (((symbol)->maybe_copied) ? get_symbol_address (symbol)     \
+   : ((symbol)->ginfo.value.address))
 #define SET_SYMBOL_VALUE_ADDRESS(symbol, new_value)	\
   ((symbol)->ginfo.value.address = (new_value))
 #define SYMBOL_VALUE_BYTES(symbol)	(symbol)->ginfo.value.bytes
@@ -671,6 +681,14 @@ struct minimal_symbol : public general_symbol_info
      the object file format may not carry that piece of information.  */
   unsigned int has_size : 1;
 
+  /* For data symbols only, if this is set, then the symbol might be
+     subject to copy relocation.  In this case, a minimal symbol
+     matching the symbol's linkage name is first looked for in the
+     main objfile.  If found, then that address is used; otherwise the
+     address in this symbol is used.  */
+
+  unsigned maybe_copied : 1;
+
   /* Minimal symbols with the same hash key are kept on a linked
      list.  This is the link.  */
 
@@ -690,6 +708,15 @@ struct minimal_symbol : public general_symbol_info
   bool text_p () const;
 };
 
+/* Return the address of MINSYM, which comes from OBJF.  The
+   MAYBE_COPIED flag must be set on MINSYM.  If MINSYM appears in the
+   main program's minimal symbols, then that minsym's address is
+   returned; otherwise, MINSYM's address is returned.  This should
+   generally only be used via the MSYMBOL_VALUE_ADDRESS macro.  */
+
+extern CORE_ADDR get_msymbol_address (struct objfile *objf,
+				      const struct minimal_symbol *minsym);
+
 #define MSYMBOL_TARGET_FLAG_1(msymbol)  (msymbol)->target_flag_1
 #define MSYMBOL_TARGET_FLAG_2(msymbol)  (msymbol)->target_flag_2
 #define MSYMBOL_SIZE(msymbol)		((msymbol)->size + 0)
@@ -708,8 +735,9 @@ struct minimal_symbol : public general_symbol_info
 /* The relocated address of the minimal symbol, using the section
    offsets from OBJFILE.  */
 #define MSYMBOL_VALUE_ADDRESS(objfile, symbol)				\
-  ((symbol)->value.address					\
-   + ANOFFSET ((objfile)->section_offsets, ((symbol)->section)))
+  (((symbol)->maybe_copied) ? get_msymbol_address (objfile, symbol)	\
+   : ((symbol)->value.address						\
+      + ANOFFSET ((objfile)->section_offsets, ((symbol)->section))))
 /* For a bound minsym, we can easily compute the address directly.  */
 #define BMSYMBOL_VALUE_ADDRESS(symbol) \
   MSYMBOL_VALUE_ADDRESS ((symbol).objfile, (symbol).minsym)
@@ -1112,6 +1140,14 @@ struct symbol
   /* Whether this is an inlined function (class LOC_BLOCK only).  */
   unsigned is_inlined : 1;
 
+  /* For LOC_STATIC only, if this is set, then the symbol might be
+     subject to copy relocation.  In this case, a minimal symbol
+     matching the symbol's linkage name is first looked for in the
+     main objfile.  If found, then that address is used; otherwise the
+     address in this symbol is used.  */
+
+  unsigned maybe_copied : 1;
+
   /* The concrete type of this symbol.  */
 
   ENUM_BITFIELD (symbol_subclass_kind) subclass : 2;
-- 
2.20.1

Re: [PATCH v2 0/8] Handle copy relocations and add $_ada_exception

[Andrew Burgess]

* Tom Tromey <tromey@adacore.com> [2019-09-20 13:20:09 -0600]:

> This is v2 of the patch series to add copy relocation support to gdb,
> and to implement the $_ada_exception convenience variable.
> 
> v1 was here:
> 
>     https://sourceware.org/ml/gdb-patches/2019-08/msg00014.html
> 
> I believe this version addresses all the review comments.
> 
> I've merged some patches (in particular Pedro and Andrew both sent
> test case improvements, and I squashed these) and reordered the series
> a bit in an attempt to remove some intermediate regressions.
> (However, a couple still remain, and I didn't want to go all-out and
> squash most of the series.)
> 
> Let me know what you think.

I took a look through and didn't see any problems other than the
testsuite C++ issue I've mailed about separately.

Thanks,
Andrew