[Bug c++/13005] New: Pointer wrongly adjusted for derived class containing virtual function

[Bug c++/13005] New: Pointer wrongly adjusted for derived class containing virtual function

[stephenma at telus dot net]

If a base class B has no virtual functions, not even inherited ones,
but a class derived from B has at least one virtual function, then
sometimes a pointer to the derived class is wrongly adjusted.

Here is the slightly reformatted output from "g++ -v":

	Reading specs from /usr/lib/gcc-lib/i486-linux/3.3.2/specs

	Configured with: ../src/configure -v
	--enable-languages=c,c++,java,f77,pascal,objc,ada,treelang
	--prefix=/usr --mandir=/usr/share/man
	--infodir=/usr/share/info
	--with-gxx-include-dir=/usr/include/c++/3.3 --enable-shared
	--with-system-zlib --enable-nls --without-included-gettext
	--enable-__cxa_atexit --enable-clocale=gnu --enable-debug
	--enable-java-gc=boehm --enable-java-awt=xlib --enable-objc-gc
	i486-linux

	Thread model: posix
	gcc version 3.3.2 (Debian)

The following little snippet demonstrates the problem:

	class Derived;
	
	struct Base {
	  Derived* before;
	  Derived* after;
	  void insertAfter(Derived* node);
	};
	
	struct Derived: public Base {
	  virtual void foo();
	};
	
	void Base::insertAfter(Derived* node) {
	  this->before->after = node;	// Notice the double indirection.
	}

When compiled with "g++-3.3 -S z.cpp" the compiler produces the
following assembler code for Base::insertAfter():

	pushl	%ebp
	movl	%esp, %ebp
	movl	8(%ebp), %eax
	movl	(%eax), %edx
	addl	$4, %edx         <-- BOGUS INSTRUCTION
	movl	12(%ebp), %eax
	movl	%eax, 4(%edx)
	popl	%ebp
	ret

I have marked what I believe is the offending instruction.

When I delete the declaration for Derived::foo(), the "addl"
instruction magically disappears.

In this specific example, the "addl" also disappears when I compile
with "-O2", regardless of whether Derived::foo() is declared.

-- 
           Summary: Pointer wrongly adjusted for derived class containing
                    virtual function
           Product: gcc
           Version: 3.3.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: c++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: stephenma at telus dot net
                CC: gcc-bugs at gcc dot gnu dot org
  GCC host triplet: i386-pc-linux-gnu
GCC target triplet: i386-pc-linux-gnu


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer wrongly adjusted for derived class containing virtual function

[bangerth at dealii dot org]

------- Additional Comments From bangerth at dealii dot org  2003-11-11 16:01 -------
Can you come up with an example in which one can see that something
is miscompiled by running the program? E.g., that the program crashes
or that the output it generates is not what is expected.

Thanks
  Wolfgang

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer wrongly adjusted for derived class containing virtual function

[bangerth at dealii dot org]

------- Additional Comments From bangerth at dealii dot org  2003-11-11 17:54 -------
Just for the record: a simple attempt at showing this like so
--------------------
#include <iostream>

class Derived;
	
struct Base {
    Derived* before;
    Derived* after;
    void insertAfter(Derived* node);
};
	
struct Derived: public Base {
    virtual void foo();
};
	
void Base::insertAfter(Derived* node) {
  this->before->after = node;	// Notice the double indirection.
}


int main ()
{
  Derived node1, node2;
  node2.before = &node1;

  node2.insertAfter (&node2);

  std::cout << node2.before << " " << &node1 << std::endl;
  std::cout << node2.before->after << " " << &node2 << std::endl;
  std::cout << node1.after << " " << &node2 << std::endl;
}

void Derived::foo () 
{}
-----------------------------

doesn't yield anything that's wrong. I get
g/x> /home/bangerth/bin/gcc-3.3.2/bin/c++ x.cc
g/x> ./a.out
0xbffff054 0xbffff054
0xbffff044 0xbffff044
0xbffff044 0xbffff044

I.e. the data seems to get written into the right place. I think you
need to come up with something that shows the problem more clearly.

W.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer wrongly adjusted for derived class containing virtual function

[bangerth at dealii dot org]

