Hi, John!

On 19.12.2024 12:48, John Naylor wrote:
That would actually be a maintenance headache because the function is
inlined, but here's a better idea: We already have a fallback path for
when the string is not suitably aligned, or in 32-bit builds. We could
just use that under Valgrind:

  static inline size_t
  fasthash_accum_cstring(fasthash_state *hs, const char *str)
  {
-#if SIZEOF_VOID_P >= 8
+#if SIZEOF_VOID_P >= 8 && !defined(USE_VALGRIND)

Any objections?

This variant doesn't produce error and helped me to move
further beyond initdb in the some tests under valgrind
and fix a number of bugs.
Thank you very much!

Seems it is possible to exclude much less code from checking
under valgrind and get the same result by replacing the only
function call pg_rightmost_one_pos64() with a valgrind-safe
code. See the attached patch, please.

The pg_rightmost_one_pos64() itself can also be valgrind-safe
in some cases when the last version of its code works.
But i'm not sure if it's worth writing extra preprocessor instructions
to make this small piece of code also checkable under valgrind.
So in the patch i made a simple variant without it.

With the best wishes,

--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
From 7af05703e59fb8f1e51ec1d667ddc150c40756f0 Mon Sep 17 00:00:00 2001
From: "Anton A. Melnikov" <a.melni...@postgrespro.ru>
Date: Thu, 16 Jan 2025 02:56:52 +0300
Subject: [PATCH] Add valgrind-safe code to find a number of rightmost zero
 bytes.

---
 src/include/common/hashfn_unstable.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/include/common/hashfn_unstable.h b/src/include/common/hashfn_unstable.h
index a09c0b7d5ce..7674c39b1ca 100644
--- a/src/include/common/hashfn_unstable.h
+++ b/src/include/common/hashfn_unstable.h
@@ -302,13 +302,22 @@ fasthash_accum_cstring_aligned(fasthash_state *hs, const char *str)
 		size_t		remainder;
 		uint64		mask;
 
+#if !defined(USE_VALGRIND)
 		/*
 		 * The byte corresponding to the NUL will be 0x80, so the rightmost
 		 * bit position will be in the range 15, 23, ..., 63. Turn this into
 		 * byte position by dividing by 8.
 		 */
 		remainder = pg_rightmost_one_pos64(zero_byte_low) / BITS_PER_BYTE;
-
+#else
+		/* valgrind-safe variant */
+		remainder = 0;
+		while ((zero_byte_low & 0xFF) == 0)
+		{
+			zero_byte_low >>= 8;
+			++remainder;
+		}
+#endif
 		/*
 		 * Create a mask for the remaining bytes so we can combine them into
 		 * the hash. This must have the same result as mixing the remaining
-- 
2.48.1

Reply via email to