Re: Sim hangs on new target at dup_arg_p() in infinite loop.

[Alexey Makhalov <makhaloff@gmail.com>]

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

Tom, thanks for advice.
Now it works fine.


Index: sim/common/sim-options.c
===================================================================
RCS file: /cvs/src/src/sim/common/sim-options.c,v
retrieving revision 1.31
diff -u -p -r1.31 sim-options.c
--- sim/common/sim-options.c 3 Sep 2013 20:45:08 -0000 1.31
+++ sim/common/sim-options.c 20 Sep 2013 04:57:09 -0000
@@ -35,6 +35,7 @@ along with this program.  If not, see <h
 #include "sim-assert.h"

 #include "bfd.h"
+#include "hashtab.h"

 /* Add a set of options to the simulator.
    TABLE is an array of OPTIONS terminated by a NULL `opt.name' entry.
@@ -485,37 +486,40 @@ standard_install (SIM_DESC sd)
   return SIM_RC_OK;
 }

+/* A simple comparison function with opposite semantics to strcmp.  */
+
+static int
+streq (const char *lhs, const char *rhs)
+{
+  return !strcmp (lhs, rhs);
+}
+
 /* Return non-zero if arg is a duplicate argument.
    If ARG is NULL, initialize.  */

-#define ARG_HASH_SIZE 97
-#define ARG_HASH(a) ((256 * (unsigned char) a[0] + (unsigned char)
a[1]) % ARG_HASH_SIZE)
+#define ARG_HASH_SIZE 256

 static int
 dup_arg_p (const char *arg)
 {
-  int hash;
-  static const char **arg_table = NULL;
+  void **slot;
+  static htab_t arg_table = NULL;

   if (arg == NULL)
     {
       if (arg_table == NULL)
- arg_table = (const char **) xmalloc (ARG_HASH_SIZE * sizeof (char *));
-      memset (arg_table, 0, ARG_HASH_SIZE * sizeof (char *));
+        arg_table = htab_create (ARG_HASH_SIZE, htab_hash_string,
+               (int (*)(const void *, const void *)) streq, free);
+      else
+        htab_empty (arg_table);
       return 0;
     }

-  hash = ARG_HASH (arg);
-  while (arg_table[hash] != NULL)
-    {
-      if (strcmp (arg, arg_table[hash]) == 0)
- return 1;
-      /* We assume there won't be more than ARG_HASH_SIZE arguments so we
- don't check if the table is full.  */
-      if (++hash == ARG_HASH_SIZE)
- hash = 0;
-    }
-  arg_table[hash] = arg;
+  slot = htab_find_slot (arg_table, arg, INSERT);
+  if (*slot != NULL)
+    return 1;
+
+  *slot = strdup(arg);
   return 0;
 }

On Tue, Sep 17, 2013 at 2:52 PM, Alexey Makhalov <makhaloff@gmail.com> wrote:
> I'll try.
>
>
> On Tue, Sep 17, 2013 at 10:29 AM, Tom Tromey <tromey@redhat.com> wrote:
>>
>> >>>>> "Alexey" == Alexey Makhalov <makhaloff@gmail.com> writes:
>>
>> Alexey> I've added checking for hash table overflow. It's better to have
>> Alexey> internal error message instead of looping.
>>
>> Alexey> 2013-09-09 Alexey Makhalov <makhaloff@gmail.com>
>> Alexey>         * sim-options.c (dup_arg_p) Check for hash table overflow.
>>
>> Is it possible to just use the libiberty hashtab code and not have it
>> ever overflow?
>>
>> Tom
>
>

[-- Attachment #2: htab.patch --]
[-- Type: application/octet-stream, Size: 2052 bytes --]

Index: sim/common/sim-options.c
===================================================================
RCS file: /cvs/src/src/sim/common/sim-options.c,v
retrieving revision 1.31
diff -u -p -r1.31 sim-options.c
--- sim/common/sim-options.c	3 Sep 2013 20:45:08 -0000	1.31
+++ sim/common/sim-options.c	20 Sep 2013 04:57:09 -0000
@@ -35,6 +35,7 @@ along with this program.  If not, see <h
 #include "sim-assert.h"
 
 #include "bfd.h"
+#include "hashtab.h"
 
 /* Add a set of options to the simulator.
    TABLE is an array of OPTIONS terminated by a NULL `opt.name' entry.
@@ -485,37 +486,40 @@ standard_install (SIM_DESC sd)
   return SIM_RC_OK;
 }
 
+/* A simple comparison function with opposite semantics to strcmp.  */
+
+static int
+streq (const char *lhs, const char *rhs)
+{
+  return !strcmp (lhs, rhs);
+}
+
 /* Return non-zero if arg is a duplicate argument.
    If ARG is NULL, initialize.  */
 
-#define ARG_HASH_SIZE 97
-#define ARG_HASH(a) ((256 * (unsigned char) a[0] + (unsigned char) a[1]) % ARG_HASH_SIZE)
+#define ARG_HASH_SIZE 256
 
 static int
 dup_arg_p (const char *arg)
 {
-  int hash;
-  static const char **arg_table = NULL;
+  void **slot;
+  static htab_t arg_table = NULL;
 
   if (arg == NULL)
     {
       if (arg_table == NULL)
-	arg_table = (const char **) xmalloc (ARG_HASH_SIZE * sizeof (char *));
-      memset (arg_table, 0, ARG_HASH_SIZE * sizeof (char *));
+        arg_table = htab_create (ARG_HASH_SIZE, htab_hash_string, 
+               (int (*)(const void *, const void *)) streq, free);
+      else
+        htab_empty (arg_table);
       return 0;
     }
 
-  hash = ARG_HASH (arg);
-  while (arg_table[hash] != NULL)
-    {
-      if (strcmp (arg, arg_table[hash]) == 0)
-	return 1;
-      /* We assume there won't be more than ARG_HASH_SIZE arguments so we
-	 don't check if the table is full.  */
-      if (++hash == ARG_HASH_SIZE)
-	hash = 0;
-    }
-  arg_table[hash] = arg;
+  slot = htab_find_slot (arg_table, arg, INSERT);
+  if (*slot != NULL)
+    return 1;
+
+  *slot = strdup(arg);
   return 0;
 }