qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC v5 0/2] monitor: add memory search commands s, sp
@ 2015-05-15 12:25 hw.claudio
  2015-05-15 12:25 ` [Qemu-devel] [RFC v5 1/2] util: add memmem replacement function hw.claudio
  2015-05-15 12:25 ` [Qemu-devel] [RFC v5 2/2] monitor: add memory search commands s, sp hw.claudio
  0 siblings, 2 replies; 9+ messages in thread
From: hw.claudio @ 2015-05-15 12:25 UTC (permalink / raw)
  To: Luiz Capitulino, Paolo Bonzini
  Cc: Peter Maydell, Gonglei, Claudio Fontana, qemu-devel

From: Claudio Fontana <claudio.fontana@huawei.com>

Hello,

I went ahead and tried to build the general solution for replacing
memmem on systems which don't provide one (notably Windows),
detecting the presence of memmem in configure and setting CONFIG_MEMMEM,
and providing an implementation (from gnulib) for the !CONFIG_MEMMEM case.

The code imported is GPLv2 in the "or later" form, which I have seen used
in the util library already.

I have tested this in both CONFIG_MEMMEM defined/undefined scenarios,
but more feedback and testing is welcome of course.

changes from v4:
made into a series of two patches.
Introduced a memmem replacement function (import from gnulib)
and detection code in configure.

changes from v3:
initialize pointer variable to NULL to finally get rid of spurious warning

changes from v2:
move code to try to address spurious warning

changes from v1:
make checkpatch happy by adding braces here and there.

Claudio Fontana (2):
  util: add memmem replacement function
  monitor: add memory search commands s, sp

 configure            |  15 ++
 hmp-commands.hx      |  28 ++++
 include/qemu/osdep.h |   4 +
 monitor.c            | 140 ++++++++++++++++
 util/Makefile.objs   |   1 +
 util/memmem.c        |  73 +++++++++
 util/str-two-way.h   | 452 +++++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 713 insertions(+)
 create mode 100644 util/memmem.c
 create mode 100644 util/str-two-way.h

-- 
1.8.5.3

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] [RFC v5 1/2] util: add memmem replacement function
  2015-05-15 12:25 [Qemu-devel] [RFC v5 0/2] monitor: add memory search commands s, sp hw.claudio
