[Bug libobjc/48177] New: incorrect registration of typed selectors

[Bug libobjc/48177] New: incorrect registration of typed selectors

[rfm at gnu dot org]

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

           Summary: incorrect registration of typed selectors
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libobjc
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: rfm@gnu.org


In __sel_register_typed_name() we try to avoid registration of duplicate
selectors, but the code uses strcmp() to check for selectors with the same type
... unless I'm missing something, it should probably be using sel_typesMatch()

[Bug libobjc/48177] incorrect registration of typed selectors

[nicola at gcc dot gnu.org]

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

Nicola Pero <nicola at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nicola at gcc dot gnu.org

--- Comment #1 from Nicola Pero <nicola at gcc dot gnu.org> 2011-03-18 14:36:50 UTC ---
Good point.  Are you thinking of differences caused by protocol qualifiers,
or by argframe layout information ?

Thanks

[Bug libobjc/48177] incorrect registration of typed selectors

[rfm at gnu dot org]

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

--- Comment #2 from rfm at gnu dot org 2011-03-18 14:58:10 UTC ---
The problem showed up with argframe information in base ... which presumably
means that something in base is getting that wrong :-(

I wonder, could we have a runtime function to take a type encoding without
argframe info, and convert it to one with argframe info using the same
algorithm the compiler uses?  That would avoid us having to do that for
NSMethodSignature in base (and potentially getting it wrong).

[Bug libobjc/48177] incorrect registration of typed selectors

[nicola at gcc dot gnu.org]

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

Nicola Pero <nicola at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011.03.18 15:22:25
     Ever Confirmed|0                           |1

--- Comment #3 from Nicola Pero <nicola at gcc dot gnu.org> 2011-03-18 15:22:25 UTC ---

> The problem showed up with argframe information in base ... which presumably
> means that something in base is getting that wrong :-(

Ok - I see.  So, something in base is trying to create a typed selector at
runtime, and register it.  The argframe information in the generated types
doesn't  match the existing one (probably because they are generated buggy,
it's hard to generate them reliably), and the selector gets registered again.

If we were using sel_types_match(), then gnustep-base wouldn't need to provide
argframe information at all, assuming a selector with the same type already 
exists ?  Or if it provides a buggy one, it would be discarded in favour of the
existing one ?

Assuming that most compiler-generated typed selectors (which include valid
type information) are loaded in the runtime before any selector generated
by gnustep-base (or anything else) on the fly at runtime (which may include 
buggy types), then this should generally work in making sure the correct
types are used. :-)

So, I guess we should do it as it should get things to work generally better.

Is this a critical bug ?  Ie, would it actually cause any visible trouble
to users (as opposed to some inefficiency) ?  If so, we need a testcase so
we can backport it at some point to 4.6.x.


> I wonder, could we have a runtime function to take a type encoding without
> argframe info, and convert it to one with argframe info using the same
> algorithm the compiler uses?

If we could do that, then we would not need the argframe info in the selectors
at all. ;-)

In fact, maybe we should get rid of it, or hide it "more" inside the runtime.

It would be nice to audit exactly when and how it is used, and what the 
relationships are between the various parts.  Ideally we'd get rid of the
need for gnustep-base or even end-users to see or know about the argframe
layout information.  Let's have a chat about that offline.

Thanks

[Bug libobjc/48177] incorrect registration of typed selectors

[rfm at gnu dot org]

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

--- Comment #4 from rfm at gnu dot org 2011-03-21 10:08:31 UTC ---
The bug did cause proxies to fail ... but I implemented a workaround for that
in base, so it's not critical for gnustep once the new base is released (and
older versions of base use the old runtime api anyway).

As far as argframe stuff goes in base ... it would be nice to be able to remove
it ... perhaps we can.  It's part of the public API ... but not of the OSX API
as far as I know.  maybe it could be deprecated and removed soon.

[Bug libobjc/48177] incorrect registration of typed selectors

[nicola at gcc dot gnu.org]

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

--- Comment #6 from Nicola Pero <nicola at gcc dot gnu.org> 2011-05-24 21:43:47 UTC ---
I applied the fix.  Waiting for approval to apply it for the 4.6 branch as well 
so that it gets into 4.6.1. :-)

Thanks

[Bug libobjc/48177] incorrect registration of typed selectors

[nicola at gcc dot gnu.org]

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

--- Comment #5 from Nicola Pero <nicola at gcc dot gnu.org> 2011-05-24 21:39:29 UTC ---
Author: nicola
Date: Tue May 24 21:39:24 2011
New Revision: 174143

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=174143
Log:
In libobjc/:
2011-05-24  Nicola Pero  <nicola.pero@meta-innovation.com>

    PR libobjc/48177
    * selector.c (__sel_register_typed_name): Use sel_types_match()
    instead of strcmp() to compare selector types (Suggestion by
    Richard Frith-Macdonald <rfm@gnu.org>).

In gcc/testsuite/:
2011-05-24  Nicola Pero  <nicola.pero@meta-innovation.com>

    PR libobjc/48177
    * objc.dg/pr48177.m: New testcase.    

Added:
    trunk/gcc/testsuite/objc.dg/pr48177.m
Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/libobjc/ChangeLog
    trunk/libobjc/selector.c

[Bug libobjc/48177] incorrect registration of typed selectors

[nicola at gcc dot gnu.org]

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

--- Comment #7 from Nicola Pero <nicola at gcc dot gnu.org> 2011-05-25 09:08:00 UTC ---
Author: nicola
Date: Wed May 25 09:07:57 2011
New Revision: 174176

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=174176
Log:
In libobjc/:
2011-05-25  Nicola Pero  <nicola.pero@meta-innovation.com>

    Backport from mainline
    2011-05-24  Nicola Pero  <nicola.pero@meta-innovation.com>

    PR libobjc/48177
    * selector.c (__sel_register_typed_name): Use sel_types_match()
    instead of strcmp() to compare selector types (Suggestion by
    Richard Frith-Macdonald <rfm@gnu.org>).

In gcc/testsuite/:
2011-05-25  Nicola Pero  <nicola.pero@meta-innovation.com>

    Backport from mainline    
    2011-05-24  Nicola Pero  <nicola.pero@meta-innovation.com>

    PR libobjc/48177
    * objc.dg/pr48177.m: New testcase.    

Added:
    branches/gcc-4_6-branch/gcc/testsuite/objc.dg/pr48177.m
Modified:
    branches/gcc-4_6-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_6-branch/libobjc/ChangeLog
    branches/gcc-4_6-branch/libobjc/selector.c

[Bug libobjc/48177] incorrect registration of typed selectors

[nicola at gcc dot gnu.org]

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

Nicola Pero <nicola at gcc dot gnu.org> changed:

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

--- Comment #8 from Nicola Pero <nicola at gcc dot gnu.org> 2011-05-25 09:13:32 UTC ---
Applied to the 4.6 branch as well.  4.6.1 will include this fix.

Thanks