[Bug libstdc++/5625] exception unwinding creates invalid pointer on mips

[Bug libstdc++/5625] exception unwinding creates invalid pointer on mips

[echristo at redhat dot com]

PLEASE REPLY TO gcc-bugzilla@gcc.gnu.org ONLY, *NOT* gcc-bugs@gcc.gnu.org.

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


echristo at redhat dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


------- Additional Comments From echristo at redhat dot com  2003-10-22 05:52 -------
Pretty sure this is fixed now.

[Bug libstdc++/5625] [mips] exception unwinding creates invalid pointer on mips

[wilson at gcc dot gnu dot org]

------- Additional Comments From wilson at gcc dot gnu dot org  2004-02-05 02:39 -------
This bug still exists on the gcc-3.4 branch, and presumably also mainline.  It
can be reproduced by configuring for an sb1-elf target which defaults to the O64
ABI, linking with the idt.ld (or idt64.ld) linker script, and running the g++
testsuite on the simulator.

I don't see any evidence that anything was done to fix this problem.  It was
closed because test results from cgd did not show the bug, but this test was
flawed.  The problem shows up only if you have 64-bit code generation, an ILP32
type system, pointers with the high bit set, and pointers that do not extend
unsigned.  The test that cgd did used the mipsisa64 target, which uses an LP64
type system by default and thus does not show the bug.  Also, cgd tested
mipsisa64 with the -mips32 flag which generates 32-bit code, and thus does not
show the bug.  In order to reproduce the bug, you have to use the O64 or N32
ABIs, or use the -mlong32 option with one of the true 64-bit ABIs.  The problem
is easiest to see if you use an embedded target that defaults to the O64 ABI.

I tried to fix the problem in the personality routine by replacing the
_Unwind_Ptr cast with an _Unwind_Word cast.  The cast is there only to suppress
a compiler warning about a cast from pointer to integer.  Unfortunately, this
gives us a compiler warning about casting from a pointer to an integer of a
different size which is unfortunate.  Also, this introduces a new problem, in
that targets which need unsigned extension for pointers are now broken.  The
convert() function always does a signed extension when converting pointers to a
larger sized int.  This is probably a bug, and is documented as such.  What we
really need here is machine dependent conversion based on the value of the
POINTER_EXTEND_UNSIGNED macro.

The only place in the compiler that does conversions based on
POINTER_EXTEND_UNSIGNED is the convert_memory_address function which operates on
RTL.  It does machine dependent conversions from ptr_mode to Pmode and vice
versa.  So a possible solution here is to add a builtin function that calls
convert_memory_address, in order to get the proper pointer extension.  This is
fairly easy to do, but it requires that the target define Pmode to the word
size, which strangely the MIPS port does not do.  The MIPS port is the only
64-bit port that I am aware of which sets Pmode to SImode when generating ILP32
code.  This is arguably a bug in the mips port.

So now we need two patches, one which adds the __builtin_extend_pointer
function, and modifies libsupc++ to use it, and one which changes the MIPS
backend to use a DImode Pmode when generating 64-bit code.  The first patch is
easy.  The second one seems to be hard.  I tried making the obvious changes, and
ended up with problems with mixed-modes in address arithmetic.  At the moment, I
am not sure if I missed something, or if there are latent bugs in the MIPS
backend.  I ended up writing a hackish patch that worked well enough to compile
libstdc++ and newlib, which I used as a proof of concept patch to test my ideas.
 The patch is pretty ugly though, in a number of places, I had to give up and
use GET_MODE instead of hard wiring in Pmode and/or ptr_mode as the code used to
do.  I will attach these patches to the PR.

I did my experiments using the g++.old-deja/g++.eh/flow1.C testcase, as that
seemed to be one of the simplest EH testcases that was failing.

I applied my hacked up DImode Pmode patch, built a toolchain, and compiled the
flow1.C test.  The personality routine still stores a zero-extended pointer when
it calls _Unwind_SetGR.  However, the testcase now works on the simulator.  I
noticed a couple of reasons for this.  Because Pmode is now DImode, and we have
PROMOTE_MODES defined, we are now generating daddiu where we used to generate
addiu, so there is no longer a chance of tripping unpredictable behaviour where
we did before.  Also, we have PROMOTE_PROTOTYPES defined, which causes the
pointer to be sign-extended in main right before the __cxa_begin_catch call, so
there is no longer a problem there either.