@ 2015-05-15 12:25 ` hw.claudio
  2015-05-15 13:57   ` Claudio Fontana
  2015-05-15 12:25 ` [Qemu-devel] [RFC v5 2/2] monitor: add memory search commands s, sp hw.claudio
  1 sibling, 1 reply; 9+ messages in thread
From: hw.claudio @ 2015-05-15 12:25 UTC (permalink / raw)
  To: Luiz Capitulino, Paolo Bonzini
  Cc: Peter Maydell, Gonglei, Claudio Fontana, qemu-devel

From: Claudio Fontana <claudio.fontana@huawei.com>

if the memmem function is missing, provide the gnulib replacement.

Signed-off-by: Claudio Fontana <claudio.fontana@huawei.com>
---
 configure            |  15 ++
 include/qemu/osdep.h |   4 +
 util/Makefile.objs   |   1 +
 util/memmem.c        |  73 +++++++++
 util/str-two-way.h   | 452 +++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 545 insertions(+)
 create mode 100644 util/memmem.c
 create mode 100644 util/str-two-way.h

diff --git a/configure b/configure
index 1f0f485..feb55b1 100755
--- a/configure
+++ b/configure
@@ -3078,6 +3078,17 @@ if compile_prog "" "" ; then
 fi
 
 ##########################################
+# memmem probe
+cat > $TMPC <<EOF
+#include <string.h>
+int main(int argc, char *argv[]) { return memmem(argv[0], 0, argv[0], 0) != argv[0]; }
+EOF
+memmem=no
+if compile_prog "" "" ; then
+  memmem=yes
+fi
+
+##########################################
 # fdt probe
 # fdt support is mandatory for at least some target architectures,
 # so insist on it if we're building those system emulators.
@@ -4431,6 +4442,7 @@ echo "RDMA support      $rdma"
 echo "TCG interpreter   $tcg_interpreter"
 echo "fdt support       $fdt"
 echo "preadv support    $preadv"
+echo "memmem support    $memmem"
 echo "fdatasync         $fdatasync"
 echo "madvise           $madvise"
 echo "posix_madvise     $posix_madvise"
@@ -4780,6 +4792,9 @@ fi
 if test "$preadv" = "yes" ; then
   echo "CONFIG_PREADV=y" >> $config_host_mak
 fi
+if test "$memmem" = "yes" ; then
+  echo "CONFIG_MEMMEM=y" >> $config_host_mak
+fi
 if test "$fdt" = "yes" ; then
   echo "CONFIG_FDT=y" >> $config_host_mak
 fi
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index b3300cc..d74ddff 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -201,6 +201,10 @@ ssize_t writev(int fd, const struct iovec *iov, int iov_cnt);
 #include <sys/uio.h>
 #endif
 
+#ifndef CONFIG_MEMMEM
+void *memmem(const void *hay, size_t hay_len, const void *s, size_t s_len);
+#endif /* !CONFIG_MEMMEM */
+
 #ifdef _WIN32
 static inline void qemu_timersub(const struct timeval *val1,
                                  const struct timeval *val2,
diff --git a/util/Makefile.objs b/util/Makefile.objs
index ceaba30..628242f 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -1,6 +1,7 @@
 util-obj-y = osdep.o cutils.o unicode.o qemu-timer-common.o
 util-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o event_notifier-win32.o
 util-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o event_notifier-posix.o qemu-openpty.o
+util-obj-$(call lnot,$(CONFIG_MEMMEM)) += memmem.o
 util-obj-y += envlist.o path.o module.o
 util-obj-$(call lnot,$(CONFIG_INT128)) += host-utils.o
 util-obj-y += bitmap.o bitops.o hbitmap.o
diff --git a/util/memmem.c b/util/memmem.c
new file mode 100644
index 0000000..951d0ec
--- /dev/null
+++ b/util/memmem.c
@@ -0,0 +1,73 @@
+/* Copyright (C) 1991-1994, 1996-1998, 2000, 2004, 2007-2015 Free Software
+   Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License along
+   with this program; if not, see <http://www.gnu.org/licenses/>.  */
+
+/* This particular implementation was written by Eric Blake, 2008.  */
+
+/* QEMU notes:
+ * slighly modified for integration into QEMU, Claudio Fontana, 2015:
+ *  - use qemu-common.h
+ *  - removed check for CHAR_BIT for setting LONG_NEEDLE_THRESHOLD.
+ */
+
+#include <qemu-common.h>
+
+#define RETURN_TYPE void *
+#define AVAILABLE(h, h_l, j, n_l) ((j) <= (h_l) - (n_l))
+#define LONG_NEEDLE_THRESHOLD 32U
+#include "str-two-way.h"
+
+/* Return the first occurrence of NEEDLE in HAYSTACK.  Return HAYSTACK
+   if NEEDLE_LEN is 0, otherwise NULL if NEEDLE is not found in
+   HAYSTACK.  */
+void *
+memmem (const void *haystack_start, size_t haystack_len,
+        const void *needle_start, size_t needle_len)
+{
+  /* Abstract memory is considered to be an array of 'unsigned char' values,
+     not an array of 'char' values.  See ISO C 99 section 6.2.6.1.  */
+  const unsigned char *haystack = (const unsigned char *) haystack_start;
+  const unsigned char *needle = (const unsigned char *) needle_start;
+
+  if (needle_len == 0)
+    /* The first occurrence of the empty string is deemed to occur at
+       the beginning of the string.  */
+    return (void *) haystack;
+
+  /* Sanity check, otherwise the loop might search through the whole
+     memory.  */
+  if (__builtin_expect (haystack_len < needle_len, 0))
+    return NULL;
+
+  /* Use optimizations in memchr when possible, to reduce the search
+     size of haystack using a linear algorithm with a smaller
+     coefficient.  However, avoid memchr for long needles, since we
+     can often achieve sublinear performance.  */
+  if (needle_len < LONG_NEEDLE_THRESHOLD)
+    {
+      haystack = memchr (haystack, *needle, haystack_len);
+      if (!haystack || __builtin_expect (needle_len == 1, 0))
+        return (void *) haystack;
+      haystack_len -= haystack - (const unsigned char *) haystack_start;
+      if (haystack_len < needle_len)
+        return NULL;
+      return two_way_short_needle (haystack, haystack_len, needle, needle_len);
+    }
+  else
+    return two_way_long_needle (haystack, haystack_len, needle, needle_len);
+}
+
+#undef LONG_NEEDLE_THRESHOLD
diff --git a/util/str-two-way.h b/util/str-two-way.h
new file mode 100644
index 0000000..d8741dd
--- /dev/null
+++ b/util/str-two-way.h
@@ -0,0 +1,452 @@
+/* Byte-wise substring search, using the Two-Way algorithm.
+   Copyright (C) 2008-2015 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Written by Eric Blake <ebb9@byu.net>, 2008.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License along
+   with this program; if not, see <http://www.gnu.org/licenses/>.  */
+
+/* QEMU notes:
+ * slighly modified for integration into QEMU, Claudio Fontana, 2015:
+ *  - removed check for CHAR_BIT for setting LONG_NEEDLE_THRESHOLD.
+ *  - adjust comments accordingly.
+ */
+
+/* Before including this file, you need to include qemu-common.h and define:
+     RESULT_TYPE             A macro that expands to the return type.
+     AVAILABLE(h, h_l, j, n_l)
+                             A macro that returns nonzero if there are
+                             at least N_L bytes left starting at H[J].
+                             H is 'unsigned char *', H_L, J, and N_L
+                             are 'size_t'; H_L is an lvalue.  For
+                             NUL-terminated searches, H_L can be
+                             modified each iteration to avoid having
+                             to compute the end of H up front.
+
+  For case-insensitivity, you may optionally define:
+     CMP_FUNC(p1, p2, l)     A macro that returns 0 iff the first L
+                             characters of P1 and P2 are equal.
+     CANON_ELEMENT(c)        A macro that canonicalizes an element right after
+                             it has been fetched from one of the two strings.
+                             The argument is an 'unsigned char'; the result
+                             must be an 'unsigned char' as well.
+     LONG_NEEDLE_THRESHOLD   Point at which computing a bad-byte shift table
+                             is likely to be worthwhile (default is 32 bytes).
+*/
+
+#include <limits.h>
+#include <stdint.h>
+
+/* We use the Two-Way string matching algorithm (also known as
+   Chrochemore-Perrin), which guarantees linear complexity with
+   constant space.  Additionally, for long needles, we also use a bad
+   character shift table similar to the Boyer-Moore algorithm to
+   achieve improved (potentially sub-linear) performance.
+
+   See http://www-igm.univ-mlv.fr/~lecroq/string/node26.html#SECTION00260,
+   http://en.wikipedia.org/wiki/Boyer-Moore_string_search_algorithm,
+   http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.34.6641&rep=rep1&type=pdf
+*/
+
+/* Point at which computing a bad-byte shift table is likely to be
+   worthwhile.  Small needles should not compute a table, since it
+   adds (1 << CHAR_BIT) + NEEDLE_LEN computations of preparation for a
+   speedup no greater than a factor of NEEDLE_LEN.  The larger the
+   needle, the better the potential performance gain. */
+#ifndef LONG_NEEDLE_THRESHOLD
+# define LONG_NEEDLE_THRESHOLD 32U
+#endif
+
+#ifndef MAX
+# define MAX(a, b) ((a < b) ? (b) : (a))
+#endif
+
+#ifndef CANON_ELEMENT
+# define CANON_ELEMENT(c) c
+#endif
+#ifndef CMP_FUNC
+# define CMP_FUNC memcmp
+#endif
+
+/* Perform a critical factorization of NEEDLE, of length NEEDLE_LEN.
+   Return the index of the first byte in the right half, and set
+   *PERIOD to the global period of the right half.
+
+   The global period of a string is the smallest index (possibly its
+   length) at which all remaining bytes in the string are repetitions
+   of the prefix (the last repetition may be a subset of the prefix).
+
+   When NEEDLE is factored into two halves, a local period is the
+   length of the smallest word that shares a suffix with the left half
+   and shares a prefix with the right half.  All factorizations of a
+   non-empty NEEDLE have a local period of at least 1 and no greater
+   than NEEDLE_LEN.
+
+   A critical factorization has the property that the local period
+   equals the global period.  All strings have at least one critical
+   factorization with the left half smaller than the global period.
+   And while some strings have more than one critical factorization,
+   it is provable that with an ordered alphabet, at least one of the
+   critical factorizations corresponds to a maximal suffix.
+
+   Given an ordered alphabet, a critical factorization can be computed
+   in linear time, with 2 * NEEDLE_LEN comparisons, by computing the
+   shorter of two ordered maximal suffixes.  The ordered maximal
+   suffixes are determined by lexicographic comparison while tracking
+   periodicity.  */
+static size_t
+critical_factorization (const unsigned char *needle, size_t needle_len,
+                        size_t *period)
+{
+  /* Index of last byte of left half, or SIZE_MAX.  */
+  size_t max_suffix, max_suffix_rev;
+  size_t j; /* Index into NEEDLE for current candidate suffix.  */
+  size_t k; /* Offset into current period.  */
+  size_t p; /* Intermediate period.  */
+  unsigned char a, b; /* Current comparison bytes.  */
+
+  /* Special case NEEDLE_LEN of 1 or 2 (all callers already filtered
+     out 0-length needles.  */
+  if (needle_len < 3)
+    {
+      *period = 1;
+      return needle_len - 1;
+    }
+
+  /* Invariants:
+     0 <= j < NEEDLE_LEN - 1
+     -1 <= max_suffix{,_rev} < j (treating SIZE_MAX as if it were signed)
+     min(max_suffix, max_suffix_rev) < global period of NEEDLE
+     1 <= p <= global period of NEEDLE
+     p == global period of the substring NEEDLE[max_suffix{,_rev}+1...j]
+     1 <= k <= p
+  */
+
+  /* Perform lexicographic search.  */
+  max_suffix = SIZE_MAX;
+  j = 0;
+  k = p = 1;
+  while (j + k < needle_len)
+    {
+      a = CANON_ELEMENT (needle[j + k]);
+      b = CANON_ELEMENT (needle[max_suffix + k]);
+      if (a < b)
+        {
+          /* Suffix is smaller, period is entire prefix so far.  */
+          j += k;
+          k = 1;
+          p = j - max_suffix;
+        }
+      else if (a == b)
+        {
+          /* Advance through repetition of the current period.  */
+          if (k != p)
+            ++k;
+          else
+            {
+              j += p;
+              k = 1;
+            }
+        }
+      else /* b < a */
+        {
+          /* Suffix is larger, start over from current location.  */
+          max_suffix = j++;
+          k = p = 1;
+        }
+    }
+  *period = p;
+
+  /* Perform reverse lexicographic search.  */
+  max_suffix_rev = SIZE_MAX;
+  j = 0;
+  k = p = 1;
+  while (j + k < needle_len)
+    {
+      a = CANON_ELEMENT (needle[j + k]);
+      b = CANON_ELEMENT (needle[max_suffix_rev + k]);
+      if (b < a)
+        {
+          /* Suffix is smaller, period is entire prefix so far.  */
+          j += k;
+          k = 1;
+          p = j - max_suffix_rev;
+        }
+      else if (a == b)
+        {
+          /* Advance through repetition of the current period.  */
+          if (k != p)
+            ++k;
+          else
+            {
+              j += p;
+              k = 1;
+            }
+        }
+      else /* a < b */
+        {
+          /* Suffix is larger, start over from current location.  */
+          max_suffix_rev = j++;
+          k = p = 1;
+        }
+    }
+
+  /* Choose the shorter suffix.  Return the index of the first byte of
+     the right half, rather than the last byte of the left half.
+
+     For some examples, 'banana' has two critical factorizations, both
+     exposed by the two lexicographic extreme suffixes of 'anana' and
+     'nana', where both suffixes have a period of 2.  On the other
+     hand, with 'aab' and 'bba', both strings have a single critical
+     factorization of the last byte, with the suffix having a period
+     of 1.  While the maximal lexicographic suffix of 'aab' is 'b',
+     the maximal lexicographic suffix of 'bba' is 'ba', which is not a
+     critical factorization.  Conversely, the maximal reverse
+     lexicographic suffix of 'a' works for 'bba', but not 'ab' for
+     'aab'.  The shorter suffix of the two will always be a critical
+     factorization.  */
+  if (max_suffix_rev + 1 < max_suffix + 1)
+    return max_suffix + 1;
+  *period = p;
+  return max_suffix_rev + 1;
+}
+
+/* Return the first location of non-empty NEEDLE within HAYSTACK, or
+   NULL.  HAYSTACK_LEN is the minimum known length of HAYSTACK.  This
+   method is optimized for NEEDLE_LEN < LONG_NEEDLE_THRESHOLD.
+   Performance is guaranteed to be linear, with an initialization cost
+   of 2 * NEEDLE_LEN comparisons.
+
+   If AVAILABLE does not modify HAYSTACK_LEN (as in memmem), then at
+   most 2 * HAYSTACK_LEN - NEEDLE_LEN comparisons occur in searching.
+   If AVAILABLE modifies HAYSTACK_LEN (as in strstr), then at most 3 *
+   HAYSTACK_LEN - NEEDLE_LEN comparisons occur in searching.  */
+static RETURN_TYPE
+two_way_short_needle (const unsigned char *haystack, size_t haystack_len,
+                      const unsigned char *needle, size_t needle_len)
+{
+  size_t i; /* Index into current byte of NEEDLE.  */
+  size_t j; /* Index into current window of HAYSTACK.  */
+  size_t period; /* The period of the right half of needle.  */
+  size_t suffix; /* The index of the right half of needle.  */
+
+  /* Factor the needle into two halves, such that the left half is
+     smaller than the global period, and the right half is
+     periodic (with a period as large as NEEDLE_LEN - suffix).  */
+  suffix = critical_factorization (needle, needle_len, &period);
+
+  /* Perform the search.  Each iteration compares the right half
+     first.  */
+  if (CMP_FUNC (needle, needle + period, suffix) == 0)
+    {
+      /* Entire needle is periodic; a mismatch in the left half can
+         only advance by the period, so use memory to avoid rescanning
+         known occurrences of the period in the right half.  */
+      size_t memory = 0;
+      j = 0;
+      while (AVAILABLE (haystack, haystack_len, j, needle_len))
+        {
+          /* Scan for matches in right half.  */
+          i = MAX (suffix, memory);
+          while (i < needle_len && (CANON_ELEMENT (needle[i])
+                                    == CANON_ELEMENT (haystack[i + j])))
+            ++i;
+          if (needle_len <= i)
+            {
+              /* Scan for matches in left half.  */
+              i = suffix - 1;
+              while (memory < i + 1 && (CANON_ELEMENT (needle[i])
+                                        == CANON_ELEMENT (haystack[i + j])))
+                --i;
+              if (i + 1 < memory + 1)
+                return (RETURN_TYPE) (haystack + j);
+              /* No match, so remember how many repetitions of period
+                 on the right half were scanned.  */
+              j += period;
+              memory = needle_len - period;
+            }
+          else
+            {
+              j += i - suffix + 1;
+              memory = 0;
+            }
+        }
+    }
+  else
+    {
+      /* The two halves of needle are distinct; no extra memory is
+         required, and any mismatch results in a maximal shift.  */
+      period = MAX (suffix, needle_len - suffix) + 1;
+      j = 0;
+      while (AVAILABLE (haystack, haystack_len, j, needle_len))
+        {
+          /* Scan for matches in right half.  */
+          i = suffix;
+          while (i < needle_len && (CANON_ELEMENT (needle[i])
+                                    == CANON_ELEMENT (haystack[i + j])))
+            ++i;
+          if (needle_len <= i)
+            {
+              /* Scan for matches in left half.  */
+              i = suffix - 1;
+              while (i != SIZE_MAX && (CANON_ELEMENT (needle[i])
+                                       == CANON_ELEMENT (haystack[i + j])))
+                --i;
+              if (i == SIZE_MAX)
+                return (RETURN_TYPE) (haystack + j);
+              j += period;
+            }
+          else
+            j += i - suffix + 1;
+        }
+    }
+  return NULL;
+}
+
+/* Return the first location of non-empty NEEDLE within HAYSTACK, or
+   NULL.  HAYSTACK_LEN is the minimum known length of HAYSTACK.  This
+   method is optimized for LONG_NEEDLE_THRESHOLD <= NEEDLE_LEN.
+   Performance is guaranteed to be linear, with an initialization cost
+   of 3 * NEEDLE_LEN + (1 << CHAR_BIT) operations.
+
+   If AVAILABLE does not modify HAYSTACK_LEN (as in memmem), then at
+   most 2 * HAYSTACK_LEN - NEEDLE_LEN comparisons occur in searching,
+   and sublinear performance O(HAYSTACK_LEN / NEEDLE_LEN) is possible.
+   If AVAILABLE modifies HAYSTACK_LEN (as in strstr), then at most 3 *
+   HAYSTACK_LEN - NEEDLE_LEN comparisons occur in searching, and
+   sublinear performance is not possible.  */
+static RETURN_TYPE
+two_way_long_needle (const unsigned char *haystack, size_t haystack_len,
+                     const unsigned char *needle, size_t needle_len)
+{
+  size_t i; /* Index into current byte of NEEDLE.  */
+  size_t j; /* Index into current window of HAYSTACK.  */
+  size_t period; /* The period of the right half of needle.  */
+  size_t suffix; /* The index of the right half of needle.  */
+  size_t shift_table[1U << CHAR_BIT]; /* See below.  */
+
+  /* Factor the needle into two halves, such that the left half is
+     smaller than the global period, and the right half is
+     periodic (with a period as large as NEEDLE_LEN - suffix).  */
+  suffix = critical_factorization (needle, needle_len, &period);
+
+  /* Populate shift_table.  For each possible byte value c,
+     shift_table[c] is the distance from the last occurrence of c to
+     the end of NEEDLE, or NEEDLE_LEN if c is absent from the NEEDLE.
+     shift_table[NEEDLE[NEEDLE_LEN - 1]] contains the only 0.  */
+  for (i = 0; i < 1U << CHAR_BIT; i++)
+    shift_table[i] = needle_len;
+  for (i = 0; i < needle_len; i++)
+    shift_table[CANON_ELEMENT (needle[i])] = needle_len - i - 1;
+
+  /* Perform the search.  Each iteration compares the right half
+     first.  */
+  if (CMP_FUNC (needle, needle + period, suffix) == 0)
+    {
+      /* Entire needle is periodic; a mismatch in the left half can
+         only advance by the period, so use memory to avoid rescanning
+         known occurrences of the period in the right half.  */
+      size_t memory = 0;
+      size_t shift;
+      j = 0;
+      while (AVAILABLE (haystack, haystack_len, j, needle_len))
+        {
+          /* Check the last byte first; if it does not match, then
+             shift to the next possible match location.  */
+          shift = shift_table[CANON_ELEMENT (haystack[j + needle_len - 1])];
+          if (0 < shift)
+            {
+              if (memory && shift < period)
+                {
+                  /* Since needle is periodic, but the last period has
+                     a byte out of place, there can be no match until
+                     after the mismatch.  */
+                  shift = needle_len - period;
+                }
+              memory = 0;
+              j += shift;
+              continue;
+            }
+          /* Scan for matches in right half.  The last byte has
+             already been matched, by virtue of the shift table.  */
+          i = MAX (suffix, memory);
+          while (i < needle_len - 1 && (CANON_ELEMENT (needle[i])
+                                        == CANON_ELEMENT (haystack[i + j])))
+            ++i;
+          if (needle_len - 1 <= i)
+            {
+              /* Scan for matches in left half.  */
+              i = suffix - 1;
+              while (memory < i + 1 && (CANON_ELEMENT (needle[i])
+                                        == CANON_ELEMENT (haystack[i + j])))
+                --i;
+              if (i + 1 < memory + 1)
+                return (RETURN_TYPE) (haystack + j);
+              /* No match, so remember how many repetitions of period
+                 on the right half were scanned.  */
+              j += period;
+              memory = needle_len - period;
+            }
+          else
+            {
+              j += i - suffix + 1;
+              memory = 0;
+            }
+        }
+    }
+  else
+    {
+      /* The two halves of needle are distinct; no extra memory is
+         required, and any mismatch results in a maximal shift.  */
+      size_t shift;
+      period = MAX (suffix, needle_len - suffix) + 1;
+      j = 0;
+      while (AVAILABLE (haystack, haystack_len, j, needle_len))
+        {
+          /* Check the last byte first; if it does not match, then
+             shift to the next possible match location.  */
+          shift = shift_table[CANON_ELEMENT (haystack[j + needle_len - 1])];
+          if (0 < shift)
+            {
+              j += shift;
+              continue;
+            }
+          /* Scan for matches in right half.  The last byte has
+             already been matched, by virtue of the shift table.  */
+          i = suffix;
+          while (i < needle_len - 1 && (CANON_ELEMENT (needle[i])
+                                        == CANON_ELEMENT (haystack[i + j])))
+            ++i;
+          if (needle_len - 1 <= i)
+            {
+              /* Scan for matches in left half.  */
+              i = suffix - 1;
+              while (i != SIZE_MAX && (CANON_ELEMENT (needle[i])
+                                       == CANON_ELEMENT (haystack[i + j])))
+                --i;
+              if (i == SIZE_MAX)
+                return (RETURN_TYPE) (haystack + j);
+              j += period;
+            }
+          else
+            j += i - suffix + 1;
+        }
+    }
+  return NULL;
+}
+
+#undef AVAILABLE
+#undef CANON_ELEMENT
+#undef CMP_FUNC
+#undef MAX
+#undef RETURN_TYPE
-- 
1.8.5.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Qemu-devel] [RFC v5 2/2] monitor: add memory search commands s, sp
  2015-05-15 12:25 [Qemu-devel] [RFC v5 0/2] monitor: add memory search commands s, sp hw.claudio
  2015-05-15 12:25 ` [Qemu-devel] [RFC v5 1/2] util: add memmem replacement function hw.claudio
