From 6db53b26e6c3d973295df51bb1934d5b42fb6ebe Mon Sep 17 00:00:00 2001
From: Cyrille Bagard <nocbos@gmail.com>
Date: Fri, 6 Jan 2017 05:12:24 +0100
Subject: Checked if a symbol exists right before adding it to the symbol list.

---
 ChangeLog               | 13 +++++++++++++
 src/arch/arm/v7/fetch.c |  6 ++++--
 src/common/sort.c       |  2 +-
 src/format/format.c     | 51 +++++++++++++++++++++++++++++++++++--------------
 src/format/format.h     |  2 +-
 src/format/symbol.h     | 28 +++++++++++++--------------
 6 files changed, 70 insertions(+), 32 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index cb87097..729c4ad 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+17-01-06  Cyrille Bagard <nocbos@gmail.com>
+
+	* src/arch/arm/v7/fetch.c:
+	* src/common/sort.c:
+	Update code.
+
+	* src/format/format.c:
+	* src/format/format.h:
+	Check if a symbol exists right before adding it to the symbol list.
+
+	* src/format/symbol.h:
+	Update code.
+
 17-01-04  Cyrille Bagard <nocbos@gmail.com>
 
 	* src/glibext/gbufferline.c:
diff --git a/src/arch/arm/v7/fetch.c b/src/arch/arm/v7/fetch.c
index c0004b4..15707e3 100644
--- a/src/arch/arm/v7/fetch.c
+++ b/src/arch/arm/v7/fetch.c
@@ -363,6 +363,7 @@ void help_fetching_with_instruction_ldr_literal_with_orig(GArchInstruction *inst
     GArchInstruction *sym_instr;            /* Instruction de symbole      */
     GBinSymbol *symbol;                     /* Nouveau symbole construit   */
     GDbComment *comment;                    /* Définition de commentaire   */
+    bool added;                             /* Bilan de l'insertion        */
     GArchOperand *new;                      /* Instruction de ciblage      */
 
     /* Récupération de l'adresse visée par le chargement */
@@ -439,13 +440,14 @@ void help_fetching_with_instruction_ldr_literal_with_orig(GArchInstruction *inst
     vmpa2_virt_to_string(get_mrange_addr(range), MDS_32_BITS, loc, NULL);
     snprintf(name, name_len, _("Value used @ %s"), loc);
 
-    ADD_RAW_AS_SYM(G_BIN_FORMAT(format), symbol, sym_instr, comment, name);
+    added = ADD_RAW_AS_SYM(G_BIN_FORMAT(format), symbol, sym_instr, comment, name);
 
     free(name);
 
 
 
-    g_proc_context_push_new_symbol_at(G_PROC_CONTEXT(context), &sym_addr);
+    if (added)
+        g_proc_context_push_new_symbol_at(G_PROC_CONTEXT(context), &sym_addr);
 
 
 
diff --git a/src/common/sort.c b/src/common/sort.c
index bd97cef..750034b 100644
--- a/src/common/sort.c
+++ b/src/common/sort.c
@@ -194,7 +194,7 @@ void *qinsert(void *base, size_t *nmemb, size_t size, __compar_fn_t compar, void
 
 #ifndef NDEBUG
     found = bsearch_index(new, base, *nmemb, size, compar, &index);
-    //assert(!found); FIXME (symboles)
+    //assert(!found); FIXME (portions)
 #else
     bsearch_index(new, base, *nmemb, size, compar, &index);
 #endif
diff --git a/src/format/format.c b/src/format/format.c
index 6f2c03e..be522cb 100644
--- a/src/format/format.c
+++ b/src/format/format.c
@@ -286,12 +286,14 @@ void g_binary_format_setup_disassembling_context(GBinFormat *format, GProcContex
 *                                                                             *
 ******************************************************************************/
 
-void g_binary_format_add_symbol(GBinFormat *format, GBinSymbol *symbol)
+bool g_binary_format_add_symbol(GBinFormat *format, GBinSymbol *symbol)
 {
+    bool result;                            /* Statut d'ajout à retourner  */
 #ifndef NDEBUG
     const mrange_t *range;                  /* Couverture du symbole       */
     const vmpa2t *addr;                     /* Emplacement du symbole      */
 #endif
+    size_t index;                           /* Indice du point d'insertion */
     GBinRoutine *routine;                   /* Nouvelle routine à insérer  */
 
     /**
@@ -309,33 +311,54 @@ void g_binary_format_add_symbol(GBinFormat *format, GBinSymbol *symbol)
     range = g_binary_symbol_get_range(symbol);
     addr = get_mrange_addr(range);
 
-    assert(get_phy_addr(addr) != VMPA_NO_PHYSICAL && get_virt_addr(addr) != VMPA_NO_VIRTUAL);
+    assert(!is_invalid_vmpa(addr));
 #endif
 
     g_rw_lock_writer_lock(&format->syms_lock);
 
-    format->symbols = qinsert(format->symbols, &format->symbols_count,
-                              sizeof(GBinSymbol *), (__compar_fn_t)g_binary_symbol_cmp, &symbol);
+    /**
+     * Avec tous les traitements parallèles, il est possible que plusieurs chemins d'exécution
+     * amènent à la création d'un même symbole.
+     *
+     * Plutôt que de verrouiller la liste des symboles en amont (et donc assez longtemps)
+     * pour faire une vérification avant construction puis ajout, on préfère limiter
+     * l'état figé à cette seule fonction, quitte à annuler le travail fourni pour la
+     * construction du symbole dans les cas peu fréquents où le symbole était déjà en place.
+     */
 
-    switch (g_binary_symbol_get_target_type(symbol))
+    result = bsearch_index(&symbol, format->symbols, format->symbols_count,
+                           sizeof(GBinSymbol *), (__compar_fn_t)g_binary_symbol_cmp, &index);
+
+    if (!result)
     {
-        case STP_ROUTINE:
-        case STP_ENTRY_POINT:
+        format->symbols = _qinsert(format->symbols, &format->symbols_count,
+                                   sizeof(GBinSymbol *), &symbol, index);
 
-            routine = g_binary_symbol_get_routine(symbol);
+        switch (g_binary_symbol_get_target_type(symbol))
+        {
+            case STP_ROUTINE:
+            case STP_ENTRY_POINT:
 
-            format->routines = qinsert(format->routines, &format->routines_count,
-                                       sizeof(GBinRoutine *), (__compar_fn_t)g_binary_routine_compare,
-                                       &routine);
-            break;
+                routine = g_binary_symbol_get_routine(symbol);
 
-        default:
-            break;
+                format->routines = qinsert(format->routines, &format->routines_count,
+                                           sizeof(GBinRoutine *), (__compar_fn_t)g_binary_routine_compare,
+                                           &routine);
+                break;
+
+            default:
+                break;
+
+        }
 
     }
+    else
+        g_object_unref(G_OBJECT(symbol));
 
     g_rw_lock_writer_unlock(&format->syms_lock);
 
+    return result;
+
 }
 
 
diff --git a/src/format/format.h b/src/format/format.h
index 81f1776..d97f542 100644
--- a/src/format/format.h
+++ b/src/format/format.h
@@ -68,7 +68,7 @@ void g_binary_format_register_code_point(GBinFormat *, virt_t, bool);
 void g_binary_format_setup_disassembling_context(GBinFormat *, GProcContext *);
 
 /* Ajoute un symbole à la collection du format binaire. */
-void g_binary_format_add_symbol(GBinFormat *, GBinSymbol *);
+bool g_binary_format_add_symbol(GBinFormat *, GBinSymbol *);
 
 /* Retire un symbole de la collection du format binaire. */
 void g_binary_format_remove_symbol(GBinFormat *, GBinSymbol *);
diff --git a/src/format/symbol.h b/src/format/symbol.h
index 67696f4..4a48cf8 100644
--- a/src/format/symbol.h
+++ b/src/format/symbol.h
@@ -144,8 +144,8 @@ GDbComment *g_binary_symbol_get_comment(const GBinSymbol *);
     while (0)
 
 #define ADD_RAW_AS_SYM(_fmt, _sym, _ins, _cmt, _txt)                        \
-    do                                                                      \
-    {                                                                       \
+    ({                                                                      \
+        bool __result;                                                      \
         const vmpa2t *__addr;                                               \
         __addr = get_mrange_addr(g_arch_instruction_get_range(_ins));       \
         _cmt = g_db_comment_new_inlined(__addr, BLF_HAS_CODE, _txt, false); \
@@ -153,18 +153,18 @@ GDbComment *g_binary_symbol_get_comment(const GBinSymbol *);
         _sym = g_binary_symbol_new(STP_DATA);                               \
         g_binary_symbol_attach_instruction(_sym, _ins);                     \
         g_binary_symbol_set_comment(_sym, _cmt);                            \
-        g_binary_format_add_symbol(G_BIN_FORMAT(_fmt), _sym);               \
-    }                                                                       \
-    while (0)
-
-#define ADD_STR_AS_SYM(_fmt, _sym, _ins)                        \
-    do                                                          \
-    {                                                           \
-        _sym = g_binary_symbol_new(STP_RO_STRING);              \
-        g_binary_symbol_attach_instruction(_sym, _ins);         \
-        g_binary_format_add_symbol(G_BIN_FORMAT(_fmt), _sym);   \
-    }                                                           \
-    while (0)
+        __result = g_binary_format_add_symbol(G_BIN_FORMAT(_fmt), _sym);    \
+        __result;                                                           \
+    })
+
+#define ADD_STR_AS_SYM(_fmt, _sym, _ins)                                    \
+    ({                                                                      \
+        bool __result;                                                      \
+        _sym = g_binary_symbol_new(STP_RO_STRING);                          \
+        g_binary_symbol_attach_instruction(_sym, _ins);                     \
+        __result = g_binary_format_add_symbol(G_BIN_FORMAT(_fmt), _sym);    \
+        __result;                                                           \
+    })
 
 
 
-- 
cgit v0.11.2-87-g4458