I then applied by __builtin_extend_pointer patch, and now I get the desired
signed pointer extension in the personality routine before the _Unwind_SetGR
call.  However, based on the results of the above, I think that this patch is
unnecessary.

Since I can't think of anyway to solve the problem without changing Pmode to
DImode, and this change alone makes the testcase work, I don't see any real need
for any libsupc++ change in order to fix this problem.  I think the mips backend
change is enough.  Converting the MIPS backend looks like it may be a pain
though.  I ran into problems when I tried a naive approach.  Also, this will
require testing on a number of different systems, some of which I may not have
access to.  I don't have any immediate plans to work on this, but may need to co
me back to it later.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wilson at specifixinc dot
                   |                            |com
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |


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

[Bug libstdc++/5625] [mips] exception unwinding creates invalid pointer on mips

[wilson at gcc dot gnu dot org]

------- Additional Comments From wilson at gcc dot gnu dot org  2004-02-05 02:52 -------
I forgot to mention... there may have been some testing confusion due to changes
to default ABIs.  I think cgd may have used a toolchain whose default ABI
changed on the mips-rewrite branch.  Thus the problem appeared to have been
fixed when it was in fact still present with the old ABI.

-- 


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

[Bug libstdc++/5625] [mips] exception unwinding creates invalid pointer on mips

[cgd at broadcom dot com]

------- Additional Comments From cgd at broadcom dot com  2004-02-05 06:25 -------
Subject: Re:  [mips] exception unwinding creates
 invalid pointer on mips

At 5 Feb 2004 02:45:07 -0000, wilson at gcc dot gnu dot org wrote:
> This patch changes the MIPS backend to use DImode for Pmode when generating
> 64-bit ILP32 code, instead of SImode as it currently does.

I'm a bit concerned about this comment, because SImode values in
registers on MIPS should be in their properly sign-extended forms.

If they aren't, patterns like addsi3 (-> addu) will not work properly
under all conditions.  (addu and other 32-bit arithmetic ops are
UNPREDICTABLE if operands don't have bits 63:32 matching bit 31, i.e.,
if they're not sign extended.  mipsisa64 sim should be reporting that,
as I believe you've noticed.)


Sorry if i manage to help get bug closed prematurely.  That's what
happens when a PHB tries to do real work. 8-)


chris



-- 


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

[Bug libstdc++/5625] [mips] exception unwinding creates invalid pointer on mips

[wilson at specifixinc dot com]

------- Additional Comments From wilson at specifixinc dot com  2004-02-05 07:41 -------
Subject: Re:  [mips] exception unwinding creates invalid
	pointer on mips

On Wed, 2004-02-04 at 22:25, cgd at broadcom dot com wrote:
> I'm a bit concerned about this comment, because SImode values in
> registers on MIPS should be in their properly sign-extended forms.

The MIPS backend knows that SImode values need to be sign-extended, and
it does take care of this.

However, the EH code is doing something very special, similar to what
longjmp does.  Except instead of reading values from a context saved by
a setjmp, we are computing values at runtime, and then storing them into
a context, which later gets stuffed into registers.  When we store
values into the context, if the pointer size does not equal the register
size, then we have a problem.  The code currently assumes that pointers
extend unsigned, which is a reasonable assumption in general, but breaks
the MIPS convention of requiring SImode values to be sign-extended. 
There are actually 3 options here, signed-extension, unsigned-extension,
and pointer swizzling (IA-64).  We need some way to get a machine
dependent conversion of a pointer to a possibly larger integer which
represents a register, an operation which is not part of the C
language.  Gcc does have a convention already that mostly solves this
problem.  You define Pmode to the register size, POINTER_SIZE to the
pointer size, and POINTERS_EXTEND_UNSIGNED to indicate how to convert
pointers.  This is the obvious approach, except that changing the MIPS
backend looks like a lot of work.