@ 2015-05-15 12:25 ` hw.claudio
  1 sibling, 0 replies; 9+ messages in thread
From: hw.claudio @ 2015-05-15 12:25 UTC (permalink / raw)
  To: Luiz Capitulino, Paolo Bonzini
  Cc: Peter Maydell, Gonglei, Claudio Fontana, qemu-devel

From: Claudio Fontana <claudio.fontana@huawei.com>

usage is similar to the commands x, xp.

Example with string: looking for "ELF" header in memory:

(qemu) s/1000000cb 0x40001000 "ELF"
searching memory area [0000000040001000-00000000400f5240]
0000000040090001
(qemu) x/20b 0x40090000
0000000040090000: '\x7f' 'E' 'L' 'F' '\x02' '\x01' '\x01' '\x03'
0000000040090008: '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00' '\x00'
0000000040090010: '\x02' '\x00' '\xb7' '\x00'

Example with value: looking for 64bit variable value 0x990088

(qemu) s/1000000xg 0xffff900042000000 0x990088
searching memory area [ffff900042000000-ffff9000427a1200]
ffff9000424b3000
ffff9000424c1000

Signed-off-by: Claudio Fontana <claudio.fontana@huawei.com>
---
 hmp-commands.hx |  28 ++++++++++++
 monitor.c       | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 168 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e864a6c..badf1f5 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -430,6 +430,34 @@ Start gdbserver session (default @var{port}=1234)
 ETEXI
 
     {
+        .name       = "s",
+        .args_type  = "fmt:/,addr:l,data:s",
+        .params     = "/fmt addr data",
+        .help       = "search virtual memory starting at 'addr' for 'data'",
+        .mhandler.cmd = hmp_memory_search,
+    },
+
+STEXI
+@item s/fmt @var{addr} @var{data}
+@findex s
+Virtual memory search starting at @var{addr} for data described by @var{data}.
+ETEXI
+
+    {
+        .name       = "sp",
+        .args_type  = "fmt:/,addr:l,data:s",
+        .params     = "/fmt addr data",
+        .help       = "search physical memory starting at 'addr' for 'data'",
+        .mhandler.cmd = hmp_physical_memory_search,
+    },
+
+STEXI
+@item sp/fmt @var{addr} @var{data}
+@findex sp
+Physical memory search starting at @var{addr} for data described by @var{data}.
+ETEXI
+
+    {
         .name       = "x",
         .args_type  = "fmt:/,addr:l",
         .params     = "/fmt addr",
diff --git a/monitor.c b/monitor.c
index b2561e1..fde383e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1205,6 +1205,124 @@ static void monitor_printc(Monitor *mon, int c)
     monitor_printf(mon, "'");
 }
 
+static void monitor_print_addr(Monitor *mon, hwaddr addr, bool is_physical)
+{
+    if (is_physical) {
+        monitor_printf(mon, TARGET_FMT_plx "\n", addr);
+    } else {
+        monitor_printf(mon, TARGET_FMT_lx "\n", (target_ulong)addr);
+    }
+}
+
+/* simple memory search for a byte sequence. The sequence is generated from
+ * a numeric value to look for in guest memory, or from a string.
+ */
+static void memory_search(Monitor *mon, int count, int format, int wsize,
+                          hwaddr addr, const char *data_str, bool is_physical)
+{
+    int pos, len;              /* pos in the search area, len of area */
+    char *hay;                 /* buffer for haystack */
+    int hay_size;              /* haystack size. Needle size is wsize. */
+    const char *needle = NULL; /* needle to search in the haystack */
+    const char *format_str;    /* numeric input format string */
+    char value_raw[8];         /* numeric input converted to raw data */
+#define MONITOR_S_CHUNK_SIZE 16000
+
+    len = wsize * count;
+    if (len < 1) {
+        monitor_printf(mon, "invalid search area length.\n");
+        return;
+    }
+    switch (format) {
+    case 'i':
+        monitor_printf(mon, "format '%c' not supported.\n", format);
+        return;
+    case 'c':
+        needle = data_str;
+        wsize = strlen(data_str);
+        if (wsize > MONITOR_S_CHUNK_SIZE) {
+            monitor_printf(mon, "search string too long [max %d].\n",
+                           MONITOR_S_CHUNK_SIZE);
+            return;
+        }
+        break;
+    case 'o':
+        format_str = "%" SCNo64;
+        break;
+    default:
+    case 'x':
+        format_str = "%" SCNx64;
+        break;
+    case 'u':
+        format_str = "%" SCNu64;
+        break;
+    case 'd':
+        format_str = "%" SCNd64;
+        break;
+    }
+    if (format != 'c') {
+        uint64_t value;      /* numeric input value */
+        void *from = &value;
+        if (sscanf(data_str, format_str, &value) != 1) {
+            monitor_printf(mon, "could not parse search string "
+                           "\"%s\" as format '%c'.\n", data_str, format);
+            return;
+        }
+#if defined(HOST_WORDS_BIGENDIAN) != defined(TARGET_WORDS_BIGENDIAN)
+        value = bswap64(value);
+#endif
+#if defined(TARGET_WORDS_BIGENDIAN)
+        from += 8 - wsize;
+#endif
+        memcpy(value_raw, from, wsize);
+        needle = value_raw;
+    }
+    monitor_printf(mon, "searching memory area ");
+    if (is_physical) {
+        monitor_printf(mon, "[" TARGET_FMT_plx "-" TARGET_FMT_plx "]\n",
+                       addr, addr + len);
+    } else {
+        monitor_printf(mon, "[" TARGET_FMT_lx "-" TARGET_FMT_lx "]\n",
+                       (target_ulong)addr, (target_ulong)addr + len);
+    }
+    hay_size = len < MONITOR_S_CHUNK_SIZE ? len : MONITOR_S_CHUNK_SIZE;
+    hay = g_malloc0(hay_size);
+
+    for (pos = 0; pos < len;) {
+        char *mark, *match; /* mark new starting position, eventual match */
+        int l, todo;        /* total length to be processed in current chunk */
+        l = len - pos;
+        if (l > hay_size) {
+            l = hay_size;
+        }
+        if (is_physical) {
+            cpu_physical_memory_read(addr, hay, l);
+        } else if (cpu_memory_rw_debug(ENV_GET_CPU(mon_get_cpu()), addr,
+                                       (uint8_t *)hay, l, 0) < 0) {
+            monitor_printf(mon, " Cannot access memory\n");
+            break;
+        }
+        for (mark = hay, todo = l; todo >= wsize;) {
+            match = memmem(mark, todo, needle, wsize);
+            if (!match) {
+                break;
+            }
+            monitor_print_addr(mon, addr + (match - hay), is_physical);
+            mark = match + 1;
+            todo = mark - hay;
+        }
+        if (pos + l < len) {
+            /* catch potential matches across chunks. */
+            pos += l - (wsize - 1);
+            addr += l - (wsize - 1);
+        } else {
+            pos += l;
+            addr += l;
+        }
+    }
+    g_free(hay);
+}
+
 static void memory_dump(Monitor *mon, int count, int format, int wsize,
                         hwaddr addr, int is_physical)
 {
@@ -1329,6 +1447,28 @@ static void memory_dump(Monitor *mon, int count, int format, int wsize,
     }
 }
 
+static void hmp_memory_search(Monitor *mon, const QDict *qdict)
+{
+    int count = qdict_get_int(qdict, "count");
+    int format = qdict_get_int(qdict, "format");
+    int size = qdict_get_int(qdict, "size");
+    target_long addr = qdict_get_int(qdict, "addr");
+    const char *data_str = qdict_get_str(qdict, "data");
+
+    memory_search(mon, count, format, size, addr, data_str, false);
+}
+
+static void hmp_physical_memory_search(Monitor *mon, const QDict *qdict)
+{
+    int count = qdict_get_int(qdict, "count");
+    int format = qdict_get_int(qdict, "format");
+    int size = qdict_get_int(qdict, "size");
+    hwaddr addr = qdict_get_int(qdict, "addr");
+    const char *data_str = qdict_get_str(qdict, "data");
+
+    memory_search(mon, count, format, size, addr, data_str, true);
+}
+
 static void hmp_memory_dump(Monitor *mon, const QDict *qdict)
 {
     int count = qdict_get_int(qdict, "count");
-- 
1.8.5.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [RFC v5 1/2] util: add memmem replacement function
  2015-05-15 12:25 ` [Qemu-devel] [RFC v5 1/2] util: add memmem replacement function hw.claudio
