[parisc-linux] more on canonicalize_funcptr_for_compare

[parisc-linux] more on canonicalize_funcptr_for_compare

[Randolph Chung]

The latest glibc on hppa (2.3.1-17) was compiled with a gcc-3.2 which
has the __canonicalize_funcptr_for_compare patch backported from gcc-3.3
... with this new version, some programs will die on startup (e.g. vim).
I traced to a segfault in __canonicalize_funcptr_for_compare. 

The testcase below illustrates the problem:

=====
typedef void (*func_t)(void);

#define N ((func_t)2)

void foo(void) {};

int main(int argc, char **argv)
{
        return (foo == N);
}
=====

this program segfaults when run with:

(gdb) run
Starting program: /home/tausq/sig

Program received signal SIGSEGV, Segmentation fault.
0x00010530 in __canonicalize_funcptr_for_compare ()
(gdb) bt
#0  0x00010530 in __canonicalize_funcptr_for_compare ()
#1  0x000104bc in main (argc=1, argv=0xfaf001b8) at sig.c:9

it seems like the 2 in the constant (N) confuses the comparision
routine. If I change it to 1, everything is ok.... I don't really
understand this tho, because the code for __c_f_f_c has something like:

if ((int) fptr == -1 || (unsigned int) fptr < 4096 || !((int) fptr & 2))
    return (unsigned int) fptr;

so, why doesn't that match the second || case and exit?  (gdb
disassmbly shows that the code tries to dereference the fptr argument
and segfaults)

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [parisc-linux] more on canonicalize_funcptr_for_compare

[Matthew Wilcox]

On Tue, May 13, 2003 at 10:52:25PM -0700, Randolph Chung wrote:
> The latest glibc on hppa (2.3.1-17) was compiled with a gcc-3.2 which
> has the __canonicalize_funcptr_for_compare patch backported from gcc-3.3
> ... with this new version, some programs will die on startup (e.g. vim).
> I traced to a segfault in __canonicalize_funcptr_for_compare. 
> 
> The testcase below illustrates the problem:
> 
> =====
> typedef void (*func_t)(void);
> 
> #define N ((func_t)2)
> 
> void foo(void) {};
> 
> int main(int argc, char **argv)
> {
>         return (foo == N);
> }
> =====
> 
> this program segfaults when run with:
> 
> (gdb) run
> Starting program: /home/tausq/sig
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x00010530 in __canonicalize_funcptr_for_compare ()
> (gdb) bt
> #0  0x00010530 in __canonicalize_funcptr_for_compare ()
> #1  0x000104bc in main (argc=1, argv=0xfaf001b8) at sig.c:9
> 
> it seems like the 2 in the constant (N) confuses the comparision
> routine. If I change it to 1, everything is ok.... I don't really
> understand this tho, because the code for __c_f_f_c has something like:
> 
> if ((int) fptr == -1 || (unsigned int) fptr < 4096 || !((int) fptr & 2))
>     return (unsigned int) fptr;
> 
> so, why doesn't that match the second || case and exit?  (gdb
> disassmbly shows that the code tries to dereference the fptr argument
> and segfaults)

Even if this were fixed, it seems like a quality of implementation issue.
Basically, we're saying that if none of these conditions are met, it's
safe to dereference this pointer, and I'm sure we'll find people stuffing
other magic values into pointers.

I see three options:

1) Continue with this, accepting that some badly written software won't
   run.

2) Install a signal handler that handles the segfault (we can lose two of
   the tests this way, so it'll be faster in the common case)

3) Change the ABI.  Make it so we always have unique PLabels.  Recompile
   anything necessary.

-- 
"It's not Hollywood.  War is real, war is primarily not about defeat or
victory, it is about death.  I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk

Re: [parisc-linux] more on canonicalize_funcptr_for_compare

[Randolph Chung]

> if ((int) fptr == -1 || (unsigned int) fptr < 4096 || !((int) fptr & 2))
>     return (unsigned int) fptr;
> 
> so, why doesn't that match the second || case and exit?  (gdb
> disassmbly shows that the code tries to dereference the fptr argument
> and segfaults)

sigh, after some sleep i realize that i was looking at the real 3.3 tree
instead of my backport, and in the current 3.2 patch the second test is
missing....  (because it was derived from an earlier snapshot of jda's
patch)

i'll send an update to debian-gcc... sorry about this.

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/

Re: [parisc-linux] more on canonicalize_funcptr_for_compare

[John David Anglin]

> > if ((int) fptr == -1 || (unsigned int) fptr < 4096 || !((int) fptr & 2))
> >     return (unsigned int) fptr;
> > 
> > so, why doesn't that match the second || case and exit?  (gdb
> > disassmbly shows that the code tries to dereference the fptr argument
> > and segfaults)
> 
> Even if this were fixed, it seems like a quality of implementation issue.
> Basically, we're saying that if none of these conditions are met, it's
> safe to dereference this pointer, and I'm sure we'll find people stuffing
> other magic values into pointers.

The magic values were arbitrarily chosen to be the same as under hpux.

> I see three options:
> 
> 1) Continue with this, accepting that some badly written software won't
>    run.

I'm not going to lose sleep on this one.  Obviously, using implementation
dependent features of function pointers is not portable.

> 2) Install a signal handler that handles the segfault (we can lose two of
>    the tests this way, so it'll be faster in the common case)
> 
> 3) Change the ABI.  Make it so we always have unique PLabels.  Recompile
>    anything necessary.

It would be nice to have the 32 and 64 bit ABIs the same in this
respect.  However, this requires non trivial changes to the dynamic
loader and linker.  I'm not sure what the extra overhead would be.
Function pointer comparisons aren't done very often in user code.

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)