* [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107).
@ 2018-08-29 8:31 Martin Liška
2018-08-29 9:21 ` Richard Biener
0 siblings, 1 reply; 19+ messages in thread
From: Martin Liška @ 2018-08-29 8:31 UTC (permalink / raw)
To: gcc-patches; +Cc: Joseph Myers, Michael Matz, Alexander Monakov
[-- Attachment #1: Type: text/plain, Size: 2127 bytes --]
Hello.
Moving the thread from gcc ML into gcc-patches. That's first implementation
of shared libgcov library. Currently inclusion of t-libgcov is added only
to *-linux targets. Is it fine to add to all configurations that already
include 't-slibgcc t-slibgcc-elf-ver'?
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
Martin
libgcc/ChangeLog:
2018-08-28 Martin Liska <mliska@suse.cz>
PR gcov-profile/84107
* Makefile.in: Build libgcov.so objects separately,
add rule for libgcov.map and how the library
is linked.
* config.host: Add t-libgcov and t-libgcov-elf-ver
to *-linux targets.
* config/t-libgcov: New file.
* config/t-libgcov-elf-ver: New file.
* libgcov-driver.c: Declare function if new L_gcov_shared
is defined.
* libgcov-interface.c: Likewise.
* libgcov-merge.c: Likewise.
* libgcov-profiler.c: Likewise.
* libgcov-std.ver.in: New file.
* libgcov.h (__gcov_init): Remove ATTRIBUTE_HIDDEN.
(__gcov_exit): Likewise.
(__gcov_merge_add): Likewise.
(__gcov_merge_time_profile): Likewise.
(__gcov_merge_single): Likewise.
(__gcov_merge_ior): Likewise.
(__gcov_merge_icall_topn): Likewise.
(gcov_sort_n_vals): Likewise.
(__gcov_fork): Likewise.
(__gcov_execl): Likewise.
(__gcov_execlp): Likewise.
(__gcov_execle): Likewise.
(__gcov_execv): Likewise.
(__gcov_execvp): Likewise.
(__gcov_execve): Likewise.
---
libgcc/Makefile.in | 56 ++++++++++++++++++++++++++++++---
libgcc/config.host | 2 +-
libgcc/config/t-libgcov | 46 +++++++++++++++++++++++++++
libgcc/config/t-libgcov-elf-ver | 3 ++
libgcc/libgcov-driver.c | 4 +--
libgcc/libgcov-interface.c | 28 ++++++++---------
libgcc/libgcov-merge.c | 14 ++++-----
libgcc/libgcov-profiler.c | 34 ++++++++++----------
libgcc/libgcov-std.ver.in | 53 +++++++++++++++++++++++++++++++
libgcc/libgcov.h | 31 +++++++++---------
10 files changed, 209 insertions(+), 62 deletions(-)
create mode 100644 libgcc/config/t-libgcov
create mode 100644 libgcc/config/t-libgcov-elf-ver
create mode 100644 libgcc/libgcov-std.ver.in
[-- Attachment #2: 0001-Introduce-libgcov.so-run-time-library-PR-gcov-profil.patch --]
[-- Type: text/x-patch, Size: 24785 bytes --]
diff --git a/libgcc/Makefile.in b/libgcc/Makefile.in
index 0c5b264f717..7307ad14418 100644
--- a/libgcc/Makefile.in
+++ b/libgcc/Makefile.in
@@ -912,15 +912,25 @@ libgcov-objects = $(libgcov-merge-objects) $(libgcov-profiler-objects) \
$(libgcov-interface-objects) $(libgcov-driver-objects)
$(libgcov-merge-objects): %$(objext): $(srcdir)/libgcov-merge.c $(srcdir)/gcov.h $(srcdir)/libgcov.h
- $(gcc_compile) -DL$* -c $(srcdir)/libgcov-merge.c
+ $(gcc_compile) -DL$* -c $(srcdir)/libgcov-merge.c $(vis_hide)
$(libgcov-profiler-objects): %$(objext): $(srcdir)/libgcov-profiler.c $(srcdir)/gcov.h $(srcdir)/libgcov.h
- $(gcc_compile) -DL$* -c $(srcdir)/libgcov-profiler.c
+ $(gcc_compile) -DL$* -c $(srcdir)/libgcov-profiler.c $(vis_hide)
$(libgcov-interface-objects): %$(objext): $(srcdir)/libgcov-interface.c $(srcdir)/gcov.h $(srcdir)/libgcov.h
- $(gcc_compile) -DL$* -c $(srcdir)/libgcov-interface.c
+ $(gcc_compile) -DL$* -c $(srcdir)/libgcov-interface.c $(vis_hide)
$(libgcov-driver-objects): %$(objext): $(srcdir)/libgcov-driver.c \
$(srcdir)/libgcov-driver-system.c $(srcdir)/gcov.h $(srcdir)/libgcov.h
- $(gcc_compile) -DL$* -c $(srcdir)/libgcov-driver.c
+ $(gcc_compile) -DL$* -c $(srcdir)/libgcov-driver.c $(vis_hide)
+libgcov-s-merge$(objext): $(srcdir)/libgcov-merge.c $(srcdir)/gcov.h $(srcdir)/libgcov.h
+ $(gcc_compile) -DL_gcov_shared -c $(srcdir)/libgcov-merge.c
+libgcov-s-profiler$(objext): $(srcdir)/libgcov-profiler.c $(srcdir)/gcov.h $(srcdir)/libgcov.h
+ $(gcc_compile) -DL_gcov_shared -c $(srcdir)/libgcov-profiler.c
+libgcov-s-interface$(objext): $(srcdir)/libgcov-interface.c $(srcdir)/gcov.h $(srcdir)/libgcov.h
+ $(gcc_compile) -DL_gcov_shared -c $(srcdir)/libgcov-interface.c
+libgcov-s-driver$(objext): $(srcdir)/libgcov-driver.c $(srcdir)/libgcov-driver-system.c $(srcdir)/gcov.h $(srcdir)/libgcov.h
+ $(gcc_compile) -DL_gcov_shared -c $(srcdir)/libgcov-driver.c
+
+libgcov-s-objects = libgcov-s-merge${objext} libgcov-s-profiler${objext} libgcov-s-interface${objext} libgcov-s-driver${objext}
# Static libraries.
libgcc.a: $(libgcc-objects)
@@ -944,7 +954,7 @@ libgcc.a libgcov.a libunwind.a libgcc_eh.a:
all: libgcc.a
ifeq ($(enable_gcov),yes)
-all: libgcov.a
+all: libgcov.a libgcov$(SHLIB_EXT)
endif
ifneq ($(LIBUNWIND),)
@@ -1006,6 +1016,38 @@ libunwind$(SHLIB_EXT): $(libunwind-s-objects) $(extra-parts)
@shlib_base_name@,libunwind,$(subst \
@shlib_slibdir_qual@,$(MULTIOSSUBDIR),$(SHLIBUNWIND_LINK))))))
+# Map-file generation.
+libgcov.map.in: $(SHLIBGCOV_MAPFILES)
+ { cat $(SHLIBGCOV_MAPFILES) \
+ | sed -e '/^[ ]*#/d' \
+ -e 's/^%\(if\|else\|elif\|endif\|define\)/#\1/' \
+ | $(gcc_compile_bare) -E -xassembler-with-cpp -; \
+ } > tmp-$@
+ mv tmp-$@ $@
+libgcov.map: $(SHLIB_MKMAP) libgcov.map.in $(libgcov-objects)
+ { $(NM) $(SHLIBGCOV_NM_FLAGS) $(libgcov-objects); echo %%; \
+ cat libgcov.map.in; \
+ } | $(AWK) -f $(SHLIB_MKMAP) $(SHLIB_MKMAP_OPTS) > tmp-$@
+ mv tmp-$@ $@
+libgcov$(SHLIB_EXT): libgcov.map
+libgcov_mapfile = libgcov.map
+
+libgcov-std.ver: $(srcdir)/libgcov-std.ver.in
+ cat < $< > $@
+
+libgcov$(SHLIB_EXT): $(libgcov-s-objects) libgcc_s$(SHLIB_EXT)
+ # @multilib_flags@ is still needed because this may use
+ # $(GCC_FOR_TARGET) and $(LIBGCC2_CFLAGS) directly.
+ # @multilib_dir@ is not really necessary, but sometimes it has
+ # more uses than just a directory name.
+ $(mkinstalldirs) $(MULTIDIR)
+ $(subst @multilib_flags@,$(CFLAGS) -B./,$(subst \
+ @multilib_dir@,$(MULTIDIR),$(subst \
+ @shlib_objs@,$(objects),$(subst \
+ @shlib_base_name@,libgcov,$(subst \
+ @shlib_map_file@,$(libgcov_mapfile),$(subst \
+ @shlib_slibdir_qual@,$(MULTIOSSUBDIR),$(SHLIBGCOV_LINK)))))))
+
endif
# Build the standard GCC startfiles and endfiles.
@@ -1172,6 +1214,10 @@ ifeq ($(enable_gcov),yes)
$(INSTALL_DATA) libgcov.a $(DESTDIR)$(inst_libdir)/
chmod 644 $(DESTDIR)$(inst_libdir)/libgcov.a
$(RANLIB) $(DESTDIR)$(inst_libdir)/libgcov.a
+
+ $(subst @multilib_dir@,$(MULTIDIR),$(subst \
+ @shlib_base_name@,libgcov,$(subst \
+ @shlib_slibdir_qual@,$(MULTISUBDIR),$(SHLIBGCOV_INSTALL))))
endif
parts="$(INSTALL_PARTS)"; \
diff --git a/libgcc/config.host b/libgcc/config.host
index 029f6569caf..6c35d13dd2c 100644
--- a/libgcc/config.host
+++ b/libgcc/config.host
@@ -239,7 +239,7 @@ case ${host} in
extra_parts="crtbegin.o crtend.o"
;;
*-*-linux* | frv-*-*linux* | *-*-kfreebsd*-gnu | *-*-gnu* | *-*-kopensolaris*-gnu)
- tmake_file="$tmake_file t-crtstuff-pic t-libgcc-pic t-eh-dw2-dip t-slibgcc t-slibgcc-gld t-slibgcc-elf-ver t-linux"
+ tmake_file="$tmake_file t-crtstuff-pic t-libgcc-pic t-eh-dw2-dip t-slibgcc t-slibgcc-gld t-slibgcc-elf-ver t-linux t-libgcov t-libgcov-elf-ver"
extra_parts="crtbegin.o crtbeginS.o crtbeginT.o crtend.o crtendS.o"
if test x$enable_vtable_verify = xyes; then
extra_parts="$extra_parts vtv_start.o vtv_end.o vtv_start_preinit.o vtv_end_preinit.o"
diff --git a/libgcc/config/t-libgcov b/libgcc/config/t-libgcov
new file mode 100644
index 00000000000..075b25148c7
--- /dev/null
+++ b/libgcc/config/t-libgcov
@@ -0,0 +1,46 @@
+# Copyright (C) 2018 Free Software Foundation, Inc.
+#
+# This file is part of GCC.
+#
+# GCC 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 3, or (at your option)
+# any later version.
+#
+# GCC 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 GCC; see the file COPYING3. If not see
+# <http://www.gnu.org/licenses/>.
+
+# Build libgcov for ELF with the GNU linker.
+
+SHLIBGCOV_SOVERSION = 1
+SHLIBGCOV_SONAME = @shlib_base_name@.so.$(SHLIBGCOV_SOVERSION)
+
+SHLIBGCOV_LINK = $(CC) $(LIBGCC2_CFLAGS) -shared \
+ $(SHLIB_LDFLAGS) \
+ -nodefaultlibs -Wl,-h,$(SHLIBGCOV_SONAME) \
+ -Wl,-z,text -Wl,-z,defs -Wl,-z,now -o $(SHLIB_DIR)/$(SHLIBGCOV_SONAME).tmp \
+ @multilib_flags@ $(SHLIB_OBJS) -lc -lgcc && \
+ rm -f $(SHLIB_DIR)/$(SHLIB_SOLINK) && \
+ if [ -f $(SHLIB_DIR)/$(SHLIBGCOV_SONAME) ]; then \
+ mv -f $(SHLIB_DIR)/$(SHLIBGCOV_SONAME) \
+ $(SHLIB_DIR)/$(SHLIBGCOV_SONAME).backup; \
+ else true; fi && \
+ mv $(SHLIB_DIR)/$(SHLIBGCOV_SONAME).tmp \
+ $(SHLIB_DIR)/$(SHLIBGCOV_SONAME) && \
+ $(LN_S) $(SHLIBGCOV_SONAME) $(SHLIB_DIR)/$(SHLIB_SOLINK)
+
+SHLIBGCOV_INSTALL_SOLINK = $(LN_S) $(DESTDIR)$(slibdir)$(SHLIB_SLIBDIR_QUAL)/$(SHLIBGCOV_SONAME) \
+ $(DESTDIR)$(libsubdir)$(SHLIB_SLIBDIR_QUAL)/$(SHLIB_SOLINK)
+
+SHLIBGCOV_INSTALL = \
+ $(mkinstalldirs) $(DESTDIR)$(slibdir)$(SHLIB_SLIBDIR_QUAL); \
+ $(INSTALL_SHLIB) $(SHLIB_DIR)/$(SHLIB_SONAME) \
+ $(DESTDIR)$(slibdir)$(SHLIB_SLIBDIR_QUAL)/$(SHLIB_SONAME); \
+ rm -f $(DESTDIR)$(libsubdir)$(SHLIB_SLIBDIR_QUAL)/$(SHLIB_SOLINK); \
+ $(SHLIBGCOV_INSTALL_SOLINK)
diff --git a/libgcc/config/t-libgcov-elf-ver b/libgcc/config/t-libgcov-elf-ver
new file mode 100644
index 00000000000..07bec315c3f
--- /dev/null
+++ b/libgcc/config/t-libgcov-elf-ver
@@ -0,0 +1,3 @@
+# Build a shared libgcc library for ELF with symbol versioning.
+
+SHLIBGCOV_MAPFILES = libgcov-std.ver
diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c
index 1f2c4a74298..f886b056aa6 100644
--- a/libgcc/libgcov-driver.c
+++ b/libgcc/libgcov-driver.c
@@ -28,7 +28,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
#if defined(inhibit_libc)
/* If libc and its header files are not available, provide dummy functions. */
-#if defined(L_gcov)
+#if defined(L_gcov) || defined(L_gcov_shared)
void __gcov_init (struct gcov_info *p __attribute__ ((unused))) {}
#endif
@@ -41,7 +41,7 @@ void __gcov_init (struct gcov_info *p __attribute__ ((unused))) {}
#include <sys/stat.h>
#endif
-#ifdef L_gcov
+#if defined(L_gcov) || defined(L_gcov_shared)
/* A utility function for outputting errors. */
static int gcov_error (const char *, ...);
diff --git a/libgcc/libgcov-interface.c b/libgcc/libgcov-interface.c
index 227a5a04ccb..3f38e9ad868 100644
--- a/libgcc/libgcov-interface.c
+++ b/libgcc/libgcov-interface.c
@@ -28,15 +28,15 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
#if defined(inhibit_libc)
-#ifdef L_gcov_flush
+#if defined(L_gcov_flush) || defined(L_gcov_shared)
void __gcov_flush (void) {}
#endif
-#ifdef L_gcov_reset
+#if defined(L_gcov_reset) || defined(L_gcov_shared)
void __gcov_reset (void) {}
#endif
-#ifdef L_gcov_dump
+#if defined(L_gcov_dump) || defined(L_gcov_shared)
void __gcov_dump (void) {}
#endif
@@ -54,8 +54,8 @@ void __gcov_dump (void) {}
extern __gthread_mutex_t __gcov_flush_mx ATTRIBUTE_HIDDEN;
extern __gthread_mutex_t __gcov_flush_mx ATTRIBUTE_HIDDEN;
-#ifdef L_gcov_flush
-#ifdef __GTHREAD_MUTEX_INIT
+#if defined(L_gcov_flush) || defined(L_gcov_shared)
+#if defined(__GTHREAD_MUTEX_INIT) || defined(L_gcov_shared)
__gthread_mutex_t __gcov_flush_mx = __GTHREAD_MUTEX_INIT;
#define init_mx_once()
#else
@@ -93,7 +93,7 @@ __gcov_flush (void)
#endif /* L_gcov_flush */
-#ifdef L_gcov_reset
+#if defined(L_gcov_reset) || defined(L_gcov_shared)
/* Reset all counters to zero. */
@@ -148,7 +148,7 @@ ALIAS_void_fn (__gcov_reset_int, __gcov_reset);
#endif /* L_gcov_reset */
-#ifdef L_gcov_dump
+#if defined(L_gcov_dump) || defined(L_gcov_shared)
/* Function that can be called from application to write profile collected
so far, in order to collect profile in region of interest. */
@@ -168,7 +168,7 @@ ALIAS_void_fn (__gcov_dump_int, __gcov_dump);
#endif /* L_gcov_dump */
-#ifdef L_gcov_fork
+#if defined(L_gcov_fork) || defined(L_gcov_shared)
/* A wrapper for the fork function. Flushes the accumulated profiling data, so
that they are not counted twice. */
@@ -184,7 +184,7 @@ __gcov_fork (void)
}
#endif
-#ifdef L_gcov_execl
+#if defined(L_gcov_execl) || defined(L_gcov_shared)
/* A wrapper for the execl function. Flushes the accumulated
profiling data, so that they are not lost. */
@@ -215,7 +215,7 @@ __gcov_execl (const char *path, char *arg, ...)
}
#endif
-#ifdef L_gcov_execlp
+#if defined(L_gcov_execlp) || defined(L_gcov_shared)
/* A wrapper for the execlp function. Flushes the accumulated
profiling data, so that they are not lost. */
@@ -246,7 +246,7 @@ __gcov_execlp (const char *path, char *arg, ...)
}
#endif
-#ifdef L_gcov_execle
+#if defined(L_gcov_execle) || defined(L_gcov_shared)
/* A wrapper for the execle function. Flushes the accumulated
profiling data, so that they are not lost. */
@@ -279,7 +279,7 @@ __gcov_execle (const char *path, char *arg, ...)
}
#endif
-#ifdef L_gcov_execv
+#if defined(L_gcov_execv) || defined(L_gcov_shared)
/* A wrapper for the execv function. Flushes the accumulated
profiling data, so that they are not lost. */
@@ -291,7 +291,7 @@ __gcov_execv (const char *path, char *const argv[])
}
#endif
-#ifdef L_gcov_execvp
+#if defined(L_gcov_execvp) || defined(L_gcov_shared)
/* A wrapper for the execvp function. Flushes the accumulated
profiling data, so that they are not lost. */
@@ -303,7 +303,7 @@ __gcov_execvp (const char *path, char *const argv[])
}
#endif
-#ifdef L_gcov_execve
+#if defined(L_gcov_execve) || defined(L_gcov_shared)
/* A wrapper for the execve function. Flushes the accumulated
profiling data, so that they are not lost. */
diff --git a/libgcc/libgcov-merge.c b/libgcc/libgcov-merge.c
index 3ab509a8d7f..412c01820cf 100644
--- a/libgcc/libgcov-merge.c
+++ b/libgcc/libgcov-merge.c
@@ -28,19 +28,19 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
#if defined(inhibit_libc)
/* If libc and its header files are not available, provide dummy functions. */
-#ifdef L_gcov_merge_add
+#if defined(L_gcov_merge_add) || defined(L_gcov_shared)
void __gcov_merge_add (gcov_type *counters __attribute__ ((unused)),
unsigned n_counters __attribute__ ((unused))) {}
#endif
-#ifdef L_gcov_merge_single
+#if defined(L_gcov_merge_single) || defined(L_gcov_shared)
void __gcov_merge_single (gcov_type *counters __attribute__ ((unused)),
unsigned n_counters __attribute__ ((unused))) {}
#endif
#else
-#ifdef L_gcov_merge_add
+#if defined(L_gcov_merge_add) || defined(L_gcov_shared)
/* The profile merging function that just adds the counters. It is given
an array COUNTERS of N_COUNTERS old counters and it reads the same number
of counters from the gcov file. */
@@ -52,7 +52,7 @@ __gcov_merge_add (gcov_type *counters, unsigned n_counters)
}
#endif /* L_gcov_merge_add */
-#ifdef L_gcov_merge_ior
+#if defined(L_gcov_merge_ior) || defined(L_gcov_shared)
/* The profile merging function that just adds the counters. It is given
an array COUNTERS of N_COUNTERS old counters and it reads the same number
of counters from the gcov file. */
@@ -64,7 +64,7 @@ __gcov_merge_ior (gcov_type *counters, unsigned n_counters)
}
#endif
-#ifdef L_gcov_merge_time_profile
+#if defined(L_gcov_merge_time_profile) || defined(L_gcov_shared)
/* Time profiles are merged so that minimum from all valid (greater than zero)
is stored. There could be a fork that creates new counters. To have
the profile stable, we chosen to pick the smallest function visit time. */
@@ -84,7 +84,7 @@ __gcov_merge_time_profile (gcov_type *counters, unsigned n_counters)
}
#endif /* L_gcov_merge_time_profile */
-#ifdef L_gcov_merge_single
+#if defined(L_gcov_merge_single) || defined(L_gcov_shared)
/* The profile merging function for choosing the most common value.
It is given an array COUNTERS of N_COUNTERS old counters and it
reads the same number of counters from the gcov file. The counters
@@ -122,7 +122,7 @@ __gcov_merge_single (gcov_type *counters, unsigned n_counters)
}
#endif /* L_gcov_merge_single */
-#ifdef L_gcov_merge_icall_topn
+#if defined(L_gcov_merge_icall_topn) || defined(L_gcov_shared)
/* The profile merging function used for merging indirect call counts
This function is given array COUNTERS of N_COUNTERS old counters and it
reads the same number of counters from the gcov file. */
diff --git a/libgcc/libgcov-profiler.c b/libgcc/libgcov-profiler.c
index 7e208d75d86..4ee886e6184 100644
--- a/libgcc/libgcov-profiler.c
+++ b/libgcc/libgcov-profiler.c
@@ -37,7 +37,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
#endif
#endif
-#ifdef L_gcov_interval_profiler
+#if defined(L_gcov_interval_profiler) || defined(L_gcov_shared)
/* If VALUE is in interval <START, START + STEPS - 1>, then increases the
corresponding counter in COUNTERS. If the VALUE is above or below
the interval, COUNTERS[STEPS] or COUNTERS[STEPS + 1] is increased
@@ -57,7 +57,7 @@ __gcov_interval_profiler (gcov_type *counters, gcov_type value,
}
#endif
-#if defined(L_gcov_interval_profiler_atomic) && GCOV_SUPPORTS_ATOMIC
+#if (defined(L_gcov_interval_profiler_atomic) || defined(L_gcov_shared)) && GCOV_SUPPORTS_ATOMIC
/* If VALUE is in interval <START, START + STEPS - 1>, then increases the
corresponding counter in COUNTERS. If the VALUE is above or below
the interval, COUNTERS[STEPS] or COUNTERS[STEPS + 1] is increased
@@ -77,7 +77,7 @@ __gcov_interval_profiler_atomic (gcov_type *counters, gcov_type value,
}
#endif
-#ifdef L_gcov_pow2_profiler
+#if defined(L_gcov_pow2_profiler) || defined(L_gcov_shared)
/* If VALUE is a power of two, COUNTERS[1] is incremented. Otherwise
COUNTERS[0] is incremented. */
@@ -91,7 +91,7 @@ __gcov_pow2_profiler (gcov_type *counters, gcov_type value)
}
#endif
-#if defined(L_gcov_pow2_profiler_atomic) && GCOV_SUPPORTS_ATOMIC
+#if (defined(L_gcov_pow2_profiler_atomic) || defined(L_gcov_shared)) && GCOV_SUPPORTS_ATOMIC
/* If VALUE is a power of two, COUNTERS[1] is incremented. Otherwise
COUNTERS[0] is incremented. Function is thread-safe. */
@@ -137,7 +137,7 @@ __gcov_one_value_profiler_body (gcov_type *counters, gcov_type value,
counters[2]++;
}
-#ifdef L_gcov_one_value_profiler
+#if defined(L_gcov_one_value_profiler) || defined(L_gcov_shared)
void
__gcov_one_value_profiler (gcov_type *counters, gcov_type value)
{
@@ -145,7 +145,7 @@ __gcov_one_value_profiler (gcov_type *counters, gcov_type value)
}
#endif
-#if defined(L_gcov_one_value_profiler_atomic) && GCOV_SUPPORTS_ATOMIC
+#if (defined(L_gcov_one_value_profiler_atomic) || defined(L_gcov_shared)) && GCOV_SUPPORTS_ATOMIC
/* Update one value profilers (COUNTERS) for a given VALUE.
@@ -163,7 +163,7 @@ __gcov_one_value_profiler_atomic (gcov_type *counters, gcov_type value)
}
#endif
-#ifdef L_gcov_indirect_call_topn_profiler
+#if defined(L_gcov_indirect_call_topn_profiler) || defined(L_gcov_shared)
/* Tries to keep track the most frequent N values in the counters where
N is specified by parameter TOPN_VAL. To track top N values, 2*N counter
entries are used.
@@ -271,14 +271,14 @@ __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;
+gcov_type *__gcov_indirect_call_topn_counters;
#if defined(HAVE_CC_TLS) && !defined (USE_EMUTLS)
__thread
#endif
-void *__gcov_indirect_call_topn_callee ATTRIBUTE_HIDDEN;
+void *__gcov_indirect_call_topn_callee;
-#ifdef TARGET_VTABLE_USES_DESCRIPTORS
+#if defined(TARGET_VTABLE_USES_DESCRIPTORS) || defined(L_gcov_shared)
#define VTABLE_USES_DESCRIPTORS 1
#else
#define VTABLE_USES_DESCRIPTORS 0
@@ -301,7 +301,7 @@ __gcov_indirect_call_topn_profiler (gcov_type value, void* cur_func)
}
#endif
-#ifdef L_gcov_indirect_call_profiler_v2
+#if defined(L_gcov_indirect_call_profiler_v2) || defined(L_gcov_shared)
/* These two variables are used to actually track caller and callee. Keep
them in TLS memory so races are not common (they are written to often).
@@ -341,14 +341,14 @@ __gcov_indirect_call_profiler_v2 (gcov_type value, void* cur_func)
}
#endif
-#ifdef L_gcov_time_profiler
+#if defined(L_gcov_time_profiler) || defined(L_gcov_shared)
/* Counter for first visit of each function. */
-gcov_type __gcov_time_profiler_counter ATTRIBUTE_HIDDEN;
+gcov_type __gcov_time_profiler_counter;
#endif
-#ifdef L_gcov_average_profiler
+#if defined(L_gcov_average_profiler) || defined(L_gcov_shared)
/* Increase corresponding COUNTER by VALUE. FIXME: Perhaps we want
to saturate up. */
@@ -360,7 +360,7 @@ __gcov_average_profiler (gcov_type *counters, gcov_type value)
}
#endif
-#if defined(L_gcov_average_profiler_atomic) && GCOV_SUPPORTS_ATOMIC
+#if (defined(L_gcov_average_profiler_atomic) || defined(L_gcov_shared)) && GCOV_SUPPORTS_ATOMIC
/* Increase corresponding COUNTER by VALUE. FIXME: Perhaps we want
to saturate up. Function is thread-safe. */
@@ -372,7 +372,7 @@ __gcov_average_profiler_atomic (gcov_type *counters, gcov_type value)
}
#endif
-#ifdef L_gcov_ior_profiler
+#if defined(L_gcov_ior_profiler) || defined(L_gcov_shared)
/* Bitwise-OR VALUE into COUNTER. */
void
@@ -382,7 +382,7 @@ __gcov_ior_profiler (gcov_type *counters, gcov_type value)
}
#endif
-#if defined(L_gcov_ior_profiler_atomic) && GCOV_SUPPORTS_ATOMIC
+#if (defined(L_gcov_ior_profiler_atomic) || defined(L_gcov_shared)) && GCOV_SUPPORTS_ATOMIC
/* Bitwise-OR VALUE into COUNTER. Function is thread-safe. */
void
diff --git a/libgcc/libgcov-std.ver.in b/libgcc/libgcov-std.ver.in
new file mode 100644
index 00000000000..71fa43e26c9
--- /dev/null
+++ b/libgcc/libgcov-std.ver.in
@@ -0,0 +1,53 @@
+# Copyright (C) 2018 Free Software Foundation, Inc.
+#
+# This file is part of GCC.
+#
+# GCC 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 3, or (at your option)
+# any later version.
+#
+# GCC 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 GCC; see the file COPYING3. If not see
+# <http://www.gnu.org/licenses/>.
+
+GCC_9.1.0 {
+ __gcov_init
+ __gcov_exit
+ __gcov_merge_add
+ __gcov_merge_time_profile
+ __gcov_merge_single
+ __gcov_merge_ior
+ __gcov_merge_icall_topn
+ __gcov_interval_profiler
+ __gcov_interval_profiler_atomic
+ __gcov_pow2_profiler
+ __gcov_pow2_profiler_atomic
+ __gcov_one_value_profiler
+ __gcov_one_value_profiler_atomic
+ __gcov_indirect_call_profiler_v2
+ __gcov_time_profiler
+ __gcov_time_profiler_atomic
+ __gcov_average_profiler
+ __gcov_average_profiler_atomic
+ __gcov_ior_profiler
+ __gcov_ior_profiler_atomic
+ __gcov_indirect_call_topn_profiler
+
+ __gcov_fork
+ __gcov_execl
+ __gcov_execlp
+ __gcov_execle
+ __gcov_execv
+ __gcov_execvp
+ __gcov_execve
+
+ __gcov_indirect_call_callee
+ __gcov_indirect_call_counters
+ __gcov_time_profiler_counter
+}
diff --git a/libgcc/libgcov.h b/libgcc/libgcov.h
index 21422873cf2..d47e3fec703 100644
--- a/libgcc/libgcov.h
+++ b/libgcc/libgcov.h
@@ -234,10 +234,10 @@ extern struct gcov_master __gcov_master;
extern void __gcov_dump_one (struct gcov_root *) ATTRIBUTE_HIDDEN;
/* Register a new object file module. */
-extern void __gcov_init (struct gcov_info *) ATTRIBUTE_HIDDEN;
+extern void __gcov_init (struct gcov_info *);
/* GCOV exit function registered via a static destructor. */
-extern void __gcov_exit (void) ATTRIBUTE_HIDDEN;
+extern void __gcov_exit (void);
/* Function to reset all counters to 0. Both externally visible (and
overridable) and internal version. */
@@ -247,19 +247,19 @@ extern void __gcov_reset_int (void) ATTRIBUTE_HIDDEN;
extern void __gcov_dump_int (void) ATTRIBUTE_HIDDEN;
/* The merge function that just sums the counters. */
-extern void __gcov_merge_add (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;
+extern void __gcov_merge_add (gcov_type *, unsigned);
/* The merge function to select the minimum valid counter value. */
-extern void __gcov_merge_time_profile (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;
+extern void __gcov_merge_time_profile (gcov_type *, unsigned);
/* The merge function to choose the most common value. */
-extern void __gcov_merge_single (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;
+extern void __gcov_merge_single (gcov_type *, unsigned);
/* The merge function that just ors the counters together. */
-extern void __gcov_merge_ior (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;
+extern void __gcov_merge_ior (gcov_type *, unsigned);
/* The merge function is used for topn indirect call counters. */
-extern void __gcov_merge_icall_topn (gcov_type *, unsigned) ATTRIBUTE_HIDDEN;
+extern void __gcov_merge_icall_topn (gcov_type *, unsigned);
/* The profiler functions. */
extern void __gcov_interval_profiler (gcov_type *, gcov_type, int, unsigned);
@@ -277,18 +277,17 @@ extern void __gcov_average_profiler_atomic (gcov_type *, gcov_type);
extern void __gcov_ior_profiler (gcov_type *, gcov_type);
extern void __gcov_ior_profiler_atomic (gcov_type *, gcov_type);
extern void __gcov_indirect_call_topn_profiler (gcov_type, void *);
-extern void gcov_sort_n_vals (gcov_type *, int);
+extern void gcov_sort_n_vals (gcov_type *, int) ATTRIBUTE_HIDDEN;
#ifndef inhibit_libc
/* The wrappers around some library functions.. */
-extern pid_t __gcov_fork (void) ATTRIBUTE_HIDDEN;
-extern int __gcov_execl (const char *, char *, ...) ATTRIBUTE_HIDDEN;
-extern int __gcov_execlp (const char *, char *, ...) ATTRIBUTE_HIDDEN;
-extern int __gcov_execle (const char *, char *, ...) ATTRIBUTE_HIDDEN;
-extern int __gcov_execv (const char *, char *const []) ATTRIBUTE_HIDDEN;
-extern int __gcov_execvp (const char *, char *const []) ATTRIBUTE_HIDDEN;
-extern int __gcov_execve (const char *, char *const [], char *const [])
- ATTRIBUTE_HIDDEN;
+extern pid_t __gcov_fork (void);
+extern int __gcov_execl (const char *, char *, ...);
+extern int __gcov_execlp (const char *, char *, ...);
+extern int __gcov_execle (const char *, char *, ...);
+extern int __gcov_execv (const char *, char *const []);
+extern int __gcov_execvp (const char *, char *const []);
+extern int __gcov_execve (const char *, char *const [], char *const []);
/* Functions that only available in libgcov. */
GCOV_LINKAGE int gcov_open (const char */*name*/) ATTRIBUTE_HIDDEN;
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107).
2018-08-29 8:31 [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107) Martin Liška
@ 2018-08-29 9:21 ` Richard Biener
2018-08-29 10:44 ` Martin Liška
0 siblings, 1 reply; 19+ messages in thread
From: Richard Biener @ 2018-08-29 9:21 UTC (permalink / raw)
To: Martin Liška
Cc: GCC Patches, Joseph S. Myers, Michael Matz, Alexander Monakov
On Wed, Aug 29, 2018 at 10:31 AM Martin Liška <mliska@suse.cz> wrote:
>
> Hello.
>
> Moving the thread from gcc ML into gcc-patches. That's first implementation
> of shared libgcov library. Currently inclusion of t-libgcov is added only
> to *-linux targets. Is it fine to add to all configurations that already
> include 't-slibgcc t-slibgcc-elf-ver'?
>
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
I understand this is to make profiling work with multiple DSOs (all
instrumented).
But does this also make the case work when those DSOs are dlopened/dlclosed
multiple times? I understand that with the shared libgcov implementation this
library must be finalized last? Wouldn't it be better to have some additional
object that lives in an executable only? Or do we want profiled DSOs without
profiling the executable using it? Does that case work with the shared libgcov
(with dlopen/dlclose)? Does it work when DSO1 is compiled with GCC N
and DSO2 is compiled with GCC N+1 where the libgcov ABI changed?
I think we need a better understanding of the situation before jumping to
the conclusion that a shared libgcov is the solution. Maybe better isolating
the individual instances is better?
Richard.
> Martin
>
> libgcc/ChangeLog:
>
> 2018-08-28 Martin Liska <mliska@suse.cz>
>
> PR gcov-profile/84107
> * Makefile.in: Build libgcov.so objects separately,
> add rule for libgcov.map and how the library
> is linked.
> * config.host: Add t-libgcov and t-libgcov-elf-ver
> to *-linux targets.
> * config/t-libgcov: New file.
> * config/t-libgcov-elf-ver: New file.
> * libgcov-driver.c: Declare function if new L_gcov_shared
> is defined.
> * libgcov-interface.c: Likewise.
> * libgcov-merge.c: Likewise.
> * libgcov-profiler.c: Likewise.
> * libgcov-std.ver.in: New file.
> * libgcov.h (__gcov_init): Remove ATTRIBUTE_HIDDEN.
> (__gcov_exit): Likewise.
> (__gcov_merge_add): Likewise.
> (__gcov_merge_time_profile): Likewise.
> (__gcov_merge_single): Likewise.
> (__gcov_merge_ior): Likewise.
> (__gcov_merge_icall_topn): Likewise.
> (gcov_sort_n_vals): Likewise.
> (__gcov_fork): Likewise.
> (__gcov_execl): Likewise.
> (__gcov_execlp): Likewise.
> (__gcov_execle): Likewise.
> (__gcov_execv): Likewise.
> (__gcov_execvp): Likewise.
> (__gcov_execve): Likewise.
> ---
> libgcc/Makefile.in | 56 ++++++++++++++++++++++++++++++---
> libgcc/config.host | 2 +-
> libgcc/config/t-libgcov | 46 +++++++++++++++++++++++++++
> libgcc/config/t-libgcov-elf-ver | 3 ++
> libgcc/libgcov-driver.c | 4 +--
> libgcc/libgcov-interface.c | 28 ++++++++---------
> libgcc/libgcov-merge.c | 14 ++++-----
> libgcc/libgcov-profiler.c | 34 ++++++++++----------
> libgcc/libgcov-std.ver.in | 53 +++++++++++++++++++++++++++++++
> libgcc/libgcov.h | 31 +++++++++---------
> 10 files changed, 209 insertions(+), 62 deletions(-)
> create mode 100644 libgcc/config/t-libgcov
> create mode 100644 libgcc/config/t-libgcov-elf-ver
> create mode 100644 libgcc/libgcov-std.ver.in
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107).
2018-08-29 9:21 ` Richard Biener
@ 2018-08-29 10:44 ` Martin Liška
2018-08-29 11:19 ` Richard Biener
0 siblings, 1 reply; 19+ messages in thread
From: Martin Liška @ 2018-08-29 10:44 UTC (permalink / raw)
To: Richard Biener
Cc: GCC Patches, Joseph S. Myers, Michael Matz, Alexander Monakov
On 08/29/2018 11:17 AM, Richard Biener wrote:
> On Wed, Aug 29, 2018 at 10:31 AM Martin Liška <mliska@suse.cz> wrote:
>>
>> Hello.
>>
>> Moving the thread from gcc ML into gcc-patches. That's first implementation
>> of shared libgcov library. Currently inclusion of t-libgcov is added only
>> to *-linux targets. Is it fine to add to all configurations that already
>> include 't-slibgcc t-slibgcc-elf-ver'?
>>
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>
> I understand this is to make profiling work with multiple DSOs (all
> instrumented).
Yes.
> But does this also make the case work when those DSOs are dlopened/dlclosed
> multiple times?
That works fine even now, when a DSO is loaded, __gcov_init is called and
it registers gcov_info into __gcov_root.
Following sample:
#include <dlfcn.h>
#include <assert.h>
#include <unistd.h>
int main()
{
for (int i = 0; i < 10; i++)
{
void *handle = dlopen ("libfoo.so", RTLD_NOW);
assert (handle);
void (*fn) (void) = dlsym (handle, "foo");
assert (fn);
fn ();
sleep (1);
}
return 0;
}
has:
$ gcov-dump -l foo.gcda
foo.gcda:data:magic `gcda':version `A82*'
foo.gcda:stamp 2235622999
foo.gcda: a3000000: 22:PROGRAM_SUMMARY checksum=0x02bfe640
foo.gcda: counts=2, runs=1, sum_all=20, run_max=10, sum_max=10
foo.gcda: counter histogram:
foo.gcda: 9: num counts=2, min counter=10, cum_counter=20
foo.gcda: 01000000: 3:FUNCTION ident=1636255671, lineno_checksum=0x2172766e, cfg_checksum=0xc0bbb23e
foo.gcda: 01a10000: 4:COUNTERS arcs 2 counts
foo.gcda: 0: 10 10
foo.gcda: 01af0000: 2:COUNTERS time_profiler 1 counts
foo.gcda: 0: 1
which is fine, runs == 1 and arcs counter executed 10x.
I understand that with the shared libgcov implementation this
> library must be finalized last?
If you do profiling, then you depend on libgcov, thus it's loaded before a DSO (or executable)
is loaded.
Wouldn't it be better to have some additional
> object that lives in an executable only? Or do we want profiled DSOs without
> profiling the executable using it?
That should be rare, but it will be possible as __gcov_exit will be eventually called.
Does that case work with the shared libgcov
> (with dlopen/dlclose)?
Yes, I've just tested that.
Does it work when DSO1 is compiled with GCC N
> and DSO2 is compiled with GCC N+1 where the libgcov ABI changed?
For such case we have:
/* Exactly one of these will be live in the process image. */
struct gcov_master __gcov_master =
{GCOV_VERSION, 0};
...
and it's check in __gcov_init.
>
> I think we need a better understanding of the situation before jumping to
> the conclusion that a shared libgcov is the solution. Maybe better isolating
> the individual instances is better?
It's undesired as seen in the PR. When doing profiling of indirect jumps
we need have exactly one __gcov_indirect_call_callee. That's because one can
do indirect calls that cross DSO boundaries.
Martin
>
> Richard.
>
>> Martin
>>
>> libgcc/ChangeLog:
>>
>> 2018-08-28 Martin Liska <mliska@suse.cz>
>>
>> PR gcov-profile/84107
>> * Makefile.in: Build libgcov.so objects separately,
>> add rule for libgcov.map and how the library
>> is linked.
>> * config.host: Add t-libgcov and t-libgcov-elf-ver
>> to *-linux targets.
>> * config/t-libgcov: New file.
>> * config/t-libgcov-elf-ver: New file.
>> * libgcov-driver.c: Declare function if new L_gcov_shared
>> is defined.
>> * libgcov-interface.c: Likewise.
>> * libgcov-merge.c: Likewise.
>> * libgcov-profiler.c: Likewise.
>> * libgcov-std.ver.in: New file.
>> * libgcov.h (__gcov_init): Remove ATTRIBUTE_HIDDEN.
>> (__gcov_exit): Likewise.
>> (__gcov_merge_add): Likewise.
>> (__gcov_merge_time_profile): Likewise.
>> (__gcov_merge_single): Likewise.
>> (__gcov_merge_ior): Likewise.
>> (__gcov_merge_icall_topn): Likewise.
>> (gcov_sort_n_vals): Likewise.
>> (__gcov_fork): Likewise.
>> (__gcov_execl): Likewise.
>> (__gcov_execlp): Likewise.
>> (__gcov_execle): Likewise.
>> (__gcov_execv): Likewise.
>> (__gcov_execvp): Likewise.
>> (__gcov_execve): Likewise.
>> ---
>> libgcc/Makefile.in | 56 ++++++++++++++++++++++++++++++---
>> libgcc/config.host | 2 +-
>> libgcc/config/t-libgcov | 46 +++++++++++++++++++++++++++
>> libgcc/config/t-libgcov-elf-ver | 3 ++
>> libgcc/libgcov-driver.c | 4 +--
>> libgcc/libgcov-interface.c | 28 ++++++++---------
>> libgcc/libgcov-merge.c | 14 ++++-----
>> libgcc/libgcov-profiler.c | 34 ++++++++++----------
>> libgcc/libgcov-std.ver.in | 53 +++++++++++++++++++++++++++++++
>> libgcc/libgcov.h | 31 +++++++++---------
>> 10 files changed, 209 insertions(+), 62 deletions(-)
>> create mode 100644 libgcc/config/t-libgcov
>> create mode 100644 libgcc/config/t-libgcov-elf-ver
>> create mode 100644 libgcc/libgcov-std.ver.in
>>
>>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107).
2018-08-29 10:44 ` Martin Liška
@ 2018-08-29 11:19 ` Richard Biener
2018-08-29 12:13 ` Martin Liška
0 siblings, 1 reply; 19+ messages in thread
From: Richard Biener @ 2018-08-29 11:19 UTC (permalink / raw)
To: Martin Liška
Cc: GCC Patches, Joseph S. Myers, Michael Matz, Alexander Monakov
On Wed, Aug 29, 2018 at 12:44 PM Martin Liška <mliska@suse.cz> wrote:
>
> On 08/29/2018 11:17 AM, Richard Biener wrote:
> > On Wed, Aug 29, 2018 at 10:31 AM Martin Liška <mliska@suse.cz> wrote:
> >>
> >> Hello.
> >>
> >> Moving the thread from gcc ML into gcc-patches. That's first implementation
> >> of shared libgcov library. Currently inclusion of t-libgcov is added only
> >> to *-linux targets. Is it fine to add to all configurations that already
> >> include 't-slibgcc t-slibgcc-elf-ver'?
> >>
> >> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> >
> > I understand this is to make profiling work with multiple DSOs (all
> > instrumented).
>
> Yes.
>
> > But does this also make the case work when those DSOs are dlopened/dlclosed
> > multiple times?
>
> That works fine even now, when a DSO is loaded, __gcov_init is called and
> it registers gcov_info into __gcov_root.
>
> Following sample:
>
> #include <dlfcn.h>
> #include <assert.h>
> #include <unistd.h>
>
> int main()
> {
> for (int i = 0; i < 10; i++)
> {
> void *handle = dlopen ("libfoo.so", RTLD_NOW);
> assert (handle);
>
> void (*fn) (void) = dlsym (handle, "foo");
> assert (fn);
>
> fn ();
> sleep (1);
> }
>
> return 0;
> }
That doesn't dlclose ;)
> has:
>
> $ gcov-dump -l foo.gcda
> foo.gcda:data:magic `gcda':version `A82*'
> foo.gcda:stamp 2235622999
> foo.gcda: a3000000: 22:PROGRAM_SUMMARY checksum=0x02bfe640
> foo.gcda: counts=2, runs=1, sum_all=20, run_max=10, sum_max=10
> foo.gcda: counter histogram:
> foo.gcda: 9: num counts=2, min counter=10, cum_counter=20
> foo.gcda: 01000000: 3:FUNCTION ident=1636255671, lineno_checksum=0x2172766e, cfg_checksum=0xc0bbb23e
> foo.gcda: 01a10000: 4:COUNTERS arcs 2 counts
> foo.gcda: 0: 10 10
> foo.gcda: 01af0000: 2:COUNTERS time_profiler 1 counts
> foo.gcda: 0: 1
>
> which is fine, runs == 1 and arcs counter executed 10x.
>
>
> I understand that with the shared libgcov implementation this
> > library must be finalized last?
>
> If you do profiling, then you depend on libgcov, thus it's loaded before a DSO (or executable)
> is loaded.
>
> Wouldn't it be better to have some additional
> > object that lives in an executable only? Or do we want profiled DSOs without
> > profiling the executable using it?
>
> That should be rare, but it will be possible as __gcov_exit will be eventually called.
I'm not too familiar but I guess each DSO _fini will call __gcov_exit?
> Does that case work with the shared libgcov
> > (with dlopen/dlclose)?
>
> Yes, I've just tested that.
>
> Does it work when DSO1 is compiled with GCC N
> > and DSO2 is compiled with GCC N+1 where the libgcov ABI changed?
>
> For such case we have:
>
> /* Exactly one of these will be live in the process image. */
> struct gcov_master __gcov_master =
> {GCOV_VERSION, 0};
> ...
> and it's check in __gcov_init.
So it doesn't work. Too bad, in the past we shipped a profiling (-pg)
glibc build
but that would then only work for the same compiler.
I guess using a shared libgcov makes profiling the dynamic loader DSO
impossible?
So we'd likely want a -static-libgcov option as well?
> >
> > I think we need a better understanding of the situation before jumping to
> > the conclusion that a shared libgcov is the solution. Maybe better isolating
> > the individual instances is better?
>
> It's undesired as seen in the PR. When doing profiling of indirect jumps
> we need have exactly one __gcov_indirect_call_callee. That's because one can
> do indirect calls that cross DSO boundaries.
But usually you don't inline across DSO boundaries and after devirtualizing
you still need to go through the PLT...
Btw, when you dlopen/dlcose a DSO multiple times the same function can
end up at different addresses, does gcov somehow deal with this? It seems
that the profile functions get the callee address.
Can you shortly tell why the testcase in the PR segfaults? Does the issue
only affect indirect call profiling?
Richard.
> Martin
>
> >
> > Richard.
> >
> >> Martin
> >>
> >> libgcc/ChangeLog:
> >>
> >> 2018-08-28 Martin Liska <mliska@suse.cz>
> >>
> >> PR gcov-profile/84107
> >> * Makefile.in: Build libgcov.so objects separately,
> >> add rule for libgcov.map and how the library
> >> is linked.
> >> * config.host: Add t-libgcov and t-libgcov-elf-ver
> >> to *-linux targets.
> >> * config/t-libgcov: New file.
> >> * config/t-libgcov-elf-ver: New file.
> >> * libgcov-driver.c: Declare function if new L_gcov_shared
> >> is defined.
> >> * libgcov-interface.c: Likewise.
> >> * libgcov-merge.c: Likewise.
> >> * libgcov-profiler.c: Likewise.
> >> * libgcov-std.ver.in: New file.
> >> * libgcov.h (__gcov_init): Remove ATTRIBUTE_HIDDEN.
> >> (__gcov_exit): Likewise.
> >> (__gcov_merge_add): Likewise.
> >> (__gcov_merge_time_profile): Likewise.
> >> (__gcov_merge_single): Likewise.
> >> (__gcov_merge_ior): Likewise.
> >> (__gcov_merge_icall_topn): Likewise.
> >> (gcov_sort_n_vals): Likewise.
> >> (__gcov_fork): Likewise.
> >> (__gcov_execl): Likewise.
> >> (__gcov_execlp): Likewise.
> >> (__gcov_execle): Likewise.
> >> (__gcov_execv): Likewise.
> >> (__gcov_execvp): Likewise.
> >> (__gcov_execve): Likewise.
> >> ---
> >> libgcc/Makefile.in | 56 ++++++++++++++++++++++++++++++---
> >> libgcc/config.host | 2 +-
> >> libgcc/config/t-libgcov | 46 +++++++++++++++++++++++++++
> >> libgcc/config/t-libgcov-elf-ver | 3 ++
> >> libgcc/libgcov-driver.c | 4 +--
> >> libgcc/libgcov-interface.c | 28 ++++++++---------
> >> libgcc/libgcov-merge.c | 14 ++++-----
> >> libgcc/libgcov-profiler.c | 34 ++++++++++----------
> >> libgcc/libgcov-std.ver.in | 53 +++++++++++++++++++++++++++++++
> >> libgcc/libgcov.h | 31 +++++++++---------
> >> 10 files changed, 209 insertions(+), 62 deletions(-)
> >> create mode 100644 libgcc/config/t-libgcov
> >> create mode 100644 libgcc/config/t-libgcov-elf-ver
> >> create mode 100644 libgcc/libgcov-std.ver.in
> >>
> >>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107).
2018-08-29 11:19 ` Richard Biener
@ 2018-08-29 12:13 ` Martin Liška
2018-08-29 12:21 ` Richard Biener
2018-08-29 16:08 ` Alexander Monakov
0 siblings, 2 replies; 19+ messages in thread
From: Martin Liška @ 2018-08-29 12:13 UTC (permalink / raw)
To: Richard Biener
Cc: GCC Patches, Joseph S. Myers, Michael Matz, Alexander Monakov
On 08/29/2018 01:18 PM, Richard Biener wrote:
> On Wed, Aug 29, 2018 at 12:44 PM Martin Liška <mliska@suse.cz> wrote:
>>
>> On 08/29/2018 11:17 AM, Richard Biener wrote:
>>> On Wed, Aug 29, 2018 at 10:31 AM Martin Liška <mliska@suse.cz> wrote:
>>>>
>>>> Hello.
>>>>
>>>> Moving the thread from gcc ML into gcc-patches. That's first implementation
>>>> of shared libgcov library. Currently inclusion of t-libgcov is added only
>>>> to *-linux targets. Is it fine to add to all configurations that already
>>>> include 't-slibgcc t-slibgcc-elf-ver'?
>>>>
>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>
>>> I understand this is to make profiling work with multiple DSOs (all
>>> instrumented).
>>
>> Yes.
>>
>>> But does this also make the case work when those DSOs are dlopened/dlclosed
>>> multiple times?
>>
>> That works fine even now, when a DSO is loaded, __gcov_init is called and
>> it registers gcov_info into __gcov_root.
>>
>> Following sample:
>>
>> #include <dlfcn.h>
>> #include <assert.h>
>> #include <unistd.h>
>>
>> int main()
>> {
>> for (int i = 0; i < 10; i++)
>> {
>> void *handle = dlopen ("libfoo.so", RTLD_NOW);
>> assert (handle);
>>
>> void (*fn) (void) = dlsym (handle, "foo");
>> assert (fn);
>>
>> fn ();
>> sleep (1);
>> }
>>
>> return 0;
>> }
>
> That doesn't dlclose ;)
Yep, I tested that but with compiler (static libgcov) :/
I noticed that make check for postgres generated invalid *.gcda files.
If's caused by fact that:
libgcov-driver.c:
/* Per-dynamic-object gcov state. */
struct gcov_root __gcov_root;
is not longer per-DSO, but global. So when a shared library is unloaded
with dlclose, it computes histogram of all gcov_info files. As the program
still runs, arcs counters are being updated.
>
>> has:
>>
>> $ gcov-dump -l foo.gcda
>> foo.gcda:data:magic `gcda':version `A82*'
>> foo.gcda:stamp 2235622999
>> foo.gcda: a3000000: 22:PROGRAM_SUMMARY checksum=0x02bfe640
>> foo.gcda: counts=2, runs=1, sum_all=20, run_max=10, sum_max=10
>> foo.gcda: counter histogram:
>> foo.gcda: 9: num counts=2, min counter=10, cum_counter=20
>> foo.gcda: 01000000: 3:FUNCTION ident=1636255671, lineno_checksum=0x2172766e, cfg_checksum=0xc0bbb23e
>> foo.gcda: 01a10000: 4:COUNTERS arcs 2 counts
>> foo.gcda: 0: 10 10
>> foo.gcda: 01af0000: 2:COUNTERS time_profiler 1 counts
>> foo.gcda: 0: 1
>>
>> which is fine, runs == 1 and arcs counter executed 10x.
>>
>>
>> I understand that with the shared libgcov implementation this
>>> library must be finalized last?
>>
>> If you do profiling, then you depend on libgcov, thus it's loaded before a DSO (or executable)
>> is loaded.
>>
>> Wouldn't it be better to have some additional
>>> object that lives in an executable only? Or do we want profiled DSOs without
>>> profiling the executable using it?
>>
>> That should be rare, but it will be possible as __gcov_exit will be eventually called.
>
> I'm not too familiar but I guess each DSO _fini will call __gcov_exit?
Yes.
>
>> Does that case work with the shared libgcov
>>> (with dlopen/dlclose)?
>>
>> Yes, I've just tested that.
>>
>> Does it work when DSO1 is compiled with GCC N
>>> and DSO2 is compiled with GCC N+1 where the libgcov ABI changed?
>>
>> For such case we have:
>>
>> /* Exactly one of these will be live in the process image. */
>> struct gcov_master __gcov_master =
>> {GCOV_VERSION, 0};
>> ...
>> and it's check in __gcov_init.
>
> So it doesn't work. Too bad, in the past we shipped a profiling (-pg)
> glibc build
> but that would then only work for the same compiler.
>
> I guess using a shared libgcov makes profiling the dynamic loader DSO
> impossible?
You mean ld.so?
> So we'd likely want a -static-libgcov option as well?
If you use -static, that works now.
>
>>>
>>> I think we need a better understanding of the situation before jumping to
>>> the conclusion that a shared libgcov is the solution. Maybe better isolating
>>> the individual instances is better?
>>
>> It's undesired as seen in the PR. When doing profiling of indirect jumps
>> we need have exactly one __gcov_indirect_call_callee. That's because one can
>> do indirect calls that cross DSO boundaries.
>
> But usually you don't inline across DSO boundaries and after devirtualizing
> you still need to go through the PLT...
>
> Btw, when you dlopen/dlcose a DSO multiple times the same function can
> end up at different addresses, does gcov somehow deal with this? It seems
> that the profile functions get the callee address.
That's fine, profile_id is saved:
if (__gcov_indirect_call_callee != NULL)
__gcov_indirect_call_profiler_v2 (profile_id, ¤t_function_decl);
tree_uid = build_int_cst
(gcov_type_node,
cgraph_node::get (current_function_decl)->profile_id);
>
> Can you shortly tell why the testcase in the PR segfaults? Does the issue
> only affect indirect call profiling?
What happens is that there will exist 2 instances of:
void * __gcov_indirect_call_callee;
one in main executable, and one another in DSO loaded via dlopen.
Then caller sets __gcov_indirect_call_callee to a function pointer, but when
the actual function is called it has it's own __gcov_indirect_call_callee instance
and it's zero. Then following dereferencing happens:
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);
Well, yes, it's only related to indirect profiling. And only benefit is that we maybe
miss a target of an indirect call. Profile for indirect call is beneficial only
for speculative inlining, which will not be possible because the callee lives in
a different DSO. Thus the code of such function will not be probably available
in compilation unit of a caller.
That said, it looks the shared library libgcov.so is overkill.
Alexander may explain how that can be beneficial?
Note that I can fix the segfault being caused by inter-DSO calls.
Martin
>
> Richard.
>
>> Martin
>>
>>>
>>> Richard.
>>>
>>>> Martin
>>>>
>>>> libgcc/ChangeLog:
>>>>
>>>> 2018-08-28 Martin Liska <mliska@suse.cz>
>>>>
>>>> PR gcov-profile/84107
>>>> * Makefile.in: Build libgcov.so objects separately,
>>>> add rule for libgcov.map and how the library
>>>> is linked.
>>>> * config.host: Add t-libgcov and t-libgcov-elf-ver
>>>> to *-linux targets.
>>>> * config/t-libgcov: New file.
>>>> * config/t-libgcov-elf-ver: New file.
>>>> * libgcov-driver.c: Declare function if new L_gcov_shared
>>>> is defined.
>>>> * libgcov-interface.c: Likewise.
>>>> * libgcov-merge.c: Likewise.
>>>> * libgcov-profiler.c: Likewise.
>>>> * libgcov-std.ver.in: New file.
>>>> * libgcov.h (__gcov_init): Remove ATTRIBUTE_HIDDEN.
>>>> (__gcov_exit): Likewise.
>>>> (__gcov_merge_add): Likewise.
>>>> (__gcov_merge_time_profile): Likewise.
>>>> (__gcov_merge_single): Likewise.
>>>> (__gcov_merge_ior): Likewise.
>>>> (__gcov_merge_icall_topn): Likewise.
>>>> (gcov_sort_n_vals): Likewise.
>>>> (__gcov_fork): Likewise.
>>>> (__gcov_execl): Likewise.
>>>> (__gcov_execlp): Likewise.
>>>> (__gcov_execle): Likewise.
>>>> (__gcov_execv): Likewise.
>>>> (__gcov_execvp): Likewise.
>>>> (__gcov_execve): Likewise.
>>>> ---
>>>> libgcc/Makefile.in | 56 ++++++++++++++++++++++++++++++---
>>>> libgcc/config.host | 2 +-
>>>> libgcc/config/t-libgcov | 46 +++++++++++++++++++++++++++
>>>> libgcc/config/t-libgcov-elf-ver | 3 ++
>>>> libgcc/libgcov-driver.c | 4 +--
>>>> libgcc/libgcov-interface.c | 28 ++++++++---------
>>>> libgcc/libgcov-merge.c | 14 ++++-----
>>>> libgcc/libgcov-profiler.c | 34 ++++++++++----------
>>>> libgcc/libgcov-std.ver.in | 53 +++++++++++++++++++++++++++++++
>>>> libgcc/libgcov.h | 31 +++++++++---------
>>>> 10 files changed, 209 insertions(+), 62 deletions(-)
>>>> create mode 100644 libgcc/config/t-libgcov
>>>> create mode 100644 libgcc/config/t-libgcov-elf-ver
>>>> create mode 100644 libgcc/libgcov-std.ver.in
>>>>
>>>>
>>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107).
2018-08-29 12:13 ` Martin Liška
@ 2018-08-29 12:21 ` Richard Biener
2018-08-29 12:50 ` Martin Liška
2018-08-29 16:08 ` Alexander Monakov
1 sibling, 1 reply; 19+ messages in thread
From: Richard Biener @ 2018-08-29 12:21 UTC (permalink / raw)
To: Martin Liška
Cc: GCC Patches, Joseph S. Myers, Michael Matz, Alexander Monakov
On Wed, Aug 29, 2018 at 2:13 PM Martin Liška <mliska@suse.cz> wrote:
>
> On 08/29/2018 01:18 PM, Richard Biener wrote:
> > On Wed, Aug 29, 2018 at 12:44 PM Martin Liška <mliska@suse.cz> wrote:
> >>
> >> On 08/29/2018 11:17 AM, Richard Biener wrote:
> >>> On Wed, Aug 29, 2018 at 10:31 AM Martin Liška <mliska@suse.cz> wrote:
> >>>>
> >>>> Hello.
> >>>>
> >>>> Moving the thread from gcc ML into gcc-patches. That's first implementation
> >>>> of shared libgcov library. Currently inclusion of t-libgcov is added only
> >>>> to *-linux targets. Is it fine to add to all configurations that already
> >>>> include 't-slibgcc t-slibgcc-elf-ver'?
> >>>>
> >>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> >>>
> >>> I understand this is to make profiling work with multiple DSOs (all
> >>> instrumented).
> >>
> >> Yes.
> >>
> >>> But does this also make the case work when those DSOs are dlopened/dlclosed
> >>> multiple times?
> >>
> >> That works fine even now, when a DSO is loaded, __gcov_init is called and
> >> it registers gcov_info into __gcov_root.
> >>
> >> Following sample:
> >>
> >> #include <dlfcn.h>
> >> #include <assert.h>
> >> #include <unistd.h>
> >>
> >> int main()
> >> {
> >> for (int i = 0; i < 10; i++)
> >> {
> >> void *handle = dlopen ("libfoo.so", RTLD_NOW);
> >> assert (handle);
> >>
> >> void (*fn) (void) = dlsym (handle, "foo");
> >> assert (fn);
> >>
> >> fn ();
> >> sleep (1);
> >> }
> >>
> >> return 0;
> >> }
> >
> > That doesn't dlclose ;)
>
> Yep, I tested that but with compiler (static libgcov) :/
>
> I noticed that make check for postgres generated invalid *.gcda files.
> If's caused by fact that:
>
> libgcov-driver.c:
>
> /* Per-dynamic-object gcov state. */
> struct gcov_root __gcov_root;
>
> is not longer per-DSO, but global. So when a shared library is unloaded
> with dlclose, it computes histogram of all gcov_info files. As the program
> still runs, arcs counters are being updated.
>
>
> >
> >> has:
> >>
> >> $ gcov-dump -l foo.gcda
> >> foo.gcda:data:magic `gcda':version `A82*'
> >> foo.gcda:stamp 2235622999
> >> foo.gcda: a3000000: 22:PROGRAM_SUMMARY checksum=0x02bfe640
> >> foo.gcda: counts=2, runs=1, sum_all=20, run_max=10, sum_max=10
> >> foo.gcda: counter histogram:
> >> foo.gcda: 9: num counts=2, min counter=10, cum_counter=20
> >> foo.gcda: 01000000: 3:FUNCTION ident=1636255671, lineno_checksum=0x2172766e, cfg_checksum=0xc0bbb23e
> >> foo.gcda: 01a10000: 4:COUNTERS arcs 2 counts
> >> foo.gcda: 0: 10 10
> >> foo.gcda: 01af0000: 2:COUNTERS time_profiler 1 counts
> >> foo.gcda: 0: 1
> >>
> >> which is fine, runs == 1 and arcs counter executed 10x.
> >>
> >>
> >> I understand that with the shared libgcov implementation this
> >>> library must be finalized last?
> >>
> >> If you do profiling, then you depend on libgcov, thus it's loaded before a DSO (or executable)
> >> is loaded.
> >>
> >> Wouldn't it be better to have some additional
> >>> object that lives in an executable only? Or do we want profiled DSOs without
> >>> profiling the executable using it?
> >>
> >> That should be rare, but it will be possible as __gcov_exit will be eventually called.
> >
> > I'm not too familiar but I guess each DSO _fini will call __gcov_exit?
>
> Yes.
>
> >
> >> Does that case work with the shared libgcov
> >>> (with dlopen/dlclose)?
> >>
> >> Yes, I've just tested that.
> >>
> >> Does it work when DSO1 is compiled with GCC N
> >>> and DSO2 is compiled with GCC N+1 where the libgcov ABI changed?
> >>
> >> For such case we have:
> >>
> >> /* Exactly one of these will be live in the process image. */
> >> struct gcov_master __gcov_master =
> >> {GCOV_VERSION, 0};
> >> ...
> >> and it's check in __gcov_init.
> >
> > So it doesn't work. Too bad, in the past we shipped a profiling (-pg)
> > glibc build
> > but that would then only work for the same compiler.
> >
> > I guess using a shared libgcov makes profiling the dynamic loader DSO
> > impossible?
>
> You mean ld.so?
>
> > So we'd likely want a -static-libgcov option as well?
>
> If you use -static, that works now.
>
> >
> >>>
> >>> I think we need a better understanding of the situation before jumping to
> >>> the conclusion that a shared libgcov is the solution. Maybe better isolating
> >>> the individual instances is better?
> >>
> >> It's undesired as seen in the PR. When doing profiling of indirect jumps
> >> we need have exactly one __gcov_indirect_call_callee. That's because one can
> >> do indirect calls that cross DSO boundaries.
> >
> > But usually you don't inline across DSO boundaries and after devirtualizing
> > you still need to go through the PLT...
> >
> > Btw, when you dlopen/dlcose a DSO multiple times the same function can
> > end up at different addresses, does gcov somehow deal with this? It seems
> > that the profile functions get the callee address.
>
> That's fine, profile_id is saved:
>
> if (__gcov_indirect_call_callee != NULL)
> __gcov_indirect_call_profiler_v2 (profile_id, ¤t_function_decl);
>
> tree_uid = build_int_cst
> (gcov_type_node,
> cgraph_node::get (current_function_decl)->profile_id);
>
> >
> > Can you shortly tell why the testcase in the PR segfaults? Does the issue
> > only affect indirect call profiling?
>
> What happens is that there will exist 2 instances of:
> void * __gcov_indirect_call_callee;
Ah. Isn't there a trick to commonize those even across DSOs by using weakrefs
and/or weak definitions...?
> one in main executable, and one another in DSO loaded via dlopen.
> Then caller sets __gcov_indirect_call_callee to a function pointer, but when
> the actual function is called it has it's own __gcov_indirect_call_callee instance
> and it's zero. Then following dereferencing happens:
>
> 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);
>
> Well, yes, it's only related to indirect profiling. And only benefit is that we maybe
> miss a target of an indirect call. Profile for indirect call is beneficial only
> for speculative inlining, which will not be possible because the callee lives in
> a different DSO. Thus the code of such function will not be probably available
> in compilation unit of a caller.
>
> That said, it looks the shared library libgcov.so is overkill.
It feels like that somehow.
> Alexander may explain how that can be beneficial?
>
> Note that I can fix the segfault being caused by inter-DSO calls.
I think that would be good and also amenable for backports. Just
check for NULL before
dereferencing.
Richard.
> Martin
>
> >
> > Richard.
> >
> >> Martin
> >>
> >>>
> >>> Richard.
> >>>
> >>>> Martin
> >>>>
> >>>> libgcc/ChangeLog:
> >>>>
> >>>> 2018-08-28 Martin Liska <mliska@suse.cz>
> >>>>
> >>>> PR gcov-profile/84107
> >>>> * Makefile.in: Build libgcov.so objects separately,
> >>>> add rule for libgcov.map and how the library
> >>>> is linked.
> >>>> * config.host: Add t-libgcov and t-libgcov-elf-ver
> >>>> to *-linux targets.
> >>>> * config/t-libgcov: New file.
> >>>> * config/t-libgcov-elf-ver: New file.
> >>>> * libgcov-driver.c: Declare function if new L_gcov_shared
> >>>> is defined.
> >>>> * libgcov-interface.c: Likewise.
> >>>> * libgcov-merge.c: Likewise.
> >>>> * libgcov-profiler.c: Likewise.
> >>>> * libgcov-std.ver.in: New file.
> >>>> * libgcov.h (__gcov_init): Remove ATTRIBUTE_HIDDEN.
> >>>> (__gcov_exit): Likewise.
> >>>> (__gcov_merge_add): Likewise.
> >>>> (__gcov_merge_time_profile): Likewise.
> >>>> (__gcov_merge_single): Likewise.
> >>>> (__gcov_merge_ior): Likewise.
> >>>> (__gcov_merge_icall_topn): Likewise.
> >>>> (gcov_sort_n_vals): Likewise.
> >>>> (__gcov_fork): Likewise.
> >>>> (__gcov_execl): Likewise.
> >>>> (__gcov_execlp): Likewise.
> >>>> (__gcov_execle): Likewise.
> >>>> (__gcov_execv): Likewise.
> >>>> (__gcov_execvp): Likewise.
> >>>> (__gcov_execve): Likewise.
> >>>> ---
> >>>> libgcc/Makefile.in | 56 ++++++++++++++++++++++++++++++---
> >>>> libgcc/config.host | 2 +-
> >>>> libgcc/config/t-libgcov | 46 +++++++++++++++++++++++++++
> >>>> libgcc/config/t-libgcov-elf-ver | 3 ++
> >>>> libgcc/libgcov-driver.c | 4 +--
> >>>> libgcc/libgcov-interface.c | 28 ++++++++---------
> >>>> libgcc/libgcov-merge.c | 14 ++++-----
> >>>> libgcc/libgcov-profiler.c | 34 ++++++++++----------
> >>>> libgcc/libgcov-std.ver.in | 53 +++++++++++++++++++++++++++++++
> >>>> libgcc/libgcov.h | 31 +++++++++---------
> >>>> 10 files changed, 209 insertions(+), 62 deletions(-)
> >>>> create mode 100644 libgcc/config/t-libgcov
> >>>> create mode 100644 libgcc/config/t-libgcov-elf-ver
> >>>> create mode 100644 libgcc/libgcov-std.ver.in
> >>>>
> >>>>
> >>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107).
2018-08-29 12:21 ` Richard Biener
@ 2018-08-29 12:50 ` Martin Liška
2018-08-29 14:58 ` Alexander Monakov
0 siblings, 1 reply; 19+ messages in thread
From: Martin Liška @ 2018-08-29 12:50 UTC (permalink / raw)
To: Richard Biener
Cc: GCC Patches, Joseph S. Myers, Michael Matz, Alexander Monakov
On 08/29/2018 02:20 PM, Richard Biener wrote:
> On Wed, Aug 29, 2018 at 2:13 PM Martin Liška <mliska@suse.cz> wrote:
>>
>> On 08/29/2018 01:18 PM, Richard Biener wrote:
>>> On Wed, Aug 29, 2018 at 12:44 PM Martin Liška <mliska@suse.cz> wrote:
>>>>
>>>> On 08/29/2018 11:17 AM, Richard Biener wrote:
>>>>> On Wed, Aug 29, 2018 at 10:31 AM Martin Liška <mliska@suse.cz> wrote:
>>>>>>
>>>>>> Hello.
>>>>>>
>>>>>> Moving the thread from gcc ML into gcc-patches. That's first implementation
>>>>>> of shared libgcov library. Currently inclusion of t-libgcov is added only
>>>>>> to *-linux targets. Is it fine to add to all configurations that already
>>>>>> include 't-slibgcc t-slibgcc-elf-ver'?
>>>>>>
>>>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>>>
>>>>> I understand this is to make profiling work with multiple DSOs (all
>>>>> instrumented).
>>>>
>>>> Yes.
>>>>
>>>>> But does this also make the case work when those DSOs are dlopened/dlclosed
>>>>> multiple times?
>>>>
>>>> That works fine even now, when a DSO is loaded, __gcov_init is called and
>>>> it registers gcov_info into __gcov_root.
>>>>
>>>> Following sample:
>>>>
>>>> #include <dlfcn.h>
>>>> #include <assert.h>
>>>> #include <unistd.h>
>>>>
>>>> int main()
>>>> {
>>>> for (int i = 0; i < 10; i++)
>>>> {
>>>> void *handle = dlopen ("libfoo.so", RTLD_NOW);
>>>> assert (handle);
>>>>
>>>> void (*fn) (void) = dlsym (handle, "foo");
>>>> assert (fn);
>>>>
>>>> fn ();
>>>> sleep (1);
>>>> }
>>>>
>>>> return 0;
>>>> }
>>>
>>> That doesn't dlclose ;)
>>
>> Yep, I tested that but with compiler (static libgcov) :/
>>
>> I noticed that make check for postgres generated invalid *.gcda files.
>> If's caused by fact that:
>>
>> libgcov-driver.c:
>>
>> /* Per-dynamic-object gcov state. */
>> struct gcov_root __gcov_root;
>>
>> is not longer per-DSO, but global. So when a shared library is unloaded
>> with dlclose, it computes histogram of all gcov_info files. As the program
>> still runs, arcs counters are being updated.
>>
>>
>>>
>>>> has:
>>>>
>>>> $ gcov-dump -l foo.gcda
>>>> foo.gcda:data:magic `gcda':version `A82*'
>>>> foo.gcda:stamp 2235622999
>>>> foo.gcda: a3000000: 22:PROGRAM_SUMMARY checksum=0x02bfe640
>>>> foo.gcda: counts=2, runs=1, sum_all=20, run_max=10, sum_max=10
>>>> foo.gcda: counter histogram:
>>>> foo.gcda: 9: num counts=2, min counter=10, cum_counter=20
>>>> foo.gcda: 01000000: 3:FUNCTION ident=1636255671, lineno_checksum=0x2172766e, cfg_checksum=0xc0bbb23e
>>>> foo.gcda: 01a10000: 4:COUNTERS arcs 2 counts
>>>> foo.gcda: 0: 10 10
>>>> foo.gcda: 01af0000: 2:COUNTERS time_profiler 1 counts
>>>> foo.gcda: 0: 1
>>>>
>>>> which is fine, runs == 1 and arcs counter executed 10x.
>>>>
>>>>
>>>> I understand that with the shared libgcov implementation this
>>>>> library must be finalized last?
>>>>
>>>> If you do profiling, then you depend on libgcov, thus it's loaded before a DSO (or executable)
>>>> is loaded.
>>>>
>>>> Wouldn't it be better to have some additional
>>>>> object that lives in an executable only? Or do we want profiled DSOs without
>>>>> profiling the executable using it?
>>>>
>>>> That should be rare, but it will be possible as __gcov_exit will be eventually called.
>>>
>>> I'm not too familiar but I guess each DSO _fini will call __gcov_exit?
>>
>> Yes.
>>
>>>
>>>> Does that case work with the shared libgcov
>>>>> (with dlopen/dlclose)?
>>>>
>>>> Yes, I've just tested that.
>>>>
>>>> Does it work when DSO1 is compiled with GCC N
>>>>> and DSO2 is compiled with GCC N+1 where the libgcov ABI changed?
>>>>
>>>> For such case we have:
>>>>
>>>> /* Exactly one of these will be live in the process image. */
>>>> struct gcov_master __gcov_master =
>>>> {GCOV_VERSION, 0};
>>>> ...
>>>> and it's check in __gcov_init.
>>>
>>> So it doesn't work. Too bad, in the past we shipped a profiling (-pg)
>>> glibc build
>>> but that would then only work for the same compiler.
>>>
>>> I guess using a shared libgcov makes profiling the dynamic loader DSO
>>> impossible?
>>
>> You mean ld.so?
>>
>>> So we'd likely want a -static-libgcov option as well?
>>
>> If you use -static, that works now.
>>
>>>
>>>>>
>>>>> I think we need a better understanding of the situation before jumping to
>>>>> the conclusion that a shared libgcov is the solution. Maybe better isolating
>>>>> the individual instances is better?
>>>>
>>>> It's undesired as seen in the PR. When doing profiling of indirect jumps
>>>> we need have exactly one __gcov_indirect_call_callee. That's because one can
>>>> do indirect calls that cross DSO boundaries.
>>>
>>> But usually you don't inline across DSO boundaries and after devirtualizing
>>> you still need to go through the PLT...
>>>
>>> Btw, when you dlopen/dlcose a DSO multiple times the same function can
>>> end up at different addresses, does gcov somehow deal with this? It seems
>>> that the profile functions get the callee address.
>>
>> That's fine, profile_id is saved:
>>
>> if (__gcov_indirect_call_callee != NULL)
>> __gcov_indirect_call_profiler_v2 (profile_id, ¤t_function_decl);
>>
>> tree_uid = build_int_cst
>> (gcov_type_node,
>> cgraph_node::get (current_function_decl)->profile_id);
>>
>>>
>>> Can you shortly tell why the testcase in the PR segfaults? Does the issue
>>> only affect indirect call profiling?
>>
>> What happens is that there will exist 2 instances of:
>> void * __gcov_indirect_call_callee;
>
> Ah. Isn't there a trick to commonize those even across DSOs by using weakrefs
> and/or weak definitions...?
I'm aware of -Wl,--dynamic-list-data which we have documented in gcov manual.
But this trick does not work for the test-case Alexander wrote.
>
>> one in main executable, and one another in DSO loaded via dlopen.
>> Then caller sets __gcov_indirect_call_callee to a function pointer, but when
>> the actual function is called it has it's own __gcov_indirect_call_callee instance
>> and it's zero. Then following dereferencing happens:
>>
>> 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);
>>
>> Well, yes, it's only related to indirect profiling. And only benefit is that we maybe
>> miss a target of an indirect call. Profile for indirect call is beneficial only
>> for speculative inlining, which will not be possible because the callee lives in
>> a different DSO. Thus the code of such function will not be probably available
>> in compilation unit of a caller.
>>
>> That said, it looks the shared library libgcov.so is overkill.
>
> It feels like that somehow.
>
>> Alexander may explain how that can be beneficial?
>>
>> Note that I can fix the segfault being caused by inter-DSO calls.
>
> I think that would be good and also amenable for backports. Just
> check for NULL before
> dereferencing.
Sure. Let's wait for Alexander. Maybe he's got something we missed.
Martin
>
> Richard.
>
>> Martin
>>
>>>
>>> Richard.
>>>
>>>> Martin
>>>>
>>>>>
>>>>> Richard.
>>>>>
>>>>>> Martin
>>>>>>
>>>>>> libgcc/ChangeLog:
>>>>>>
>>>>>> 2018-08-28 Martin Liska <mliska@suse.cz>
>>>>>>
>>>>>> PR gcov-profile/84107
>>>>>> * Makefile.in: Build libgcov.so objects separately,
>>>>>> add rule for libgcov.map and how the library
>>>>>> is linked.
>>>>>> * config.host: Add t-libgcov and t-libgcov-elf-ver
>>>>>> to *-linux targets.
>>>>>> * config/t-libgcov: New file.
>>>>>> * config/t-libgcov-elf-ver: New file.
>>>>>> * libgcov-driver.c: Declare function if new L_gcov_shared
>>>>>> is defined.
>>>>>> * libgcov-interface.c: Likewise.
>>>>>> * libgcov-merge.c: Likewise.
>>>>>> * libgcov-profiler.c: Likewise.
>>>>>> * libgcov-std.ver.in: New file.
>>>>>> * libgcov.h (__gcov_init): Remove ATTRIBUTE_HIDDEN.
>>>>>> (__gcov_exit): Likewise.
>>>>>> (__gcov_merge_add): Likewise.
>>>>>> (__gcov_merge_time_profile): Likewise.
>>>>>> (__gcov_merge_single): Likewise.
>>>>>> (__gcov_merge_ior): Likewise.
>>>>>> (__gcov_merge_icall_topn): Likewise.
>>>>>> (gcov_sort_n_vals): Likewise.
>>>>>> (__gcov_fork): Likewise.
>>>>>> (__gcov_execl): Likewise.
>>>>>> (__gcov_execlp): Likewise.
>>>>>> (__gcov_execle): Likewise.
>>>>>> (__gcov_execv): Likewise.
>>>>>> (__gcov_execvp): Likewise.
>>>>>> (__gcov_execve): Likewise.
>>>>>> ---
>>>>>> libgcc/Makefile.in | 56 ++++++++++++++++++++++++++++++---
>>>>>> libgcc/config.host | 2 +-
>>>>>> libgcc/config/t-libgcov | 46 +++++++++++++++++++++++++++
>>>>>> libgcc/config/t-libgcov-elf-ver | 3 ++
>>>>>> libgcc/libgcov-driver.c | 4 +--
>>>>>> libgcc/libgcov-interface.c | 28 ++++++++---------
>>>>>> libgcc/libgcov-merge.c | 14 ++++-----
>>>>>> libgcc/libgcov-profiler.c | 34 ++++++++++----------
>>>>>> libgcc/libgcov-std.ver.in | 53 +++++++++++++++++++++++++++++++
>>>>>> libgcc/libgcov.h | 31 +++++++++---------
>>>>>> 10 files changed, 209 insertions(+), 62 deletions(-)
>>>>>> create mode 100644 libgcc/config/t-libgcov
>>>>>> create mode 100644 libgcc/config/t-libgcov-elf-ver
>>>>>> create mode 100644 libgcc/libgcov-std.ver.in
>>>>>>
>>>>>>
>>>>
>>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107).
2018-08-29 12:50 ` Martin Liška
@ 2018-08-29 14:58 ` Alexander Monakov
0 siblings, 0 replies; 19+ messages in thread
From: Alexander Monakov @ 2018-08-29 14:58 UTC (permalink / raw)
To: Martin Liška
Cc: Richard Biener, GCC Patches, Joseph S. Myers, Michael Matz
[-- Attachment #1: Type: text/plain, Size: 1922 bytes --]
On Wed, 29 Aug 2018, Martin Liška wrote:
> >> That said, it looks the shared library libgcov.so is overkill.
> >
> > It feels like that somehow.
> >
> >> Alexander may explain how that can be beneficial?
> >>
> >> Note that I can fix the segfault being caused by inter-DSO calls.
> >
> > I think that would be good and also amenable for backports. Just
> > check for NULL before
> > dereferencing.
Note if you have a call chain foo->bar->baz such that foo and baz are
in the same DSO but bar is in another, with the NULL fix instead of
segfaulting you'll probably record baz as direct callee of foo, which
is not correct (but is a pre-existing problem, in the sense that it
would also happen if the DSO with 'bar' was not instrumented at all).
> Sure. Let's wait for Alexander. Maybe he's got something we missed.
Well, here's the backstory. When people were discussing PR 83879
(multiple instances of __gcov_master due to dlopen) on IRC, with
the fix being "advise the user to pass --dynamic-list-data", I butted in
to say,
>> imho it's a design mistake: gcov symbols that need to work across dso
>> boundaries should have been a part of (currently non-existent) libgcov.so
>> indirect call profiling across exe/library boundary is broken for a similar reason
and later filed PR 84107 for the latter claim.
I think in general if a design requires an object to exist in one instance
like __gcov_master, for dynamic linking it should reside in a dynamic library.
Otherwise, it puts the onus on users to ensure that it's exported from the
main executable, not concealed in libraries by their version scripts. And
even then it still might come up in two copies with dlopen (RTLD_LOCAL).
In theory, another approach would be to have all gcov symbols have hidden
visibility, no exceptions, and redesign the logic around __gcov_master to
work properly when multiple instances are active in one program.
Alexander
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107).
2018-08-29 12:13 ` Martin Liška
2018-08-29 12:21 ` Richard Biener
@ 2018-08-29 16:08 ` Alexander Monakov
2018-09-11 13:56 ` Martin Liška
1 sibling, 1 reply; 19+ messages in thread
From: Alexander Monakov @ 2018-08-29 16:08 UTC (permalink / raw)
To: Martin Liška
Cc: Richard Biener, GCC Patches, Joseph S. Myers, Michael Matz
[-- Attachment #1: Type: text/plain, Size: 537 bytes --]
On Wed, 29 Aug 2018, Martin Liška wrote:
> > Can you shortly tell why the testcase in the PR segfaults? Does the issue
> > only affect indirect call profiling?
>
> What happens is that there will exist 2 instances of:
> void * __gcov_indirect_call_callee;
>
> one in main executable, and one another in DSO loaded via dlopen.
Sorry, but no, there's no dlopen in the testcase, only plain dynamic linking,
and the reason for segfault is more subtle than that.
Preparing the testcase was quite an enlightening experience.
Alexander
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107).
2018-08-29 16:08 ` Alexander Monakov
@ 2018-09-11 13:56 ` Martin Liška
2018-09-11 15:08 ` Alexander Monakov
0 siblings, 1 reply; 19+ messages in thread
From: Martin Liška @ 2018-09-11 13:56 UTC (permalink / raw)
To: Alexander Monakov
Cc: Richard Biener, GCC Patches, Joseph S. Myers, Michael Matz
[-- Attachment #1: Type: text/plain, Size: 2618 bytes --]
On 08/29/2018 06:08 PM, Alexander Monakov wrote:
> On Wed, 29 Aug 2018, Martin Liška wrote:
>>> Can you shortly tell why the testcase in the PR segfaults? Does the issue
>>> only affect indirect call profiling?
>>
>> What happens is that there will exist 2 instances of:
>> void * __gcov_indirect_call_callee;
>>
>> one in main executable, and one another in DSO loaded via dlopen.
>
> Sorry, but no, there's no dlopen in the testcase, only plain dynamic linking,
> and the reason for segfault is more subtle than that.
>
> Preparing the testcase was quite an enlightening experience.
>
> Alexander
>
Hello.
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.
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:
data16 leaq __gcov_indirect_call_callee@tlsgd(%rip), %rdi
.value 0x6666
rex64
call __tls_get_addr@PLT
which eventually points to a stack location:
│0x7ffff7fee4ce <bazar+4> data16 lea 0x4ad2(%rip),%rdi # 0x7ffff7ff2fa8 │
>│0x7ffff7fee4d6 <bazar+12> data16 data16 callq 0x7ffff7fee190 <__tls_get_addr@plt> │
(gdb) p /x $rdi
$2 = 0x7ffff7ff2fa8
It's probably known limitation of TLS global variables.
So my next focus was on removal of TLS of variables and making the code not racy.
It's implemented in my patch where I check that both *_callee and *_counter variables
are non-null and if so profiler value is updated. I'm aware of racy of the code, but
price for it seems reasonable for me.
Doing 1000x calls from 2 concurrent threads of an empty function produce following
number of collisions:
$ gcov-dump -l bar.gcda
...
bar.gcda: 01a90000: 6:COUNTERS indirect_call 3 counts
bar.gcda: 0: 1452010722 1837 1841
...
That said I would like to go this direction. I would be happy to receive
a feedback. Then I'll test the patch.
Thanks,
Martin
[-- Attachment #2: 0001-Fix-indirect-call-instrumentation-in-between-DSOs-PR.patch --]
[-- Type: text/x-patch, Size: 7219 bytes --]
From b90cc7b270cf73d6fd376ab7b105c5207128ece0 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 11 Sep 2018 15:41:24 +0200
Subject: [PATCH] Fix indirect call instrumentation in between DSOs (PR
gcov-profile/84107).
---
gcc/doc/gcov.texi | 3 ++
gcc/tree-profile.c | 4 ---
libgcc/libgcov-profiler.c | 70 ++++++++++++++++++++++-----------------
3 files changed, 43 insertions(+), 34 deletions(-)
diff --git a/gcc/doc/gcov.texi b/gcc/doc/gcov.texi
index 98f4a876293..e0291804344 100644
--- a/gcc/doc/gcov.texi
+++ b/gcc/doc/gcov.texi
@@ -798,8 +798,11 @@ Instrumented applications use a static destructor with priority 99
to invoke the @code{__gcov_dump} function. Thus @code{__gcov_dump}
is executed after all user defined static destructors,
as well as handlers registered with @code{atexit}.
+
If an executable loads a dynamic shared object via dlopen functionality,
@option{-Wl,--dynamic-list-data} is needed to dump all profile data.
+Same option is needed in order to properly instrument destinations
+of indirect calls that happen in between multiple dynamic shared objects.
Profiling run-time library reports various errors related to profile
manipulation and profile saving. Errors are printed into standard error output
diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
index f96bd4b9704..477995020fd 100644
--- a/gcc/tree-profile.c
+++ b/gcc/tree-profile.c
@@ -94,8 +94,6 @@ init_ic_make_global_vars (void)
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));
gcov_type_ptr = build_pointer_type (get_gcov_type ());
@@ -111,8 +109,6 @@ init_ic_make_global_vars (void)
TREE_STATIC (ic_gcov_type_ptr_var) = 1;
DECL_ARTIFICIAL (ic_gcov_type_ptr_var) = 1;
DECL_INITIAL (ic_gcov_type_ptr_var) = NULL;
- if (targetm.have_tls)
- set_decl_tls_model (ic_gcov_type_ptr_var, decl_default_tls_model (ic_gcov_type_ptr_var));
}
/* Create the type and function decls for the interface with gcov. */
diff --git a/libgcc/libgcov-profiler.c b/libgcc/libgcov-profiler.c
index 7e208d75d86..6cd23442fb2 100644
--- a/libgcc/libgcov-profiler.c
+++ b/libgcc/libgcov-profiler.c
@@ -263,20 +263,14 @@ __gcov_topn_value_profiler_body (gcov_type *counters, gcov_type value)
}
}
-/* These two variables are used to actually track caller and callee. Keep
- them in TLS memory so races are not common (they are written to often).
+/* These two variables are used to actually track caller and callee.
+ We do not use TLS variables because they don't work properly
+ with -Wl,--dynamic-list-data.
The variables are set directly by GCC instrumented code, so declaration
here must match one in tree-profile.c. */
-#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;
+gcov_type *__gcov_indirect_call_topn_counters;
+void *__gcov_indirect_call_topn_callee;
#ifdef TARGET_VTABLE_USES_DESCRIPTORS
#define VTABLE_USES_DESCRIPTORS 1
@@ -284,37 +278,42 @@ void *__gcov_indirect_call_topn_callee ATTRIBUTE_HIDDEN;
#define VTABLE_USES_DESCRIPTORS 0
#endif
-/* This fucntion is instrumented at function entry to track topn indirect
- calls to CUR_FUNC. */
+/* This function is instrumented at function entry to track topn indirect
+ calls to CUR_FUNC. As multiple threads can run concurrently,
+ we have to carefull about __gcov_indirect_call_topn_callee and
+ __gcov_indirect_call_topn_counters global variables. It is possible
+ that a data race will confuse indirect call counter, but it should
+ be relatively rare. */
void
__gcov_indirect_call_topn_profiler (gcov_type value, void* cur_func)
{
void *callee_func = __gcov_indirect_call_topn_callee;
+ void *counters = __gcov_indirect_call_topn_counters;
+
/* 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);
+ if ((cur_func == callee_func
+ || (VTABLE_USES_DESCRIPTORS && callee_func
+ && *(void **) cur_func == *(void **) callee_func))
+ && counters != NULL)
+ __gcov_topn_value_profiler_body (counters, value);
+
+ __gcov_indirect_call_topn_callee = NULL;
+ __gcov_indirect_call_topn_counters = NULL;
}
#endif
#ifdef L_gcov_indirect_call_profiler_v2
-/* These two variables are used to actually track caller and callee. Keep
- them in TLS memory so races are not common (they are written to often).
+/* These two variables are used to actually track caller and callee.
+ We do not use TLS variables because they don't work properly
+ with -Wl,--dynamic-list-data.
The variables are set directly by GCC instrumented code, so declaration
here must match one in tree-profile.c */
-#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;
/* By default, the C++ compiler will use function addresses in the
@@ -323,21 +322,32 @@ gcov_type * __gcov_indirect_call_counters;
of this macro says how many words wide the descriptor is (normally 2).
It is assumed that the address of a function descriptor may be treated
- as a pointer to a function. */
+ as a pointer to a function.
+
+ As multiple threads can run concurrently,
+ we have to carefull about __gcov_indirect_call_callee and
+ __gcov_indirect_call_counters global variables. It is possible
+ that a data race will confuse indirect call counter, but it should
+ be relatively rare. */
/* Tries to determine the most common value among its inputs. */
void
__gcov_indirect_call_profiler_v2 (gcov_type value, void* cur_func)
{
+ void *callee = __gcov_indirect_call_callee;
+ void *counters = __gcov_indirect_call_counters;
+
/* 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
- || (__LIBGCC_VTABLE_USES_DESCRIPTORS__
- && *(void **) cur_func == *(void **) __gcov_indirect_call_callee))
- __gcov_one_value_profiler_body (__gcov_indirect_call_counters, value, 0);
+ if ((cur_func == callee
+ || (__LIBGCC_VTABLE_USES_DESCRIPTORS__
+ && *(void **) cur_func == *(void **) callee))
+ && counters != NULL)
+ __gcov_one_value_profiler_body (counters, value, 0);
__gcov_indirect_call_callee = NULL;
+ __gcov_indirect_call_counters = NULL;
}
#endif
--
2.18.0
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107).
2018-09-11 13:56 ` Martin Liška
@ 2018-09-11 15:08 ` Alexander Monakov
2018-09-12 9:27 ` Martin Liška
0 siblings, 1 reply; 19+ messages in thread
From: Alexander Monakov @ 2018-09-11 15:08 UTC (permalink / raw)
To: Martin Liška
Cc: Richard Biener, GCC Patches, Joseph S. Myers, Michael Matz
[-- Attachment #1: Type: text/plain, Size: 1354 bytes --]
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.
> 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)
> 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.
Alexander
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107).
2018-09-11 15:08 ` Alexander Monakov
@ 2018-09-12 9:27 ` Martin Liška
2018-09-12 14:48 ` Richard Biener
2018-09-12 15:02 ` Alexander Monakov
0 siblings, 2 replies; 19+ messages in thread
From: Martin Liška @ 2018-09-12 9:27 UTC (permalink / raw)
To: Alexander Monakov
Cc: Richard Biener, GCC Patches, Joseph S. Myers, Michael Matz
[-- Attachment #1: Type: text/plain, Size: 1776 bytes --]
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
>
[-- Attachment #2: 0001-Patch-candidate.patch --]
[-- Type: text/x-patch, Size: 9462 bytes --]
From ce708b5d3f3742b3e8b930aa6eb74181273efd9f Mon Sep 17 00:00:00 2001
From: marxin <mliska@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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107).
2018-09-12 9:27 ` Martin Liška
@ 2018-09-12 14:48 ` Richard Biener
2018-09-17 11:40 ` Martin Liška
2018-09-12 15:02 ` Alexander Monakov
1 sibling, 1 reply; 19+ messages in thread
From: Richard Biener @ 2018-09-12 14:48 UTC (permalink / raw)
To: Martin Liška
Cc: Alexander Monakov, GCC Patches, Joseph S. Myers, Michael Matz
On Wed, Sep 12, 2018 at 11:27 AM Martin Liška <mliska@suse.cz> wrote:
>
> 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.
Sounds good. I notice the code had this before but...
+ TREE_PUBLIC (ic_tuple_var) = 1;
+ DECL_EXTERNAL (ic_tuple_var) = 1;
+ TREE_STATIC (ic_tuple_var) = 1;
that's one flag too much?! I suggest to drop DECL_EXTERNAL.
Richard.
> Martin
>
> >
> > Alexander
> >
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107).
2018-09-12 9:27 ` Martin Liška
2018-09-12 14:48 ` Richard Biener
@ 2018-09-12 15:02 ` Alexander Monakov
2018-09-12 15:13 ` Richard Biener
2018-09-17 11:43 ` Martin Liška
1 sibling, 2 replies; 19+ messages in thread
From: Alexander Monakov @ 2018-09-12 15:02 UTC (permalink / raw)
To: Martin Liška
Cc: Richard Biener, GCC Patches, Joseph S. Myers, Michael Matz
[-- Attachment #1: Type: text/plain, Size: 1099 bytes --]
On Wed, 12 Sep 2018, Martin Liška wrote:
> 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?
No, for this issue dlopen doesn't pose extra complications as far as I know.
> > 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.
Ack, I think this is a good way to solve the specific issue in the PR.
I'd like to remind that the point of the PR was to draw attention to larger
design issues in libgcov.
There's no decision on the topic of gcov.so. At the Cauldron I had the chance
to talk to you and Richi separately, but we didn't meet together. Would it
help if I started a new thread summarizing the current status from my point of
view?
Alexander
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107).
2018-09-12 15:02 ` Alexander Monakov
@ 2018-09-12 15:13 ` Richard Biener
2018-09-17 11:43 ` Martin Liška
1 sibling, 0 replies; 19+ messages in thread
From: Richard Biener @ 2018-09-12 15:13 UTC (permalink / raw)
To: Alexander Monakov
Cc: Martin Liška, GCC Patches, Joseph S. Myers, Michael Matz
On Wed, Sep 12, 2018 at 5:02 PM Alexander Monakov <amonakov@ispras.ru> wrote:
>
> On Wed, 12 Sep 2018, Martin Liška wrote:
> > 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?
>
> No, for this issue dlopen doesn't pose extra complications as far as I know.
>
> > > 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.
>
> Ack, I think this is a good way to solve the specific issue in the PR.
>
> I'd like to remind that the point of the PR was to draw attention to larger
> design issues in libgcov.
>
> There's no decision on the topic of gcov.so. At the Cauldron I had the chance
> to talk to you and Richi separately, but we didn't meet together. Would it
> help if I started a new thread summarizing the current status from my point of
> view?
Definitely.
Richard.
> Alexander
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107).
2018-09-12 14:48 ` Richard Biener
@ 2018-09-17 11:40 ` Martin Liška
2018-09-18 8:17 ` Martin Liška
0 siblings, 1 reply; 19+ messages in thread
From: Martin Liška @ 2018-09-17 11:40 UTC (permalink / raw)
To: Richard Biener
Cc: Alexander Monakov, GCC Patches, Joseph S. Myers, Michael Matz
[-- Attachment #1: Type: text/plain, Size: 2376 bytes --]
On 9/12/18 4:48 PM, Richard Biener wrote:
> On Wed, Sep 12, 2018 at 11:27 AM Martin Liška <mliska@suse.cz> wrote:
>>
>> 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.
>
> Sounds good. I notice the code had this before but...
>
> + TREE_PUBLIC (ic_tuple_var) = 1;
> + DECL_EXTERNAL (ic_tuple_var) = 1;
> + TREE_STATIC (ic_tuple_var) = 1;
>
> that's one flag too much?! I suggest to drop DECL_EXTERNAL.
I've done that. I'm sending updated version of the patch.
I'm currently testing the patch. Ready after it finishes tests?
Martin
>
> Richard.
>
>> Martin
>>
>>>
>>> Alexander
>>>
>>
[-- Attachment #2: 0001-Fix-divergence-in-indirect-profiling-PR-gcov-profile.patch --]
[-- Type: text/x-patch, Size: 9715 bytes --]
From 7b6c90b291141095072f916b0626e37ee9f2f40a Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Wed, 12 Sep 2018 11:24:59 +0200
Subject: [PATCH] Fix divergence in indirect profiling (PR gcov-profile/84107).
gcc/ChangeLog:
2018-09-17 Martin Liska <mliska@suse.cz>
PR gcov-profile/84107
* tree-profile.c (init_ic_make_global_vars):
Remove ic_void_ptr_var and ic_gcov_type_ptr_var.
Come up with new ic_tuple* variables. Emit
__gcov_indirect_call{,_topn} variables.
(gimple_gen_ic_profiler): Access the variable
and emit gimple.
(gimple_gen_ic_func_profiler): Access
__gcov_indirect_call.callee field.
libgcc/ChangeLog:
2018-09-17 Martin Liska <mliska@suse.cz>
PR gcov-profile/84107
* libgcov-profiler.c (__gcov_indirect_call):
Change type to indirect_call_tuple.
(struct indirect_call_tuple): New struct.
(__gcov_indirect_call_topn_profiler): Change type.
(__gcov_indirect_call_profiler_v2): Use the new
variables.
* libgcov.h (struct indirect_call_tuple): New struct
definition.
---
gcc/tree-profile.c | 78 ++++++++++++++++++++++-----------------
libgcc/libgcov-profiler.c | 25 ++++---------
libgcc/libgcov.h | 9 +++++
3 files changed, 62 insertions(+), 50 deletions(-)
diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
index f96bd4b9704..2b11e60cd53 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,35 @@ 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;
+ 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. */
@@ -388,22 +388,30 @@ gimple_gen_ic_profiler (histogram_value value, unsigned tag, unsigned base)
/* Insert code:
- stmt1: __gcov_indirect_call_counters = get_relevant_counter_ptr ();
+ stmt1: __gcov_indirect_call.counters = get_relevant_counter_ptr ();
stmt2: tmp1 = (void *) (indirect call argument value)
- stmt3: __gcov_indirect_call_callee = tmp1;
+ stmt3: __gcov_indirect_call.callee = tmp1;
Example:
f_1 = foo;
- __gcov_indirect_call_counters = &__gcov4.main[0];
+ __gcov_indirect_call.counters = &__gcov4.main[0];
PROF_9 = f_1;
__gcov_indirect_call_callee = PROF_9;
_4 = f_1 ();
*/
- stmt1 = gimple_build_assign (ic_gcov_type_ptr_var, ref_ptr);
+ 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);
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 callee_ref = build3 (COMPONENT_REF, ptr_void,
+ ic_tuple_var, ic_tuple_callee_field, NULL_TREE);
+ stmt3 = gimple_build_assign (callee_ref, tmp1);
gsi_insert_before (&gsi, stmt1, GSI_SAME_STMT);
gsi_insert_before (&gsi, stmt2, GSI_SAME_STMT);
@@ -461,7 +469,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..7a5e50009ce 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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107).
2018-09-12 15:02 ` Alexander Monakov
2018-09-12 15:13 ` Richard Biener
@ 2018-09-17 11:43 ` Martin Liška
1 sibling, 0 replies; 19+ messages in thread
From: Martin Liška @ 2018-09-17 11:43 UTC (permalink / raw)
To: Alexander Monakov
Cc: Richard Biener, GCC Patches, Joseph S. Myers, Michael Matz
On 9/12/18 5:02 PM, Alexander Monakov wrote:
> On Wed, 12 Sep 2018, Martin Liška wrote:
>> 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?
>
> No, for this issue dlopen doesn't pose extra complications as far as I know.
>
>>> 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.
>
> Ack, I think this is a good way to solve the specific issue in the PR.
>
> I'd like to remind that the point of the PR was to draw attention to larger
> design issues in libgcov.
>
> There's no decision on the topic of gcov.so. At the Cauldron I had the chance
> to talk to you and Richi separately, but we didn't meet together. Would it
> help if I started a new thread summarizing the current status from my point of
> view?
Yes please. Note that we have also fully working version of the shared library right now.
The only question is whether it worth coming up with it? I'm open for introduction of it.
Martin
>
> Alexander
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107).
2018-09-17 11:40 ` Martin Liška
@ 2018-09-18 8:17 ` Martin Liška
2018-10-04 12:47 ` Martin Liška
0 siblings, 1 reply; 19+ messages in thread
From: Martin Liška @ 2018-09-18 8:17 UTC (permalink / raw)
To: Richard Biener
Cc: Alexander Monakov, GCC Patches, Joseph S. Myers, Michael Matz
[-- Attachment #1: Type: text/plain, Size: 2720 bytes --]
On 9/17/18 1:39 PM, Martin Liška wrote:
> On 9/12/18 4:48 PM, Richard Biener wrote:
>> On Wed, Sep 12, 2018 at 11:27 AM Martin Liška <mliska@suse.cz> wrote:
>>>
>>> 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.
>>
>> Sounds good. I notice the code had this before but...
>>
>> + TREE_PUBLIC (ic_tuple_var) = 1;
>> + DECL_EXTERNAL (ic_tuple_var) = 1;
>> + TREE_STATIC (ic_tuple_var) = 1;
>>
>> that's one flag too much?! I suggest to drop DECL_EXTERNAL.
>
> I've done that. I'm sending updated version of the patch.
> I'm currently testing the patch. Ready after it finishes tests?
>
> Martin
>
>>
>> Richard.
>>
>>> Martin
>>>
>>>>
>>>> Alexander
>>>>
>>>
>
Hi.
This is tested version where DECL_EXTERNAL is needed for LTO partitioning
to work properly. Note that the previous variables emitted in tree-profile.c were
also DECL_EXTERNAL.
Patch survives tests on ppcl64-linux-gnu.
Martin
[-- Attachment #2: 0001-Fix-divergence-in-indirect-profiling-PR-gcov-profile.patch --]
[-- Type: text/x-patch, Size: 9752 bytes --]
From 181c59e8e6474edadb23bcc7d3b387a854eb73a9 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Wed, 12 Sep 2018 11:24:59 +0200
Subject: [PATCH] Fix divergence in indirect profiling (PR gcov-profile/84107).
gcc/ChangeLog:
2018-09-17 Martin Liska <mliska@suse.cz>
PR gcov-profile/84107
* tree-profile.c (init_ic_make_global_vars):
Remove ic_void_ptr_var and ic_gcov_type_ptr_var.
Come up with new ic_tuple* variables. Emit
__gcov_indirect_call{,_topn} variables.
(gimple_gen_ic_profiler): Access the variable
and emit gimple.
(gimple_gen_ic_func_profiler): Access
__gcov_indirect_call.callee field.
libgcc/ChangeLog:
2018-09-17 Martin Liska <mliska@suse.cz>
PR gcov-profile/84107
* libgcov-profiler.c (__gcov_indirect_call):
Change type to indirect_call_tuple.
(struct indirect_call_tuple): New struct.
(__gcov_indirect_call_topn_profiler): Change type.
(__gcov_indirect_call_profiler_v2): Use the new
variables.
* libgcov.h (struct indirect_call_tuple): New struct
definition.
---
gcc/tree-profile.c | 79 +++++++++++++++++++++++----------------
libgcc/libgcov-profiler.c | 25 ++++---------
libgcc/libgcov.h | 9 +++++
3 files changed, 63 insertions(+), 50 deletions(-)
diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
index f96bd4b9704..8e1b966be1e 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,36 @@ 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;
+ TREE_STATIC (ic_tuple_var) = 1;
+ DECL_ARTIFICIAL (ic_tuple_var) = 1;
+ DECL_INITIAL (ic_tuple_var) = NULL;
+ DECL_EXTERNAL (ic_tuple_var) = 1;
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. */
@@ -388,22 +389,30 @@ gimple_gen_ic_profiler (histogram_value value, unsigned tag, unsigned base)
/* Insert code:
- stmt1: __gcov_indirect_call_counters = get_relevant_counter_ptr ();
+ stmt1: __gcov_indirect_call.counters = get_relevant_counter_ptr ();
stmt2: tmp1 = (void *) (indirect call argument value)
- stmt3: __gcov_indirect_call_callee = tmp1;
+ stmt3: __gcov_indirect_call.callee = tmp1;
Example:
f_1 = foo;
- __gcov_indirect_call_counters = &__gcov4.main[0];
+ __gcov_indirect_call.counters = &__gcov4.main[0];
PROF_9 = f_1;
__gcov_indirect_call_callee = PROF_9;
_4 = f_1 ();
*/
- stmt1 = gimple_build_assign (ic_gcov_type_ptr_var, ref_ptr);
+ 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);
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 callee_ref = build3 (COMPONENT_REF, ptr_void,
+ ic_tuple_var, ic_tuple_callee_field, NULL_TREE);
+ stmt3 = gimple_build_assign (callee_ref, tmp1);
gsi_insert_before (&gsi, stmt1, GSI_SAME_STMT);
gsi_insert_before (&gsi, stmt2, GSI_SAME_STMT);
@@ -461,7 +470,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..7a5e50009ce 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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107).
2018-09-18 8:17 ` Martin Liška
@ 2018-10-04 12:47 ` Martin Liška
0 siblings, 0 replies; 19+ messages in thread
From: Martin Liška @ 2018-10-04 12:47 UTC (permalink / raw)
To: Richard Biener
Cc: Alexander Monakov, GCC Patches, Joseph S. Myers, Michael Matz
[-- Attachment #1: Type: text/plain, Size: 3049 bytes --]
On 9/18/18 10:00 AM, Martin Liška wrote:
> On 9/17/18 1:39 PM, Martin Liška wrote:
>> On 9/12/18 4:48 PM, Richard Biener wrote:
>>> On Wed, Sep 12, 2018 at 11:27 AM Martin Liška <mliska@suse.cz> wrote:
>>>>
>>>> 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.
>>>
>>> Sounds good. I notice the code had this before but...
>>>
>>> + TREE_PUBLIC (ic_tuple_var) = 1;
>>> + DECL_EXTERNAL (ic_tuple_var) = 1;
>>> + TREE_STATIC (ic_tuple_var) = 1;
>>>
>>> that's one flag too much?! I suggest to drop DECL_EXTERNAL.
>>
>> I've done that. I'm sending updated version of the patch.
>> I'm currently testing the patch. Ready after it finishes tests?
>>
>> Martin
>>
>>>
>>> Richard.
>>>
>>>> Martin
>>>>
>>>>>
>>>>> Alexander
>>>>>
>>>>
>>
>
> Hi.
>
> This is tested version where DECL_EXTERNAL is needed for LTO partitioning
> to work properly. Note that the previous variables emitted in tree-profile.c were
> also DECL_EXTERNAL.
>
> Patch survives tests on ppcl64-linux-gnu.
>
> Martin
>
Ok.
I'm sending final version of the patch that I'm going to install.
I removed TREE_STATIC and also removed few places where ptr_type_node
can replace build_pointer_type (void_type_node).
Martin
[-- Attachment #2: 0001-Fix-divergence-in-indirect-profiling-PR-gcov-profile.patch --]
[-- Type: text/x-patch, Size: 11518 bytes --]
From 3aa0d5a783bcef796d2dee09ff788d736ab0672d Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Wed, 12 Sep 2018 11:24:59 +0200
Subject: [PATCH] Fix divergence in indirect profiling (PR gcov-profile/84107).
gcc/ChangeLog:
2018-09-17 Martin Liska <mliska@suse.cz>
PR gcov-profile/84107
* tree-profile.c (init_ic_make_global_vars):
Remove ic_void_ptr_var and ic_gcov_type_ptr_var.
Come up with new ic_tuple* variables. Emit
__gcov_indirect_call{,_topn} variables.
(gimple_gen_ic_profiler): Access the variable
and emit gimple.
(gimple_gen_ic_func_profiler): Access
__gcov_indirect_call.callee field.
(gimple_init_gcov_profiler): Use ptr_type_node.
* value-prof.c (gimple_ic): Use ptr_type_node.
libgcc/ChangeLog:
2018-09-17 Martin Liska <mliska@suse.cz>
PR gcov-profile/84107
* libgcov-profiler.c (__gcov_indirect_call):
Change type to indirect_call_tuple.
(struct indirect_call_tuple): New struct.
(__gcov_indirect_call_topn_profiler): Change type.
(__gcov_indirect_call_profiler_v2): Use the new
variables.
* libgcov.h (struct indirect_call_tuple): New struct
definition.
---
gcc/tree-profile.c | 84 +++++++++++++++++++++------------------
gcc/value-prof.c | 7 ++--
libgcc/libgcov-profiler.c | 25 ++++--------
libgcc/libgcov.h | 9 +++++
4 files changed, 66 insertions(+), 59 deletions(-)
diff --git a/gcc/tree-profile.c b/gcc/tree-profile.c
index f96bd4b9704..d8f2a3b1ba4 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,9 +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 ptr_void;
+static GTY(()) tree ic_tuple_var;
+static GTY(()) tree ic_tuple_counters_field;
+static GTY(()) tree ic_tuple_callee_field;
/* Do initialization work for the edge profiler. */
@@ -80,39 +82,35 @@ 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_type_node);
- 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_ARTIFICIAL (ic_tuple_var) = 1;
+ DECL_INITIAL (ic_tuple_var) = NULL;
+ DECL_EXTERNAL (ic_tuple_var) = 1;
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. */
@@ -185,7 +183,7 @@ gimple_init_gcov_profiler (void)
ic_profiler_fn_type
= build_function_type_list (void_type_node,
gcov_type_node,
- ptr_void,
+ ptr_type_node,
NULL_TREE);
profiler_fn_name = "__gcov_indirect_call_profiler_v2";
if (PARAM_VALUE (PARAM_INDIR_CALL_TOPN_PROFILE))
@@ -388,22 +386,29 @@ gimple_gen_ic_profiler (histogram_value value, unsigned tag, unsigned base)
/* Insert code:
- stmt1: __gcov_indirect_call_counters = get_relevant_counter_ptr ();
+ stmt1: __gcov_indirect_call.counters = get_relevant_counter_ptr ();
stmt2: tmp1 = (void *) (indirect call argument value)
- stmt3: __gcov_indirect_call_callee = tmp1;
+ stmt3: __gcov_indirect_call.callee = tmp1;
Example:
f_1 = foo;
- __gcov_indirect_call_counters = &__gcov4.main[0];
+ __gcov_indirect_call.counters = &__gcov4.main[0];
PROF_9 = f_1;
__gcov_indirect_call_callee = PROF_9;
_4 = f_1 ();
*/
- stmt1 = gimple_build_assign (ic_gcov_type_ptr_var, ref_ptr);
- tmp1 = make_temp_ssa_name (ptr_void, NULL, "PROF");
+ 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);
+ tmp1 = make_temp_ssa_name (ptr_type_node, 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 callee_ref = build3 (COMPONENT_REF, ptr_type_node,
+ ic_tuple_var, ic_tuple_callee_field, NULL_TREE);
+ stmt3 = gimple_build_assign (callee_ref, tmp1);
gsi_insert_before (&gsi, stmt1, GSI_SAME_STMT);
gsi_insert_before (&gsi, stmt2, GSI_SAME_STMT);
@@ -459,9 +464,12 @@ gimple_gen_ic_func_profiler (void)
resetting __gcov_indirect_call_callee to NULL. */
gimple_stmt_iterator gsi = gsi_start_bb (cond_bb);
- void0 = build_int_cst (build_pointer_type (void_type_node), 0);
+ void0 = build_int_cst (ptr_type_node, 0);
+
+ tree callee_ref = build3 (COMPONENT_REF, ptr_type_node,
+ ic_tuple_var, ic_tuple_callee_field, NULL_TREE);
- tree ref = force_gimple_operand_gsi (&gsi, ic_void_ptr_var, true, 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/gcc/value-prof.c b/gcc/value-prof.c
index 29489e0f85f..60b982793d1 100644
--- a/gcc/value-prof.c
+++ b/gcc/value-prof.c
@@ -1290,7 +1290,6 @@ gimple_ic (gcall *icall_stmt, struct cgraph_node *direct_call,
gcond *cond_stmt;
tree tmp0, tmp1, tmp;
basic_block cond_bb, dcall_bb, icall_bb, join_bb = NULL;
- tree optype = build_pointer_type (void_type_node);
edge e_cd, e_ci, e_di, e_dj = NULL, e_ij;
gimple_stmt_iterator gsi;
int lp_nr, dflags;
@@ -1300,13 +1299,13 @@ gimple_ic (gcall *icall_stmt, struct cgraph_node *direct_call,
cond_bb = gimple_bb (icall_stmt);
gsi = gsi_for_stmt (icall_stmt);
- tmp0 = make_temp_ssa_name (optype, NULL, "PROF");
- tmp1 = make_temp_ssa_name (optype, NULL, "PROF");
+ tmp0 = make_temp_ssa_name (ptr_type_node, NULL, "PROF");
+ tmp1 = make_temp_ssa_name (ptr_type_node, NULL, "PROF");
tmp = unshare_expr (gimple_call_fn (icall_stmt));
load_stmt = gimple_build_assign (tmp0, tmp);
gsi_insert_before (&gsi, load_stmt, GSI_SAME_STMT);
- tmp = fold_convert (optype, build_addr (direct_call->decl));
+ tmp = fold_convert (ptr_type_node, build_addr (direct_call->decl));
load_stmt = gimple_build_assign (tmp1, tmp);
gsi_insert_before (&gsi, load_stmt, GSI_SAME_STMT);
diff --git a/libgcc/libgcov-profiler.c b/libgcc/libgcov-profiler.c
index 7e208d75d86..7a5e50009ce 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.19.0
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2018-10-04 12:40 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-29 8:31 [PATCH] Introduce libgcov.so run-time library (PR gcov-profile/84107) Martin Liška
2018-08-29 9:21 ` Richard Biener
2018-08-29 10:44 ` Martin Liška
2018-08-29 11:19 ` Richard Biener
2018-08-29 12:13 ` Martin Liška
2018-08-29 12:21 ` Richard Biener
2018-08-29 12:50 ` Martin Liška
2018-08-29 14:58 ` Alexander Monakov
2018-08-29 16:08 ` Alexander Monakov
2018-09-11 13:56 ` Martin Liška
2018-09-11 15:08 ` Alexander Monakov
2018-09-12 9:27 ` Martin Liška
2018-09-12 14:48 ` Richard Biener
2018-09-17 11:40 ` Martin Liška
2018-09-18 8:17 ` Martin Liška
2018-10-04 12:47 ` Martin Liška
2018-09-12 15:02 ` Alexander Monakov
2018-09-12 15:13 ` Richard Biener
2018-09-17 11:43 ` Martin Liška
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 read-only IMAP folder(s) and NNTP newsgroup(s).