From 9d0ff2c2d02bee25ada2b3907c026130cf69b4d6 Mon Sep 17 00:00:00 2001
From: nmeum <soeren+git@soeren-tempel.net>
Date: Sat, 11 Jun 2022 17:33:19 +0000
Subject: [PATCH] Don't rely on signed integer overflow in symhash (#625)

Without this patch, the loop body can easily cause an integer overflow.
In this case, it is possible for the local variable h to take a negative
value. The bitwise-and with 0x7fffffff is supposed to handle this case
by setting the sign-bit to zero. However, GCC may optimize out the
bitwise-and if char is an unsigned value since it assumes that signed
integer overflow does not take place and in this case arithmetic on h
would only be performed on unsigned values. This is, for example, the
case on the ARMv6 architecture. On ARMv6, symhash might thus return
negative values, which can lead to a segmentation fault when they are
used to index vectors etc.

This commit attempts to address this issue by rewriting the code in a
way that it doesn't rely on undefined behavior (i.e. signed integer
overflow). Alternatively, it might also be possible to cast the
local variable s explicitly to a signed type to prevent the
bitwise-and with 0x7fffffff from being optimized out. However,
such a change would still rely on undefined behavior.

Fixes #622
---
 c/foreign.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/c/foreign.c b/c/foreign.c
index 1a4dbf7..fbe9a12 100644
--- a/c/foreign.c
+++ b/c/foreign.c
@@ -51,7 +51,7 @@
 #endif /* LOAD_SHARED_OBJECT */
 
 /* locally defined functions */
-static iptr symhash(const char *s);
+static uptr symhash(const char *s);
 static ptr lookup_static(const char *s);
 static ptr lookup_dynamic(const char *s, ptr tbl);
 static ptr lookup(const char *s);
@@ -85,16 +85,16 @@ static ptr bvstring(const char *s) {
 }
 
 /* multiplier weights each character, h = n factors in the length */
-static iptr symhash(const char *s) {
-  iptr n, h;
+static uptr symhash(const char *s) {
+  uptr n, h;
 
   h = n = strlen(s);
-  while (n--) h = h * multiplier + *s++;
-  return (h & 0x7fffffff) % buckets;
+  while (n--) h = h * multiplier + (unsigned char)*s++;
+  return h % buckets;
 }
 
 static ptr lookup_static(const char *s) {
-  iptr b; ptr p;
+  uptr b; ptr p;
 
   b = symhash(s);
   for (p = Svector_ref(S_G.foreign_static, b); p != Snil; p = Scdr(p))
@@ -180,7 +180,7 @@ void Sforeign_symbol(const char *s, void *v) {
 /* like Sforeign_symbol except it silently redefines the symbol
    if it's already in S_G.foreign_static */
 void Sregister_symbol(const char *s, void *v) {
-  iptr b; ptr p;
+  uptr b; ptr p;
 
   tc_mutex_acquire()
 
@@ -198,7 +198,7 @@ void Sregister_symbol(const char *s, void *v) {
 }
 
 static ptr remove_foreign_entry(const char *s) {
-    iptr b;
+    uptr b;
     ptr tbl, p1, p2;
 
     tc_mutex_acquire()