[PATCH] libobjc: Properly handle classes without instance variables in class_copyIvarList ().

[PATCH] libobjc: Properly handle classes without instance variables in class_copyIvarList ().

[Dimitris Papavasiliou]

[-- Attachment #1: Type: text/plain, Size: 1230 bytes --]

Hello,

The attached patch fixes an issue reported a couple of years ago in Bug 
51891 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51891).  The problem 
is caused because classes without instance variables have no ivar list 
at all, so that their ivars pointer is NULL, but the code in 
class_copyIvarList () is unaware of this.

That this is in fact so can be easily verified by checking the code of 
class_addIvar in the same source file, where the ivars list is allocated 
when the first ivar is added.  The code there also checks for a NULL 
ivars pointer.

The patch also adds a simple test-case for this issue.  I think that the 
ChangeLog entry should be something along the lines of:

    2014-12-24  Dimitris Papavasiliou  <dpapavas@gmail.com>

          PR libobjc/51891
          * libobjc/ivars.c: Add a check for classes without instance
	variables, which have a NULL ivar list pointer.
          * gcc/testsuite/objc.dg/gnu-api-2-class.m: Add a test case
	for the above change.

I hope I got the formatting right.  I've run make -k check-objc and all 
tests pass without problems.

Let me know if there are any problems, or if I can do anything else to 
facilitate the acceptance of the patch.

Regards,
Dimitris


[-- Attachment #2: class_copyIvarList_bugfix.diff --]
[-- Type: text/x-patch, Size: 1167 bytes --]

Index: gcc/testsuite/objc.dg/gnu-api-2-class.m
===================================================================
--- gcc/testsuite/objc.dg/gnu-api-2-class.m	(revision 219054)
+++ gcc/testsuite/objc.dg/gnu-api-2-class.m	(working copy)
@@ -239,6 +239,19 @@
       abort ();
   }
 