------- Additional Comments From bangerth at dealii dot org  2003-11-11 17:57 -------
Oh, yes, and one more datapoint: without optimization, I get this assembler code:
	pushl	%ebp
	movl	%esp, %ebp
	movl	8(%ebp), %eax
	movl	(%eax), %edx
	addl	$4, %edx
	movl	12(%ebp), %eax
	movl	%eax, 4(%edx)
	popl	%ebp
	ret
That is, the addl is there. On the other hand, _with_ optimization, we get
	pushl	%ebp
	movl	%esp, %ebp
	movl	8(%ebp), %eax
	movl	(%eax), %edx
	movl	12(%ebp), %eax
	movl	%eax, 8(%edx)
	popl	%ebp
	ret
The addl is gone, but note that in the first case the result is written
into 4(%edx), while in the second into 8(%edx). My assembler times have
long gone for good, but to me this seems functionally equivalent.

W.

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] [aliasing] Pointer wrongly adjusted for derived class containing virtual function

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2003-11-15 09:10 -------
unconfirmed.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |UNCONFIRMED
         Resolution|INVALID                     |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] [aliasing] Pointer wrongly adjusted for derived class containing virtual function

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2003-11-15 09:10 -------
The problem is an aliasing problem but I do not know rules for C++ aliasing at all, can someone 
comment on this one?

Moving to invalid to change to ...

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |critical
             Status|WAITING                     |RESOLVED
         Resolution|                            |INVALID
            Summary|Pointer wrongly adjusted for|[aliasing] Pointer wrongly
                   |derived class containing    |adjusted for derived class
                   |virtual function            |containing virtual function


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] [aliasing] Pointer wrongly adjusted for derived class containing virtual function

[geoffk at gcc dot gnu dot org]

------- Additional Comments From geoffk at gcc dot gnu dot org  2003-11-16 12:04 -------
I believe this program is OK.  [expr.cast] says:
- a pointer to an object of non-virtual base class type ... may be explicitly converted to a pointer... 
of a derived class type...


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|                            |1
   Last reconfirmed|0000-00-00 00:00:00         |2003-11-16 12:04:48
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] [3.3/3.4 Regresssion] [aliasing] Pointer wrongly adjusted for derived class containing virtual function

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2003-12-04 18:38 -------
I think the problem for the C++ front-end is that Derived is being forward declared and not see 
that Derived is a sub-class of Base so it is marking them as two different aliasing groups (but then 
again I could be wrong).

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
            Summary|[aliasing] Pointer wrongly  |[3.3/3.4 Regresssion]
                   |adjusted for derived class  |[aliasing] Pointer wrongly
                   |containing virtual function |adjusted for derived class
                   |                            |containing virtual function
   Target Milestone|---                         |3.3.3


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] [3.3/3.4 Regresssion] [aliasing] Pointer wrongly adjusted for derived class containing virtual function

[jazzdaq at yahoo dot com]

------- Additional Comments From jazzdaq at yahoo dot com  2003-12-24 11:13 -------
I have some code that corrupts data due to this bug.  Non-virtual base class,
virtual derived class.  If base class is made virtual, the bug goes away.  If
derived class is made non-virtual, the bug also goes away.

The bug occurs with both gcc 3.3.2 and 3.2.3.

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] [3.3/3.4 Regresssion] [aliasing] Pointer wrongly adjusted for derived class containing virtual function

[gdr at gcc dot gnu dot org]

------- Additional Comments From gdr at gcc dot gnu dot org  2003-12-24 21:53 -------
Nathan --
Can you look into this, please?

-- Gaby


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nathan at codesourcery dot
                   |                            |com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] [3.3/3.4 Regresssion] [aliasing] Pointer wrongly adjusted for derived class containing virtual function

[nathan at gcc dot gnu dot org]

------- Additional Comments From nathan at gcc dot gnu dot org  2003-12-29 19:15 -------
AFAICT the report so far is that Base is not at offset zero within
Derived. That is not a bug. The original test case contains this

    Base() : before((Derived*)this), after((Derived*)this) {}
 
which is unspecified. At that point Derived is incomplete.
So it is unspecified as to whether the old-style cast is treated as
a static_cast or a reinterpret_cast.  See [5.4]/6



