This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 3/4] Use std::vector in solib-target lm_info


Something else I realized is that these lm_info structures
are all defined differently in the different solib-*.c files.

It was OK in C, but in C++, that's an ODR violation
("-flto -Wodr" will tell you about them).

I think that as long as these types are PODs, it isn't a 
problem in practice, exactly due to C compatibility.
But as soon as we add non-trivial ctors/dtors (with external linkage),
then I think we're likely to run into problems as the linker picks
just one of the (different) types' ctor/dtor definitions assuming
they're all the same.

#1 - One way around it would be to make "struct lm_info" an
empty struct

  struct lm_info {};

in solist.h, instead of an opaque struct:

  struct lm_info;

and then make all lm_info implementations inherit from that
instead of redefining it.  You then need to add casts from
lm_info * to the appropriate type within each of the
solib-*.c implementations.

The downside is that this requires changing every port at the
same time, otherwise the solib-*.c files won't compile due to
lm_info redefinitions.

#2 - An alternative way, is to instead keep struct lm_info opaque,
and rename each solib-*.c lm_info's name.  E.g., do:

 -struct lm_info
 +struct solib_target_lm_info

And then add the casts between "solib_target_lm_info *"
and "lm_info *" in each solib-*.c as we convert them.
So you'd just do it in solib-target.c for now.  This is more
casts than the other approach (likely just one or two where
the lm_info object is constructed), because you now need to
cast in the solib_target_lm_info -> lm_info direction too.
However, if we changed so_list::lm_info to "void *" [1], we
would avoid those casts and only need the
lm_info -> solib_target_lm_info casts, as in the other approach.

[1] - the type is already opaque to common code so we'd not
really be adding any type hole at all.

#3 - Yet another approach is to get rid of the lm_info type and
replace it with inheriting the so_list type directly:

 struct solib_target_lib : so_list 
 {
    // old lm_info fields go here.
 };

This is I suspect, the cleanest.  It is already the responsibility
of the solib-*.c backend to create/destroy so_list objects.
It could also be done incrementally, I think.  We'd have to start by
touching all solib-*.c files to use new/delete instead of malloc/free
to allocate so_list objects, but after that, I think we could
progress one backend at a time as we find a need...

Thanks,
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]