[Bug build/23399] New: Wodr warning in agent.c

[mliska at suse dot cz]

https://sourceware.org/bugzilla/show_bug.cgi?id=23399

Bug ID: 23399
Summary: Wodr warning in agent.c
Product: gdb
Version: unknown
Status: UNCONFIRMED
Severity: normal
Priority: P2
Component: build
Assignee: unassigned at sourceware dot org
Reporter: mliska at suse dot cz
Target Milestone: ---

When building gdb with -flto and -Wodr I see:

[ 802s] ../../../gdb/gdbserver/../common/agent.c:51:8: error: type 'struct
ipa_sym_addresses' violates the C++ One Definition Rule [-Werror=odr]
[ 802s] struct ipa_sym_addresses
[ 802s] ^
[ 802s] ../../../gdb/gdbserver/tracepoint.c:144:8: note: a different type is
defined in another translation unit
[ 802s] struct ipa_sym_addresses
[ 802s] ^
[ 802s] ../../../gdb/gdbserver/../common/agent.c:53:13: note: the first
difference of corresponding definitions is field 'addr_helper_thread_id'
[ 802s] CORE_ADDR addr_helper_thread_id;
[ 802s] ^
[ 802s] ../../../gdb/gdbserver/tracepoint.c:146:13: note: a field with
different name is defined in another translation unit
[ 802s] CORE_ADDR addr_gdb_tp_heap_buffer;
[ 802s] ^
[ 802s] lto1: all warnings being treated as errors


$ cat -n ./rpmbuild/BUILD/gdb-8.1/gdb/common/agent.c | head -n100
...

48 /* Addresses of in-process agent's symbols both GDB and GDBserver cares
49 about. */
50
51 struct ipa_sym_addresses
52 {
53 CORE_ADDR addr_helper_thread_id;
54 CORE_ADDR addr_cmd_buf;
55 CORE_ADDR addr_capability;
56 };

...

$ cat -n ./rpmbuild/BUILD/gdb-8.1/gdb/gdbserver/tracepoint.c | head -n 200
...
144 struct ipa_sym_addresses
145 {
146 CORE_ADDR addr_gdb_tp_heap_buffer;
147 CORE_ADDR addr_gdb_jump_pad_buffer;
148 CORE_ADDR addr_gdb_jump_pad_buffer_end;
149 CORE_ADDR addr_gdb_trampoline_buffer;
150 CORE_ADDR addr_gdb_trampoline_buffer_end;
151 CORE_ADDR addr_gdb_trampoline_buffer_error;
152 CORE_ADDR addr_collecting;
153 CORE_ADDR addr_gdb_collect_ptr;
154 CORE_ADDR addr_stop_tracing;
155 CORE_ADDR addr_flush_trace_buffer;
156 CORE_ADDR addr_about_to_request_buffer_space;
157 CORE_ADDR addr_trace_buffer_is_full;
158 CORE_ADDR addr_stopping_tracepoint;
159 CORE_ADDR addr_expr_eval_result;
160 CORE_ADDR addr_error_tracepoint;
161 CORE_ADDR addr_tracepoints;
162 CORE_ADDR addr_tracing;
163 CORE_ADDR addr_trace_buffer_ctrl;
164 CORE_ADDR addr_trace_buffer_ctrl_curr;
165 CORE_ADDR addr_trace_buffer_lo;
166 CORE_ADDR addr_trace_buffer_hi;
167 CORE_ADDR addr_traceframe_read_count;
168 CORE_ADDR addr_traceframe_write_count;
169 CORE_ADDR addr_traceframes_created;
170 CORE_ADDR addr_trace_state_variables;
171 CORE_ADDR addr_get_raw_reg_ptr;
172 CORE_ADDR addr_get_trace_state_variable_value_ptr;
173 CORE_ADDR addr_set_trace_state_variable_value_ptr;
174 CORE_ADDR addr_ust_loaded;
175 CORE_ADDR addr_ipa_tdesc_idx;
176 };
...

Which is really a different layout of the struct.

[simon.marchi at ericsson dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=23399

Simon Marchi <simon.marchi at ericsson dot com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |simon.marchi at ericsson dot com

--- Comment #1 from Simon Marchi <simon.marchi at ericsson dot com> ---
Isn't it ok to have multiple definitions of a struct, if they are all POD?

[palves at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=23399

Pedro Alves <palves at redhat dot com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |palves at redhat dot com

--- Comment #2 from Pedro Alves <palves at redhat dot com> ---
IIRC, it's valid C but not C++. May be OK in practice, but it'd be safer to
fix it.

[simon.marchi at ericsson dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=23399

--- Comment #3 from Simon Marchi <simon.marchi at ericsson dot com> ---
Proposed patch here:
https://sourceware.org/ml/gdb-patches/2018-08/msg00614.html

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=23399

--- Comment #4 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Simon Marchi <***@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=ad202fcc2277f351d41d4d4e897ad4f6d7c9f043

commit ad202fcc2277f351d41d4d4e897ad4f6d7c9f043
Author: Simon Marchi <***@polymtl.ca>
Date: Tue Aug 28 13:29:32 2018 -0400

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

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.

[simon.marchi at ericsson dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=23399

Simon Marchi <simon.marchi at ericsson dot com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |RESOLVED
Resolution|--- |FIXED

--- Comment #5 from Simon Marchi <simon.marchi at ericsson dot com> ---
Should be fixed be the mentioned commit.

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=23399

Tom Tromey <tromey at sourceware dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |tromey at sourceware dot org
Target Milestone|--- |8.3