-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |INVALID


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] [3.3/3.4 Regresssion] [aliasing] Pointer wrongly adjusted for derived class containing virtual function

[stephenma at telus dot net]

------- Additional Comments From stephenma at telus dot net  2003-12-31 08:53 -------
Nathan Sidwell refers to [5.4]/6's explicit disclaimer about the casting of
incomplete types.  He is technically correct, but only with respect to the Draft
ANSI C++ Standard (1996).

There is an enormous legacy of *working* programs that follow the older ARM
standard, as defined in the Annotated C++ Reference Manual by Ellis and
Stroustrup.  Silently breaking these programs, as g++ 3.x does in the case I
stumbled upon, is probably asking for trouble.  Perhaps g++ needs an -fARM
switch for compatibility.

Here is some evidence that my test program is correct with respect to the ARM. 
Section 10.3c begins by saying "With multiple inheritance, casting may change
the value of a pointer".  Section 5.4 contains the following sentence: "It
follows that where multiple inheritance is involved casting to and from
undefined types is best avoided" (page 69 of the hardcover edition).  The clear
implication is that when there is only single inheritance, casting back and
forth between base and derived pointers always yields valid pointers, even when
the types are incomplete.  And indeed, this works properly in g++ 2.95.

A minor consideration is that by locating Base at a nonzero offset in Derived,
g++ 3.3 is imposing multiple inheritance overhead even in the single inheritance
case.  The situation I discovered may not be all that rare, so the inefficiency
could be uncomfortable to some people.

I realize that changing the object layout again will not be fun.  And yet we
have all these legacy programs to worry about.  How can we solve this problem?

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|INVALID                     |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] [3.3/3.4 Regresssion] [aliasing] Pointer wrongly adjusted for derived class containing virtual function

[nathan at gcc dot gnu dot org]

------- Additional Comments From nathan at gcc dot gnu dot org  2003-12-31 09:55 -------
Base is at a non-zero offset in Derived, due to an *optimization* of the ABI.

Does the ARM have the new-style casts at all? I don't have a copy of it, but my
C++ Primer (2nd ed) does not appear to list them.

One solution might be to mark incomplete types that have been old-style casted
between, and then issue a warning if it later turns out that there is an
inheritance relationship between them. That would be an enhancement

g++ will not change to make such old-style casts of incomplete types do an
adjustment.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|critical                    |enhancement


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] [3.3/3.4 Regresssion] [aliasing] Pointer wrongly adjusted for derived class containing virtual function

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2003-12-31 17:20 -------
No the new style cast of re9nterpret_cast was not in the ARM but only in standard C++.
Also ARM based code just needs to be updated, sorry.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |INVALID


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] [3.3/3.4 Regresssion] [aliasing] Pointer wrongly adjusted for derived class containing virtual function

[stephenma at telus dot net]

------- Additional Comments From stephenma at telus dot net  2004-01-01 01:01 -------
Andrew Pinski wrote:
> Also ARM based code just needs to be updated, sorry.

Sorry to spoil your New Year's Eve party.  I think it is risky for g++ to
silently break a previously working, ARM-conforming program.

If g++ does not warn about the use of deprecated features, it is asking too much
of people to manually locate the broken needles in their million-line haystacks.  

In an ideal world, it would be best to have Base at an offset of zero in
Derived, when only single inheritance is involved.  From my narrow point of
view, this would be optimal: it would conform to the ARM and to the C++
standard, and it would be more efficient.  And no warnings would be necessary.

However, if g++ cannot issue warnings, and if the layout of objects cannot be
reformed, a solution is still possible.

Oops, I will be killed if I miss tonight's party; I will discuss the proposed
solution in a day or two, after I recover.  :)


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|INVALID                     |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] [3.3/3.4 Regresssion] [aliasing] Pointer wrongly adjusted for derived class containing virtual function

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-01-01 01:09 -------
Well one more thing the C++ standard does not say this needs to be diagnosted so there is 
nothing here to see, try to compile your program on any other standard compiler and it might 
compile it right or it might compile it right who knows as it is undefined.

The reason why the standard was done is remove some of the ambousities in ARM and compilers; 
ARM is not a standard.  A lot of ARM conferming code cannot be compiled with standard C++ 
compilers.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |INVALID


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] [3.3/3.4 Regresssion] [aliasing] Pointer wrongly adjusted for derived class containing virtual function

