Re: Bounds checking for TREE_VEC_ELT and related bugs (PR c++/8511)

[Gabriel Dos Reis <gdr at integrable-solutions dot net>]

Zack Weinberg <zack@codesourcery.com> writes:

| Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
| 
| > Zack Weinberg <zack@codesourcery.com> writes:
| > | Index: cp/search.c
| > | --- cp/search.c	4 Nov 2002 01:45:55 -0000	1.240
| > | +++ cp/search.c	14 Nov 2002 19:54:51 -0000
| > | @@ -2544,7 +2544,8 @@ dfs_push_decls (binfo, data)
| > |  	  
| > |        method_vec = (CLASS_TYPE_P (type) 
| > |  		    ? CLASSTYPE_METHOD_VEC (type) : NULL_TREE);
| > | -      if (method_vec)
| > | +
| > | +      if (method_vec && TREE_VEC_LENGTH (method_vec) >= 3)
| >
| > I would probably put an assert here is if we have a method_vec then it
| > size ought to be at least 3.
| 
| We get a method_vec of size 2 compiling libstdc++-v3:

Incredible Zack :-) :-)

| /home/zack/src/gcc/vanilla/build/gcc/xgcc -shared-libgcc -B/home/zack/src/gcc/vanilla/build/gcc/ -nostdinc++ -L/home/zack/src/gcc/vanilla/build/i686-pc-linux-gnu/libstdc++-v3/src -L/home/zack/src/gcc/vanilla/build/i686-pc-linux-gnu/libstdc++-v3/src/.libs -B/home/zack/src/i/gcc_ni/i686-pc-linux-gnu/bin/ -B/home/zack/src/i/gcc_ni/i686-pc-linux-gnu/lib/ -isystem /home/zack/src/i/gcc_ni/i686-pc-linux-gnu/include -nostdinc++ -I/home/zack/src/gcc/vanilla/build/i686-pc-linux-gnu/libstdc++-v3/include/i686-pc-linux-gnu -I/home/zack/src/gcc/vanilla/build/i686-pc-linux-gnu/libstdc++-v3/include -I/home/zack/src/gcc/vanilla/libstdc++-v3/libsupc++ -I/home/zack/src/gcc/vanilla/libstdc++-v3/libmath -fno-implicit-templates -Wall -Wno-format -W -Wwrite-strings -Winline -fdiagnostics-show-location=once -ffunction-sections -fdata-sections -g -c /home/zack/src/gcc/vanilla/libstdc++-v3/src/bitset.cc  -fPIC -DPIC -o .libs/bitset.o
| In file included from /home/zack/src/gcc/vanilla/build/i686-pc-linux-gnu/libstdc++-v3/include/memory:55,
|                  from /home/zack/src/gcc/vanilla/build/i686-pc-linux-gnu/libstdc++-v3/include/string:48,
|                  from /home/zack/src/gcc/vanilla/build/i686-pc-linux-gnu/libstdc++-v3/include/bitset:55,
|                  from /home/zack/src/gcc/vanilla/libstdc++-v3/src/bitset.cc:43:
| /home/zack/src/gcc/vanilla/build/i686-pc-linux-gnu/libstdc++-v3/include/bits/stl_alloc.h:381: internal compiler error: tree
|    check: accessed elt 3 of tree_vec with 2 elts in dfs_push_decls, at 
|    cp/search.c:2556

So, you just uncovered another latent bug with your tree_vec bound-checking.
Lovely. 

However, I spoke nonsense in my previous message.  I mistakenly
thought that we always allocate at least 3 slot for the following
usage:
     * one slot for the constructors
     * one slot for the desctructor
     * one slot for the copy-and-assignment operator

My memory was failing since this very comment in cp/cp-tree.h settles
the issue:

  /* Vector member functions defined in this class.  Each element is
     either a FUNCTION_DECL, a TEMPLATE_DECL, or an OVERLOAD.  All
     functions with the same name end up in the same slot.  The first
     two elements are for constructors, and destructors, respectively.
     All template conversion operators to innermost template dependent
     types are overloaded on the next slot, if they exist.  Note, the
     names for these functions will not all be the same.  The
     non-template conversion operators & templated conversions to
     non-innermost template types are next, followed by ordinary member
     functions.  There may be empty entries at the end of the vector.
     The conversion operators are unsorted. The ordinary member
     functions are sorted, once the class is complete.  */

This other comment in cp/class.c:

 if (!CLASSTYPE_METHOD_VEC (type))
    /* Make a new method vector.  We start with 8 entries.  We must
       allocate at least two (for constructors and destructors), and
       we're going to end up with an assignment operator at some point
       as well.
       
       We could use a TREE_LIST for now, and convert it to a TREE_VEC
       in finish_struct, but we would probably waste more memory
       making the links in the list than we would by over-allocating
       the size of the vector here.  Furthermore, we would complicate
       all the code that expects this to be a vector.  */
    CLASSTYPE_METHOD_VEC (type) = make_tree_vec (8);

makes it that the assert should be for 2 not 3.  Sorry for the
confusion. 

I'm still wondering why the copy-and-assignment operator is left out.
Mark can you enlighten me?

-- Gaby