summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniel Kolesa <d.kolesa@samsung.com>2019-11-14 04:02:09 +0100
committerDaniel Kolesa <d.kolesa@samsung.com>2019-11-14 04:18:46 +0100
commit9ca573f40f1065cc717c0c5aabb787671bab852b (patch)
treecb5b82f2feb569da20169692b9255457630d1749
parent288218527d494e1ca3146b0321b0967842fecae6 (diff)
eo: fix UB in the eo event code (edje_cc hangs etc.)
Today I started experiencing mysterious hanging of edje_cc during build. "The French are at it again" I thought, and after spending a while bisecting, I found the culprit. It's 7f53d9158395504151e1ff3dcae715a799d913a8. So, what is happening in here? The idea here was fundamentally sound; compute a special hash value for event descriptors, taking range between 0 and 63 (on 64-bit systems) and 0 and 31 (on 32-bit systems), then use a mask sized 32-bit or 64-bit (depending on your system) and check early if to bail out from callback_call, saving some resources. So far so good. The problem is in the way the mask is handled. We're applying the hash as the shift value like, `x |= (1 << hash)`. On 32-bit systems this is okay, but on 64-bit systems, C's dumb silent coercion rules kick in, since the left side of the expression is 1, a literal with type signed int; that means our shifting range is limited to 31 and what we get is... undefined behavior. This is obviously not what we want, so take a 1ULL value as a base. The previous thing seemingly worked on x86_64 (nobody reported any issues) and interestingly it worked for me too for a while (on ppc64le), but undefined behavior can be unpredictable, so... This shouldn't need a commit message as long as this, but I'm making it proportional to the amount of time I wasted on this.
-rw-r--r--src/lib/eo/eo_base_class.c4
1 files changed, 2 insertions, 2 deletions
diff --git a/src/lib/eo/eo_base_class.c b/src/lib/eo/eo_base_class.c
index 06cd482..8d147d3 100644
--- a/src/lib/eo/eo_base_class.c
+++ b/src/lib/eo/eo_base_class.c
@@ -1263,7 +1263,7 @@ _special_event_count_inc(Eo *obj_id, Efl_Object_Data *pd, const Efl_Callback_Arr
1263 1263
1264 event_hash = _pointer_hash((uintptr_t) it->desc); 1264 event_hash = _pointer_hash((uintptr_t) it->desc);
1265 1265
1266 pd->callbacks_mask |= 1 << event_hash; 1266 pd->callbacks_mask |= 1ULL << event_hash;
1267 1267
1268 EFL_OBJECT_EVENT_CB_INC(obj_id, it, pd, EFL_EVENT_CALLBACK_ADD) 1268 EFL_OBJECT_EVENT_CB_INC(obj_id, it, pd, EFL_EVENT_CALLBACK_ADD)
1269 else EFL_OBJECT_EVENT_CB_INC(obj_id, it, pd, EFL_EVENT_CALLBACK_DEL) 1269 else EFL_OBJECT_EVENT_CB_INC(obj_id, it, pd, EFL_EVENT_CALLBACK_DEL)
@@ -2009,7 +2009,7 @@ _event_callback_call(Eo *obj_id, Efl_Object_Data *pd,
2009 if (!legacy_compare) 2009 if (!legacy_compare)
2010 { 2010 {
2011 event_hash = _pointer_hash((uintptr_t) desc); 2011 event_hash = _pointer_hash((uintptr_t) desc);
2012 if (!(pd->callbacks_mask & (1 << event_hash))) 2012 if (!(pd->callbacks_mask & (1ULL << event_hash)))
2013 return EINA_TRUE; 2013 return EINA_TRUE;
2014 } 2014 }
2015 2015