[stephenma at telus dot net]

------- Additional Comments From stephenma at telus dot net  2004-01-08 02:01 -------
I will attach a patch against the mainline snapshot gcc-3.4-20031231.   But
first, a bit of discussion.

Andrew Pinski wrote:
> A lot of ARM conferming code cannot be compiled with standard C++ compilers.

I am not suggesting that g++ needs to support the sort of gratuitous
incompatibilities that a certain company in Redmond likes to introduce.  A
program that breaks the rules gets no sympathy from me.

However, there are billions of lines of *working* code that conform to the old
rules (the ARM).  Perfectly compiling all of it may not be possible, but
fairness to these law-abiding programs should compel us to handle as many of
them as we can, or at least to have the compiler warn the programmer when
failure is probable.  Retroactively breaking a working, standard-conforming
program would be unfair, especially if the breakage happens silently.

In my patch I have taken the "warning" approach.  A longer-term solution may
also be possible, as I have hinted before, but the patch is adequate for now.


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|INVALID                     |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer wrongly adjusted for derived class containing virtual function

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-01-08 02:23 -------
What patch are you taking about, can you post it to gcc-patches if you have a patch.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|ABI, wrong-code             |diagnostic
            Summary|[3.3/3.4 Regresssion]       |Pointer wrongly adjusted for
                   |[aliasing] Pointer wrongly  |derived class containing
                   |adjusted for derived class  |virtual function
                   |containing virtual function |
   Target Milestone|3.3.3                       |---


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer wrongly adjusted for derived class containing virtual function

[falk dot hueffner at student dot uni-tuebingen dot de]

------- Additional Comments From falk dot hueffner at student dot uni-tuebingen dot de  2004-01-08 08:10 -------
Subject: Re:  [3.3/3.4 Regresssion] [aliasing] Pointer wrongly adjusted for derived class containing virtual function

"stephenma at telus dot net" <gcc-bugzilla@gcc.gnu.org> writes:

> ------- Additional Comments From stephenma at telus dot net  2004-01-08 02:01 -------
> I will attach a patch against the mainline snapshot gcc-3.4-20031231.


+  if (warn_cast_incomplete)
+    if (!COMPLETE_TYPE_P (TREE_TYPE (type))  ||  !COMPLETE_TYPE_P (TREE_TYPE (otype)))
+      warning ("casting from/to an incomplete type is risky; see -Wno-cast-incomplete");
+

Lines must not be longer than 78 characters. Also leave out the "see",
we don't do that for any other warning.

+class Beta;        *b;   // Incomplete class

Spurious ;.