I was looking at other 64-bit targets like Alpha and IA-64, but it
occurs to me that MIPS is somewhat different, in that we have a complete
set of both 32-bit and 64-bit operations (or at least we can pretend
that we do have them all), and thus maybe an SImode Pmode makes more
sense for 64-bit ILP32 MIPS than it does for other targets.

If we don't modify the mips backend, then another possible solution
would be to modify the convert_memory_address function so that we can
specify conversions from ptr_mode to word_mode.  Or alternatively create
a specialized version of the function that only does conversions from
ptr_mode to word_mode, for use in the EH code.  This specialized version
could be a lot simpler than convert_memory_address, as it would not have
to handle arbitrary addresses, it only needs to handle MEM and REG I
think.  Actually, looking at this again, I think all we need to do is
call convert_modes.  I think I gave up too early when I looked at this
the first time.  I will have to revisit this option.

I suppose another way to solve this is to modify the code that
saves/restores the context.  When we store a pointer into the context,
we could mark that we stored a pointer value, and then when we load the
context into registers, we do a pointer sized load instead of a word
sized load.  I am not sure whether this can actually work, because
contexts are created in different ways in different places.  There is
also the problem that this requires changes to the standard Unwind API,
and there are already alternative implementations (HPUX libunwind, David
Mosberger linux libunwind) that we would be incompatible with if we go
down this route, so this seems to be a very poor option.


-- 


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

[Bug libstdc++/5625] [mips] exception unwinding creates invalid pointer on mips

[rsandifo at redhat dot com]

------- Additional Comments From rsandifo at redhat dot com  2004-02-05 20:42 -------
Subject: Re:  [mips] exception unwinding creates invalid
 pointer on mips

"wilson at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:
> The MIPS port is the only 64-bit port that I am aware of which sets
> Pmode to SImode when generating ILP32 code.  This is arguably a bug in
> the mips port.

And later:

> I was looking at other 64-bit targets like Alpha and IA-64, but it
> occurs to me that MIPS is somewhat different, in that we have a complete
> set of both 32-bit and 64-bit operations (or at least we can pretend
> that we do have them all), and thus maybe an SImode Pmode makes more
> sense for 64-bit ILP32 MIPS than it does for other targets.

Right.  The Pmode == ptr_mode thing is deliberate: if you change Pmode
to DImode for 32-bit ptr_mode, then the loop optimisers are no longer
able to recognise ivs properly.  Any pointer + integer will involve
a sign-extension.

It was suggested that you can reduce this effect by making PROMOTE_MODE
promote to DImode.  But we (well, OK, Eric ;) tried that for mips-rewrite
and it resulted in a chronic loss of performance.  This is because we can
only enforce the "SImode registers are sign-extended" rule via
TRULY_NOOP_TRUNCATION.  Promoting everything to DImode introduces
a _lot_ of truncations.

FWIW, I like your suggestion of introducing a special builtin-function.

Richard



-- 


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

[Bug libstdc++/5625] [mips] exception unwinding creates invalid pointer on mips

[rsandifo at redhat dot com]

------- Additional Comments From rsandifo at redhat dot com  2004-02-05 20:51 -------
Subject: Re:  [mips] exception unwinding creates invalid
 pointer on mips

Richard Sandiford <rsandifo@redhat.com> writes:
> Right.  The Pmode == ptr_mode thing is deliberate: if you change Pmode
> to DImode for 32-bit ptr_mode, then the loop optimisers are no longer
> able to recognise ivs properly.  Any pointer + integer will involve
> a sign-extension.

Sorry, this was poorly phrased.  I wasn't trying to suggest there was
anything special about 32-bit ptr_mode here: the same problem applies
for 64-bit ptr_mode too.  It's just not a problem we want to spread.

Richard


-- 


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

[Bug libstdc++/5625] [mips] exception unwinding creates invalid pointer on mips

[wilson at gcc dot gnu dot org]

------- Additional Comments From wilson at gcc dot gnu dot org  2004-02-06 21:23 -------
Patch posted to gcc-patches for comment.
    http://gcc.gnu.org/ml/gcc-patches/2004-02/msg00606.html

