Bug 27177 - [4.1/4.2/4.3 Regression] ICE in build_simple_base_path, at cp/class.c:474
Summary: [4.1/4.2/4.3 Regression] ICE in build_simple_base_path, at cp/class.c:474
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.1.0
: P1 normal
Target Milestone: 4.2.3
Assignee: Jason Merrill
URL:
Keywords: ice-on-valid-code, monitored, rejects-valid
: 34978 34984 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-04-16 09:30 UTC by Paul R. Nash
Modified: 2008-01-28 02:20 UTC (History)
12 users (show)

See Also:
Host:
Target:
Build:
Known to work: 3.4.4
Known to fail: 4.1.0 4.2.0 4.3.0
Last reconfirmed: 2008-01-15 09:47:36


Attachments
Condensed .cpp file. (1.80 KB, text/plain)
2006-04-16 09:32 UTC, Paul R. Nash
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Paul R. Nash 2006-04-16 09:30:27 UTC
This is my first gcc bug, so please be kind...

I'm in the process of upgrading my company's toolchain from 3.4.4 to 4.1.0 (mostly because we want ARM EABI support...but that's not relevant, it turns out this bug happens on both i686 and ARM targets).

I narrowed down a particular usage of Loki's Type Manipulation templates combined with a common _C_ASSERT trick that was causing an ICE:

ice-cp-class_c_474.cpp: In instantiation of 'Loki::Conversion<const volatile Event*, const volatile IBar*>':
ice-cp-class_c_474.cpp:62:   instantiated from 'Loki::Conversion<const volatile IBar*, const volatile Event*>'
ice-cp-class_c_474.cpp:118:   instantiated from 'Loki::SuperSubclassStrict<IBar, Event>'
ice-cp-class_c_474.cpp:201:   instantiated from here
ice-cp-class_c_474.cpp:58: internal compiler error: in build_simple_base_path, at cp/class.c:474
Please submit a full bug report,

I will attach my one-page self-contained .cpp file.

This is known to work on gcc-3.4.4.

My compiler is configured thusly:

/usr/local/i686/4.1.0-1a/bin/g++ -v ice-cp-class_c_474.cpp
Using built-in specs.
Target: i686-pc-linux-gnu
Configured with: .././gcc-4.1.0-pass2/configure --prefix=/usr/local/i686/4.1.0-1a --libexecdir=/usr/local/i686/4.1.0-1a/lib --with-local-prefix=/usr/local/i686/4.1.0-1a --enable-threads=posix --enable-c99 --enable-clocale=gnu --enable-shared --enable-__cxa_atexit --enable-languages=c,c++ --enable-long-long --disable-libstdcxx-pch
Thread model: posix
gcc version 4.1.0
Comment 1 Paul R. Nash 2006-04-16 09:32:39 UTC
Created attachment 11280 [details]
Condensed .cpp file.

Here's a repro scenario...I do not know exactly if every class in the inheritance tree is strictly necessary.

This very closely resembles the code in my product that is actually tripping on this.
Comment 2 Paul R. Nash 2006-04-16 09:34:32 UTC
Known to work on 3.4.4
Comment 3 Andrew Pinski 2006-04-16 23:13:40 UTC
Confirmed, reduced testcase:
template <class T, class U>
struct Conversion
{
    static T a;
    enum { exists = sizeof((U)(a)) };
};
template<class T,class U>
struct SuperSubclassStrict
{
    enum { value = (Conversion<U*, T*>::exists)};
};
struct IObject
{
    virtual unsigned long A(void) = 0;
};
struct IFoo : IObject{};
struct IBar : IObject {};
struct Component : IFoo, IBar
{
};
struct Event : Component
{
  static const int t = SuperSubclassStrict<IBar, Event>::value;
};
Comment 4 Volker Reichelt 2006-06-02 22:38:32 UTC
Here's an even shorter testcase:

=============================================
struct X {};

struct Y : virtual X {};
struct Z : virtual X {};

struct A : Y, Z {};

struct B : A
{
  static const int i = sizeof((Z*)(B*)0);
};
=============================================

A simpler testcase was fixed (or maybe only papered over) in GCC 4.0.3:

=============================================
struct A { virtual ~A(); };

struct B : A
{
  static const int i = sizeof((A*)(B*)0);
};
=============================================

A slight modification results in a testcase that crashes with a segfault
since GCC 3.2:

=============================================
struct A {};

struct B : virtual A
{
  static const int i = sizeof((A*)(B*)0);
};
=============================================
Comment 5 Mark Mitchell 2006-06-06 21:39:03 UTC
Subject: Bug 27177

Author: mmitchel
Date: Tue Jun  6 21:38:54 2006
New Revision: 114448

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=114448
Log:
	PR c++/27177
	* call.c (standard_conversion): Require that the derived type be
	complete when performing a derived-to-base conversion.
	PR c++/27177
	* g++.dg/expr/cast7.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/expr/cast7.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/call.c
    trunk/gcc/testsuite/ChangeLog

Comment 6 Mark Mitchell 2006-06-06 21:39:43 UTC
Subject: Bug 27177

Author: mmitchel
Date: Tue Jun  6 21:39:33 2006
New Revision: 114449

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=114449
Log:
	PR c++/27177
	* call.c (standard_conversion): Require that the derived type be
	complete when performing a derived-to-base conversion.
	PR c++/27177
	* g++.dg/expr/cast7.C: New test.

Added:
    branches/gcc-4_1-branch/gcc/testsuite/g++.dg/expr/cast7.C
Modified:
    branches/gcc-4_1-branch/gcc/cp/ChangeLog
    branches/gcc-4_1-branch/gcc/cp/call.c
    branches/gcc-4_1-branch/gcc/testsuite/ChangeLog

Comment 7 Mark Mitchell 2006-06-06 21:42:57 UTC
Fixed in 4.1.2, 4.2.0.
Comment 8 zak.kipling 2006-08-18 11:21:06 UTC
However, that patch breaks the following code, which requires *implicit* derived-to-base conversion:

================================
struct Z {};
struct A : Z {};

Z* implicitToZ (Z*);

struct B : A
{
          static const int i = sizeof(implicitToZ((B*)0));
};
================================

Without the patch, the above compiles cleanly (as it does in 4.0.3, 4.1.1 and Comeau). With the patch, I get:

$ g++ -c dtob.cc
dtob.cc:8: error: invalid conversion from 'B*' to 'Z*'
dtob.cc:8: error:   initializing argument 1 of 'Z* implicitToZ(Z*)'


The standard (in 4.10 [conv.ptr], paragraph 3) states that a program requiring a derived-to-base conversion is ill-formed if the base class is inaccessible or ambiguous; it does not mention any requirement that the derived class must be complete.


This breaks code like this using boost::is_base_and_derived (this is cut down from a real-world example):

================================
#include <boost/type_traits/is_base_and_derived.hpp>
#include <boost/static_assert.hpp>

struct Base { };

template< typename T >
struct A {
        BOOST_STATIC_ASSERT(( boost::is_base_and_derived< Base, T >::value ));
        struct Inner { };
};

struct Derived : Base {
        typedef A< Derived >::Inner Inner;
};
================================

Comment 9 Mark Mitchell 2006-09-09 03:17:03 UTC
The case in Comment #8 is now broken on 4.1/4.2.
Comment 10 Mark Mitchell 2006-09-10 22:04:05 UTC
I am not convinced that the code in Comment #8 is valid.

Although the operand of sizeof is not in fact evaluated, it seems odd to permit an operand which cannot, even in principle, be evaluated.  This is not even a case in which evaluating the operand would lead to undefined behavior; there is simply no way to evaluate the operand at all.  If there is an implicit conversion from B* to Z* at this point, then we must know how to performn the conversion, but we cannot, since B is not complete.

Are you arguing that in:

  struct B {};
  struct D : public B {                                      
    static const int i = sizeof((B*)(D*)0);                  
  };

the conversion from D* to B* is a static_cast?

Has anyone asked about this case on the core reflector?
Comment 11 Gabriel Dos Reis 2007-02-03 16:51:32 UTC
Won't fix in GCC-4.0.x.  Adjusting milestone.
Comment 12 Lawrence Crowl 2007-05-02 02:24:20 UTC
(In reply to comment #10)
> I am not convinced that the code in Comment #8 is valid.
> 
> Although the operand of sizeof is not in fact evaluated, it seems odd to
> permit an operand which cannot, even in principle, be evaluated.  This is
> not even a case in which evaluating the operand would lead to undefined
> behavior; there is simply no way to evaluate the operand at all.  If there
> is an implicit conversion from B* to Z* at this point, then we must know
> how to perform the conversion, but we cannot, since B is not complete.

While that view has merit, I find no requirement in the standard that
requires a complete class.  Setting that aside s possibly unreasonable,
I think 4.10 paragraph 3 "The null pointer value is converted to the null
pointer value of the destination type." enables conversion of null pointers
when the pointer type has known bases but is not yet complete.

> Are you arguing that in:
> 
>   struct B {};
>   struct D : public B {                                      
>     static const int i = sizeof((B*)(D*)0);                  
>   };
> 
> the conversion from D* to B* is a static_cast?

I think (B*)(D*)0 is a conversion under 4.10.

> Has anyone asked about this case on the core reflector?

Would you like me to?

Comment 13 Mark Mitchell 2007-05-02 20:02:28 UTC
Subject: Re:  [4.0/4.1/4.2/4.3 Regression] ICE in build_simple_base_path,
 at cp/class.c:474

crowl at google dot com wrote:

> I think (B*)(D*)0 is a conversion under 4.10.
> 
>> Has anyone asked about this case on the core reflector?
> 
> Would you like me to?

Yes, please!

If this is considered valid, it's important to understand why: is it
your NULL pointer argument, or that this is a static_cast, even though D
isn't complete yet?  That will affect the validity of:

  struct B {};
  struct D;
  D* p;
  struct D: public B {
    static const int i = sizeof ((B*)p);
  };

which is similar, but does not involve the NULL pointer.

Thanks,

Comment 14 Steven Bosscher 2007-12-16 23:21:30 UTC
Ping!
Comment 15 Mark Mitchell 2007-12-18 05:38:14 UTC
Lawrence, was there any feedback on the core reflector about this issue?
Comment 16 Richard Biener 2008-01-14 11:19:15 UTC
Ping!
Comment 17 Lawrence Crowl 2008-01-14 21:29:01 UTC
Subject: Re:  [4.1/4.2/4.3 Regression] ICE in build_simple_base_path, at cp/class.c:474

The consensus of the C++ standards reflector is that all three
following code snippets are well-formed.

explicit:
   struct B {};
   struct D : public B {
       static const int i = sizeof((B*)(D*)0);
   };

implicit:
   struct Z {};
   struct A : Z {};
   Z* implicitToZ (Z*);
   struct B : A {
       static const int i = sizeof(implicitToZ((B*)0));
   };

non-null:
   struct B {};
   struct D;
   D* p;
   struct D: public B {
       static const int i = sizeof ((B*)p);
   };

The rational is that even though the classes are not complete
within their body, the bases must be known.  The reason is that
other features of the language, like co-variant returns, would fail.
Since the bases are known, the conversions are well-formed.

Comment 18 Richard Biener 2008-01-15 09:47:36 UTC
Thanks.
Comment 19 Mark Mitchell 2008-01-21 02:00:04 UTC
Lawrence, thanks for looking into this.  

Was there any consensus on whether or not these are static_casts in this context?

I'm guessing that the eventual resolution is going to be something like saying that a cast is a static_cast even if the class is incomplete, so long as the bases are known, but that such a static_cast can only be used in an un-evaluated context.  Is that correct?
Comment 20 Lawrence Crowl 2008-01-21 20:49:50 UTC
Subject: Re:  [4.1/4.2/4.3 Regression] ICE in build_simple_base_path, at cp/class.c:474

On 21 Jan 2008 02:00:07 -0000, mmitchel at gcc dot gnu dot org
<gcc-bugzilla@gcc.gnu.org> wrote:
>
>
> ------- Comment #19 from mmitchel at gcc dot gnu dot org  2008-01-21 02:00 -------
> Lawrence, thanks for looking into this.
>
> Was there any consensus on whether or not these are static_casts in this
> context?

The discussion didn't say so explicitly, but the obvious conclusion was
that they were static_casts.

> I'm guessing that the eventual resolution is going to be something like
> saying that a cast is a static_cast even if the class is incomplete, so long
> as the bases are known, but that such a static_cast can only be used
> in an un-evaluated context.  Is that correct?

If I understand correctly, this issue can only arise in constant expressions.
In any other context (as in default arguments or member bodies) the class
would be complete, and hence the issue is moot.  Is there something that
you had in mind?

Comment 21 Richard Biener 2008-01-25 15:45:06 UTC
Jason, can you coordinate with Mark and help with the remaining P1 C++ regressions?
Comment 22 Jason Merrill 2008-01-26 00:42:35 UTC
Subject: Bug 27177

Author: jason
Date: Sat Jan 26 00:41:49 2008
New Revision: 131855

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=131855
Log:
        PR c++/27177
        * class.c (build_base_path): Don't mess with virtual access if
        skip_evaluation.
        * call.c (standard_conversion): Don't check whether source type
        is complete.

Added:
    trunk/gcc/testsuite/g++.dg/expr/cast9.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/call.c
    trunk/gcc/cp/class.c

Comment 23 Jakub Jelinek 2008-01-26 08:31:04 UTC
Fixed on the trunk.
Comment 24 Richard Biener 2008-01-26 11:40:27 UTC
*** Bug 34978 has been marked as a duplicate of this bug. ***
Comment 25 Richard Biener 2008-01-26 11:41:53 UTC
The g++.dg/expr/cast7.C failure re-appeared with the last patch.  Re-opening
the regression against 4.3.
Comment 26 Richard Biener 2008-01-26 18:38:39 UTC
*** Bug 34984 has been marked as a duplicate of this bug. ***
Comment 27 Jason Merrill 2008-01-28 02:20:25 UTC
Subject: Bug 27177

Author: jason
Date: Mon Jan 28 02:19:38 2008
New Revision: 131899

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=131899
Log:
        PR c++/27177
        * class.c (build_base_path): Fix previous change.

Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/class.c

Comment 28 Jason Merrill 2008-01-28 02:20:38 UTC
Really fixed.
Comment 29 Jason Merrill 2008-01-28 16:19:59 UTC
Subject: Bug 27177

Author: jason
Date: Mon Jan 28 16:18:56 2008
New Revision: 131905

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=131905
Log:
        PR c++/27177
        * class.c (build_base_path): Fix previous change.

        PR c++/27177
        * class.c (build_base_path): Don't mess with virtual access if
        skip_evaluation.
        * call.c (standard_conversion): Don't check whether source type
        is complete.

Modified:
    branches/gcc-4_2-branch/gcc/cp/ChangeLog
    branches/gcc-4_2-branch/gcc/cp/call.c
    branches/gcc-4_2-branch/gcc/cp/class.c

Comment 30 Jason Merrill 2008-01-28 16:28:04 UTC
Subject: Bug 27177

Author: jason
Date: Mon Jan 28 16:27:17 2008
New Revision: 131907

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=131907
Log:
        PR c++/27177
        * class.c (build_base_path): Fix previous change.

        PR c++/27177
        * class.c (build_base_path): Don't mess with virtual access if
        skip_evaluation.
        * call.c (standard_conversion): Don't check whether source type
        is complete.

        PR c++/33959
        * pt.c (tsubst_aggr_type): Make sure our context is complete.

Modified:
    branches/gcc-4_1-branch/gcc/cp/ChangeLog
    branches/gcc-4_1-branch/gcc/cp/call.c
    branches/gcc-4_1-branch/gcc/cp/class.c
    branches/gcc-4_1-branch/gcc/cp/pt.c