The changelog lacks file references. Also, you need test cases
(http://gcc.gnu.org/codingconventions.html).  If you have that, I
suggest you post your patch to gcc-patches.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer wrongly adjusted for derived class containing virtual function

[gdr at integrable-solutions dot net]

------- Additional Comments From gdr at integrable-solutions dot net  2004-01-08 18:13 -------
Subject: Re:  [3.3/3.4 Regresssion] [aliasing] Pointer wrongly adjusted for derived class containing virtual function

"stephenma at telus dot net" <gcc-bugzilla@gcc.gnu.org> writes:

| Nathan Sidwell refers to [5.4]/6's explicit disclaimer about the casting of
| incomplete types.  He is technically correct, but only with respect to the Draft
| ANSI C++ Standard (1996).

Not only does the draft say that but both versions of C++93 and C++03
says the same thing.

| There is an enormous legacy of *working* programs that follow the older ARM
| standard, as defined in the Annotated C++ Reference Manual by Ellis and
| Stroustrup.

I have a copy of that book here with me; can you tell me which section
you're looking at? 


| Silently breaking these programs, as g++ 3.x does in the case I
| stumbled upon, is probably asking for trouble.  Perhaps g++ needs an -fARM
| switch for compatibility.
| 
| Here is some evidence that my test program is correct with respect to the ARM. 
| Section 10.3c begins by saying "With multiple inheritance, casting may change
| the value of a pointer".  Section 5.4 contains the following sentence: "It
| follows that where multiple inheritance is involved casting to and from
| undefined types is best avoided" (page 69 of the hardcover edition).

I don't see those sentences as evidence that the behaviour is
guaranteed in the single inheritance case.  That happened to work only
by accident (actually the accident is the use of a particular object
layout which was by no means imposed by ARM).

| The clear
| implication is that when there is only single inheritance, casting back and
| forth between base and derived pointers always yields valid pointers, even when
| the types are incomplete.

I disagree with  the assertion "clear implication".  I don't think it
was even an implication.  

The ARM specification did not impose any requirement on the object
layout.  Sorry.


-- Gaby


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer wrongly adjusted for derived class containing virtual function

[stephenma at telus dot net]

------- Additional Comments From stephenma at telus dot net  2004-01-09 01:33 -------
Falk Hueffner wrote:

> +      warning ("casting from/to an incomplete type is risky; see
-Wno-cast-incomplete");
> Lines must not be longer than 78 characters.

Heh, I knew somebody would mention this.  I went from short to long lines for
readability: several other lines in that file (gcc/cp/typeck.c) are much longer
than 100 characters.


> Also leave out the "see", we don't do that for any other warning.

I think the reference is necessary, because the reason for the warning is rather
subtle.  The intent is to point the programmer to the documentation for
-Wno-cast-incomplete, which is extensive.


> +class Beta;        *b;   // Incomplete class
>
> Spurious ;.

Thanks for catching that typo in the documentation.  I will take this
opportunity to reword a few other things too.


> The changelog lacks file references.

I thought "cvs diff" and "cvs annotate" would make that requirement obsolete,
but OK.


> Also, you need test cases (http://gcc.gnu.org/codingconventions.html).

Ugh.  OK.


> If you have that, I suggest you post your patch to gcc-patches.

Thanks for looking at the patch.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer wrongly adjusted for derived class containing virtual function

[stephenma at telus dot net]

------- Additional Comments From stephenma at telus dot net  2004-01-09 01:46 -------
> Not only does the draft say that but both versions of C++93 and C++03
> says the same thing.

Good.  But I wasn't comparing the 1996 Draft to the '93 or '03 standards, I was
comparing it to the ARM.


> | There is an enormous legacy of *working* programs that follow the older ARM
> | standard, as defined in the Annotated C++ Reference Manual by Ellis and
> | Stroustrup.
> 
> I have a copy of that book here with me; can you tell me which section
> you're looking at? 

I took the quotations from sections 5.4 and 10.3c of the 1990 hardcover edition.


> | Here is some evidence that my test program is correct with respect
> | to the ARM. Section 10.3c begins by saying "With multiple
> | inheritance, casting may change the value of a pointer".  Section
> | 5.4 contains the following sentence: "It follows that where
> | multiple inheritance is involved casting to and from undefined
> | types is best avoided" (page 69 of the hardcover edition).
> 
> I don't see those sentences as evidence that the behaviour is
> guaranteed in the single inheritance case.  That happened to work
> only by accident (actually the accident is the use of a particular
> object layout which was by no means imposed by ARM).
> 
> | The clear implication is that when there is only single
> | inheritance, casting back and forth between base and derived
> | pointers always yields valid pointers, even when the types are
> | incomplete.
> 
> I disagree with  the assertion "clear implication".  I don't think it
> was even an implication.  
> 
> The ARM specification did not impose any requirement on the object
> layout.  Sorry.

If the ARM did not mean to imply what I said it implied, then why was it so
consistent in apparently implying it -- at least twice?  If the intention was to
avoid expressing any preference on object layout, why did the authors even
bother to make the quoted remarks?

The only answer is that the authors were blessing the universal practice of the
time, namely, that a base would always be at offset zero in a derived, when only
single inheritance is involved.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer wrongly adjusted for derived class containing virtual function

[gdr at integrable-solutions dot net]

------- Additional Comments From gdr at integrable-solutions dot net  2004-01-09 04:17 -------
Subject: Re:  Pointer wrongly adjusted for derived class containing virtual function

"stephenma at telus dot net" <gcc-bugzilla@gcc.gnu.org> writes:

| Falk Hueffner wrote:
| 
| > +      warning ("casting from/to an incomplete type is risky; see
| -Wno-cast-incomplete");
| > Lines must not be longer than 78 characters.
| 
| Heh, I knew somebody would mention this.  I went from short to long lines for
| readability: several other lines in that file (gcc/cp/typeck.c) are much longer
| than 100 characters.

yes, but they are not examples to duplicate.

| > Also leave out the "see", we don't do that for any other warning.
| 
| I think the reference is necessary, because the reason for the warning is rather
| subtle.  The intent is to point the programmer to the documentation for
| -Wno-cast-incomplete, which is extensive.

As a matter of existing practrice, we don't say "see xxxx".


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer wrongly adjusted for derived class containing virtual function

[gdr at integrable-solutions dot net]

------- Additional Comments From gdr at integrable-solutions dot net  2004-01-09 04:24 -------
Subject: Re:  Pointer wrongly adjusted for derived class containing virtual function

"stephenma at telus dot net" <gcc-bugzilla@gcc.gnu.org> writes:

[...]

| > | The clear implication is that when there is only single
| > | inheritance, casting back and forth between base and derived
| > | pointers always yields valid pointers, even when the types are
| > | incomplete.
| > 
| > I disagree with  the assertion "clear implication".  I don't think it
| > was even an implication.  
| > 
| > The ARM specification did not impose any requirement on the object
| > layout.  Sorry.
| 
| If the ARM did not mean to imply what I said it implied, then why was it so
| consistent in apparently implying it -- at least twice?

Saying twice it implies, does not actually make it do so. 

| If the intention was to
| avoid expressing any preference on object layout, why did the authors even
| bother to make the quoted remarks?

You probably realized that the problem was more acute for multiple
inheritance -- and at the time, multiple inheritance was most  probably
one of the "feature" people happen to pick on.

| The only answer is that the authors were blessing the universal
| practice of the time, namely, that a base would always be at offset
| zero in a derived, when only single inheritance is involved.

I happen to work with one of the authors.  He never implies to impose
a Single True Object Model.

GCC changed object model it adopted the new ABI since GCC-3.0.
Any code hat relies on a particular preivous object model was broken
since then.

-- Gaby


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer wrongly adjusted for derived class containing virtual function

[stephenma at telus dot net]

------- Additional Comments From stephenma at telus dot net  2004-01-09 08:40 -------
Gabriel Dos Reis wrote:

> | If the ARM did not mean to imply what I said it implied, then why
> | was it so consistent in apparently implying it -- at least twice?
> 
> Saying twice it implies, does not actually make it do so. 

The ARM never explicitly says that "1+1 == 2" is a C++ expression that
evaluates to 1, yet everyone knows it is so.  The standard
mathematical properties are assumed (absent overflow).

My point: when the implications are unambiguous and clear, they are
just as important and just as valid as the explicit statements.


> You probably realized that the problem was more acute for multiple
> inheritance -- and at the time, multiple inheritance was most  probably
> one of the "feature" people happen to pick on.

I can certainly imagine the loudness of the complaints from those who
wanted multiple inheritance to work.  (Personally, I avoid multiple
inheritance like the plague.)  Overall, the g++ people have done well.


> | The only answer is that the authors were blessing the universal
> | practice of the time, namely, that a base would always be at offset
> | zero in a derived, when only single inheritance is involved.
> 
> I happen to work with one of the authors.

Do you want me to congratulate you or feel sorry for you?  Hopefully,
Stroustrup's influence hasn't damaged you too badly.  :)

Are you on the C++ committee?  If so, I appreciate the work you must
have put into it.  The committee has done a huge job cleaning up C++.


> He never implies to impose a Single True Object Model.

He may not have intended to completely prescribe all features of the
object layout.  But that is not the same as saying that the ARM is
absolutely silent on the subject.  My contention is that the ARM is
clear on at least one feature, namely the zero adjustment used in
casting between a base and a derived (in single inheritance).

Even if Stroustrup never intended to prescribe the zero adjustment,
it's too late to retract that feature from the ARM.


> GCC changed object model it adopted the new ABI since GCC-3.0.
> Any code hat relies on a particular preivous object model was broken
> since then.

That's true.  But as I said earlier, let's have some pity for the
maintainers of old ARM code, which is now broken.  The patch for
-Wcast-incomplete is offered in that spirit.

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer wrongly adjusted for derived class containing virtual function

[stephenma at telus dot net]

------- Additional Comments From stephenma at telus dot net  2004-01-09 08:46 -------
Gabriel Dos Reis wrote:

> | Heh, I knew somebody would mention this.  I went from short to
> | long lines for readability: several other lines in that file
> | (gcc/cp/typeck.c) are much longer than 100 characters.
> 
> yes, but they are not examples to duplicate.

OK, I will make the really, really trivial change.


> | > Also leave out the "see", we don't do that for any other warning.
> | 
> | I think the reference is necessary, because the reason for the
> | warning is rather subtle.  The intent is to point the programmer
> | to the documentation for -Wno-cast-incomplete, which is extensive.
> 
> As a matter of existing practrice, we don't say "see xxxx".

Omitting the "see xxx" would do more harm than good.  The g++
maintainers would be flooded with questions about the warning.

If you intend to be rigid about this, please tell me now.  I would
prefer to withdraw the patch rather than be blamed by people who do
not understand what the warning means.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer wrongly adjusted for derived class containing virtual function

[bangerth at dealii dot org]

------- Additional Comments From bangerth at dealii dot org  2004-01-09 15:15 -------
I think that if we usually don't refer to a specific flag in warnings 
and error messages, we should keep to that here as well. However, a good 
way to avoid getting lots of questions is to do the following: 
- list the exact error message text in the manual where you 
  describe this flag, for example when showing a small example 
- leave a small note in the code where the error is issued that 
  the exact wording is mirrored in the manual (so as to remind people 
  changing the wording of the message that they also need to change 
  the manual). 
 
W.    

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer wrongly adjusted for derived class containing virtual function

[gdr at integrable-solutions dot net]

------- Additional Comments From gdr at integrable-solutions dot net  2004-01-10 02:13 -------
Subject: Re:  Pointer wrongly adjusted for derived class containing virtual function

"stephenma at telus dot net" <gcc-bugzilla@gcc.gnu.org> writes:

| ------- Additional Comments From stephenma at telus dot net  2004-01-09 08:40 -------
| Gabriel Dos Reis wrote:
| 
| > | If the ARM did not mean to imply what I said it implied, then why
| > | was it so consistent in apparently implying it -- at least twice?
| > 
| > Saying twice it implies, does not actually make it do so. 
| 
| The ARM never explicitly says that "1+1 == 2" is a C++ expression that
| evaluates to 1, yet everyone knows it is so. 

Proof by analogy is fraud -- dixit one of authors of ARM :-)
You need hard data, not just what analogy you think was in the mind of
the author of ARM -- oh, by the way, the authorS of ARM were actually
the author of the ARM. 

[...]

| > | The only answer is that the authors were blessing the universal
| > | practice of the time, namely, that a base would always be at offset
| > | zero in a derived, when only single inheritance is involved.
| > 
| > I happen to work with one of the authors.
| 
| Do you want me to congratulate you or feel sorry for you?  Hopefully,
| Stroustrup's influence hasn't damaged you too badly.  :)

As you can see, I'm surviving, so I would say that his influence is
doing me more good than bad :-)

