gnustep/libs-base
0
0
Fork 0
mirror of https://github.com/gnustep/libs-base.git synced 2025-04-23 00:41:02 +00:00
Code Issues Projects Releases Packages Wiki Activity

Rework some of the weak handling for NS{Map,Hash}Table.

Browse source 
The existing code was assuming that weak object pointers were safe to
read directly, without going via the read barrier, which is incorrect.
It was also attempting to retain the result in some places.  The new
code is hopefully somewhat more correct.
This commit is contained in:
David Chisnall 2019-11-24 12:15:11 +00:00
parent 69f7130fa7
commit f957b21972
3 changed files with 37 additions and 35 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
Headers/GNUstepBase/GSIMap.h
View file

@ -450,7 +450,7 @@ GSIMapAddNodeToMap(GSIMapTable map, GSIMapNode node)
{
GSIMapBucket bucket;
bucket = GSIMapBucketForKey(map, node->key);
bucket = GSIMapBucketForKey(map, GSI_MAP_READ_KEY(map, &node->key));
GSIMapAddNodeToBucket(bucket, node);
map->nodeCount++;
}
@ -543,7 +543,7 @@ GSIMapRemangleBuckets(GSIMapTable map,
GSIMapBucket bkt;
GSIMapRemoveNodeFromBucket(old_buckets, node);
bkt = GSIMapPickBucket(GSI_MAP_HASH(map, node->key),
bkt = GSIMapPickBucket(GSI_MAP_HASH(map, GSI_MAP_READ_KEY(map, &node->key)),
new_buckets, new_bucketCount);
GSIMapAddNodeToBucket(bkt, node);
}
@ -561,7 +561,7 @@ GSIMapRemangleBuckets(GSIMapTable map,
GSIMapBucket bkt;
GSIMapRemoveNodeFromBucket(old_buckets, node);
bkt = GSIMapPickBucket(GSI_MAP_HASH(map, node->key),
bkt = GSIMapPickBucket(GSI_MAP_HASH(map, GSI_MAP_READ_KEY(map, &node->key)),
new_buckets, new_bucketCount);
GSIMapAddNodeToBucket(bkt, node);
}

21
Source/NSConcreteHashTable.m
View file

@ -75,7 +75,9 @@ typedef GSIMapNode_t *GSIMapNode;
#define GSI_MAP_KTYPES GSUNION_PTR | GSUNION_OBJ
#define GSI_MAP_TABLE_T NSConcreteHashTable
#define GSI_MAP_TABLE_S instanceSize
#define IS_WEAK(M) \
M->cb.pf.options & (NSPointerFunctionsZeroingWeakMemory | NSPointerFunctionsWeakMemory)
#define GSI_MAP_HASH(M, X)\
(M->legacy ? M->cb.old.hash(M, X.ptr) \
: pointerFunctionsHash(&M->cb.pf, X.ptr))
@ -84,13 +86,13 @@ typedef GSIMapNode_t *GSIMapNode;
: pointerFunctionsEqual(&M->cb.pf, X.ptr, Y.ptr))
#define GSI_MAP_RELEASE_KEY(M, X)\
(M->legacy ? M->cb.old.release(M, X.ptr) \
: pointerFunctionsRelinquish(&M->cb.pf, &X.ptr))
: IS_WEAK(M) ? nil : pointerFunctionsRelinquish(&M->cb.pf, &X.ptr))
#define GSI_MAP_RETAIN_KEY(M, X)\
(M->legacy ? M->cb.old.retain(M, X.ptr) \
: pointerFunctionsAcquire(&M->cb.pf, &X.ptr, X.ptr))
: IS_WEAK(M) ? nil : pointerFunctionsAcquire(&M->cb.pf, &X.ptr, X.ptr))
#define GSI_MAP_ZEROED(M)\
(M->legacy ? 0 \
: ((M->cb.pf.options & NSPointerFunctionsZeroingWeakMemory) ? YES : NO))
: (IS_WEAK(M) ? YES : NO))
#define GSI_MAP_WRITE_KEY(M, addr, x) \
if (M->legacy) \
@ -100,9 +102,6 @@ typedef GSIMapNode_t *GSIMapNode;
#define GSI_MAP_READ_KEY(M,addr) \
(M->legacy ? *(addr) :\
(typeof(*addr))pointerFunctionsRead(&M->cb.pf, (void**)addr))
#define GSI_MAP_ZEROED(M)\
(M->legacy ? 0 \
: ((M->cb.pf.options & NSPointerFunctionsZeroingWeakMemory) ? YES : NO))
#define GSI_MAP_ENUMERATOR NSHashEnumerator
@ -628,7 +627,13 @@ NSNextHashEnumeratorItem(NSHashEnumerator *enumerator)
}
else
{
return n->key.ptr;
NSConcreteHashTable *map = enumerator->map;
GSIMapKey k = GSI_MAP_READ_KEY(map, &n->key);
if (k.ptr == NULL)
{
return NSNextHashEnumeratorItem(enumerator);
}
return k.ptr;
}
}
else if (enumerator->node != 0) // Got an enumerator object

45
Source/NSConcreteMapTable.m
View file