@ 2015-05-15 13:57   ` Claudio Fontana
  2015-05-15 14:12     ` Paolo Bonzini
  0 siblings, 1 reply; 9+ messages in thread
From: Claudio Fontana @ 2015-05-15 13:57 UTC (permalink / raw)
  To: hw.claudio, Luiz Capitulino, Paolo Bonzini
  Cc: Peter Maydell, Gonglei, qemu-devel, karl

Hmm some licensing weirdness:

On 15.05.2015 14:25, hw.claudio@gmail.com wrote:
> From: Claudio Fontana <claudio.fontana@huawei.com>
> 
> if the memmem function is missing, provide the gnulib replacement.
> 
> Signed-off-by: Claudio Fontana <claudio.fontana@huawei.com>
> ---
>  configure            |  15 ++
>  include/qemu/osdep.h |   4 +
>  util/Makefile.objs   |   1 +
>  util/memmem.c        |  73 +++++++++
>  util/str-two-way.h   | 452 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 545 insertions(+)
>  create mode 100644 util/memmem.c
>  create mode 100644 util/str-two-way.h
> 
> diff --git a/configure b/configure
> index 1f0f485..feb55b1 100755
> --- a/configure
> +++ b/configure
> @@ -3078,6 +3078,17 @@ if compile_prog "" "" ; then
>  fi
>  
>  ##########################################
> +# memmem probe
> +cat > $TMPC <<EOF
> +#include <string.h>
> +int main(int argc, char *argv[]) { return memmem(argv[0], 0, argv[0], 0) != argv[0]; }
> +EOF
> +memmem=no
> +if compile_prog "" "" ; then
> +  memmem=yes
> +fi
> +
> +##########################################
>  # fdt probe
>  # fdt support is mandatory for at least some target architectures,
>  # so insist on it if we're building those system emulators.
> @@ -4431,6 +4442,7 @@ echo "RDMA support      $rdma"
>  echo "TCG interpreter   $tcg_interpreter"
>  echo "fdt support       $fdt"
>  echo "preadv support    $preadv"
> +echo "memmem support    $memmem"
>  echo "fdatasync         $fdatasync"
>  echo "madvise           $madvise"
>  echo "posix_madvise     $posix_madvise"
> @@ -4780,6 +4792,9 @@ fi
>  if test "$preadv" = "yes" ; then
>    echo "CONFIG_PREADV=y" >> $config_host_mak
>  fi
> +if test "$memmem" = "yes" ; then
> +  echo "CONFIG_MEMMEM=y" >> $config_host_mak
> +fi
>  if test "$fdt" = "yes" ; then
>    echo "CONFIG_FDT=y" >> $config_host_mak
>  fi
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index b3300cc..d74ddff 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -201,6 +201,10 @@ ssize_t writev(int fd, const struct iovec *iov, int iov_cnt);
>  #include <sys/uio.h>
>  #endif
>  
> +#ifndef CONFIG_MEMMEM
> +void *memmem(const void *hay, size_t hay_len, const void *s, size_t s_len);
> +#endif /* !CONFIG_MEMMEM */
> +
>  #ifdef _WIN32
>  static inline void qemu_timersub(const struct timeval *val1,
>                                   const struct timeval *val2,
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index ceaba30..628242f 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -1,6 +1,7 @@
>  util-obj-y = osdep.o cutils.o unicode.o qemu-timer-common.o
>  util-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o event_notifier-win32.o
>  util-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o event_notifier-posix.o qemu-openpty.o
> +util-obj-$(call lnot,$(CONFIG_MEMMEM)) += memmem.o
>  util-obj-y += envlist.o path.o module.o
>  util-obj-$(call lnot,$(CONFIG_INT128)) += host-utils.o
>  util-obj-y += bitmap.o bitops.o hbitmap.o
> diff --git a/util/memmem.c b/util/memmem.c
> new file mode 100644
> index 0000000..951d0ec
> --- /dev/null
> +++ b/util/memmem.c
> @@ -0,0 +1,73 @@
> +/* Copyright (C) 1991-1994, 1996-1998, 2000, 2004, 2007-2015 Free Software
> +   Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 2, or (at your option)
> +   any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License along
> +   with this program; if not, see <http://www.gnu.org/licenses/>.  */
> +
> +/* This particular implementation was written by Eric Blake, 2008.  */

The header here mentions GPLv2+, but the module data for memmem in gnulib mentions LGPLv2+.

Very confusing.

The COPYING file in gnulib mentions:

"The files in here are mostly copyright (C) Free Software Foundation, and
are under assorted licenses.  Mostly, but not entirely, GPL.

Many modules are provided dual-license, either GPL or LGPL at your
option.  The headers of files in the lib directory (e.g., lib/error.c)
state GPL for convenience, since the bulk of current gnulib users are
GPL'd programs.  But the files in the modules directory (e.g.,
modules/error) state the true license of each file, [...]
"

So if one would want to include the gnulib code without using the gnulib esoteric modules mechanisms, what should one do?

CCing Karl Berry since he probably knows what's going on here..

Ciao,

Claudio