| Are you on the C++ committee?  If so, I appreciate the work you must
| have put into it.  The committee has done a huge job cleaning up C++.

yes, I'm on the committee but I did not write those clauses ;-)
Yes, the committee, overall, did a good job.

(My main pet peeves are those rules that are long list of special cases
-- see templates for example)
 
| > He never implies to impose a Single True Object Model.
| 
| He may not have intended to completely prescribe all features of the
| object layout.

I mean he -explicilty- wanted to open the door for different object
model. 

[...]

| > GCC changed object model it adopted the new ABI since GCC-3.0.
| > Any code hat relies on a particular preivous object model was broken
| > since then.
| 
| That's true.  But as I said earlier, let's have some pity for the
| maintainers of old ARM code, which is now broken.  The patch for
| -Wcast-incomplete is offered in that spirit.

I suspect that more diagnostics are always desirable -- where appropriate. 

-- Gaby


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer wrongly adjusted for derived class containing virtual function

[gdr at integrable-solutions dot net]

------- Additional Comments From gdr at integrable-solutions dot net  2004-01-10 02:16 -------
Subject: Re:  Pointer wrongly adjusted for derived class containing virtual function

"stephenma at telus dot net" <gcc-bugzilla@gcc.gnu.org> writes:

| > As a matter of existing practrice, we don't say "see xxxx".
| 
| Omitting the "see xxx" would do more harm than good.  The g++
| maintainers would be flooded with questions about the warning.
| 
| If you intend to be rigid about this, please tell me now.  I would
| prefer to withdraw the patch rather than be blamed by people who do
| not understand what the warning means.

