This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


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

Re: C++ PATCH: offsetof with multiple bases


Jason Merrill wrote:
> 
> Since this is an extension, it must generate a pedwarn when used.
We currently don't warn about other undefined uses of offsetof (single
inheritance, non-pod etc), and if you recall, my attempt at doing so ended in
failure with too many false-positives. I don't believe the standard mandates us
to pedwarn, because this is undefined behaviour. Though it would be nice to
pedwarn, I think we should pedwarn on all bad offsetofs, or none -- somewhere
in between would be confusing.

> Weakening the interface to convert_pointer_to_real bothers me; converting
> (foo*)0 to (bar*) must always produce (bar*)0.  Rather than do that, I'd
> prefer that you change it in build_component_ref to be relative to some
> other value -- say, 1 -- and adjust afterward.
I tried this, but the code was so ugly I'd be ashamed to put my name to it. In
addition build_scoped_ref would also need the same adjustment, as otherwise
something like offsetof(D, A::m) fails.

> Also, your last chunk would break code that constructs an object at a
> specific address, to interface with hardware.  Unlikely, but I don't see a
> reason to break it.
Um, how could one sanely use a virtual inheritance lattice to interface to a
hardware device?

>  This stuff should be diagnosed in build_component_ref instead.
Diagnosing this (virtual inheritance) in build_component_ref would have the
same effect of preventing using virtual inheritance at hardware addresses.
Also, if we do want to allow non-virtual multiple base, then
build_component_ref has to trog down the inheritance lattice to ensure
nothing's virtual. This is duplicating the functionality of
convert_pointer_to_real and build_vbase_path -- so why not let them do the
diagnosing?

The basic problem with this offsetof stuff is that only the first non-zero
adjustment gets to see that it was based off a NULL ptr, later adjustments see
a non-null pointer and think everything's ok. This leads to inconsistent
behaviour. Here's an example,

struct A { int am;};
struct B : virtual A {};
struct C : A {};
struct D { int dm; B db; C dc;};
struct E { B eb; int em;};
struct F { C fc; int fm;};

offsetof(D,db.am) -> compile ok, segfault at execution. (should reject)
offsetof(E,eb.am) -> compile error. (should reject)
offsetof(B,A::am) -> ICE 191 (should reject)
offsetof(C,A::am) -> ICE 191 (should accept)
offsetof(D,db.A::am) -> compile ok, segfault at execution (should reject)
offsetof(D,dc.A::am) -> compile ok (should accept)
offsetof(E,eb.A::am) -> ICE 191 (should reject)
offsetof(F,fc.A::am) -> ICE 191 (should accept)

This is bad.

Some other ways to avoid weakening convert_pointer_to_real so that we never
mistakenly give it a NULL pointer are
1) give it another arg saying zero-valued pointers are ok
2) change offsetof to be
  #define offsetof(T,m) ((size_t)(((char *)((T *)1)->m) - (char *)1))
which avoids ever using a NULL ptr. Unfortunately I beleive offsetof is
effectively outside our control.
3) provide a new routine with a weaker constraint which convert_pointer_to_real
can call after ensuring it's not got a null ptr. But then what do we call it
(really_convert_pointer_to_real??).

I don't have any preference either way for these, but I'm pretty much averse to
hacking around in build_component_ref and build_scoped_ref to adjust null
pointers.

Open to suggestions...

nathan

-- 
Dr Nathan Sidwell :: Computer Science Department :: Bristol University
        I have seen the death of PhotoShop -- it is called GIMP
nathan@acm.org  http://www.cs.bris.ac.uk/~nathan/  nathan@cs.bris.ac.uk


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