[PATCH v2 3/4] perf hashmap: Fix strict aliasing violations in hashmap

Ian Rogers posted 4 patches 2 weeks, 1 day ago
[PATCH v2 3/4] perf hashmap: Fix strict aliasing violations in hashmap
Posted by Ian Rogers 2 weeks, 1 day ago
The hashmap in perf util (copied from libbpf) contained strict
aliasing violations.  Specifically, the hashmap_cast_ptr(p) macro was
casting pointers (such as void **) to long *, and these were
subsequently dereferenced in functions like hashmap_insert(),
hashmap_find(), and hashmap_delete().

C's strict aliasing rules (C11 6.5/7) prohibit accessing an object
through an lvalue of an incompatible type. Dereferencing a long * to
write to a void * object is a violation, even if they share the same
size, as they are not compatible types. This can lead to undefined
behavior, especially with aggressive compiler optimizations.

Fix this by:
1. Updating hashmap_insert(), hashmap_find(), and hashmap_delete() to
   take void * for their output parameters (old_key, old_value, and
   value).
2. Modifying the implementation to use memcpy() and memset() for
   accessing these output parameters. Accessing an object as an array of
   characters (as done by memcpy) is a permitted exception to the
   strict aliasing rules.
3. Updating the hashmap_cast_ptr(p) macro to return void *, ensuring
   compatibility with the new function signatures while preserving the
   static assertion that ensures the pointed-to type matches the size of
   a long.

Input parameters (key and value) remain as long, as they involve value
conversion rather than incompatible pointer dereferencing, which is safe
under strict aliasing rules.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/hashmap.c | 21 +++++++++++----------
 tools/perf/util/hashmap.h |  8 ++++----
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/hashmap.c b/tools/perf/util/hashmap.c
index 8c4b1f2af3ed..d90ef4ed384d 100644
--- a/tools/perf/util/hashmap.c
+++ b/tools/perf/util/hashmap.c
@@ -8,6 +8,7 @@
 #include <stdint.h>
 #include <stdlib.h>
 #include <stdio.h>
+#include <string.h>
 #include <errno.h>
 #include <linux/err.h>
 #include "hashmap.h"
@@ -153,24 +154,24 @@ static bool perf_hashmap_find_entry(const struct perf_hashmap *map,
 
 int perf_hashmap_insert(struct perf_hashmap *map, long key, long value,
 		   enum perf_hashmap_insert_strategy strategy,
-		   long *old_key, long *old_value)
+		   void *old_key, void *old_value)
 {
 	struct perf_hashmap_entry *entry;
 	size_t h;
 	int err;
 
 	if (old_key)
-		*old_key = 0;
+		memset(old_key, 0, sizeof(long));
 	if (old_value)
-		*old_value = 0;
+		memset(old_value, 0, sizeof(long));
 
 	h = hash_bits(map->hash_fn(key, map->ctx), map->cap_bits);
 	if (strategy != PERF_HASHMAP_APPEND &&
 	    perf_hashmap_find_entry(map, key, h, NULL, &entry)) {
 		if (old_key)
-			*old_key = entry->key;
+			memcpy(old_key, &entry->key, sizeof(long));
 		if (old_value)
-			*old_value = entry->value;
+			memcpy(old_value, &entry->value, sizeof(long));
 
 		if (strategy == PERF_HASHMAP_SET || strategy == PERF_HASHMAP_UPDATE) {
 			entry->key = key;
@@ -203,7 +204,7 @@ int perf_hashmap_insert(struct perf_hashmap *map, long key, long value,
 	return 0;
 }
 
-bool perf_hashmap_find(const struct perf_hashmap *map, long key, long *value)
+bool perf_hashmap_find(const struct perf_hashmap *map, long key, void *value)
 {
 	struct perf_hashmap_entry *entry;
 	size_t h;
@@ -213,12 +214,12 @@ bool perf_hashmap_find(const struct perf_hashmap *map, long key, long *value)
 		return false;
 
 	if (value)
-		*value = entry->value;
+		memcpy(value, &entry->value, sizeof(long));
 	return true;
 }
 
 bool perf_hashmap_delete(struct perf_hashmap *map, long key,
-		    long *old_key, long *old_value)
+		    void *old_key, void *old_value)
 {
 	struct perf_hashmap_entry **pprev, *entry;
 	size_t h;
@@ -228,9 +229,9 @@ bool perf_hashmap_delete(struct perf_hashmap *map, long key,
 		return false;
 
 	if (old_key)
-		*old_key = entry->key;
+		memcpy(old_key, &entry->key, sizeof(long));
 	if (old_value)
-		*old_value = entry->value;
+		memcpy(old_value, &entry->value, sizeof(long));
 
 	perf_hashmap_del_entry(pprev, entry);
 	free(entry);
diff --git a/tools/perf/util/hashmap.h b/tools/perf/util/hashmap.h
index 310b08c0b669..51ad25dd9980 100644
--- a/tools/perf/util/hashmap.h
+++ b/tools/perf/util/hashmap.h
@@ -116,7 +116,7 @@ enum perf_hashmap_insert_strategy {
 	_Static_assert((__builtin_constant_p((p)) ? (p) == NULL : 0) ||			\
 				sizeof(*(p)) == sizeof(long),				\
 		       #p " pointee should be a long-sized integer or a pointer");	\
-	(long *)(p);									\
+	(void *)(p);									\
 })
 
 /*
@@ -128,7 +128,7 @@ enum perf_hashmap_insert_strategy {
  */
 int perf_hashmap_insert(struct perf_hashmap *map, long key, long value,
 		   enum perf_hashmap_insert_strategy strategy,
-		   long *old_key, long *old_value);
+		   void *old_key, void *old_value);
 
 #define perf_hashmap__insert(map, key, value, strategy, old_key, old_value) \
 	perf_hashmap_insert((map), (long)(key), (long)(value), (strategy),  \
@@ -147,14 +147,14 @@ int perf_hashmap_insert(struct perf_hashmap *map, long key, long value,
 #define perf_hashmap__append(map, key, value) \
 	perf_hashmap__insert((map), (key), (value), PERF_HASHMAP_APPEND, NULL, NULL)
 
-bool perf_hashmap_delete(struct perf_hashmap *map, long key, long *old_key, long *old_value);
+bool perf_hashmap_delete(struct perf_hashmap *map, long key, void *old_key, void *old_value);
 
 #define perf_hashmap__delete(map, key, old_key, old_value)		       \
 	perf_hashmap_delete((map), (long)(key),			       \
 		       perf_hashmap_cast_ptr(old_key),		       \
 		       perf_hashmap_cast_ptr(old_value))
 
-bool perf_hashmap_find(const struct perf_hashmap *map, long key, long *value);
+bool perf_hashmap_find(const struct perf_hashmap *map, long key, void *value);
 
 #define perf_hashmap__find(map, key, value) \
 	perf_hashmap_find((map), (long)(key), perf_hashmap_cast_ptr(value))
-- 
2.53.0.959.g497ff81fa9-goog