+  printf ("Testing class_copyIvarList () on class with no instance variables...\n");
+  {
+    unsigned int count;
+    Ivar * list = class_copyIvarList (objc_getClass ("MyOtherSubClass"),
+                                      &count);
+
+    if (count != 0)
+      abort ();
+    
+    if (list != NULL)
+      abort ();
+  }
+
   printf ("Testing class_copyMethodList ()...\n");
   {
     unsigned int count;
Index: libobjc/ivars.c
===================================================================
--- libobjc/ivars.c	(revision 219054)
+++ libobjc/ivars.c	(working copy)
@@ -179,7 +179,7 @@
   struct objc_ivar **returnValue = NULL;
   struct objc_ivar_list* ivar_list;
 
-  if (class_ == Nil  ||  CLS_IS_IN_CONSTRUCTION (class_))
+  if (class_ == Nil  ||  CLS_IS_IN_CONSTRUCTION (class_) || !class_->ivars)
     {
       if (numberOfReturnedIvars)
 	*numberOfReturnedIvars = 0;

Re: [PATCH][PING] libobjc: Properly handle classes without instance variables in class_copyIvarList ().

[Dimitris Papavasiliou]

Ping!

On 12/24/2014 07:28 PM, Dimitris Papavasiliou wrote:
> Hello,
>
> The attached patch fixes an issue reported a couple of years ago in Bug
> 51891 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51891).  The problem
> is caused because classes without instance variables have no ivar list
> at all, so that their ivars pointer is NULL, but the code in
> class_copyIvarList () is unaware of this.
>
> That this is in fact so can be easily verified by checking the code of
> class_addIvar in the same source file, where the ivars list is allocated
> when the first ivar is added.  The code there also checks for a NULL
> ivars pointer.
>
> The patch also adds a simple test-case for this issue.  I think that the
> ChangeLog entry should be something along the lines of:
>
>     2014-12-24  Dimitris Papavasiliou  <dpapavas@gmail.com>
>
>           PR libobjc/51891
>           * libobjc/ivars.c: Add a check for classes without instance
>      variables, which have a NULL ivar list pointer.
>           * gcc/testsuite/objc.dg/gnu-api-2-class.m: Add a test case
>      for the above change.
>
> I hope I got the formatting right.  I've run make -k check-objc and all
> tests pass without problems.
>
> Let me know if there are any problems, or if I can do anything else to
> facilitate the acceptance of the patch.
>
> Regards,
> Dimitris
>

Re: [PATCH] libobjc: Properly handle classes without instance variables in class_copyIvarList ().

[Mike Stump]

[-- Attachment #1: Type: text/plain, Size: 1442 bytes --]

On Dec 24, 2014, at 9:28 AM, Dimitris Papavasiliou <dpapavas@gmail.com> wrote:
> The attached patch fixes an issue reported a couple of years ago in Bug 51891 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51891).  The problem is caused because classes without instance variables have no ivar list at all, so that their ivars pointer is NULL, but the code in class_copyIvarList () is unaware of this.
> 
> That this is in fact so can be easily verified by checking the code of class_addIvar in the same source file, where the ivars list is allocated when the first ivar is added.  The code there also checks for a NULL ivars pointer.
> 
> The patch also adds a simple test-case for this issue.  I think that the ChangeLog entry should be something along the lines of:
> 
>   2014-12-24  Dimitris Papavasiliou  <dpapavas@gmail.com>
> 
>         PR libobjc/51891
>         * libobjc/ivars.c: Add a check for classes without instance
> 	variables, which have a NULL ivar list pointer.
>         * gcc/testsuite/objc.dg/gnu-api-2-class.m: Add a test case
> 	for the above change.
> 
> I hope I got the formatting right.  I've run make -k check-objc and all tests pass without problems.
> 
> Let me know if there are any problems, or if I can do anything else to facilitate the acceptance of the patch.
> 
> Regards,
> Dimitris

So, Andrew is the reviewer for libobjc.  I’m not.  I don’t have any issue with it.

Andrew?


[-- Attachment #2: class_copyIvarList_bugfix.diff --]
[-- Type: application/octet-stream, Size: 1167 bytes --]

Index: gcc/testsuite/objc.dg/gnu-api-2-class.m
===================================================================
--- gcc/testsuite/objc.dg/gnu-api-2-class.m	(revision 219054)
+++ gcc/testsuite/objc.dg/gnu-api-2-class.m	(working copy)
@@ -239,6 +239,19 @@
       abort ();
   }
 
+  printf ("Testing class_copyIvarList () on class with no instance variables...\n");
+  {
+    unsigned int count;
+    Ivar * list = class_copyIvarList (objc_getClass ("MyOtherSubClass"),
+                                      &count);
+
+    if (count != 0)
+      abort ();
+    
+    if (list != NULL)
+      abort ();
+  }
+
   printf ("Testing class_copyMethodList ()...\n");
   {
     unsigned int count;
Index: libobjc/ivars.c
===================================================================
--- libobjc/ivars.c	(revision 219054)
+++ libobjc/ivars.c	(working copy)
@@ -179,7 +179,7 @@
   struct objc_ivar **returnValue = NULL;
   struct objc_ivar_list* ivar_list;
 
-  if (class_ == Nil  ||  CLS_IS_IN_CONSTRUCTION (class_))
+  if (class_ == Nil  ||  CLS_IS_IN_CONSTRUCTION (class_) || !class_->ivars)
     {
       if (numberOfReturnedIvars)
 	*numberOfReturnedIvars = 0;

Re: [PATCH] libobjc: Properly handle classes without instance variables in class_copyIvarList ().

[Jeff Law]

On 01/05/15 14:18, Mike Stump wrote:
> On Dec 24, 2014, at 9:28 AM, Dimitris Papavasiliou <dpapavas@gmail.com> wrote:
>> The attached patch fixes an issue reported a couple of years ago in Bug 51891 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51891).  The problem is caused because classes without instance variables have no ivar list at all, so that their ivars pointer is NULL, but the code in class_copyIvarList () is unaware of this.
>>
>> That this is in fact so can be easily verified by checking the code of class_addIvar in the same source file, where the ivars list is allocated when the first ivar is added.  The code there also checks for a NULL ivars pointer.
>>
>> The patch also adds a simple test-case for this issue.  I think that the ChangeLog entry should be something along the lines of:
>>
>>    2014-12-24  Dimitris Papavasiliou  <dpapavas@gmail.com>
>>
>>          PR libobjc/51891
>>          * libobjc/ivars.c: Add a check for classes without instance
>> 	variables, which have a NULL ivar list pointer.
>>          * gcc/testsuite/objc.dg/gnu-api-2-class.m: Add a test case
>> 	for the above change.
>>
>> I hope I got the formatting right.  I've run make -k check-objc and all tests pass without problems.
>>
>> Let me know if there are any problems, or if I can do anything else to facilitate the acceptance of the patch.
>>
>> Regards,
>> Dimitris
>
> So, Andrew is the reviewer for libobjc.  IÂ’m not.  I donÂ’t have any issue with it.
Do you want to be a reviewer for libobjc?  I don't think the load there 
is high, but having someone else who cares about the code is always a 
good thing.

jeff

Re: [PATCH] libobjc: Properly handle classes without instance variables in class_copyIvarList ().

[Andrew Pinski]

On Wed, Dec 24, 2014 at 9:28 AM, Dimitris Papavasiliou
<dpapavas@gmail.com> wrote:
> Hello,
>
> The attached patch fixes an issue reported a couple of years ago in Bug
> 51891 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51891).  The problem is
> caused because classes without instance variables have no ivar list at all,
> so that their ivars pointer is NULL, but the code in class_copyIvarList ()
> is unaware of this.
>
> That this is in fact so can be easily verified by checking the code of
> class_addIvar in the same source file, where the ivars list is allocated
> when the first ivar is added.  The code there also checks for a NULL ivars
> pointer.
>
> The patch also adds a simple test-case for this issue.  I think that the
> ChangeLog entry should be something along the lines of:
>
>    2014-12-24  Dimitris Papavasiliou  <dpapavas@gmail.com>
>
>          PR libobjc/51891
>          * libobjc/ivars.c: Add a check for classes without instance
>         variables, which have a NULL ivar list pointer.
>          * gcc/testsuite/objc.dg/gnu-api-2-class.m: Add a test case
>         for the above change.
>
> I hope I got the formatting right.  I've run make -k check-objc and all
> tests pass without problems.
>
> Let me know if there are any problems, or if I can do anything else to
> facilitate the acceptance of the patch.


This is ok.

Thanks,
Andrew

>
> Regards,
> Dimitris
>

Re: [PATCH] libobjc: Properly handle classes without instance variables in class_copyIvarList ().

[Andrew Pinski]

On Thu, Jan 8, 2015 at 9:35 PM, Jeff Law <law@redhat.com> wrote:
> On 01/05/15 14:18, Mike Stump wrote:
>>
>> On Dec 24, 2014, at 9:28 AM, Dimitris Papavasiliou <dpapavas@gmail.com>
>> wrote:
>>>
>>> The attached patch fixes an issue reported a couple of years ago in Bug
>>> 51891 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51891).  The problem is
>>> caused because classes without instance variables have no ivar list at all,
>>> so that their ivars pointer is NULL, but the code in class_copyIvarList ()
>>> is unaware of this.
>>>
>>> That this is in fact so can be easily verified by checking the code of
>>> class_addIvar in the same source file, where the ivars list is allocated
>>> when the first ivar is added.  The code there also checks for a NULL ivars
>>> pointer.
>>>
>>> The patch also adds a simple test-case for this issue.  I think that the
>>> ChangeLog entry should be something along the lines of:
>>>
>>>    2014-12-24  Dimitris Papavasiliou  <dpapavas@gmail.com>
>>>
>>>          PR libobjc/51891
>>>          * libobjc/ivars.c: Add a check for classes without instance
>>>         variables, which have a NULL ivar list pointer.
>>>          * gcc/testsuite/objc.dg/gnu-api-2-class.m: Add a test case
>>>         for the above change.
>>>
>>> I hope I got the formatting right.  I've run make -k check-objc and all
>>> tests pass without problems.
>>>
>>> Let me know if there are any problems, or if I can do anything else to
>>> facilitate the acceptance of the patch.
>>>
>>> Regards,
>>> Dimitris
>>
>>
>> So, Andrew is the reviewer for libobjc.  I’m not.  I don’t have any issue
>> with it.
>
> Do you want to be a reviewer for libobjc?  I don't think the load there is
> high, but having someone else who cares about the code is always a good
> thing.

I am a reviewer for libobjc, I had missed this patch when it came in
due to the low traffic of libobjc patches and working on other
projects at the time.  I had marked as something I needed to review
but I did not get around to it until now.

Thanks,
Andrew

>
> jeff

Re: [PATCH] libobjc: Properly handle classes without instance variables in class_copyIvarList ().

[Mike Stump]

On Jan 8, 2015, at 9:38 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>>   2014-12-24  Dimitris Papavasiliou  <dpapavas@gmail.com>
>> 
>>         PR libobjc/51891
>>         * libobjc/ivars.c: Add a check for classes without instance
>>        variables, which have a NULL ivar list pointer.
>>         * gcc/testsuite/objc.dg/gnu-api-2-class.m: Add a test case
>>        for the above change.

> This is ok.

Committed revision 219396.

Re: [PATCH] libobjc: Properly handle classes without instance variables in class_copyIvarList ().

[Mike Stump]

On Jan 8, 2015, at 9:35 PM, Jeff Law <law@redhat.com> wrote:
> Do you want to be a reviewer for libobjc?

I think things are fine as is.  If things were pinged and there were no response, or if someone wanted to do major updated on the library to bring it up a decade, then we might want to change things.  If someone wanted to do a major update, then, I’d recommend they step forward.