Re: Bug in base: NSMapGet()

[Daniel Ferreira (theiostream)]

Okay, I'm ashamed of how long and how many experiments it took me to
finally see this, and I'm sorry for the misleading example I sent on
the first thread e-mail that was supposed to go wrong anyway.

The issue whose fault is Base's is that despite the key barriers being
set to (NSPointerFunctionsOpaqueMemory |
NSPointerFunctionsOpaquePersonality), i.e. with nothing to do with
ARC, Base's NSConcreteMapTable was still calling objc_loadWeak() to
get the key value.

After looking at the wrong place for hours, I finally noticed that in
GSI_MAP_NODE_IS_EMPTY we were reading the table nodes' key values
using the GSI_MAP_READ_VALUE macro, which applies the *value* barriers
to whatever it's reading. This way, we were applying value barriers to
reading the keys, and since the value barriers used
NSPointerFunctionsWeakMemory, it ended up calling ARC functions on the
raw pointers that made up the keys causing all sorts of unexpected
behavior.

Here's the diff of how I fixed it. (Since how we submit code
contributions is apparently unstable with the change to Git, I suppose
I will temporarily send it here for comments, since it's a small one
anyway.)

diff --git a/Headers/GNUstepBase/GSIMap.h b/Headers/GNUstepBase/GSIMap.h
index 47fc8d3..b0ac494 100644
--- a/Headers/GNUstepBase/GSIMap.h
+++ b/Headers/GNUstepBase/GSIMap.h
@@ -157,9 +157,9 @@ extern "C" {
 #  define GSI_MAP_WRITE_VAL(M, addr, obj) (*(addr) = obj)
 #endif
 #if    GSI_MAP_HAS_VALUE
-#define GSI_MAP_NODE_IS_EMPTY(M, node) (((GSI_MAP_READ_VALUE(M,
&node->key).addr) == 0) || ((GSI_MAP_READ_VALUE(M, &node->value).addr
== 0)))
+#define GSI_MAP_NODE_IS_EMPTY(M, node) (((GSI_MAP_READ_KEY(M,
&node->key).addr) == 0) || ((GSI_MAP_READ_VALUE(M, &node->value).addr
== 0)))
 #else
-#define GSI_MAP_NODE_IS_EMPTY(M, node) (((GSI_MAP_READ_VALUE(M,
&node->key).addr) == 0))
+#define GSI_MAP_NODE_IS_EMPTY(M, node) (((GSI_MAP_READ_KEY(M,
&node->key).addr) == 0))
 #endif

-- Daniel.