Bug 30751 - [gdb/build] ODR violation for struct btrace_target_info
Summary: [gdb/build] ODR violation for struct btrace_target_info
Status: RESOLVED FIXED
Alias: None
Product: gdb
Classification: Unclassified
Component: build (show other bugs)
Version: HEAD
: P2 normal
Target Milestone: 14.1
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks: 22395
  Show dependency treegraph
 
Reported: 2023-08-14 09:27 UTC by Tom de Vries
Modified: 2023-09-11 06:55 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tom de Vries 2023-08-14 09:27:13 UTC
When building gdb with -O2 -flto -Wall we run into:
...
/data/vries/gdb/src/gdb/nat/linux-btrace.h:84:8: warning: type 'struct btrace_target_info' violates the C++ One Definition Rule [-Wodr]
 struct btrace_target_info
        ^
/data/vries/gdb/src/gdb/remote.c:14140:0: note: a different type is defined in another translation unit
 struct btrace_target_info

/data/vries/gdb/src/gdb/nat/linux-btrace.h:101:5: note: the first difference of corresponding definitions is field 'variant'
   } variant;
     ^
/data/vries/gdb/src/gdb/remote.c:14140:0: note: a type with different number of fields is defined in another translation unit
 struct btrace_target_info
...
Comment 1 Tom de Vries 2023-08-14 12:56:29 UTC
I tried renaming "struct btrace_target_info" to "struct remote_btrace_target_info" in remote.c, but that didn't work.  The two types are related somehow but not the same.

I'm not sure how to fix this.

Markus, do you have any idea how this should be fixed?
Comment 2 Markus Metzger 2023-08-14 13:20:00 UTC
(In reply to Tom de Vries from comment #1)
> I tried renaming "struct btrace_target_info" to "struct
> remote_btrace_target_info" in remote.c, but that didn't work.  The two types
> are related somehow but not the same.
> 
> I'm not sure how to fix this.
> 
> Markus, do you have any idea how this should be fixed?

The idea was that targets can attach target-specific information.  You'd get the handle when enabling btrace on a target, then you'd pass it to other btrace functions on that target.

One obvious fix would be to turn it into a void * and have targets cast it.  I declared this opaque struct to avoid exactly that.

This is similar to the private data targets attach to inferiors or threads so the same solution should work here, as well.

I can work on a patch but not right now.
Comment 3 Tom Tromey 2023-08-14 18:03:23 UTC
Another option is to use a base class and derive the per-target classes
from it.  'inferior' does this for private data.
Comment 4 Markus Metzger 2023-08-18 10:07:51 UTC
(In reply to Tom Tromey from comment #3)
> Another option is to use a base class and derive the per-target classes
> from it.  'inferior' does this for private data.

I have a patch to c++ify the btrace_target_info similar to inferior and thread private target data, but I cannot reproduce the original issue with GCC 11.4.0 on Ubuntu 22.04.

After configuring, I build GDB with:

    $ make -sj$(nproc) CFLAGS='-O2 -flto -Wall' CXXFLAGS='-O2 -flto -Wall'

and I get several fails, e.g.

.../sim/m32r/traps.c:115:43: error: type of 'm32r2f_h_psw_get' does not match original declaration [-Werror=lto-type-mismatch]
  115 |           m32r2f_h_bpsw_set (current_cpu, m32r2f_h_psw_get (current_cpu));

but no -Wodr warnings nor any other warnings related to btrace.
Comment 5 Tom Tromey 2023-08-18 14:12:08 UTC
(In reply to Markus Metzger from comment #4)

> but no -Wodr warnings nor any other warnings related to btrace.

FWIW I can definitely reproduce on x86-64 Fedora 36 using the system
gcc (12.2.1), e.g.:

../../binutils-gdb/gdb/nat/linux-btrace.h:84: warning: type 'struct btrace_target_info' violates the C++ One Definition Rule [-Wodr]
Comment 6 Tom de Vries 2023-08-18 14:29:44 UTC
(In reply to Markus Metzger from comment #4)
> but I cannot reproduce the original issue with
> GCC 11.4.0 on Ubuntu 22.04.

I'm currently trying -flto-partition=one to try to flush out more odr warnings.
Comment 7 Tom de Vries 2023-08-18 15:56:42 UTC
FWIW, if you post the patch here or on the ml, I can confirm that the warning is gone.
Comment 8 Tom de Vries 2023-08-21 06:55:14 UTC
(In reply to Markus Metzger from comment #4)
> After configuring, I build GDB with:
> 
>     $ make -sj$(nproc) CFLAGS='-O2 -flto -Wall' CXXFLAGS='-O2 -flto -Wall'
> 
> and I get several fails, e.g.
> 
> .../sim/m32r/traps.c:115:43: error: type of 'm32r2f_h_psw_get' does not
> match original declaration [-Werror=lto-type-mismatch]
>   115 |           m32r2f_h_bpsw_set (current_cpu, m32r2f_h_psw_get
> (current_cpu));
> 
> but no -Wodr warnings nor any other warnings related to btrace.

I think what may be happening is that if you're getting an error while building sim, gdb is not build at all, because all-gdb requires maybe-all-sim.

In that case, you could try building with --disable-sim.
Comment 9 Markus Metzger 2023-09-06 07:18:36 UTC
(In reply to Tom de Vries from comment #8)
> (In reply to Markus Metzger from comment #4)
> > After configuring, I build GDB with:
> > 
> >     $ make -sj$(nproc) CFLAGS='-O2 -flto -Wall' CXXFLAGS='-O2 -flto -Wall'
> > 
> > and I get several fails, e.g.
> > 
> > .../sim/m32r/traps.c:115:43: error: type of 'm32r2f_h_psw_get' does not
> > match original declaration [-Werror=lto-type-mismatch]
> >   115 |           m32r2f_h_bpsw_set (current_cpu, m32r2f_h_psw_get
> > (current_cpu));
> > 
> > but no -Wodr warnings nor any other warnings related to btrace.
> 
> I think what may be happening is that if you're getting an error while
> building sim, gdb is not build at all, because all-gdb requires
> maybe-all-sim.
> 
> In that case, you could try building with --disable-sim.

After disabling sim, I get

bfd/elf32-xtensa.c:5725:41: error: 'MEM[(struct removal_by_action_entry *)ientry_91 + -24B].offset' may be used uninitialized [-Werror=maybe-uninitialized]
 5725 |   if (ctx->map.n_entries && (ientry - 1)->offset == r->offset)
      |                                    

but when I build with 'make -k' I can now reproduce the error and confirm that it is fixed with my patch.
Comment 10 Sourceware Commits 2023-09-11 06:18:51 UTC
The master branch has been updated by Markus Metzger <mmetzger@sourceware.org>:

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

commit cdda72c2fa1e47c566c7b5768f3831a1cc11d263
Author: Markus Metzger <markus.t.metzger@intel.com>
Date:   Thu Aug 17 10:17:26 2023 +0000

    gdb: c++ify btrace_target_info
    
    Following the example of private_thread_info and private_inferior, turn
    struct btrace_target_info into a small class hierarchy.
    
    Also merge btrace_tinfo_bts with btrace_tinfo_pt and inline into
    linux_btrace_target_info.
    
    Fixes PR gdb/30751.
Comment 11 Tom de Vries 2023-09-11 06:55:21 UTC
Markus, thanks for fixing this.