casaroli commented on code in PR #21350:
URL: https://github.com/apache/nuttx/pull/21350#discussion_r4470641973
##########
libs/libbuiltin/libgcc/gcov.c:
##########
@@ -485,16 +565,10 @@ void __gcov_execle(void)
{
}
-/* GCC redirects fork() in instrumented code to __gcov_fork(), so this is
- * reachable only where unistd.h declares fork() at all.
- */
-
-#ifdef CONFIG_ARCH_HAVE_FORK
pid_t __gcov_fork(void)
Review Comment:
gcov.c does not compile with `CONFIG_COVERAGE_MINI`: qemu-armv8a:nsh_smp on
this head gives `implicit declaration of function 'lib_malloc'`, `'lib_free'`,
`'lib_get_tempbuffer'`, `'lib_put_tempbuffer'` and `'fork'`. The PR removes
`#include <nuttx/lib/lib.h>` and the `#ifdef CONFIG_ARCH_HAVE_FORK` around
`__gcov_fork()`, and master has both.
Fix: restore that include and that `#ifdef`.
##########
libs/libbuiltin/Kconfig:
##########
@@ -75,7 +75,7 @@ config COVERAGE_GCOV_DUMP_REBOOT
config COVERAGE_DEFAULT_PREFIX_STRIP
string "Default gcov dump path prefix strip"
- default "0"
+ default "99"
Review Comment:
The default of `COVERAGE_DEFAULT_PREFIX_STRIP` changes from "0" to "99".
With 99, `gcov_dump_foreach()` (gcov.c:452) skips every directory of the source
path, so all `.gcda` files land in one directory and two sources with the same
name overwrite each other. The body does not mention it.
Fix: keep the default "0".
##########
libs/libbuiltin/libgcc/Make.defs:
##########
@@ -23,7 +23,7 @@ CSRCS += profile.c
endif
ifeq ($(CONFIG_COVERAGE_MINI)$(CONFIG_ARCH_TOOLCHAIN_GCC),yy)
-CSRCS += gcov.c
+CSRCS += gcov.c gcov_write.c profile_value.c profile_write.c
Review Comment:
Make builds compile profile_value.c and profile_write.c always, and CMake
only with `CONFIG_COVERAGE_PGO`, a symbol that no Kconfig defines.
profile_value.c:38 needs `CONFIG_GCOV_COUNTER_POOL_SIZE`, which depends on that
symbol, and in profile_write.c the empty `write_topn_counters()` macro of
gcov_internal.h removes the function name. The body says that PGO is not part
of this series.
Fix: remove profile_value.c, profile_write.c, `GCOV_COUNTER_POOL_SIZE` and
Documentation/guides/pgo_instrumentation.rst from this PR.
##########
include/gcov.h:
##########
@@ -68,36 +89,76 @@ extern void __gcov_reset(void);
extern void __gcov_dump(void);
#if defined(CONFIG_COVERAGE_MINI) && defined(CONFIG_ARCH_TOOLCHAIN_GCC)
Review Comment:
`__gcov_unload()` is back, but this PR still removes the declarations of
`__gcov_init`, `__gcov_merge_add`, `__gcov_exit` and `__gcov_fork` that master
has and that libs/libc/libc.csv:5-8 exports.
Fix: keep these declarations.
##########
syscall/syscall.csv:
##########
@@ -38,6 +38,7 @@
"fsync","unistd.h","","int","int"
"ftruncate","unistd.h","","int","int","off_t"
"futimens","sys/stat.h","","int","int","const struct timespec [2]|FAR const
struct timespec *"
+"gcov","gcov.h","defined(CONFIG_COVERAGE_ALL)","int","int"
Review Comment:
`gcov()` is defined only with `CONFIG_COVERAGE_MINI` and GCC
(libgcc/Make.defs:25), but the syscall is enabled for `CONFIG_COVERAGE_ALL`. A
protected or kernel build with `CONFIG_COVERAGE_ALL` and clang or
`CONFIG_COVERAGE_TOOLCHAIN` has a stub with no target.
Fix: use `defined(CONFIG_COVERAGE_MINI) &&
defined(CONFIG_ARCH_TOOLCHAIN_GCC)`, the guard of libs/libc/libc.csv:4.
##########
Documentation/applications/system/gcov/index.rst:
##########
@@ -139,10 +129,10 @@ Examples of applicable platforms
# The -t parameter specifies the gcov version, which needs to match
the gcc version
# sim
- ./tools/gcov.py -t gcov
+ ./tools/gcov.sh -t gcov-13
Review Comment:
`./tools/gcov.sh -t gcov-13` names a tool that is not in the tree.
Fix: name tools/gcov.py with its current options.
##########
boards/sim/sim/sim/configs/kasan/defconfig:
##########
@@ -0,0 +1,52 @@
+#
Review Comment:
This file was deleted on master by 22d8a50a7b6, and 35008f4cb8f adds it
again.
Fix: drop it from this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
