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. :-)
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.).