I don't mean to be ridig about it. I'm pointing out a fact and would
like that to be taken into account, when you will be submitting a
revised version.

-- Gaby


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer wrongly adjusted for derived class containing virtual function

[stephenma at telus dot net]

------- Additional Comments From stephenma at telus dot net  2004-01-10 05:45 -------
Wolfgang Bangerth wrote:

> I think that if we usually don't refer to a specific flag in warnings
> and error messages, we should keep to that here as well. However, a good
> way to avoid getting lots of questions is to do the following:
> - list the exact error message text in the manual where you
>   describe this flag, for example when showing a small example
> - leave a small note in the code where the error is issued that
>   the exact wording is mirrored in the manual (so as to remind people
>   changing the wording of the message that they also need to change
>   the manual).

Thank you for the suggestion.  I will consider it, but I would prefer
to keep the "see -W...".

I am puzzled.  If the cross reference in the warning was misleading or
ambiguous, or if the cited documentation was incomplete, then I could
understand why people would object.  Neither is the case here, if I
may say so.  Can someone please explain?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer wrongly adjusted for derived class containing virtual function

[stephenma at telus dot net]

------- Additional Comments From stephenma at telus dot net  2004-01-10 05:48 -------
Gabriel Dos Reis wrote:

> | The ARM never explicitly says that "1+1 == 2" is a C++ expression that
> | evaluates to 1, yet everyone knows it is so. 
> 
> Proof by analogy is fraud -- dixit one of authors of ARM :-)