@ -89,16 +89,16 @@ typedef GSIMapNode_t *GSIMapNode;
: pointerFunctionsEqual(&M->cb.pf.k, X.ptr, Y.ptr))
#define GSI_MAP_RELEASE_KEY(M, X)\
(M->legacy ? M->cb.old.k.release(M, X.ptr) \
: pointerFunctionsRelinquish(&M->cb.pf.k, &X.ptr))
: IS_WEAK_KEY(M) ? nil : pointerFunctionsRelinquish(&M->cb.pf.k, &X.ptr))
#define GSI_MAP_RETAIN_KEY(M, X)\
(M->legacy ? M->cb.old.k.retain(M, X.ptr) \
: pointerFunctionsAcquire(&M->cb.pf.k, &X.ptr, X.ptr))
: IS_WEAK_KEY(M) ? nil : pointerFunctionsAcquire(&M->cb.pf.k, &X.ptr, X.ptr))
#define GSI_MAP_RELEASE_VAL(M, X)\
(M->legacy ? M->cb.old.v.release(M, X.ptr) \
: pointerFunctionsRelinquish(&M->cb.pf.v, &X.ptr))
: IS_WEAK_VALUE(M) ? nil : pointerFunctionsRelinquish(&M->cb.pf.v, &X.ptr))
#define GSI_MAP_RETAIN_VAL(M, X)\
(M->legacy ? M->cb.old.v.retain(M, X.ptr) \
: pointerFunctionsAcquire(&M->cb.pf.v, &X.ptr, X.ptr))
: IS_WEAK_VALUE(M) ? nil : pointerFunctionsAcquire(&M->cb.pf.v, &X.ptr, X.ptr))
/* 2013-05-25 Here are the macros originally added for GC/ARC ...
* but they caused map table entries to be doubly retained :-(
@ -116,16 +116,20 @@ typedef GSIMapNode_t *GSIMapNode;
else\
pointerFunctionsAssign(&M->cb.pf.v, (void**)addr, (x).obj);
*/
#define IS_WEAK_KEY(M) \
M->cb.pf.k.options & (NSPointerFunctionsZeroingWeakMemory | NSPointerFunctionsWeakMemory)
#define IS_WEAK_VALUE(M) \
M->cb.pf.v.options & (NSPointerFunctionsZeroingWeakMemory | NSPointerFunctionsWeakMemory)
#define GSI_MAP_WRITE_KEY(M, addr, x) \
if (M->legacy) \
*(addr) = x;\
else\
*(id*)(addr) = (x).obj;
(IS_WEAK_KEY(M) ? pointerFunctionsAssign(&M->cb.pf.k, (void**)addr, (x).obj) : (*(id*)(addr) = (x).obj));
#define GSI_MAP_WRITE_VAL(M, addr, x) \
if (M->legacy) \
*(addr) = x;\
else\
*(id*)(addr) = (x).obj;
(IS_WEAK_VALUE(M) ? pointerFunctionsAssign(&M->cb.pf.v, (void**)addr, (x).obj) : (*(id*)(addr) = (x).obj));
#define GSI_MAP_READ_KEY(M,addr) \
(M->legacy ? *(addr)\
: (typeof(*addr))pointerFunctionsRead(&M->cb.pf.k, (void**)addr))
@ -134,8 +138,7 @@ typedef GSIMapNode_t *GSIMapNode;
: (typeof(*addr))pointerFunctionsRead(&M->cb.pf.v, (void**)addr))
#define GSI_MAP_ZEROED(M)\
(M->legacy ? 0\
: (((M->cb.pf.k.options | M->cb.pf.v.options)\
& NSPointerFunctionsZeroingWeakMemory) ? YES : NO))
: (IS_WEAK_KEY(M) || IS_WEAK_VALUE(M)) ? YES : NO)
#define GSI_MAP_ENUMERATOR NSMapEnumerator
@ -879,23 +882,17 @@ NSNextMapEnumeratorPair(NSMapEnumerator *enumerator,
}
else
{
if (key != 0)
NSConcreteMapTable *map = enumerator->map;
GSIMapKey k = GSI_MAP_READ_KEY(map, &n->key);
GSIMapVal v = GSI_MAP_READ_VALUE(map, &n->value);
if (k.ptr == NULL || v.ptr == NULL)
{
*key = n->key.ptr;
}
else
{
NSWarnFLog(@"Null key return address");
return NSNextMapEnumeratorPair(enumerator, key, value);
}
if (value != 0)
{
*value = n->value.ptr;
}
else
{
NSWarnFLog(@"Null value return address");
}
*key = k.ptr;
*value = v.ptr;
return YES;
}
}
@ -1332,7 +1329,7 @@ const NSMapTableValueCallBacks NSOwnedPointerMapValueCallBacks =
if (node)
{
return node->value.obj;
return GSI_MAP_READ_VALUE(self, &node->value).obj;
}
}
return nil;
@ -1384,7 +1381,7 @@ const NSMapTableValueCallBacks NSOwnedPointerMapValueCallBacks =
node = GSIMapNodeForKey(self, (GSIMapKey)aKey);
if (node)
{
if (node->value.obj != anObject)
if (GSI_MAP_READ_VALUE(self, &node->value).obj != anObject)
{
GSI_MAP_RELEASE_VAL(self, node->value);
node->value.obj = anObject;