-- 


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

[Bug libstdc++/5625] [mips] exception unwinding creates invalid pointer on mips

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-02-09 20:40 -------
Subject: Bug 5625

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_4-branch
Changes by:	wilson@gcc.gnu.org	2004-02-09 20:40:37

Modified files:
	gcc            : ChangeLog builtin-types.def builtins.c 
	                 builtins.def except.c except.h 

Log message:
	PR libstdc++/5625, MIPS EH unwind failure
	* builtin-types.def (BT_WORD, BT_FN_WORD_PTR): New.
	* builtins.c (expand_builtin): Handle BUILT_IN_EXTEND_POINTER.
	* builtins.def (BUILT_IN_EXTEND_POINTER): New.
	* except.c (expand_builtin_extend_pointer): New.
	* except.h (expand_builtin_extend_pointer): Declare.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=2.2326.2.166&r2=2.2326.2.167
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/builtin-types.def.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.21&r2=1.21.10.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/builtins.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.275.2.2&r2=1.275.2.3
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/builtins.def.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.73&r2=1.73.10.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/except.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.256.2.1&r2=1.256.2.2
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/except.h.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.72&r2=1.72.10.1



-- 


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

[Bug libstdc++/5625] [mips] exception unwinding creates invalid pointer on mips

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-02-09 20:42 -------
Subject: Bug 5625

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_4-branch
Changes by:	wilson@gcc.gnu.org	2004-02-09 20:42:13

Modified files:
	libstdc++-v3   : ChangeLog 
	libstdc++-v3/libsupc++: eh_personality.cc 

Log message:
	PR libstdc++/5625
	* libsuspc++/eh_personality.cc (PERSONALITY_FUNCTION): Use
	__builtin_extend_pointer.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.2224.2.29&r2=1.2224.2.30
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/libsupc++/eh_personality.cc.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.13&r2=1.13.14.1



-- 


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

[Bug libstdc++/5625] [mips] exception unwinding creates invalid pointer on mips

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-02-09 21:18 -------
Subject: Bug 5625

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	wilson@gcc.gnu.org	2004-02-09 21:18:45

Modified files:
	gcc            : ChangeLog builtin-types.def builtins.c 
	                 builtins.def except.c except.h 

Log message:
	PR libstdc++/5625
	* builtin-types.def (BT_WORD, BT_FN_WORD_PTR): New.
	* builtins.c (expand_builtin): Handle BUILT_IN_EXTEND_POINTER.
	* builtins.def (BUILT_IN_EXTEND_POINTER): New.
	* except.c (expand_builtin_extend_pointer): New.
	* except.h (expand_builtin_extend_pointer): Declare.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.2721&r2=2.2722
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/builtin-types.def.diff?cvsroot=gcc&r1=1.22&r2=1.23
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/builtins.c.diff?cvsroot=gcc&r1=1.282&r2=1.283
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/builtins.def.diff?cvsroot=gcc&r1=1.75&r2=1.76
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/except.c.diff?cvsroot=gcc&r1=1.257&r2=1.258
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/except.h.diff?cvsroot=gcc&r1=1.72&r2=1.73



-- 


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

[Bug libstdc++/5625] [mips] exception unwinding creates invalid pointer on mips

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-02-09 21:20 -------
Subject: Bug 5625

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	wilson@gcc.gnu.org	2004-02-09 21:20:34

Modified files:
	libstdc++-v3   : ChangeLog 
	libstdc++-v3/libsupc++: eh_personality.cc 

Log message:
	PR libstdc++/5625
	* libsuspc++/eh_personality.cc (PERSONALITY_FUNCTION): Use
	__builtin_extend_pointer.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&r1=1.2318&r2=1.2319
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/libsupc++/eh_personality.cc.diff?cvsroot=gcc&r1=1.13&r2=1.14



-- 


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

[Bug libstdc++/5625] [mips] exception unwinding creates invalid pointer on mips

[wilson at gcc dot gnu dot org]

------- Additional Comments From wilson at gcc dot gnu dot org  2004-02-09 23:25 -------
Patches checked in on mainline and 3.4 branch.

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


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