> +
> +/* QEMU notes:
> + * slighly modified for integration into QEMU, Claudio Fontana, 2015:
> + *  - use qemu-common.h
> + *  - removed check for CHAR_BIT for setting LONG_NEEDLE_THRESHOLD.
> + */
> +
> +#include <qemu-common.h>
> +
> +#define RETURN_TYPE void *
> +#define AVAILABLE(h, h_l, j, n_l) ((j) <= (h_l) - (n_l))
> +#define LONG_NEEDLE_THRESHOLD 32U
> +#include "str-two-way.h"
> +
> +/* Return the first occurrence of NEEDLE in HAYSTACK.  Return HAYSTACK
> +   if NEEDLE_LEN is 0, otherwise NULL if NEEDLE is not found in
> +   HAYSTACK.  */
> +void *
> +memmem (const void *haystack_start, size_t haystack_len,
> +        const void *needle_start, size_t needle_len)
> +{
> +  /* Abstract memory is considered to be an array of 'unsigned char' values,
> +     not an array of 'char' values.  See ISO C 99 section 6.2.6.1.  */
> +  const unsigned char *haystack = (const unsigned char *) haystack_start;
> +  const unsigned char *needle = (const unsigned char *) needle_start;
> +
> +  if (needle_len == 0)
> +    /* The first occurrence of the empty string is deemed to occur at
> +       the beginning of the string.  */
> +    return (void *) haystack;
> +
> +  /* Sanity check, otherwise the loop might search through the whole
> +     memory.  */
> +  if (__builtin_expect (haystack_len < needle_len, 0))
> +    return NULL;
> +
> +  /* Use optimizations in memchr when possible, to reduce the search
> +     size of haystack using a linear algorithm with a smaller
> +     coefficient.  However, avoid memchr for long needles, since we
> +     can often achieve sublinear performance.  */
> +  if (needle_len < LONG_NEEDLE_THRESHOLD)
> +    {
> +      haystack = memchr (haystack, *needle, haystack_len);
> +      if (!haystack || __builtin_expect (needle_len == 1, 0))
> +        return (void *) haystack;
> +      haystack_len -= haystack - (const unsigned char *) haystack_start;
> +      if (haystack_len < needle_len)
> +        return NULL;
> +      return two_way_short_needle (haystack, haystack_len, needle, needle_len);
> +    }
> +  else
> +    return two_way_long_needle (haystack, haystack_len, needle, needle_len);
> +}
> +
> +#undef LONG_NEEDLE_THRESHOLD
> diff --git a/util/str-two-way.h b/util/str-two-way.h
> new file mode 100644
> index 0000000..d8741dd
> --- /dev/null
> +++ b/util/str-two-way.h
> @@ -0,0 +1,452 @@
> +/* Byte-wise substring search, using the Two-Way algorithm.
> +   Copyright (C) 2008-2015 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +   Written by Eric Blake <ebb9@byu.net>, 2008.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 2, or (at your option)
> +   any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License along
> +   with this program; if not, see <http://www.gnu.org/licenses/>.  */
> +
> +/* QEMU notes:
> + * slighly modified for integration into QEMU, Claudio Fontana, 2015:
> + *  - removed check for CHAR_BIT for setting LONG_NEEDLE_THRESHOLD.
> + *  - adjust comments accordingly.
> + */
> +
> +/* Before including this file, you need to include qemu-common.h and define:
> +     RESULT_TYPE             A macro that expands to the return type.
> +     AVAILABLE(h, h_l, j, n_l)
> +                             A macro that returns nonzero if there are
> +                             at least N_L bytes left starting at H[J].
> +                             H is 'unsigned char *', H_L, J, and N_L
> +                             are 'size_t'; H_L is an lvalue.  For
> +                             NUL-terminated searches, H_L can be
> +                             modified each iteration to avoid having
> +                             to compute the end of H up front.
> +
> +  For case-insensitivity, you may optionally define:
> +     CMP_FUNC(p1, p2, l)     A macro that returns 0 iff the first L
> +                             characters of P1 and P2 are equal.
> +     CANON_ELEMENT(c)        A macro that canonicalizes an element right after
> +                             it has been fetched from one of the two strings.
> +                             The argument is an 'unsigned char'; the result
> +                             must be an 'unsigned char' as well.
> +     LONG_NEEDLE_THRESHOLD   Point at which computing a bad-byte shift table
> +                             is likely to be worthwhile (default is 32 bytes).
> +*/
> +
> +#include <limits.h>
> +#include <stdint.h>
> +
> +/* We use the Two-Way string matching algorithm (also known as
> +   Chrochemore-Perrin), which guarantees linear complexity with
> +   constant space.  Additionally, for long needles, we also use a bad
> +   character shift table similar to the Boyer-Moore algorithm to
> +   achieve improved (potentially sub-linear) performance.
> +
> +   See http://www-igm.univ-mlv.fr/~lecroq/string/node26.html#SECTION00260,
> +   http://en.wikipedia.org/wiki/Boyer-Moore_string_search_algorithm,
> +   http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.34.6641&rep=rep1&type=pdf
> +*/
> +
> +/* Point at which computing a bad-byte shift table is likely to be
> +   worthwhile.  Small needles should not compute a table, since it
> +   adds (1 << CHAR_BIT) + NEEDLE_LEN computations of preparation for a
> +   speedup no greater than a factor of NEEDLE_LEN.  The larger the
> +   needle, the better the potential performance gain. */
> +#ifndef LONG_NEEDLE_THRESHOLD
> +# define LONG_NEEDLE_THRESHOLD 32U
> +#endif
> +
> +#ifndef MAX
> +# define MAX(a, b) ((a < b) ? (b) : (a))
> +#endif
> +
> +#ifndef CANON_ELEMENT
> +# define CANON_ELEMENT(c) c
> +#endif
> +#ifndef CMP_FUNC
> +# define CMP_FUNC memcmp
> +#endif
> +
> +/* Perform a critical factorization of NEEDLE, of length NEEDLE_LEN.
> +   Return the index of the first byte in the right half, and set
> +   *PERIOD to the global period of the right half.
> +
> +   The global period of a string is the smallest index (possibly its
> +   length) at which all remaining bytes in the string are repetitions
> +   of the prefix (the last repetition may be a subset of the prefix).
> +
> +   When NEEDLE is factored into two halves, a local period is the
> +   length of the smallest word that shares a suffix with the left half
> +   and shares a prefix with the right half.  All factorizations of a
> +   non-empty NEEDLE have a local period of at least 1 and no greater
> +   than NEEDLE_LEN.
> +
> +   A critical factorization has the property that the local period
> +   equals the global period.  All strings have at least one critical
> +   factorization with the left half smaller than the global period.
> +   And while some strings have more than one critical factorization,
> +   it is provable that with an ordered alphabet, at least one of the
> +   critical factorizations corresponds to a maximal suffix.
> +
> +   Given an ordered alphabet, a critical factorization can be computed
> +   in linear time, with 2 * NEEDLE_LEN comparisons, by computing the
> +   shorter of two ordered maximal suffixes.  The ordered maximal
> +   suffixes are determined by lexicographic comparison while tracking
> +   periodicity.  */
> +static size_t
> +critical_factorization (const unsigned char *needle, size_t needle_len,
> +                        size_t *period)
> +{
> +  /* Index of last byte of left half, or SIZE_MAX.  */
> +  size_t max_suffix, max_suffix_rev;
> +  size_t j; /* Index into NEEDLE for current candidate suffix.  */
> +  size_t k; /* Offset into current period.  */
> +  size_t p; /* Intermediate period.  */
> +  unsigned char a, b; /* Current comparison bytes.  */
> +
> +  /* Special case NEEDLE_LEN of 1 or 2 (all callers already filtered
> +     out 0-length needles.  */
> +  if (needle_len < 3)
> +    {
> +      *period = 1;
> +      return needle_len - 1;
> +    }
> +
> +  /* Invariants:
> +     0 <= j < NEEDLE_LEN - 1
> +     -1 <= max_suffix{,_rev} < j (treating SIZE_MAX as if it were signed)
> +     min(max_suffix, max_suffix_rev) < global period of NEEDLE
> +     1 <= p <= global period of NEEDLE
> +     p == global period of the substring NEEDLE[max_suffix{,_rev}+1...j]
> +     1 <= k <= p
> +  */
> +
> +  /* Perform lexicographic search.  */
> +  max_suffix = SIZE_MAX;
> +  j = 0;
> +  k = p = 1;
> +  while (j + k < needle_len)
> +    {
> +      a = CANON_ELEMENT (needle[j + k]);
> +      b = CANON_ELEMENT (needle[max_suffix + k]);
> +      if (a < b)
> +        {
> +          /* Suffix is smaller, period is entire prefix so far.  */
> +          j += k;
> +          k = 1;
> +          p = j - max_suffix;
> +        }
> +      else if (a == b)
> +        {
> +          /* Advance through repetition of the current period.  */
> +          if (k != p)
> +            ++k;
> +          else
> +            {
> +              j += p;
> +              k = 1;
> +            }
> +        }
> +      else /* b < a */
> +        {
> +          /* Suffix is larger, start over from current location.  */
> +          max_suffix = j++;
> +          k = p = 1;
> +        }
> +    }
> +  *period = p;
> +
> +  /* Perform reverse lexicographic search.  */
> +  max_suffix_rev = SIZE_MAX;
> +  j = 0;
> +  k = p = 1;
> +  while (j + k < needle_len)
> +    {
> +      a = CANON_ELEMENT (needle[j + k]);
> +      b = CANON_ELEMENT (needle[max_suffix_rev + k]);
> +      if (b < a)
> +        {
> +          /* Suffix is smaller, period is entire prefix so far.  */
> +          j += k;
> +          k = 1;
> +          p = j - max_suffix_rev;
> +        }
> +      else if (a == b)
> +        {
> +          /* Advance through repetition of the current period.  */
> +          if (k != p)
> +            ++k;
> +          else
> +            {
> +              j += p;
> +              k = 1;
> +            }
> +        }
> +      else /* a < b */
> +        {
> +          /* Suffix is larger, start over from current location.  */
> +          max_suffix_rev = j++;
> +          k = p = 1;
> +        }
> +    }
> +
> +  /* Choose the shorter suffix.  Return the index of the first byte of
> +     the right half, rather than the last byte of the left half.
> +
> +     For some examples, 'banana' has two critical factorizations, both
> +     exposed by the two lexicographic extreme suffixes of 'anana' and
> +     'nana', where both suffixes have a period of 2.  On the other
> +     hand, with 'aab' and 'bba', both strings have a single critical
> +     factorization of the last byte, with the suffix having a period
> +     of 1.  While the maximal lexicographic suffix of 'aab' is 'b',
> +     the maximal lexicographic suffix of 'bba' is 'ba', which is not a
> +     critical factorization.  Conversely, the maximal reverse
> +     lexicographic suffix of 'a' works for 'bba', but not 'ab' for
> +     'aab'.  The shorter suffix of the two will always be a critical
> +     factorization.  */
> +  if (max_suffix_rev + 1 < max_suffix + 1)
> +    return max_suffix + 1;
> +  *period = p;
> +  return max_suffix_rev + 1;
> +}
> +
> +/* Return the first location of non-empty NEEDLE within HAYSTACK, or
> +   NULL.  HAYSTACK_LEN is the minimum known length of HAYSTACK.  This
> +   method is optimized for NEEDLE_LEN < LONG_NEEDLE_THRESHOLD.
> +   Performance is guaranteed to be linear, with an initialization cost
> +   of 2 * NEEDLE_LEN comparisons.
> +
> +   If AVAILABLE does not modify HAYSTACK_LEN (as in memmem), then at
> +   most 2 * HAYSTACK_LEN - NEEDLE_LEN comparisons occur in searching.
> +   If AVAILABLE modifies HAYSTACK_LEN (as in strstr), then at most 3 *
> +   HAYSTACK_LEN - NEEDLE_LEN comparisons occur in searching.  */
> +static RETURN_TYPE
> +two_way_short_needle (const unsigned char *haystack, size_t haystack_len,
> +                      const unsigned char *needle, size_t needle_len)
> +{
> +  size_t i; /* Index into current byte of NEEDLE.  */
> +  size_t j; /* Index into current window of HAYSTACK.  */
> +  size_t period; /* The period of the right half of needle.  */
> +  size_t suffix; /* The index of the right half of needle.  */
> +
> +  /* Factor the needle into two halves, such that the left half is
> +     smaller than the global period, and the right half is
> +     periodic (with a period as large as NEEDLE_LEN - suffix).  */
> +  suffix = critical_factorization (needle, needle_len, &period);
> +
> +  /* Perform the search.  Each iteration compares the right half
> +     first.  */
> +  if (CMP_FUNC (needle, needle + period, suffix) == 0)
> +    {
> +      /* Entire needle is periodic; a mismatch in the left half can
> +         only advance by the period, so use memory to avoid rescanning
> +         known occurrences of the period in the right half.  */
> +      size_t memory = 0;
> +      j = 0;
> +      while (AVAILABLE (haystack, haystack_len, j, needle_len))
> +        {
> +          /* Scan for matches in right half.  */
> +          i = MAX (suffix, memory);
> +          while (i < needle_len && (CANON_ELEMENT (needle[i])
> +                                    == CANON_ELEMENT (haystack[i + j])))
> +            ++i;
> +          if (needle_len <= i)
> +            {
> +              /* Scan for matches in left half.  */
> +              i = suffix - 1;
> +              while (memory < i + 1 && (CANON_ELEMENT (needle[i])
> +                                        == CANON_ELEMENT (haystack[i + j])))
> +                --i;
> +              if (i + 1 < memory + 1)
> +                return (RETURN_TYPE) (haystack + j);
> +              /* No match, so remember how many repetitions of period
> +                 on the right half were scanned.  */
> +              j += period;
> +              memory = needle_len - period;
> +            }
> +          else
> +            {
> +              j += i - suffix + 1;
> +              memory = 0;
> +            }
> +        }
> +    }
> +  else
> +    {
> +      /* The two halves of needle are distinct; no extra memory is
> +         required, and any mismatch results in a maximal shift.  */
> +      period = MAX (suffix, needle_len - suffix) + 1;
> +      j = 0;
> +      while (AVAILABLE (haystack, haystack_len, j, needle_len))
> +        {
> +          /* Scan for matches in right half.  */
> +          i = suffix;
> +          while (i < needle_len && (CANON_ELEMENT (needle[i])
> +                                    == CANON_ELEMENT (haystack[i + j])))
> +            ++i;
> +          if (needle_len <= i)
> +            {
> +              /* Scan for matches in left half.  */
> +              i = suffix - 1;
> +              while (i != SIZE_MAX && (CANON_ELEMENT (needle[i])
> +                                       == CANON_ELEMENT (haystack[i + j])))
> +                --i;
> +              if (i == SIZE_MAX)
> +                return (RETURN_TYPE) (haystack + j);
> +              j += period;
> +            }
> +          else
> +            j += i - suffix + 1;
> +        }
> +    }
> +  return NULL;
> +}
> +
> +/* Return the first location of non-empty NEEDLE within HAYSTACK, or
> +   NULL.  HAYSTACK_LEN is the minimum known length of HAYSTACK.  This
> +   method is optimized for LONG_NEEDLE_THRESHOLD <= NEEDLE_LEN.
> +   Performance is guaranteed to be linear, with an initialization cost
> +   of 3 * NEEDLE_LEN + (1 << CHAR_BIT) operations.
> +
> +   If AVAILABLE does not modify HAYSTACK_LEN (as in memmem), then at
> +   most 2 * HAYSTACK_LEN - NEEDLE_LEN comparisons occur in searching,
> +   and sublinear performance O(HAYSTACK_LEN / NEEDLE_LEN) is possible.
> +   If AVAILABLE modifies HAYSTACK_LEN (as in strstr), then at most 3 *
> +   HAYSTACK_LEN - NEEDLE_LEN comparisons occur in searching, and
> +   sublinear performance is not possible.  */
> +static RETURN_TYPE
> +two_way_long_needle (const unsigned char *haystack, size_t haystack_len,
> +                     const unsigned char *needle, size_t needle_len)
> +{
> +  size_t i; /* Index into current byte of NEEDLE.  */
> +  size_t j; /* Index into current window of HAYSTACK.  */
> +  size_t period; /* The period of the right half of needle.  */
> +  size_t suffix; /* The index of the right half of needle.  */
> +  size_t shift_table[1U << CHAR_BIT]; /* See below.  */
> +
> +  /* Factor the needle into two halves, such that the left half is
> +     smaller than the global period, and the right half is
> +     periodic (with a period as large as NEEDLE_LEN - suffix).  */
> +  suffix = critical_factorization (needle, needle_len, &period);
> +
> +  /* Populate shift_table.  For each possible byte value c,
> +     shift_table[c] is the distance from the last occurrence of c to
> +     the end of NEEDLE, or NEEDLE_LEN if c is absent from the NEEDLE.
> +     shift_table[NEEDLE[NEEDLE_LEN - 1]] contains the only 0.  */
> +  for (i = 0; i < 1U << CHAR_BIT; i++)
> +    shift_table[i] = needle_len;
> +  for (i = 0; i < needle_len; i++)
> +    shift_table[CANON_ELEMENT (needle[i])] = needle_len - i - 1;
> +
> +  /* Perform the search.  Each iteration compares the right half
> +     first.  */
> +  if (CMP_FUNC (needle, needle + period, suffix) == 0)
> +    {
> +      /* Entire needle is periodic; a mismatch in the left half can
> +         only advance by the period, so use memory to avoid rescanning
> +         known occurrences of the period in the right half.  */
> +      size_t memory = 0;
> +      size_t shift;
> +      j = 0;
> +      while (AVAILABLE (haystack, haystack_len, j, needle_len))
> +        {
> +          /* Check the last byte first; if it does not match, then
> +             shift to the next possible match location.  */
> +          shift = shift_table[CANON_ELEMENT (haystack[j + needle_len - 1])];
> +          if (0 < shift)
> +            {
> +              if (memory && shift < period)
> +                {
> +                  /* Since needle is periodic, but the last period has
> +                     a byte out of place, there can be no match until
> +                     after the mismatch.  */
> +                  shift = needle_len - period;
> +                }
> +              memory = 0;
> +              j += shift;
> +              continue;
> +            }
> +          /* Scan for matches in right half.  The last byte has
> +             already been matched, by virtue of the shift table.  */
> +          i = MAX (suffix, memory);
> +          while (i < needle_len - 1 && (CANON_ELEMENT (needle[i])
> +                                        == CANON_ELEMENT (haystack[i + j])))
> +            ++i;
> +          if (needle_len - 1 <= i)
> +            {
> +              /* Scan for matches in left half.  */
> +              i = suffix - 1;
> +              while (memory < i + 1 && (CANON_ELEMENT (needle[i])
> +                                        == CANON_ELEMENT (haystack[i + j])))
> +                --i;
> +              if (i + 1 < memory + 1)
> +                return (RETURN_TYPE) (haystack + j);
> +              /* No match, so remember how many repetitions of period
> +                 on the right half were scanned.  */
> +              j += period;
> +              memory = needle_len - period;
> +            }
> +          else
> +            {
> +              j += i - suffix + 1;
> +              memory = 0;
> +            }
> +        }
> +    }
> +  else
> +    {
> +      /* The two halves of needle are distinct; no extra memory is
> +         required, and any mismatch results in a maximal shift.  */
> +      size_t shift;
> +      period = MAX (suffix, needle_len - suffix) + 1;
> +      j = 0;
> +      while (AVAILABLE (haystack, haystack_len, j, needle_len))
> +        {
> +          /* Check the last byte first; if it does not match, then
> +             shift to the next possible match location.  */
> +          shift = shift_table[CANON_ELEMENT (haystack[j + needle_len - 1])];
> +          if (0 < shift)
> +            {
> +              j += shift;
> +              continue;
> +            }
> +          /* Scan for matches in right half.  The last byte has
> +             already been matched, by virtue of the shift table.  */
> +          i = suffix;
> +          while (i < needle_len - 1 && (CANON_ELEMENT (needle[i])
> +                                        == CANON_ELEMENT (haystack[i + j])))
> +            ++i;
> +          if (needle_len - 1 <= i)
> +            {
> +              /* Scan for matches in left half.  */
> +              i = suffix - 1;
> +              while (i != SIZE_MAX && (CANON_ELEMENT (needle[i])
> +                                       == CANON_ELEMENT (haystack[i + j])))
> +                --i;
> +              if (i == SIZE_MAX)
> +                return (RETURN_TYPE) (haystack + j);
> +              j += period;
> +            }
> +          else
> +            j += i - suffix + 1;
> +        }
> +    }
> +  return NULL;
> +}
> +
> +#undef AVAILABLE
> +#undef CANON_ELEMENT
> +#undef CMP_FUNC
> +#undef MAX
> +#undef RETURN_TYPE
> 


