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]
Other format: [Raw text]

misp16 ISA bit breaks EH and pointers-to-member-functions


The fact that the least-significant bit of a pointer-to-function is
set to tell the processor to switch to mips16 mode breaks EH.  If a
function ends in a throw, such as g++.eh/unwind1.C, the labels that
marks the EH region of the function do not have the mips16 bit set,
but the return address of the caller of __cxa_throw does.  When we
subtract one so as to get a pointer that's still inside the EH region,
we get a pointer that's still outside, because the subtraction just
disabled in ISA bit.  Oops.

Evidently, since the lowest bit can be set, we can't use it to tell
pointers to code from indexes to vtable_entry arrays in pointers to
member functions in C++, so we have to use the alternate
representation, at least on mips ports that support mips16.  I've been
discussing this matter with Eric Christopher, and he kind of agrees
with me that we'd probably be better off doing it for all mips ports,
just in case some port that currently does not support mips16 at some
point starts doing so, at which point we'd have to break its ABI in
order to still allow for linking between mips16 and non-mips16 code.
Besides, I think we'd be better off using a uniform representation of
pmfs for all mips targets, so this is what the patch below
implements.

However, I'd like to give some time for people to jump in and try to
get me to change my mind before I put this in :-)

Oh, and I think it would be appropriate for this to go in the 3.1
branch too, since otherwise we'd break the mips C++ ABI between 3.1
and 3.2.

I almost forgot: I'm including a C++ patch by Jeff Knaggs that
corrects a bug I introduced back when I added support for
TARGET_PTRMEMFUNC_VBIT_LOCATION.  He posted the patch to a Red
Hat-internal mailing list just a few days ago, and Jason Merrill kind
of approved it.  I was Jeff would post the patch here soon, but I
ended up going for it myself.

Comments?

Index: gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* config/mips/mips.h (MASK_RETURN_ADDR): Define.
	(TARGET_PTRMEMFUNC_VBIT_LOCATION): Define.

Index: gcc/cp/ChangeLog
from  Jeff Knaggs  <jknaggs@redhat.com>

	* typeck.c (expand_ptrmemfunc_cst): Scale idx down to an index
	into the vtable_entry array regardless of
	TARGET_PTRMEMFUNC_VBIT_LOCATION.

Index: gcc/cp/typeck.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/cp/typeck.c,v
retrieving revision 1.392
diff -u -p -r1.392 typeck.c
--- gcc/cp/typeck.c 2002/03/16 18:30:14 1.392
+++ gcc/cp/typeck.c 2002/03/21 03:12:06
@@ -2890,19 +2890,18 @@ get_member_function_from_ptrfunc (instan
 	 load-with-sign-extend, while the second used normal load then
 	 shift to sign-extend.  An optimizer flaw, perhaps, but it's
 	 easier to make this change.  */
+      idx = cp_build_binary_op (TRUNC_DIV_EXPR, 
+				build1 (NOP_EXPR, vtable_index_type, e3),
+				TYPE_SIZE_UNIT (vtable_entry_type));
       switch (TARGET_PTRMEMFUNC_VBIT_LOCATION)
 	{
 	case ptrmemfunc_vbit_in_pfn:
-	  idx = cp_build_binary_op (TRUNC_DIV_EXPR, 
-				    build1 (NOP_EXPR, vtable_index_type, e3),
-				    TYPE_SIZE_UNIT (vtable_entry_type));
 	  e1 = cp_build_binary_op (BIT_AND_EXPR,
 				   build1 (NOP_EXPR, vtable_index_type, e3),
 				   integer_one_node);
 	  break;
 
 	case ptrmemfunc_vbit_in_delta:
-	  idx = build1 (NOP_EXPR, vtable_index_type, e3);
 	  e1 = cp_build_binary_op (BIT_AND_EXPR,
 				   delta, integer_one_node);
 	  delta = cp_build_binary_op (RSHIFT_EXPR,
Index: gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* g++.old-deja/g++.abi/ptrmem.C: Mips puts vbit in delta too.

Index: gcc/config/mips/mips.h
===================================================================
RCS file: /cvs/gcc/egcs/gcc/config/mips/mips.h,v
retrieving revision 1.170
diff -u -p -r1.170 mips.h
--- gcc/config/mips/mips.h 2002/03/18 19:15:24 1.170
+++ gcc/config/mips/mips.h 2002/03/21 03:12:15
@@ -2425,6 +2425,17 @@ extern enum reg_class mips_char_to_class
 					 RETURN_ADDRESS_POINTER_REGNUM))) \
    : (rtx) 0)
 
+/* Since the mips16 ISA mode is encoded in the least-significant bit
+   of the address, mask it off return addresses for purposes of
+   finding exception handling regions.  */
+
+#define MASK_RETURN_ADDR GEN_INT (-2)
+
+/* Similarly, don't use the least-significant bit to tell pointers to
+   code from vtable index.  */
+
+#define TARGET_PTRMEMFUNC_VBIT_LOCATION ptrmemfunc_vbit_in_delta
+
 /* Structure to be filled in by compute_frame_size with register
    save masks, and offsets for the current function.  */
 
Index: gcc/testsuite/g++.old-deja/g++.abi/ptrmem.C
===================================================================
RCS file: /cvs/gcc/egcs/gcc/testsuite/g++.old-deja/g++.abi/ptrmem.C,v
retrieving revision 1.7
diff -u -p -r1.7 ptrmem.C
--- gcc/testsuite/g++.old-deja/g++.abi/ptrmem.C 2002/02/09 03:06:46 1.7
+++ gcc/testsuite/g++.old-deja/g++.abi/ptrmem.C 2002/03/21 03:12:15
@@ -6,7 +6,7 @@
    function.  However, some platforms use all bits to encode a
    function pointer.  Such platforms use the lowest bit of the delta,
    that is shifted left by one bit.  */
-#if defined __MN10300__ || defined __SH5__ || defined __arm__ || defined __thumb__
+#if defined __MN10300__ || defined __SH5__ || defined __arm__ || defined __thumb__ || defined __mips__
 #define ADJUST_PTRFN(func, virt) ((void (*)())(func))
 #define ADJUST_DELTA(delta, virt) (((delta) << 1) + !!(virt))
 #else

-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                  aoliva@{cygnus.com, redhat.com}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist                Professional serial bug killer

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