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 0/3] Create arch_lwp_info class hierarchy


On 2017-08-14 13:53, Pedro Alves wrote:
On 08/12/2017 12:29 PM, Simon Marchi wrote:

But then I realized that I forgot to include the header for s390, and
the compiler (when building for a s390 host) didn't warn me.  This is
dangerous and fragile since we end up with two definitions of
arch_lwp_info (the s390 one and that fallback one), and nothing to warn
about it.

Not sure whether that's really such a bad problem.  It seems like the
sort of thing that'd crash quite visibly/quickly if you actually try to
run the resulting binary.  It's not like we're adding ports
every day.  :-)

How would it crash visibly? I thought it would most likely crash or corrupt memory seemingly randomly. That's irrelevant because of you suggestion below, but I'm asking just for the sake of the discussion.

So I changed it to listing explicitly the architectures that
don't defined their own arch_lwp_info:

...
#elif  defined __alpha__ || defined __powerpc__ || ...
/* Define a dummy arch_lwp_info for arches that don't define one.  */
struct arch_lwp_info {};
#else
# error "Missing arch-specific include."
#endif

That would work, but requires listing all the arches that need the
fallback definition of arch_lwp_info, so it gets pretty ugly.

Any idea to make this simple but safe? Otherwise, I'll just go with the
current version of the patch.

There's a 3rd, much simpler option.

The problem we ran into is one of blurred division of responsibility: the
arch-specific code is responsible for allocating the arch-specific
arch_lwp_info, which it must be, because it's not possible to allocate an incomplete type (must know its size), but we free the object in common code,
where the type is opaque.

We can solve the original xfree poisoning problem by simply making the
arch-specific code responsible for releasing arch_lwp_info too, where the
type is known, just like it is responsible for allocating it.

See below a proof of concept doing that for the x86 port.  Making other
archs do the same should be trivial.  Using new/delete/cdtors/in-class
initialization for arch_lwp_info would be a separate, orthogonal change
(and so would class-ification of linux_nat_new_thread, linux_target_ops,
etc.).

Yes, that makes sense.  I'll try that.  Thanks for the prototype.

Simon


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