-- 
Claudio Fontana
Server Virtualization Architect
Huawei Technologies Duesseldorf GmbH
Riesstraße 25 - 80992 München

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [RFC v5 1/2] util: add memmem replacement function
  2015-05-15 13:57   ` Claudio Fontana
@ 2015-05-15 14:12     ` Paolo Bonzini
  2015-05-15 14:56       ` Eric Blake
  0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2015-05-15 14:12 UTC (permalink / raw)
  To: Claudio Fontana, hw.claudio, Luiz Capitulino
  Cc: Peter Maydell, Gonglei, qemu-devel, karl



On 15/05/2015 15:57, Claudio Fontana wrote:
> The header here mentions GPLv2+, but the module data for memmem in gnulib mentions LGPLv2+.
> 
> Very confusing.
> 
> The COPYING file in gnulib mentions:
> 
> "The files in here are mostly copyright (C) Free Software Foundation, and
> are under assorted licenses.  Mostly, but not entirely, GPL.
> 
> Many modules are provided dual-license, either GPL or LGPL at your
> option.  The headers of files in the lib directory (e.g., lib/error.c)
> state GPL for convenience, since the bulk of current gnulib users are
> GPL'd programs.  But the files in the modules directory (e.g.,
> modules/error) state the true license of each file, [...]
> "
> 
> So if one would want to include the gnulib code without using the gnulib esoteric modules mechanisms, what should one do?

Change it manually, I guess.

Paolo

> CCing Karl Berry since he probably knows what's going on here..

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [RFC v5 1/2] util: add memmem replacement function
  2015-05-15 14:12     ` Paolo Bonzini
