On 9/11/18 5:08 PM, Alexander Monakov wrote:
> On Tue, 11 Sep 2018, Martin Liška wrote:
>> I've discussed the topic with Alexander on the Cauldron and we hoped
>> that the issue with unique __gcov_root can be handled with DECL_COMMON in
>> each DSO.
>> Apparently this approach does not work as all DSOs in an executable have
>> eventually
>> just a single instance of the __gcov_root, which is bad.
>
> No, that happens only if they have default visibility. Emitting them with
> "hidden" visibility solves this.
Ah, ok, thanks. So theoretically that way should work.
>
>> So that I returned back to catch the root of the failure. It shows that even
>> with
>> -Wl,--dynamic-list-data __gcov_indirect_call_callee point to a different
>> variable
>> among the DSOs. The reason is that the variable is TLS:
>
> (no, that the variable is TLS is not causing the bug; the issue is libraries
> carry public definitions of just one or both variables depending on if they
> have indirect calls or not, and the library state becomes "diverged" when
> one variable gets interposed while the other does not)
I see, I'm attaching patch that does that. I can confirm your test-case works
fine w/o -Wl,--dynamic-list-data.
I'm wondering if it will work as well with dlopen/dlsym machinery? Or now
the linker flag will be needed?
>
>> That said I would like to go this direction. I would be happy to receive
>> a feedback. Then I'll test the patch.
>
> Hm, this TLS change is attacking the issue from a wrong direction. What I
> suggested on the Cauldron as a simple and backportable way of solving this
> is to consolidate the two TLS variables into one TLS struct, which is then
> either interposed or not as a whole.
Got it, current I prefer the solution.
Martin
>
> Alexander
>
>From ce708b5d3f3742b3e8b930aa6eb74181273efd9f Mon Sep 17 00:00:00 2001
From: marxin <mli...@suse.cz>
Date: Wed, 12 Sep 2018 11:24:59 +0200
Subject: [PATCH] Patch candidate.
---
gcc/tree-profile.c | 88 +++++++++++++++++++++------------------
libgcc/libgcov-profiler.c | 25 ++++-------
libgcc/libgcov.h | 9 ++++
3 files changed, 64 insertions(+), 58 deletions(-)
diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
index f96bd4b9704..e2df8f005be 100644
--- a/gcc/tree-profile.c
+++ b/gcc/tree-profile.c
@@ -53,6 +53,8 @@ along with GCC; see the file COPYING3. If not see
#include "stringpool.h"
#include "attribs.h"
#include "tree-pretty-print.h"
+#include "langhooks.h"
+#include "stor-layout.h"
static GTY(()) tree gcov_type_node;
static GTY(()) tree tree_interval_profiler_fn;
@@ -64,8 +66,9 @@ static GTY(()) tree tree_ior_profiler_fn;
static GTY(()) tree tree_time_profiler_counter;
-static GTY(()) tree ic_void_ptr_var;
-static GTY(()) tree ic_gcov_type_ptr_var;
+static GTY(()) tree ic_tuple_var;
+static GTY(()) tree ic_tuple_counters_field;
+static GTY(()) tree ic_tuple_callee_field;
static GTY(()) tree ptr_void;
/* Do initialization work for the edge profiler. */
@@ -81,38 +84,34 @@ init_ic_make_global_vars (void)
tree gcov_type_ptr;
ptr_void = build_pointer_type (void_type_node);
+ gcov_type_ptr = build_pointer_type (get_gcov_type ());
- ic_void_ptr_var
- = build_decl (UNKNOWN_LOCATION, VAR_DECL,
- get_identifier (
- (PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) ?
- "__gcov_indirect_call_topn_callee" :
- "__gcov_indirect_call_callee")),
- ptr_void);
- TREE_PUBLIC (ic_void_ptr_var) = 1;
- DECL_EXTERNAL (ic_void_ptr_var) = 1;
- TREE_STATIC (ic_void_ptr_var) = 1;
- DECL_ARTIFICIAL (ic_void_ptr_var) = 1;
- DECL_INITIAL (ic_void_ptr_var) = NULL;
- if (targetm.have_tls)
- set_decl_tls_model (ic_void_ptr_var, decl_default_tls_model (ic_void_ptr_var));
+ tree tuple_type = lang_hooks.types.make_type (RECORD_TYPE);
- gcov_type_ptr = build_pointer_type (get_gcov_type ());
+ /* callee */
+ ic_tuple_callee_field = build_decl (BUILTINS_LOCATION, FIELD_DECL, NULL_TREE, ptr_void);
- ic_gcov_type_ptr_var
+ /* counters */
+ ic_tuple_counters_field = build_decl (BUILTINS_LOCATION, FIELD_DECL, NULL_TREE, gcov_type_ptr);
+ DECL_CHAIN (ic_tuple_counters_field) = ic_tuple_callee_field;
+
+ finish_builtin_struct (tuple_type, "indirect_call_tuple",
+ ic_tuple_counters_field, NULL_TREE);
+
+ ic_tuple_var
= build_decl (UNKNOWN_LOCATION, VAR_DECL,
get_identifier (
(PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE) ?
- "__gcov_indirect_call_topn_counters" :
- "__gcov_indirect_call_counters")),
- gcov_type_ptr);
- TREE_PUBLIC (ic_gcov_type_ptr_var) = 1;
- DECL_EXTERNAL (ic_gcov_type_ptr_var) = 1;
- TREE_STATIC (ic_gcov_type_ptr_var) = 1;
- DECL_ARTIFICIAL (ic_gcov_type_ptr_var) = 1;
- DECL_INITIAL (ic_gcov_type_ptr_var) = NULL;
+ "__gcov_indirect_call_topn" :
+ "__gcov_indirect_call")),
+ tuple_type);
+ TREE_PUBLIC (ic_tuple_var) = 1;
+ DECL_EXTERNAL (ic_tuple_var) = 1;
+ TREE_STATIC (ic_tuple_var) = 1;
+ DECL_ARTIFICIAL (ic_tuple_var) = 1;
+ DECL_INITIAL (ic_tuple_var) = NULL;
if (targetm.have_tls)
- set_decl_tls_model (ic_gcov_type_ptr_var, decl_default_tls_model (ic_gcov_type_ptr_var));
+ set_decl_tls_model (ic_tuple_var, decl_default_tls_model (tuple_type));
}
/* Create the type and function decls for the interface with gcov. */
@@ -371,8 +370,7 @@ gimple_gen_one_value_profiler (histogram_value value, unsigned tag, unsigned bas
void
gimple_gen_ic_profiler (histogram_value value, unsigned tag, unsigned base)
{
- tree tmp1;
- gassign *stmt1, *stmt2, *stmt3;
+ gassign *stmt1, *stmt2;
gimple *stmt = value->hvalue.stmt;
gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
tree ref_ptr = tree_coverage_counter_addr (tag, base);
@@ -388,26 +386,30 @@ gimple_gen_ic_profiler (histogram_value value, unsigned tag, unsigned base)
/* Insert code:
- stmt1: __gcov_indirect_call_counters = get_relevant_counter_ptr ();
- stmt2: tmp1 = (void *) (indirect call argument value)
- stmt3: __gcov_indirect_call_callee = tmp1;
+ stmt1: __gcov_indirect_call.counters = get_relevant_counter_ptr ();
+ stmt2: __gcov_indirect_call.callee = indirect call argument value;
Example:
f_1 = foo;
- __gcov_indirect_call_counters = &__gcov4.main[0];
- PROF_9 = f_1;
- __gcov_indirect_call_callee = PROF_9;
+ __gcov_indirect_call.counters = &__gcov4.main[0];
+ __gcov_indirect_call_callee = f_1;
_4 = f_1 ();
*/
- stmt1 = gimple_build_assign (ic_gcov_type_ptr_var, ref_ptr);
- tmp1 = make_temp_ssa_name (ptr_void, NULL, "PROF");
- stmt2 = gimple_build_assign (tmp1, unshare_expr (value->hvalue.value));
- stmt3 = gimple_build_assign (ic_void_ptr_var, gimple_assign_lhs (stmt2));
+ tree ptr_void = build_pointer_type (void_type_node);
+ tree gcov_type_ptr = build_pointer_type (get_gcov_type ());
+
+ tree counter_ref = build3 (COMPONENT_REF, gcov_type_ptr,
+ ic_tuple_var, ic_tuple_counters_field, NULL_TREE);
+
+ stmt1 = gimple_build_assign (counter_ref, ref_ptr);
+
+ tree callee_ref = build3 (COMPONENT_REF, ptr_void,
+ ic_tuple_var, ic_tuple_callee_field, NULL_TREE);
+ stmt2 = gimple_build_assign (callee_ref, unshare_expr (value->hvalue.value));
gsi_insert_before (&gsi, stmt1, GSI_SAME_STMT);
gsi_insert_before (&gsi, stmt2, GSI_SAME_STMT);
- gsi_insert_before (&gsi, stmt3, GSI_SAME_STMT);
}
@@ -461,7 +463,11 @@ gimple_gen_ic_func_profiler (void)
gimple_stmt_iterator gsi = gsi_start_bb (cond_bb);
void0 = build_int_cst (build_pointer_type (void_type_node), 0);
- tree ref = force_gimple_operand_gsi (&gsi, ic_void_ptr_var, true, NULL_TREE,
+ tree ptr_void = build_pointer_type (void_type_node);
+ tree callee_ref = build3 (COMPONENT_REF, ptr_void,
+ ic_tuple_var, ic_tuple_callee_field, NULL_TREE);
+
+ tree ref = force_gimple_operand_gsi (&gsi, callee_ref, true, NULL_TREE,
true, GSI_SAME_STMT);
gcond *cond = gimple_build_cond (NE_EXPR, ref,
diff --git a/libgcc/libgcov-profiler.c b/libgcc/libgcov-profiler.c
index 7e208d75d86..c3e05db33eb 100644
--- a/libgcc/libgcov-profiler.c
+++ b/libgcc/libgcov-profiler.c
@@ -271,12 +271,7 @@ __gcov_topn_value_profiler_body (gcov_type *counters, gcov_type value)
#if defined(HAVE_CC_TLS) && !defined (USE_EMUTLS)
__thread
#endif
-gcov_type *__gcov_indirect_call_topn_counters ATTRIBUTE_HIDDEN;
-
-#if defined(HAVE_CC_TLS) && !defined (USE_EMUTLS)
-__thread
-#endif
-void *__gcov_indirect_call_topn_callee ATTRIBUTE_HIDDEN;
+struct indirect_call_tuple __gcov_indirect_call_topn;
#ifdef TARGET_VTABLE_USES_DESCRIPTORS
#define VTABLE_USES_DESCRIPTORS 1
@@ -290,14 +285,14 @@ void *__gcov_indirect_call_topn_callee ATTRIBUTE_HIDDEN;
void
__gcov_indirect_call_topn_profiler (gcov_type value, void* cur_func)
{
- void *callee_func = __gcov_indirect_call_topn_callee;
+ void *callee_func = __gcov_indirect_call_topn.callee;
/* If the C++ virtual tables contain function descriptors then one
function may have multiple descriptors and we need to dereference
the descriptors to see if they point to the same function. */
if (cur_func == callee_func
|| (VTABLE_USES_DESCRIPTORS && callee_func
&& *(void **) cur_func == *(void **) callee_func))
- __gcov_topn_value_profiler_body (__gcov_indirect_call_topn_counters, value);
+ __gcov_topn_value_profiler_body (__gcov_indirect_call_topn.counters, value);
}
#endif
@@ -311,11 +306,7 @@ __gcov_indirect_call_topn_profiler (gcov_type value, void* cur_func)
#if defined(HAVE_CC_TLS) && !defined (USE_EMUTLS)
__thread
#endif
-void * __gcov_indirect_call_callee;
-#if defined(HAVE_CC_TLS) && !defined (USE_EMUTLS)
-__thread
-#endif
-gcov_type * __gcov_indirect_call_counters;
+struct indirect_call_tuple __gcov_indirect_call;
/* By default, the C++ compiler will use function addresses in the
vtable entries. Setting TARGET_VTABLE_USES_DESCRIPTORS to nonzero
@@ -332,12 +323,12 @@ __gcov_indirect_call_profiler_v2 (gcov_type value, void* cur_func)
/* If the C++ virtual tables contain function descriptors then one
function may have multiple descriptors and we need to dereference
the descriptors to see if they point to the same function. */
- if (cur_func == __gcov_indirect_call_callee
+ if (cur_func == __gcov_indirect_call.callee
|| (__LIBGCC_VTABLE_USES_DESCRIPTORS__
- && *(void **) cur_func == *(void **) __gcov_indirect_call_callee))
- __gcov_one_value_profiler_body (__gcov_indirect_call_counters, value, 0);
+ && *(void **) cur_func == *(void **) __gcov_indirect_call.callee))
+ __gcov_one_value_profiler_body (__gcov_indirect_call.counters, value, 0);
- __gcov_indirect_call_callee = NULL;
+ __gcov_indirect_call.callee = NULL;
}
#endif
diff --git a/libgcc/libgcov.h b/libgcc/libgcov.h
index 21422873cf2..ee05a68d2b2 100644
--- a/libgcc/libgcov.h
+++ b/libgcc/libgcov.h
@@ -226,6 +226,15 @@ struct gcov_master
gcov_unsigned_t version;
struct gcov_root *root;
};
+
+struct indirect_call_tuple
+{
+ /* Callee function. */
+ void *callee;
+
+ /* Pointer to counters. */
+ gcov_type *counters;
+};
/* Exactly one of these will be active in the process. */
extern struct gcov_master __gcov_master;
--
2.18.0
