* [PATCH] libobjc: Properly handle classes without instance variables in class_copyIvarList ().
@ 2014-12-24 17:45 Dimitris Papavasiliou
2015-01-05 9:51 ` [PATCH][PING] " Dimitris Papavasiliou
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Dimitris Papavasiliou @ 2014-12-24 17:45 UTC (permalink / raw)
To: gcc-patches@gcc.gnu.org Patches; +Cc: Mike Stump, stanshebs
[-- 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;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH][PING] libobjc: Properly handle classes without instance variables in class_copyIvarList ().
2014-12-24 17:45 [PATCH] libobjc: Properly handle classes without instance variables in class_copyIvarList () Dimitris Papavasiliou
@ 2015-01-05 9:51 ` Dimitris Papavasiliou
2015-01-05 21:18 ` [PATCH] " Mike Stump
2015-01-09 5:38 ` Andrew Pinski
2 siblings, 0 replies; 8+ messages in thread
From: Dimitris Papavasiliou @ 2015-01-05 9:51 UTC (permalink / raw)
To: gcc-patches@gcc.gnu.org Patches; +Cc: Mike Stump, stanshebs
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] libobjc: Properly handle classes without instance variables in class_copyIvarList ().
2014-12-24 17:45 [PATCH] libobjc: Properly handle classes without instance variables in class_copyIvarList () Dimitris Papavasiliou
2015-01-05 9:51 ` [PATCH][PING] " Dimitris Papavasiliou
@ 2015-01-05 21:18 ` Mike Stump
2015-01-09 5:35 ` Jeff Law
2015-01-09 5:38 ` Andrew Pinski
2 siblings, 1 reply; 8+ messages in thread
From: Mike Stump @ 2015-01-05 21:18 UTC (permalink / raw)
To: Dimitris Papavasiliou, Andrew Pinski
Cc: gcc-patches@gcc.gnu.org Patches, Stan Shebs
[-- 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;
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] libobjc: Properly handle classes without instance variables in class_copyIvarList ().
2015-01-05 21:18 ` [PATCH] " Mike Stump
@ 2015-01-09 5:35 ` Jeff Law
2015-01-09 5:40 ` Andrew Pinski
2015-01-09 19:06 ` Mike Stump
0 siblings, 2 replies; 8+ messages in thread
From: Jeff Law @ 2015-01-09 5:35 UTC (permalink / raw)
To: Mike Stump, Dimitris Papavasiliou, Andrew Pinski
Cc: gcc-patches@gcc.gnu.org Patches, Stan Shebs
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] libobjc: Properly handle classes without instance variables in class_copyIvarList ().
2015-01-09 5:35 ` Jeff Law
@ 2015-01-09 5:40 ` Andrew Pinski
2015-01-09 19:06 ` Mike Stump
1 sibling, 0 replies; 8+ messages in thread
From: Andrew Pinski @ 2015-01-09 5:40 UTC (permalink / raw)
To: Jeff Law
Cc: Mike Stump, Dimitris Papavasiliou,
gcc-patches@gcc.gnu.org Patches, Stan Shebs
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] libobjc: Properly handle classes without instance variables in class_copyIvarList ().
2015-01-09 5:35 ` Jeff Law
2015-01-09 5:40 ` Andrew Pinski
@ 2015-01-09 19:06 ` Mike Stump
1 sibling, 0 replies; 8+ messages in thread
From: Mike Stump @ 2015-01-09 19:06 UTC (permalink / raw)
To: Jeff Law
Cc: Dimitris Papavasiliou, Andrew Pinski,
gcc-patches@gcc.gnu.org Patches, Stan Shebs
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] libobjc: Properly handle classes without instance variables in class_copyIvarList ().
2014-12-24 17:45 [PATCH] libobjc: Properly handle classes without instance variables in class_copyIvarList () Dimitris Papavasiliou
2015-01-05 9:51 ` [PATCH][PING] " Dimitris Papavasiliou
2015-01-05 21:18 ` [PATCH] " Mike Stump
@ 2015-01-09 5:38 ` Andrew Pinski
2015-01-09 18:00 ` Mike Stump
2 siblings, 1 reply; 8+ messages in thread
From: Andrew Pinski @ 2015-01-09 5:38 UTC (permalink / raw)
To: Dimitris Papavasiliou
Cc: gcc-patches@gcc.gnu.org Patches, Mike Stump, Stan Shebs
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] libobjc: Properly handle classes without instance variables in class_copyIvarList ().
2015-01-09 5:38 ` Andrew Pinski
@ 2015-01-09 18:00 ` Mike Stump
0 siblings, 0 replies; 8+ messages in thread
From: Mike Stump @ 2015-01-09 18:00 UTC (permalink / raw)
To: Dimitris Papavasiliou
Cc: gcc-patches@gcc.gnu.org Patches, Stan Shebs, Andrew Pinski
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-01-09 18:34 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-24 17:45 [PATCH] libobjc: Properly handle classes without instance variables in class_copyIvarList () Dimitris Papavasiliou
2015-01-05 9:51 ` [PATCH][PING] " Dimitris Papavasiliou
2015-01-05 21:18 ` [PATCH] " Mike Stump
2015-01-09 5:35 ` Jeff Law
2015-01-09 5:40 ` Andrew Pinski
2015-01-09 19:06 ` Mike Stump
2015-01-09 5:38 ` Andrew Pinski
2015-01-09 18:00 ` Mike Stump
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).