@ 2015-05-15 14:56       ` Eric Blake
  2015-05-15 15:13         ` Claudio Fontana
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Blake @ 2015-05-15 14:56 UTC (permalink / raw)
  To: Paolo Bonzini, Claudio Fontana, hw.claudio, Luiz Capitulino
  Cc: Peter Maydell, Gonglei, qemu-devel, karl

[-- Attachment #1: Type: text/plain, Size: 3342 bytes --]

On 05/15/2015 08:12 AM, Paolo Bonzini wrote:
> 
> 
> On 15/05/2015 15:57, Claudio Fontana wrote:
>> The header here mentions GPLv2+, but the module data for memmem in gnulib mentions LGPLv2+.
>>
>> Very confusing.
>>
>> The COPYING file in gnulib mentions:
>>
>> "The files in here are mostly copyright (C) Free Software Foundation, and
>> are under assorted licenses.  Mostly, but not entirely, GPL.
>>
>> Many modules are provided dual-license, either GPL or LGPL at your
>> option.  The headers of files in the lib directory (e.g., lib/error.c)
>> state GPL for convenience, since the bulk of current gnulib users are
>> GPL'd programs.  But the files in the modules directory (e.g.,
>> modules/error) state the true license of each file, [...]
>> "
>>
>> So if one would want to include the gnulib code without using the gnulib esoteric modules mechanisms, what should one do?
> 
> Change it manually, I guess.

It's still possible to use gnulib-tool for a one-time export of the
modules in question into a temporary project with correct licensing
headers written by gnulib-tool, then copy from that temporary project
into qemu.  It's probably just as fast to do it by hand, but using
gnulib-tool in the procedure (and documenting in the commit message what
that procedure was, to make it easier for the next guy to copy) is
probably friendlier.  I'm not sure off-hand what the command line would
be, but could help if that's the approach we want to take.

Or, since gnulib's memmem is (mostly) sync'd with glibc's memmem (or at
least was in sync at one point), and glibc is under LGPLv2+, why not
copy straight from glibc instead of from gnulib, and then you don't have
to modify any headers?  (I haven't looked lately, but glibc may have
optimizations that should be backported into gnulib)

Historical note: gnulib had this particular implementation of memmem
first, as written by me under a dual license; then I got glibc to
incorporate it under just LGPLv2+; then since the initial
implementation, glibc has been better optimized by others. If you really
want to, you could observe that since I released the same original
memmem implementation under multiple licenses at the time I first wrote
it, you could therefore find and copy an older version under an even
looser license in the newlib project (but without glibc's enhancements,
as I can't dual-license someone else's follow-up work):
https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=newlib/libc/string/memmem.c;h=25704e46;hb=HEAD

Or back to the original question - why are we worrying about the O(n)
memmem implementation when the naive O(n^2) is MUCH shorter and easier
to understand, and where the scaling penalty is only apparent on really
long corner case strings?  Is our use of memmem going to hit the corner
cases where scaling matters?

[Personal note: I'm still tickled pink every time I see yet another
project adopt my O(n) code - I didn't come up with the algorithm, but
merely scraped it from public research, and was shocked to note how many
libc still had O(n^2) code at the time.  But it is still gratifying to
see that my implementation is getting such wide adoption]

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [RFC v5 1/2] util: add memmem replacement function
  2015-05-15 14:56       ` Eric Blake
@ 2015-05-15 15:13         ` Claudio Fontana
  2015-05-15 16:01           ` Eric Blake
  0 siblings, 1 reply; 9+ messages in thread
From: Claudio Fontana @ 2015-05-15 15:13 UTC (permalink / raw)
  To: Eric Blake, Paolo Bonzini, hw.claudio, Luiz Capitulino
  Cc: Peter Maydell, Gonglei, qemu-devel, karl

Hello Eric,

On 15.05.2015 16:56, Eric Blake wrote:
> On 05/15/2015 08:12 AM, Paolo Bonzini wrote:
>>
>>
>> On 15/05/2015 15:57, Claudio Fontana wrote:
>>> The header here mentions GPLv2+, but the module data for memmem in gnulib mentions LGPLv2+.
>>>
>>> Very confusing.
>>>
>>> The COPYING file in gnulib mentions:
>>>
>>> "The files in here are mostly copyright (C) Free Software Foundation, and
>>> are under assorted licenses.  Mostly, but not entirely, GPL.
>>>
>>> Many modules are provided dual-license, either GPL or LGPL at your
>>> option.  The headers of files in the lib directory (e.g., lib/error.c)
>>> state GPL for convenience, since the bulk of current gnulib users are
>>> GPL'd programs.  But the files in the modules directory (e.g.,
>>> modules/error) state the true license of each file, [...]
>>> "
>>>
>>> So if one would want to include the gnulib code without using the gnulib esoteric modules mechanisms, what should one do?
>>
>> Change it manually, I guess.
> 
> It's still possible to use gnulib-tool for a one-time export of the
> modules in question into a temporary project with correct licensing
> headers written by gnulib-tool, then copy from that temporary project
> into qemu.  It's probably just as fast to do it by hand, but using
> gnulib-tool in the procedure (and documenting in the commit message what
> that procedure was, to make it easier for the next guy to copy) is
> probably friendlier.  I'm not sure off-hand what the command line would
> be, but could help if that's the approach we want to take.
> 
> Or, since gnulib's memmem is (mostly) sync'd with glibc's memmem (or at
> least was in sync at one point), and glibc is under LGPLv2+, why not
> copy straight from glibc instead of from gnulib, and then you don't have
> to modify any headers?  (I haven't looked lately, but glibc may have
> optimizations that should be backported into gnulib)

I will check the status of glibc's version, seems a good idea to get it from there.

> 
> Historical note: gnulib had this particular implementation of memmem
> first, as written by me under a dual license; then I got glibc to
> incorporate it under just LGPLv2+; then since the initial
> implementation, glibc has been better optimized by others. If you really
> want to, you could observe that since I released the same original
> memmem implementation under multiple licenses at the time I first wrote
> it, you could therefore find and copy an older version under an even
> looser license in the newlib project (but without glibc's enhancements,
> as I can't dual-license someone else's follow-up work):
> https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=newlib/libc/string/memmem.c;h=25704e46;hb=HEAD
> 
> Or back to the original question - why are we worrying about the O(n)
> memmem implementation when the naive O(n^2) is MUCH shorter and easier
> to understand, and where the scaling penalty is only apparent on really
> long corner case strings?  Is our use of memmem going to hit the corner
> cases where scaling matters?

For the specific use case here (memory search functionality),
the haystack is normally the factor which really can scale up a lot.

I would expect the needle to be usually smallish (though for string search sometimes the needle could probably arrive at the threshold of 32).

When scanning a very large memory range, considering a binary string with roughly length around 32, do you think that the non-trivial implementation could bring some noticeable benefits?

If it's below 32, do we get any benefits compared to the trivial implementation?

> 
> [Personal note: I'm still tickled pink every time I see yet another
> project adopt my O(n) code - I didn't come up with the algorithm, but
> merely scraped it from public research, and was shocked to note how many
> libc still had O(n^2) code at the time.  But it is still gratifying to
> see that my implementation is getting such wide adoption]
> 

Well your code and its derived versions have been tried and tested a lot, so picking it up just makes sense to me, to avoid getting into the same corner cases and issues that your implementation already had to go through. And I am a lazy person of course!

Ciao,

Claudio

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [RFC v5 1/2] util: add memmem replacement function
  2015-05-15 15:13         ` Claudio Fontana
@ 2015-05-15 16:01           ` Eric Blake
  2015-05-18  8:36             ` Claudio Fontana
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Blake @ 2015-05-15 16:01 UTC (permalink / raw)
  To: Claudio Fontana, Paolo Bonzini, hw.claudio, Luiz Capitulino
  Cc: Peter Maydell, Gonglei, qemu-devel, karl

