Get rid of -Wodr warning (PR build/23399)

Message ID 20180825153828.27600-1-simon.marchi@polymtl.ca
State New
Headers show
Series
  • Get rid of -Wodr warning (PR build/23399)
Related show

Commit Message

Simon Marchi Aug. 25, 2018, 3:38 p.m.
The PR reports that building with -Wodr -flto complains about different
versions of struct ipa_sym_addresses, in common/agent.c and
gdbserver/tracepoint.c.  This patch renames the version in common to
ipa_sym_addresses_common to avoid the name clash.  Because the IPA_SYM
assumed the name ipa_sym_addresses, it now requires the includer to
define the IPA_SYM_STRUCT_NAME macro to define the name of the structure
holding the IPA symbol addresses.

gdb/ChangeLog:

	PR build/23399
	* common/agent.c (IPA_SYM_STRUCT_NAME): Define.
	(struct ipa_sym_addresses): Rename to...
	(struct ipa_sym_addresses_common): ... this.
	* common/agent.h (IPA_SYM): Use IPA_SYM_STRUCT_NAME.

gdb/gdbserver/ChangeLog:

	PR build/23399
	* tracepoint.c (IPA_SYM_STRUCT_NAME): Define.
---
 gdb/common/agent.c         | 8 +++++---
 gdb/common/agent.h         | 9 +++++++--
 gdb/gdbserver/tracepoint.c | 4 +++-
 3 files changed, 15 insertions(+), 6 deletions(-)

-- 
2.18.0

Comments

Tom Tromey Aug. 28, 2018, 5:09 p.m. | #1
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:


Simon> The PR reports that building with -Wodr -flto complains about different
Simon> versions of struct ipa_sym_addresses, in common/agent.c and
Simon> gdbserver/tracepoint.c.  This patch renames the version in common to
Simon> ipa_sym_addresses_common to avoid the name clash.  Because the IPA_SYM
Simon> assumed the name ipa_sym_addresses, it now requires the includer to
Simon> define the IPA_SYM_STRUCT_NAME macro to define the name of the structure
Simon> holding the IPA symbol addresses.

Simon> gdb/ChangeLog:

Simon> 	PR build/23399
Simon> 	* common/agent.c (IPA_SYM_STRUCT_NAME): Define.
Simon> 	(struct ipa_sym_addresses): Rename to...
Simon> 	(struct ipa_sym_addresses_common): ... this.
Simon> 	* common/agent.h (IPA_SYM): Use IPA_SYM_STRUCT_NAME.

Simon> gdb/gdbserver/ChangeLog:

Simon> 	PR build/23399
Simon> 	* tracepoint.c (IPA_SYM_STRUCT_NAME): Define.

Thanks for doing this.  This seems a bit ugly but I didn't see any
obviously better way to go about it.

I was curious if we needed to add -Wodr to the build, but according to
the gcc manual it is enabled by default, so all is well.

Tom
Simon Marchi Aug. 28, 2018, 9:25 p.m. | #2
On 2018-08-28 13:09, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

> 

> Simon> The PR reports that building with -Wodr -flto complains about 

> different

> Simon> versions of struct ipa_sym_addresses, in common/agent.c and

> Simon> gdbserver/tracepoint.c.  This patch renames the version in 

> common to

> Simon> ipa_sym_addresses_common to avoid the name clash.  Because the 

> IPA_SYM

> Simon> assumed the name ipa_sym_addresses, it now requires the includer 

> to

> Simon> define the IPA_SYM_STRUCT_NAME macro to define the name of the 

> structure

> Simon> holding the IPA symbol addresses.

> 

> Simon> gdb/ChangeLog:

> 

> Simon> 	PR build/23399

> Simon> 	* common/agent.c (IPA_SYM_STRUCT_NAME): Define.

> Simon> 	(struct ipa_sym_addresses): Rename to...

> Simon> 	(struct ipa_sym_addresses_common): ... this.

> Simon> 	* common/agent.h (IPA_SYM): Use IPA_SYM_STRUCT_NAME.

> 

> Simon> gdb/gdbserver/ChangeLog:

> 

> Simon> 	PR build/23399

> Simon> 	* tracepoint.c (IPA_SYM_STRUCT_NAME): Define.

> 

> Thanks for doing this.  This seems a bit ugly but I didn't see any

> obviously better way to go about it.


Indeed.

> I was curious if we needed to add -Wodr to the build, but according to

> the gcc manual it is enabled by default, so all is well.


Thanks, I pushed it.

Simon

Patch

diff --git a/gdb/common/agent.c b/gdb/common/agent.c
index 8f80aee53320..41884b9c96c0 100644
--- a/gdb/common/agent.c
+++ b/gdb/common/agent.c
@@ -21,9 +21,11 @@ 
 #include "target/target.h"
 #include "common/symbol.h"
 #include <unistd.h>
-#include "agent.h"
 #include "filestuff.h"
 
+#define IPA_SYM_STRUCT_NAME ipa_sym_addresses_common
+#include "agent.h"
+
 int debug_agent = 0;
 
 /* A stdarg wrapper for debug_vprintf.  */
@@ -48,7 +50,7 @@  int use_agent = 0;
 /* Addresses of in-process agent's symbols both GDB and GDBserver cares
    about.  */
 
-struct ipa_sym_addresses
+struct ipa_sym_addresses_common
 {
   CORE_ADDR addr_helper_thread_id;
   CORE_ADDR addr_cmd_buf;
@@ -69,7 +71,7 @@  static struct
   IPA_SYM(capability),
 };
 
-static struct ipa_sym_addresses ipa_sym_addrs;
+static struct ipa_sym_addresses_common ipa_sym_addrs;
 
 static int all_agent_symbols_looked_up = 0;
 
diff --git a/gdb/common/agent.h b/gdb/common/agent.h
index 66b693791e23..5be07b845be6 100644
--- a/gdb/common/agent.h
+++ b/gdb/common/agent.h
@@ -27,10 +27,15 @@  int agent_run_command (int pid, const char *cmd, int len);
 int agent_look_up_symbols (void *);
 
 #define IPA_SYM_EXPORTED_NAME(SYM) gdb_agent_ ## SYM
+
+/* Define an entry in an IPA symbol list array.  If IPA_SYM is used, the macro
+   IPA_SYM_STRUCT_NAME must be defined to the structure name holding the IPA
+   symbol addresses in that particular file, before including
+   common/agent.h.  */
 #define IPA_SYM(SYM)                                   \
   {                                                    \
-    STRINGIFY (IPA_SYM_EXPORTED_NAME (SYM)),		\
-    offsetof (struct ipa_sym_addresses, addr_ ## SYM)	\
+    STRINGIFY (IPA_SYM_EXPORTED_NAME (SYM)),           \
+    offsetof (IPA_SYM_STRUCT_NAME, addr_ ## SYM)       \
   }
 
 /* The size in bytes of the buffer used to talk to the IPA helper
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index ad2a80185a18..9959d0598dd3 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -19,7 +19,6 @@ 
 #include "server.h"
 #include "tracepoint.h"
 #include "gdbthread.h"
-#include "agent.h"
 #include "rsp-low.h"
 
 #include <ctype.h>
@@ -30,6 +29,9 @@ 
 #include "ax.h"
 #include "tdesc.h"
 
+#define IPA_SYM_STRUCT_NAME ipa_sym_addresses
+#include "agent.h"
+
 #define DEFAULT_TRACE_BUFFER_SIZE 5242880 /* 5*1024*1024 */
 
 /* This file is built for both GDBserver, and the in-process