Finding #2
I removed the lengthy claude analysis, the out of bound access looks
real to me.
Philippe
====
Discovery: found while reviewing Source/unix/GSRunLoopCtxt.m.
Fix: compute the index into the array the watcher comes from, in the
three copies of the loop. The win32 change is by inspection only (not
built or tested).
Test: Tests/base/NSRunLoop/common-watchers.m (crashes on its third test
before the fix; all 11 pass after). The rest of Tests/base/NSRunLoop
passes unchanged.
---
diff --git a/Source/unix/GSRunLoopCtxt.m b/Source/unix/GSRunLoopCtxt.m
index 557ea9a93..ab4b894e8 100644
--- a/Source/unix/GSRunLoopCtxt.m
+++ b/Source/unix/GSRunLoopCtxt.m
@@ -295,19 +295,25 @@ static void setPollfd(int fd, int event,
GSRunLoopCtxtUnix *ctxt)
GSRunLoopWatcher *info;
BOOL trigger;
GSIArray table;
+ unsigned pos;
+ /* Common mode watchers follow those of this mode: index
+ * them from the start of their own array.
+ */
if (i >= modeWatchersCount)
{
table = common->watchers;
+ pos = i - modeWatchersCount;
}
else
{
table = watchers;
+ pos = i;
}
- info = GSIArrayItemAtIndex(table, i).obj;
+ info = GSIArrayItemAtIndex(table, pos).obj;
if (info->_invalidated == YES)
{
- GSIArrayRemoveItemAtIndex(table, i);
+ GSIArrayRemoveItemAtIndex(table, pos);
}
else if ([info runLoopShouldBlock: &trigger] == NO)
{
@@ -791,20 +797,26 @@ static void setPollfd(int fd, int event,
GSRunLoopCtxtUnix *ctxt)
int fd;
BOOL trigger;
GSIArray table;
+ unsigned pos;
+ /* Common mode watchers follow those of this mode: index
+ * them from the start of their own array.
+ */
if (i >= modeWatchersCount)
{
table = common->watchers;
+ pos = i - modeWatchersCount;
}
else
{
table = watchers;
+ pos = i;
}
- info = GSIArrayItemAtIndex(table, i).obj;
+ info = GSIArrayItemAtIndex(table, pos).obj;
if (info->_invalidated == YES)
{
- GSIArrayRemoveItemAtIndex(table, i);
+ GSIArrayRemoveItemAtIndex(table, pos);
}
else if ([info runLoopShouldBlock: &trigger] == NO)
{
diff --git a/Source/win32/GSRunLoopCtxt.m b/Source/win32/GSRunLoopCtxt.m
index 9e2ffcd35..9faa7b38b 100644
--- a/Source/win32/GSRunLoopCtxt.m
+++ b/Source/win32/GSRunLoopCtxt.m
@@ -285,20 +285,26 @@
GSRunLoopWatcher *info;
BOOL trigger;
GSIArray table;
+ unsigned pos;
+ /* Common mode watchers follow those of this mode: index
+ * them from the start of their own array.
+ */
if (i >= modeWatchersCount)
{
table = common->watchers;
+ pos = i - modeWatchersCount;
}
else
{
table = watchers;
+ pos = i;
}
- info = GSIArrayItemAtIndex(table, i).obj;
+ info = GSIArrayItemAtIndex(table, pos).obj;
if (info->_invalidated == YES)
{
- GSIArrayRemoveItemAtIndex(table, i);
+ GSIArrayRemoveItemAtIndex(table, pos);
}
else if ([info runLoopShouldBlock: &trigger] == NO)
{
diff --git a/Tests/base/NSRunLoop/common-watchers.m
b/Tests/base/NSRunLoop/common-watchers.m
new file mode 100644
index 000000000..73af2bb9e
--- /dev/null
+++ b/Tests/base/NSRunLoop/common-watchers.m
@@ -0,0 +1,194 @@
+/*
+ * When a run loop runs in a common mode it watches the inputs of that
+ * mode and those of NSRunLoopCommonModes together. Check that both
+ * sets of watchers are handled when both are present, in any number,
+ * and that removing a common mode watcher works.
+ */
+#import <Foundation/Foundation.h>
+#import "Testing.h"
+#include <unistd.h>
+
+@interface Reader : NSObject <RunLoopEvents>
+{
+@public
+ int fd;
+ int writeFd;
+ int fired;
+}
+@end
+
+@implementation Reader
+- (id) init
+{
+ int p[2];
+
+ if (nil != (self = [super init]))
+ {
+ if (pipe(p) < 0)
+ {
+ DESTROY(self);
+ return nil;
+ }
+ fd = p[0];
+ writeFd = p[1];
+ }
+ return self;
+}
+- (void) dealloc
+{
+ close(fd);
+ close(writeFd);
+ [super dealloc];
+}
+- (void) makeReadable
+{
+ write(writeFd, "x", 1);
+}
+- (void) receivedEvent: (void*)data
+ type: (RunLoopEventType)type
+ extra: (void*)extra
+ forMode: (NSString*)mode
+{
+ char c;
+
+ if (read(fd, &c, 1) == 1)
+ {
+ fired++;
+ }
+}
+@end
+
+static NSMutableArray *
+watch(NSRunLoop *rl, NSString *mode, int count)
+{
+ NSMutableArray *a = [NSMutableArray array];
+
+ while (count-- > 0)
+ {
+ Reader *r = AUTORELEASE([Reader new]);
+
+ [rl addEvent: (void*)(intptr_t)r->fd
+ type: ET_RDESC
+ watcher: r
+ forMode: mode];
+ [r makeReadable];
+ [a addObject: r];
+ }
+ return a;
+}
+
+static void
+unwatch(NSRunLoop *rl, NSArray *a, NSString *mode)
+{
+ NSEnumerator *e = [a objectEnumerator];
+ Reader *r;
+
+ while ((r = [e nextObject]) != nil)
+ {
+ [rl removeEvent: (void*)(intptr_t)r->fd
+ type: ET_RDESC
+ forMode: mode
+ all: YES];
+ }
+}
+
+/* Run the loop until every reader in the arrays has fired or a second
+ * has passed.
+ */
+static BOOL
+allFire(NSRunLoop *rl, NSString *mode, NSArray *a, NSArray *b)
+{
+ NSDate *limit = [NSDate dateWithTimeIntervalSinceNow: 1.0];
+ NSArray *all = [a arrayByAddingObjectsFromArray: b];
+
+ while ([limit timeIntervalSinceNow] > 0)
+ {
+ NSEnumerator *e = [all objectEnumerator];
+ Reader *r;
+ BOOL done = YES;
+
+ [rl runMode: mode beforeDate: [NSDate dateWithTimeIntervalSinceNow:
0.1]];
+ while ((r = [e nextObject]) != nil)
+ {
+ if (r->fired == 0)
+ {
+ done = NO;
+ }
+ }
+ if (done)
+ {
+ return YES;
+ }
+ }
+ return NO;
+}
+
+static void
+both(NSRunLoop *rl, int inMode, int inCommon)
+{
+ NSMutableArray *m = watch(rl, NSDefaultRunLoopMode, inMode);
+ NSMutableArray *c = watch(rl, NSRunLoopCommonModes, inCommon);
+
+ PASS(allFire(rl, NSDefaultRunLoopMode, m, c),
+ "%d default mode and %d common mode watchers all fire",
+ inMode, inCommon)
+ unwatch(rl, m, NSDefaultRunLoopMode);
+ unwatch(rl, c, NSRunLoopCommonModes);
+}
+
+int main()
+{
+ NSAutoreleasePool *arp = [NSAutoreleasePool new];
+ NSRunLoop *rl = [NSRunLoop currentRunLoop];
+ NSMutableArray *m;
+ NSMutableArray *c;
+ Reader *gone;
+
+ START_SET("watchers in a mode and in the common modes")
+
+ both(rl, 0, 1);
+ both(rl, 1, 0);
+ both(rl, 1, 1);
+ both(rl, 1, 3);
+ both(rl, 3, 1);
+ both(rl, 4, 4);
+
+ END_SET("watchers in a mode and in the common modes")
+
+ START_SET("removing common mode watchers")
+
+ m = watch(rl, NSDefaultRunLoopMode, 2);
+ c = watch(rl, NSRunLoopCommonModes, 3);
+ gone = [c objectAtIndex: 1];
+ [rl removeEvent: (void*)(intptr_t)gone->fd
+ type: ET_RDESC
+ forMode: NSRunLoopCommonModes
+ all: YES];
+ [c removeObjectAtIndex: 1];
+ PASS(allFire(rl, NSDefaultRunLoopMode, m, c),
+ "the other watchers fire after one is removed")
+ PASS(gone->fired == 0, "the removed watcher is not called")
+ unwatch(rl, m, NSDefaultRunLoopMode);
+ unwatch(rl, c, NSRunLoopCommonModes);
+
+ END_SET("removing common mode watchers")
+
+ START_SET("a mode which is not common")
+
+ m = watch(rl, @"GSTestPrivateMode", 2);
+ c = watch(rl, NSRunLoopCommonModes, 2);
+ PASS(allFire(rl, @"GSTestPrivateMode", m, nil),
+ "the watchers of a private mode fire")
+ PASS(((Reader*)[c objectAtIndex: 0])->fired == 0
+ && ((Reader*)[c objectAtIndex: 1])->fired == 0,
+ "common mode watchers do not fire in it")
+ PASS(allFire(rl, NSDefaultRunLoopMode, nil, c),
+ "they fire in the default mode")
+ unwatch(rl, m, @"GSTestPrivateMode");
+ unwatch(rl, c, NSRunLoopCommonModes);
+
+ END_SET("a mode which is not common")
+
+ [arp release];
+ return 0;
+}
--
2.39.5
