On 08/08/16 09:59, Martin Liška wrote:
Hello.
This patch is follow-up of the series where I introduce a set of counter update
function that are thread-safe. I originally thought that majority of profile
corruptions are
caused by non-atomic updated of CFG (-fprofile-arc). But there are multiple
counters that compare
it's count to a number of execution of a basic block:
blake2s.cpp:150:40: error: corrupted value profile: value profile counter
(11301120 out of 11314388) inconsistent with basic-block count (11555117)
memcpy( S->buf + left, in, fill ); // Fill buffer
This can be seen for unrar binary: PR58306. I'm also adding a simple test-case
which reveals the inconsistency: val-profiler-threads-1.c.
I've been running regression tests, ready to install after it finishes?
+ fn_name = concat ("__gcov_interval_profiler", fn_suffix, NULL);
+ tree_interval_profiler_fn = build_fn_decl (fn_name,
+ interval_profiler_fn_type);
I like this idiom, but doesn't 'concat' call for a following 'free'?
+__gcov_pow2_profiler_atomic (gcov_type *counters, gcov_type value)
+{
+ if (value & (value - 1))
+ __atomic_fetch_add (&counters[0], 1, MEMMODEL_RELAXED);
This seems to think '0' is a power of 2. (I suspect a bug in the existing
code, not something you've introduced)
-__gcov_one_value_profiler_body (gcov_type *counters, gcov_type value)
+__gcov_one_value_profiler_body (gcov_type *counters, gcov_type value,
+ int use_atomic)
{
if (value == counters[0])
This function should be commented along the lines of the email discussion we had
last week. the 'atomic' param doesn't make it completely thread safe.
/* Counter for first visit of each function. */
-static gcov_type function_counter;
+gcov_type function_counter;
why is this no longer static? If it must be globally visible, it'll need a
suitable rename. (perhaps it's simpler to put the 2(?) fns that access it into
a single object file?)
nathan