[-- Attachment #1: Type: text/plain, Size: 5128 bytes --]

On 05/15/2015 09:13 AM, Claudio Fontana wrote:

>> Or back to the original question - why are we worrying about the O(n)
>> memmem implementation when the naive O(n^2) is MUCH shorter and easier
>> to understand, and where the scaling penalty is only apparent on really
>> long corner case strings?  Is our use of memmem going to hit the corner
>> cases where scaling matters?
> 
> For the specific use case here (memory search functionality),
> the haystack is normally the factor which really can scale up a lot.

Remember, when searching, the worst-case penalty is determined by a
combination of the size of the haystack (H) and the size of the needle
(I'll choose M to represent that, to avoid confusion with the generic n
in Big-O notation).  The most brain-dead loop (for each byte in
haystack, see if the needle matches) is H*M operations; this is
noticeable as quadratic scaling (and even then, only for certain
matching patterns, such as determining if a needle of all 'a' except for
a final 'b' occurs in a haystack of all 'a') only if both needle and
haystack can grow arbitrarily large: O(H*M) == O(n^2).  Where my code
shines is that it breaks the operation into two parts - a factorization
of the needle O(M) == O(n), then a linear search using that factored
needle O(H) == O(n), for an overall cost O(H+M) == O(n). But with the
naive algorithm, if the needle is capped to a fixed size, then you have
O(constant*H) == O(H) == O(n).

Meanwhile, remember that Big-O notation is only about scaling effects,
but that there are still constant factors to be considered - an O(n)
algorithm that requires 3 comparisons per byte of the haystack will
always be slower than an O(n) algorithm that only needs 2 comparisons
per byte of the haystack, even though both algorithms scale linearly.

> 
> I would expect the needle to be usually smallish (though for string search sometimes the needle could probably arrive at the threshold of 32).
> 
> When scanning a very large memory range, considering a binary string with roughly length around 32, do you think that the non-trivial implementation could bring some noticeable benefits?
> 
> If it's below 32, do we get any benefits compared to the trivial implementation?

In fact, one of the glibc optimizations was realizing that my two-way
algorithm code is LOUSY at short needles.  It has a high relative
overhead for finding the proper factorization of a short needle, before
it can even start searching.  A hybrid approach (brute force a short
needle, and only bother with factorizing a long needle, or even after a
heuristic of a minimum number of comparisons is first met) is ideal,
because of the observation above that the poor scaling of quadratic
behavior is only a problem for large needles, while the constant
overhead of factorization is measurably larger in proportion to the rest
of the work done on short needles.

If you are letting the user search for arbitrarily long needles, then an
O(n) scale may be worth the speed penalty on shorter needles, or the
complexity of a hybrid approach (again, I think glibc may have better
hybrid approach than gnulib at the moment).  But as this is an HMP
interface, the user has to type the search string, and is unlikely to be
typing long needles, so it is justifiable to just ditch the complexity
of a hybrid approach and use ONLY the naive O(n^2) algorithm, because as
I argued above, it is still observable only as O(n) for a bounded-size
needle.

> 
>>
>> [Personal note: I'm still tickled pink every time I see yet another
>> project adopt my O(n) code - I didn't come up with the algorithm, but
>> merely scraped it from public research, and was shocked to note how many
>> libc still had O(n^2) code at the time.  But it is still gratifying to
>> see that my implementation is getting such wide adoption]
>>
> 
> Well your code and its derived versions have been tried and tested a lot, so picking it up just makes sense to me, to avoid getting into the same corner cases and issues that your implementation already had to go through. And I am a lazy person of course!

You do have a point here - if we are going to reuse code, then reusing a
debugged implementation rather than writing from scratch is the way to
go.  But my point remains that the complexity of a reused Two-Way
algorithm may not be necessary, and the 3-line naive algorithm is
trivial to review.  What's more, it is also arguable that we only use
the naive algorithm on less-than-stellar platforms that lack memmem
natively (well, we may also use less-than-stellar performance if the
native memmem is not O(n)) - but who is going to be using the HMP
command on such a platform?  And maybe those platforms will finally be
convinced to add a decent memmem if they realize that we are slow on
their platform but not on other platforms, precisely because we are
working around their lack of memmem by intentionally using a slow but
trivial replacement.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [RFC v5 1/2] util: add memmem replacement function
  2015-05-15 16:01           ` Eric Blake
@ 2015-05-18  8:36             ` Claudio Fontana
  0 siblings, 0 replies; 9+ messages in thread
From: Claudio Fontana @ 2015-05-18  8:36 UTC (permalink / raw)
  To: Eric Blake
  Cc: Peter Maydell, Claudio Fontana, QEMU Developers, Luiz Capitulino,
	Gonglei, Paolo Bonzini, karl

Hello Eric,

On 15 May 2015 at 18:01, Eric Blake <eblake@redhat.com> wrote:
> On 05/15/2015 09:13 AM, Claudio Fontana wrote:
>
>>> Or back to the original question - why are we worrying about the O(n)
>>> memmem implementation when the naive O(n^2) is MUCH shorter and easier
>>> to understand, and where the scaling penalty is only apparent on really
>>> long corner case strings?  Is our use of memmem going to hit the corner
>>> cases where scaling matters?
>>
>> For the specific use case here (memory search functionality),
>> the haystack is normally the factor which really can scale up a lot.
>
> Remember, when searching, the worst-case penalty is determined by a
> combination of the size of the haystack (H) and the size of the needle
> (I'll choose M to represent that, to avoid confusion with the generic n
> in Big-O notation).  The most brain-dead loop (for each byte in
> haystack, see if the needle matches) is H*M operations; this is
> noticeable as quadratic scaling (and even then, only for certain
> matching patterns, such as determining if a needle of all 'a' except for
> a final 'b' occurs in a haystack of all 'a') only if both needle and
> haystack can grow arbitrarily large: O(H*M) == O(n^2).  Where my code
> shines is that it breaks the operation into two parts - a factorization
> of the needle O(M) == O(n), then a linear search using that factored
> needle O(H) == O(n), for an overall cost O(H+M) == O(n). But with the
> naive algorithm, if the needle is capped to a fixed size, then you have
> O(constant*H) == O(H) == O(n).
>
> Meanwhile, remember that Big-O notation is only about scaling effects,
> but that there are still constant factors to be considered - an O(n)
> algorithm that requires 3 comparisons per byte of the haystack will
> always be slower than an O(n) algorithm that only needs 2 comparisons
> per byte of the haystack, even though both algorithms scale linearly.
>
>>
>> I would expect the needle to be usually smallish (though for string search sometimes the needle could probably arrive at the threshold of 32).
>>
>> When scanning a very large memory range, considering a binary string with roughly length around 32, do you think that the non-trivial implementation could bring some noticeable benefits?
>>
>> If it's below 32, do we get any benefits compared to the trivial implementation?
>
> In fact, one of the glibc optimizations was realizing that my two-way
> algorithm code is LOUSY at short needles.  It has a high relative
> overhead for finding the proper factorization of a short needle, before
> it can even start searching.  A hybrid approach (brute force a short
> needle, and only bother with factorizing a long needle, or even after a
> heuristic of a minimum number of comparisons is first met) is ideal,
> because of the observation above that the poor scaling of quadratic
> behavior is only a problem for large needles, while the constant
> overhead of factorization is measurably larger in proportion to the rest
> of the work done on short needles.
>
> If you are letting the user search for arbitrarily long needles, then an
> O(n) scale may be worth the speed penalty on shorter needles, or the
> complexity of a hybrid approach (again, I think glibc may have better
> hybrid approach than gnulib at the moment).  But as this is an HMP
> interface, the user has to type the search string, and is unlikely to be
> typing long needles, so it is justifiable to just ditch the complexity
> of a hybrid approach and use ONLY the naive O(n^2) algorithm, because as
> I argued above, it is still observable only as O(n) for a bounded-size
> needle.
>
>>
>>>
>>> [Personal note: I'm still tickled pink every time I see yet another
>>> project adopt my O(n) code - I didn't come up with the algorithm, but
>>> merely scraped it from public research, and was shocked to note how many
>>> libc still had O(n^2) code at the time.  But it is still gratifying to
>>> see that my implementation is getting such wide adoption]
>>>
>>
>> Well your code and its derived versions have been tried and tested a lot, so picking it up just makes sense to me, to avoid getting into the same corner cases and issues that your implementation already had to go through. And I am a lazy person of course!
>
> You do have a point here - if we are going to reuse code, then reusing a
> debugged implementation rather than writing from scratch is the way to
> go.  But my point remains that the complexity of a reused Two-Way
> algorithm may not be necessary, and the 3-line naive algorithm is
> trivial to review.
>
> What's more, it is also arguable that we only use
> the naive algorithm on less-than-stellar platforms that lack memmem
> natively (well, we may also use less-than-stellar performance if the
> native memmem is not O(n)) - but who is going to be using the HMP
> command on such a platform?  And maybe those platforms will finally be
> convinced to add a decent memmem if they realize that we are slow on
> their platform but not on other platforms, precisely because we are
> working around their lack of memmem by intentionally using a slow but
> trivial replacement.

Good points, I will respin with a trivial implementation shortly.

Thanks!

Claudio

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-05-18  8:36 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-15 12:25 [Qemu-devel] [RFC v5 0/2] monitor: add memory search commands s, sp hw.claudio
2015-05-15 12:25 ` [Qemu-devel] [RFC v5 1/2] util: add memmem replacement function hw.claudio
2015-05-15 13:57   ` Claudio Fontana
2015-05-15 14:12     ` Paolo Bonzini
2015-05-15 14:56       ` Eric Blake
2015-05-15 15:13         ` Claudio Fontana
2015-05-15 16:01           ` Eric Blake
2015-05-18  8:36             ` Claudio Fontana
2015-05-15 12:25 ` [Qemu-devel] [RFC v5 2/2] monitor: add memory search commands s, sp hw.claudio

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).