Well, dixit Stroustrup is not the same as dixit Dominus.  :)

You can't impose mathematical rigor on the loosely-written ARM.  If
you try that, you end up concluding that the value of "1+1==2" is
undefined, as far as the ARM is concerned.

I've enjoyed the argument, but that's enough for me.  The new warning
appears to be acceptable (with a few tweaks), and that is all that
really counts.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer wrongly adjusted for derived class containing virtual function

[stephenma at telus dot net]

------- Additional Comments From stephenma at telus dot net  2004-01-10 05:53 -------
Gabriel Dos Reis wrote:

> | > As a matter of existing practrice, we don't say "see xxxx".
> |
> | Omitting the "see xxx" would do more harm than good.  The g++
> | maintainers would be flooded with questions about the warning.
> |
> | If you intend to be rigid about this, please tell me now.  I would
> | prefer to withdraw the patch rather than be blamed by people who do
> | not understand what the warning means.
> 
> I don't mean to be ridig about it. I'm pointing out a fact and would
> like that to be taken into account, when you will be submitting a
> revised version.

OK, I will consider it, but I have good reasons to prefer leaving in
the cross reference.

As I said to Wolfgang a few messages back, if you or anyone else could
explain why there is such an objection to "see xxxx", I would
appreciate it.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer wrongly adjusted for derived class containing virtual function

[gdr at integrable-solutions dot net]

------- Additional Comments From gdr at integrable-solutions dot net  2004-01-10 16:02 -------
Subject: Re:  Pointer wrongly adjusted for derived class containing virtual function

"stephenma at telus dot net" <gcc-bugzilla@gcc.gnu.org> writes:

| You can't impose mathematical rigor on the loosely-written ARM.  If
| you try that, you end up concluding that the value of "1+1==2" is
| undefined, as far as the ARM is concerned.

No.  There is a difference between both two situations.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer adjusted for derived class containing virtual function

[pinskia at gcc dot gnu dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |NEW
            Summary|Pointer wrongly adjusted for|Pointer adjusted for derived
                   |derived class containing    |class containing virtual
                   |virtual function            |function


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005

[Bug c++/13005] Pointer adjusted for derived class containing virtual function

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2005-09-23 22:34 -------
Note did you post your patch to gcc-patches@gcc.gnu.org?

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13005