[PATCH] gcov: fix clang-11+ support

[Nick Desaulniers]

LLVM changed the expected function signatures for llvm_gcda_start_file()
and llvm_gcda_emit_function() in the clang-11 release. Users of clang-11
or newer may have noticed their kernels failing to boot due to a panic
when enabling CONFIG_GCOV_KERNEL=y +CONFIG_GCOV_PROFILE_ALL=y. Fix up
the function signatures so calling these functions doesn't panic the
kernel.

When we drop clang-10 support from the kernel, we should carefully
update the original implementations to try to preserve git blame,
deleting these implementations.

Link: https://reviews.llvm.org/rGcdd683b516d147925212724b09ec6fb792a40041
Link: https://reviews.llvm.org/rG13a633b438b6500ecad9e4f936ebadf3411d0f44
Cc: Fangrui Song <mas...@google.com>
Reported-by: Prasad Sodagudi<psod...@quicinc.com>
Signed-off-by: Nick Desaulniers <ndesau...@google.com>
---
kernel/gcov/clang.c | 69 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 69 insertions(+)

diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
index c94b820a1b62..20e6760ec05d 100644
--- a/kernel/gcov/clang.c
+++ b/kernel/gcov/clang.c
@@ -75,7 +75,9 @@ struct gcov_fn_info {

u32 num_counters;
u64 *counters;
+#if __clang_major__ < 11
const char *function_name;
+#endif
};

static struct gcov_info *current_info;
@@ -105,6 +107,7 @@ void llvm_gcov_init(llvm_gcov_callback writeout, llvm_gcov_callback flush)
}
EXPORT_SYMBOL(llvm_gcov_init);

+#if __clang_major__ < 11
void llvm_gcda_start_file(const char *orig_filename, const char version[4],
u32 checksum)
{
@@ -113,7 +116,17 @@ void llvm_gcda_start_file(const char *orig_filename, const char version[4],
current_info->checksum = checksum;
}
EXPORT_SYMBOL(llvm_gcda_start_file);
+#else
+void llvm_gcda_start_file(const char *orig_filename, u32 version, u32 checksum)
+{
+ current_info->filename = orig_filename;
+ current_info->version = version;
+ current_info->checksum = checksum;
+}
+EXPORT_SYMBOL(llvm_gcda_start_file);
+#endif

+#if __clang_major__ < 11
void llvm_gcda_emit_function(u32 ident, const char *function_name,
u32 func_checksum, u8 use_extra_checksum, u32 cfg_checksum)
{
@@ -133,6 +146,24 @@ void llvm_gcda_emit_function(u32 ident, const char *function_name,
list_add_tail(&info->head, &current_info->functions);
}
EXPORT_SYMBOL(llvm_gcda_emit_function);
+#else
+void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
+ u8 use_extra_checksum, u32 cfg_checksum)
+{
+ struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
+
+ if (!info)
+ return;
+
+ INIT_LIST_HEAD(&info->head);
+ info->ident = ident;
+ info->checksum = func_checksum;
+ info->use_extra_checksum = use_extra_checksum;
+ info->cfg_checksum = cfg_checksum;
+ list_add_tail(&info->head, &current_info->functions);
+}
+EXPORT_SYMBOL(llvm_gcda_emit_function);
+#endif

void llvm_gcda_emit_arcs(u32 num_counters, u64 *counters)
{
@@ -295,6 +326,7 @@ void gcov_info_add(struct gcov_info *dst, struct gcov_info *src)
}
}

+#if __clang_major__ < 11
static struct gcov_fn_info *gcov_fn_info_dup(struct gcov_fn_info *fn)
{
size_t cv_size; /* counter values size */
@@ -322,6 +354,28 @@ static struct gcov_fn_info *gcov_fn_info_dup(struct gcov_fn_info *fn)
kfree(fn_dup);
return NULL;
}
+#else
+static struct gcov_fn_info *gcov_fn_info_dup(struct gcov_fn_info *fn)
+{
+ size_t cv_size; /* counter values size */
+ struct gcov_fn_info *fn_dup = kmemdup(fn, sizeof(*fn),
+ GFP_KERNEL);
+ if (!fn_dup)
+ return NULL;
+ INIT_LIST_HEAD(&fn_dup->head);
+
+ cv_size = fn->num_counters * sizeof(fn->counters[0]);
+ fn_dup->counters = vmalloc(cv_size);
+ if (!fn_dup->counters) {
+ kfree(fn_dup);
+ return NULL;
+ }
+
+ memcpy(fn_dup->counters, fn->counters, cv_size);
+
+ return fn_dup;
+}
+#endif

/**
* gcov_info_dup - duplicate profiling data set
@@ -362,6 +416,7 @@ struct gcov_info *gcov_info_dup(struct gcov_info *info)
* gcov_info_free - release memory for profiling data set duplicate
* @info: profiling data set duplicate to free
*/
+#if __clang_major__ < 11
void gcov_info_free(struct gcov_info *info)
{
struct gcov_fn_info *fn, *tmp;
@@ -375,6 +430,20 @@ void gcov_info_free(struct gcov_info *info)
kfree(info->filename);
kfree(info);
}
+#else
+void gcov_info_free(struct gcov_info *info)
+{
+ struct gcov_fn_info *fn, *tmp;
+
+ list_for_each_entry_safe(fn, tmp, &info->functions, head) {
+ vfree(fn->counters);
+ list_del(&fn->head);
+ kfree(fn);
+ }
+ kfree(info->filename);
+ kfree(info);
+}
+#endif

#define ITER_STRIDE PAGE_SIZE


base-commit: f78d76e72a4671ea52d12752d92077788b4f5d50
--
2.31.0.rc2.261.g7f71774620-goog

[Nathan Chancellor]

On Fri, Mar 12, 2021 at 11:21:39AM -0800, Nick Desaulniers wrote:
> LLVM changed the expected function signatures for llvm_gcda_start_file()
> and llvm_gcda_emit_function() in the clang-11 release. Users of clang-11
> or newer may have noticed their kernels failing to boot due to a panic
> when enabling CONFIG_GCOV_KERNEL=y +CONFIG_GCOV_PROFILE_ALL=y. Fix up
> the function signatures so calling these functions doesn't panic the
> kernel.
>
> When we drop clang-10 support from the kernel, we should carefully
> update the original implementations to try to preserve git blame,
> deleting these implementations.
>
> Link: https://reviews.llvm.org/rGcdd683b516d147925212724b09ec6fb792a40041
> Link: https://reviews.llvm.org/rG13a633b438b6500ecad9e4f936ebadf3411d0f44
> Cc: Fangrui Song <mas...@google.com>
> Reported-by: Prasad Sodagudi<psod...@quicinc.com>
> Signed-off-by: Nick Desaulniers <ndesau...@google.com>

I can reproduce the panic (as a boot hang) in QEMU before this patch and
it is resolved after it so:

Tested-by: Nathan Chancellor <nat...@kernel.org>

However, the duplication hurts :( would it potentially be better to just
do the full update to clang-11+ and require it for CONFIG_GCOV_KERNEL?

depends on CC_IS_GCC || CLANG_VERSION >= 110000?

[Nick Desaulniers]

On Fri, Mar 12, 2021 at 11:58 AM Nathan Chancellor <nat...@kernel.org> wrote:
>
> On Fri, Mar 12, 2021 at 11:21:39AM -0800, Nick Desaulniers wrote:
> > LLVM changed the expected function signatures for llvm_gcda_start_file()
> > and llvm_gcda_emit_function() in the clang-11 release. Users of clang-11
> > or newer may have noticed their kernels failing to boot due to a panic
> > when enabling CONFIG_GCOV_KERNEL=y +CONFIG_GCOV_PROFILE_ALL=y. Fix up
> > the function signatures so calling these functions doesn't panic the
> > kernel.
> >
> > When we drop clang-10 support from the kernel, we should carefully
> > update the original implementations to try to preserve git blame,
> > deleting these implementations.
> >
> > Link: https://reviews.llvm.org/rGcdd683b516d147925212724b09ec6fb792a40041
> > Link: https://reviews.llvm.org/rG13a633b438b6500ecad9e4f936ebadf3411d0f44
> > Cc: Fangrui Song <mas...@google.com>
> > Reported-by: Prasad Sodagudi<psod...@quicinc.com>
> > Signed-off-by: Nick Desaulniers <ndesau...@google.com>
>
> I can reproduce the panic (as a boot hang) in QEMU before this patch and
> it is resolved after it so:
>
> Tested-by: Nathan Chancellor <nat...@kernel.org>
>
> However, the duplication hurts :( would it potentially be better to just
> do the full update to clang-11+ and require it for CONFIG_GCOV_KERNEL?
>
> depends on CC_IS_GCC || CLANG_VERSION >= 110000?

I'm not opposed, and value your input on the matter. Either way, this
will need to be back ported to stable. Should we be concerned with
users of stable's branches before we mandated clang-10 as the minimum
supported version?

commit 1f7a44f63e6c ("compiler-clang: add build check for clang 10.0.1")

first landed in v5.10-rc1. Does not exist in v5.4.y. The diff you
suggest is certainly easier to review to observe the differences, and
I we don't have users of the latest Android or CrOS kernels using
older clang, but I suspect there may be older kernel versions where if
they try to upgrade their version of clang, GCOV support will regress
for them. Though, I guess that's fine since either approach will fix
this for them. I guess if they don't want to upgrade from clang-10 say
for example, then this approach can be backported to stable.

--
Thanks,
~Nick Desaulniers

[Fangrui Song]

function_name can be unconditionally deleted. It is not used by llvm-cov
gcov. You'll need to delete a few assignments to gcov_info_free but you
can then unify the gcov_fn_info_dup and gcov_info_free implementations.

> };
>
> static struct gcov_info *current_info;
>@@ -105,6 +107,7 @@ void llvm_gcov_init(llvm_gcov_callback writeout, llvm_gcov_callback flush)
> }
> EXPORT_SYMBOL(llvm_gcov_init);
>
>+#if __clang_major__ < 11
> void llvm_gcda_start_file(const char *orig_filename, const char version[4],
> u32 checksum)
> {
>@@ -113,7 +116,17 @@ void llvm_gcda_start_file(const char *orig_filename, const char version[4],
> current_info->checksum = checksum;
> }
> EXPORT_SYMBOL(llvm_gcda_start_file);
>+#else
>+void llvm_gcda_start_file(const char *orig_filename, u32 version, u32 checksum)
>+{
>+ current_info->filename = orig_filename;
>+ current_info->version = version;
>+ current_info->checksum = checksum;
>+}
>+EXPORT_SYMBOL(llvm_gcda_start_file);
>+#endif

LG. On big-endian systems, clang < 11 emitted .gcno/.gcda files do not
work with llvm-cov gcov < 11. To fix it and make .gcno/.gcda work with
gcc gcov I chose to break compatibility (and make all the breaking
changes like deleting some CC1 options) in a short window. At that time
I was not aware that there is the kernel implementation. Later on I was
CCed on a few https://github.com/ClangBuiltLinux/linux/ gcov issues but
I forgot to mention the interface change.

Now in clang 11 onward, clang --coverage defaults to the gcov 4.8
compatible format. You can specify the CC1 option (internal option,
subject to change) -coverage-version to make it compatible with other
versions' gcov.

-Xclang -coverage-version='407*' => 4.7
-Xclang -coverage-version='704*' => 7.4
-Xclang -coverage-version='B02*' => 10.2 (('B'-'A')*10 = 10)

Reviewed-by: Fangrui Song <mas...@google.com>

[Nick Desaulniers]

Thinking more about this over lunch; what are your thoughts on a V2
that does this first, then what you suggest as a second patch on top,
with the first tagged for inclusion into stable, but the second one
not? Hopefully maintainers don't consider that too much churn?

--
Thanks,
~Nick Desaulniers

[Nathan Chancellor]

Hmmm fair point, I did not realize that this support had landed in 5.2
meaning that 5.4 needs it as well at 5.10.

> suggest is certainly easier to review to observe the differences, and
> I we don't have users of the latest Android or CrOS kernels using
> older clang, but I suspect there may be older kernel versions where if
> they try to upgrade their version of clang, GCOV support will regress
> for them. Though, I guess that's fine since either approach will fix
> this for them. I guess if they don't want to upgrade from clang-10 say
> for example, then this approach can be backported to stable.

If people are happy with this approach, it is the more "stable friendly"
change because it fixes it for all versions of clang that should have
been supported at their respective times. Maybe it is worthwhile to do
both? This change gets picked up with a Cc: sta...@vger.kernel.org then
in a follow up patch, we remove the #ifdef's and gate GCOV on clang-11?
The CLANG_VERSION string is usually what we will search for when
removing old workarounds. Additionally, your patch could just use

#if CLANG_VERSION <= 110000

to more easily see this. I have no strong opinion one way or the other
though. If people are happy with this approach, let's do it.

Cheers,
Nathan

[Nick Desaulniers]

Sounds like we're on the same page; will send a v2 with your
recommendation on top.

> Additionally, your patch could just use
>
> #if CLANG_VERSION <= 110000
>
> to more easily see this. I have no strong opinion one way or the other
> though. If people are happy with this approach, let's do it.

Err that would be nicer, but:
kernel/gcov/clang.c:78:5: warning: 'CLANG_VERSION' is not defined,
evaluates to 0 [-Wundef]
#if CLANG_VERSION < 110000
^
kernel/gcov/clang.c:110:5: warning: 'CLANG_VERSION' is not defined,
evaluates to 0 [-Wundef]
#if CLANG_VERSION < 110000
^
kernel/gcov/clang.c:130:5: warning: 'CLANG_VERSION' is not defined,
evaluates to 0 [-Wundef]
#if CLANG_VERSION < 110000
^
kernel/gcov/clang.c:330:5: warning: 'CLANG_VERSION' is not defined,
evaluates to 0 [-Wundef]
#if CLANG_VERSION < 110000
^
kernel/gcov/clang.c:420:5: warning: 'CLANG_VERSION' is not defined,
evaluates to 0 [-Wundef]
#if CLANG_VERSION < 110000
^

Did we just break this in commit aec6c60a01d3 ("kbuild: check the
minimum compiler version in Kconfig") in v5.12-rc1? So I'll keep it
as is for v2, but we should discuss with Masahiro and Miguel if we
should be removing CLANG_VERSION even if there are no in tree users at
the moment. (I guess I could re-introduce it in my series for v2, but
that will unnecessarily complicate the backports, so I won't). My
fault for not catching that in code review.

--
Thanks,
~Nick Desaulniers

[Nick Desaulniers]

On Fri, Mar 12, 2021 at 12:25 PM 'Fangrui Song' via Clang Built Linux
<clang-bu...@googlegroups.com> wrote:
>
> function_name can be unconditionally deleted. It is not used by llvm-cov
> gcov. You'll need to delete a few assignments to gcov_info_free but you
> can then unify the gcov_fn_info_dup and gcov_info_free implementations.
>

> LG. On big-endian systems, clang < 11 emitted .gcno/.gcda files do not
> work with llvm-cov gcov < 11. To fix it and make .gcno/.gcda work with
> gcc gcov I chose to break compatibility (and make all the breaking
> changes like deleting some CC1 options) in a short window. At that time
> I was not aware that there is the kernel implementation. Later on I was
> CCed on a few https://github.com/ClangBuiltLinux/linux/ gcov issues but
> I forgot to mention the interface change.

These are all good suggestions. Since in v2 I'll drop support for
clang < 11, I will skip additional patches to disable GCOV when using
older clang for BE, and the function_name cleanup.

> Now in clang 11 onward, clang --coverage defaults to the gcov 4.8
> compatible format. You can specify the CC1 option (internal option,
> subject to change) -coverage-version to make it compatible with other
> versions' gcov.
>
> -Xclang -coverage-version='407*' => 4.7
> -Xclang -coverage-version='704*' => 7.4
> -Xclang -coverage-version='B02*' => 10.2 (('B'-'A')*10 = 10)

How come LLVM doesn't default to 10.2 format, if it can optionally
produce it? We might be able to reuse more code in the kernel between
the two impelementations, though I expect the symbols the runtime is
expected to provide will still differ. Seeing the `B` in `B02*` is
also curious.

Thanks for the review, will include your tag in v2.
--
Thanks,
~Nick Desaulniers

[Nathan Chancellor]

Ah sorry, CONFIG_CLANG_VERSION.

> Did we just break this in commit aec6c60a01d3 ("kbuild: check the
> minimum compiler version in Kconfig") in v5.12-rc1? So I'll keep it
> as is for v2, but we should discuss with Masahiro and Miguel if we
> should be removing CLANG_VERSION even if there are no in tree users at
> the moment. (I guess I could re-introduce it in my series for v2, but
> that will unnecessarily complicate the backports, so I won't). My
> fault for not catching that in code review.

Technically yes, but the {CLANG,GCC}_VERSION macros are not portable
because they are only defined in their respective headers, resulting in
problems like commit df3da04880b4 ("mips: Fix unroll macro when building
with Clang").

Cheers,
Nathan

> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-li...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAKwvOdmV5co%2BmMSBbnnXyBXiwOha%3DS987PMA68Xe9jP8gJYkdw%40mail.gmail.com.

[Fangrui Song]

On 2021-03-12, Nick Desaulniers wrote:

>On Fri, Mar 12, 2021 at 12:25 PM 'Fangrui Song' via Clang Built Linux
><clang-bu...@googlegroups.com> wrote:
>>
>> function_name can be unconditionally deleted. It is not used by llvm-cov
>> gcov. You'll need to delete a few assignments to gcov_info_free but you
>> can then unify the gcov_fn_info_dup and gcov_info_free implementations.
>>
>> LG. On big-endian systems, clang < 11 emitted .gcno/.gcda files do not
>> work with llvm-cov gcov < 11. To fix it and make .gcno/.gcda work with
>> gcc gcov I chose to break compatibility (and make all the breaking
>> changes like deleting some CC1 options) in a short window. At that time
>> I was not aware that there is the kernel implementation. Later on I was
>> CCed on a few https://github.com/ClangBuiltLinux/linux/ gcov issues but
>> I forgot to mention the interface change.
>
>These are all good suggestions. Since in v2 I'll drop support for
>clang < 11, I will skip additional patches to disable GCOV when using
>older clang for BE, and the function_name cleanup.

Only llvm_gcda_start_file & llvm_gcda_emit_function need version
dispatch. In that case (since there will just be two #if in the file) we don't even need

depends on CC_IS_GCC || CLANG_VERSION >= 110000

>> Now in clang 11 onward, clang --coverage defaults to the gcov 4.8
>> compatible format. You can specify the CC1 option (internal option,
>> subject to change) -coverage-version to make it compatible with other
>> versions' gcov.
>>
>> -Xclang -coverage-version='407*' => 4.7
>> -Xclang -coverage-version='704*' => 7.4
>> -Xclang -coverage-version='B02*' => 10.2 (('B'-'A')*10 = 10)
>
>How come LLVM doesn't default to 10.2 format, if it can optionally
>produce it? We might be able to reuse more code in the kernel between
>the two impelementations, though I expect the symbols the runtime is
>expected to provide will still differ. Seeing the `B` in `B02*` is
>also curious.
>
>Thanks for the review, will include your tag in v2.

4.8 has the widest range of compiler support. gcov 4.8~7.* use the same format.

clang instrumentation does not support the column field (useless in my opinion) introduced in gcov 9, so it just writes zeros.

[Nick Desaulniers]

LLVM changed the expected function signatures for llvm_gcda_start_file()
and llvm_gcda_emit_function() in the clang-11 release. Users of clang-11
or newer may have noticed their kernels failing to boot due to a panic
when enabling CONFIG_GCOV_KERNEL=y +CONFIG_GCOV_PROFILE_ALL=y. Fix up
the function signatures so calling these functions doesn't panic the
kernel.

The first patch should allow us to backport it to stable; the second
drops support for older toolchains.

Link: https://reviews.llvm.org/rGcdd683b516d147925212724b09ec6fb792a40041
Link: https://reviews.llvm.org/rG13a633b438b6500ecad9e4f936ebadf3411d0f44

Nick Desaulniers (2):
gcov: fix clang-11+ support
gcov: clang: drop support for clang-10 and older

kernel/gcov/Kconfig | 1 +
kernel/gcov/clang.c | 32 ++++++++------------------------
2 files changed, 9 insertions(+), 24 deletions(-)


base-commit: f78d76e72a4671ea52d12752d92077788b4f5d50
--
2.31.0.rc2.261.g7f71774620-goog

[Nick Desaulniers]

LLVM changed the expected function signatures for llvm_gcda_start_file()
and llvm_gcda_emit_function() in the clang-11 release. Users of clang-11
or newer may have noticed their kernels failing to boot due to a panic
when enabling CONFIG_GCOV_KERNEL=y +CONFIG_GCOV_PROFILE_ALL=y. Fix up
the function signatures so calling these functions doesn't panic the
kernel.

Link: https://reviews.llvm.org/rGcdd683b516d147925212724b09ec6fb792a40041
Link: https://reviews.llvm.org/rG13a633b438b6500ecad9e4f936ebadf3411d0f44

Cc: sta...@vger.kernel.org # 5.4
Reported-by: Prasad Sodagudi <psod...@quicinc.com>
Suggested-by: Nathan Chancellor <nat...@kernel.org>
Reviewed-by: Fangrui Song <mas...@google.com>
Signed-off-by: Nick Desaulniers <ndesau...@google.com>
Tested-by: Nathan Chancellor <nat...@kernel.org>
---
Changes V1 -> V2:
* Use CONFIG_CLANG_VERSION instead of __clang_major__.
* Pick up and retain Suggested-by, Tested-by, and Reviewed-by tags.
* Drop note from commit message about `git blame`; I did what was
sugguested in V1, but it still looks to git like I wrote those
functions. Oh well.

kernel/gcov/clang.c | 69 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 69 insertions(+)

diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c

index c94b820a1b62..8743150db2ac 100644

--- a/kernel/gcov/clang.c
+++ b/kernel/gcov/clang.c
@@ -75,7 +75,9 @@ struct gcov_fn_info {

u32 num_counters;
u64 *counters;

+#if CONFIG_CLANG_VERSION < 110000

const char *function_name;
+#endif
};

static struct gcov_info *current_info;
@@ -105,6 +107,7 @@ void llvm_gcov_init(llvm_gcov_callback writeout, llvm_gcov_callback flush)
}
EXPORT_SYMBOL(llvm_gcov_init);

+#if CONFIG_CLANG_VERSION < 110000

void llvm_gcda_start_file(const char *orig_filename, const char version[4],
u32 checksum)
{
@@ -113,7 +116,17 @@ void llvm_gcda_start_file(const char *orig_filename, const char version[4],
current_info->checksum = checksum;
}
EXPORT_SYMBOL(llvm_gcda_start_file);
+#else
+void llvm_gcda_start_file(const char *orig_filename, u32 version, u32 checksum)
+{
+ current_info->filename = orig_filename;
+ current_info->version = version;
+ current_info->checksum = checksum;
+}
+EXPORT_SYMBOL(llvm_gcda_start_file);
+#endif

+#if CONFIG_CLANG_VERSION < 110000

+#if CONFIG_CLANG_VERSION < 110000

+#if CONFIG_CLANG_VERSION < 110000

[Nick Desaulniers]

LLVM changed the expected function signatures for llvm_gcda_start_file()

and llvm_gcda_emit_function() in the clang-11 release. Drop the older
implementations and require folks to upgrade their compiler if they're
interested in GCOV support.

Link: https://reviews.llvm.org/rGcdd683b516d147925212724b09ec6fb792a40041
Link: https://reviews.llvm.org/rG13a633b438b6500ecad9e4f936ebadf3411d0f44

Suggested-by: Nathan Chancellor <nat...@kernel.org>

Signed-off-by: Nick Desaulniers <ndesau...@google.com>
---

For an easier time reviewing this series, reviewers may want to apply
these patches, then check the overall diff with `git diff origin/HEAD`.

kernel/gcov/Kconfig | 1 +
kernel/gcov/clang.c | 85 ---------------------------------------------
2 files changed, 1 insertion(+), 85 deletions(-)

diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
index f62de2dea8a3..58f87a3092f3 100644
--- a/kernel/gcov/Kconfig
+++ b/kernel/gcov/Kconfig
@@ -4,6 +4,7 @@ menu "GCOV-based kernel profiling"
config GCOV_KERNEL
bool "Enable gcov-based kernel profiling"
depends on DEBUG_FS
+ depends on !CC_IS_CLANG || CLANG_VERSION >= 110000
select CONSTRUCTORS
default n
help
diff --git a/kernel/gcov/clang.c b/kernel/gcov/clang.c
index 8743150db2ac..14de5644b5cc 100644
--- a/kernel/gcov/clang.c
+++ b/kernel/gcov/clang.c
@@ -75,9 +75,6 @@ struct gcov_fn_info {

u32 num_counters;
u64 *counters;
-#if CONFIG_CLANG_VERSION < 110000
- const char *function_name;
-#endif
};

static struct gcov_info *current_info;
@@ -107,16 +104,6 @@ void llvm_gcov_init(llvm_gcov_callback writeout, llvm_gcov_callback flush)
}
EXPORT_SYMBOL(llvm_gcov_init);

-#if CONFIG_CLANG_VERSION < 110000
-void llvm_gcda_start_file(const char *orig_filename, const char version[4],
- u32 checksum)
-{
- current_info->filename = orig_filename;
- memcpy(&current_info->version, version, sizeof(current_info->version));
- current_info->checksum = checksum;
-}
-EXPORT_SYMBOL(llvm_gcda_start_file);
-#else

void llvm_gcda_start_file(const char *orig_filename, u32 version, u32 checksum)

{
current_info->filename = orig_filename;
@@ -124,29 +111,7 @@ void llvm_gcda_start_file(const char *orig_filename, u32 version, u32 checksum)

current_info->checksum = checksum;
}
EXPORT_SYMBOL(llvm_gcda_start_file);

-#endif
-
-#if CONFIG_CLANG_VERSION < 110000
-void llvm_gcda_emit_function(u32 ident, const char *function_name,
- u32 func_checksum, u8 use_extra_checksum, u32 cfg_checksum)
-{
- struct gcov_fn_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
-
- if (!info)
- return;

- INIT_LIST_HEAD(&info->head);
- info->ident = ident;
- info->checksum = func_checksum;
- info->use_extra_checksum = use_extra_checksum;
- info->cfg_checksum = cfg_checksum;
- if (function_name)
- info->function_name = kstrdup(function_name, GFP_KERNEL);
-
- list_add_tail(&info->head, &current_info->functions);
-}
-EXPORT_SYMBOL(llvm_gcda_emit_function);
-#else
void llvm_gcda_emit_function(u32 ident, u32 func_checksum,
u8 use_extra_checksum, u32 cfg_checksum)
{
@@ -163,7 +128,6 @@ void llvm_gcda_emit_function(u32 ident, u32 func_checksum,

list_add_tail(&info->head, &current_info->functions);
}
EXPORT_SYMBOL(llvm_gcda_emit_function);

-#endif

void llvm_gcda_emit_arcs(u32 num_counters, u64 *counters)
{

@@ -326,7 +290,6 @@ void gcov_info_add(struct gcov_info *dst, struct gcov_info *src)
}
}

-#if CONFIG_CLANG_VERSION < 110000

static struct gcov_fn_info *gcov_fn_info_dup(struct gcov_fn_info *fn)
{
size_t cv_size; /* counter values size */

@@ -335,47 +298,15 @@ static struct gcov_fn_info *gcov_fn_info_dup(struct gcov_fn_info *fn)
if (!fn_dup)
return NULL;
INIT_LIST_HEAD(&fn_dup->head);
-
- fn_dup->function_name = kstrdup(fn->function_name, GFP_KERNEL);
- if (!fn_dup->function_name)
- goto err_name;
-
- cv_size = fn->num_counters * sizeof(fn->counters[0]);
- fn_dup->counters = vmalloc(cv_size);
- if (!fn_dup->counters)
- goto err_counters;
- memcpy(fn_dup->counters, fn->counters, cv_size);
-
- return fn_dup;
-
-err_counters:
- kfree(fn_dup->function_name);
-err_name:
- kfree(fn_dup);
- return NULL;
-}
-#else
-static struct gcov_fn_info *gcov_fn_info_dup(struct gcov_fn_info *fn)
-{
- size_t cv_size; /* counter values size */
- struct gcov_fn_info *fn_dup = kmemdup(fn, sizeof(*fn),
- GFP_KERNEL);
- if (!fn_dup)
- return NULL;
- INIT_LIST_HEAD(&fn_dup->head);
-

cv_size = fn->num_counters * sizeof(fn->counters[0]);

fn_dup->counters = vmalloc(cv_size);
if (!fn_dup->counters) {
kfree(fn_dup);
return NULL;
}
-

memcpy(fn_dup->counters, fn->counters, cv_size);

-
return fn_dup;
}
-#endif

/**
* gcov_info_dup - duplicate profiling data set

@@ -416,21 +347,6 @@ struct gcov_info *gcov_info_dup(struct gcov_info *info)

* gcov_info_free - release memory for profiling data set duplicate
* @info: profiling data set duplicate to free
*/

-#if CONFIG_CLANG_VERSION < 110000
-void gcov_info_free(struct gcov_info *info)
-{
- struct gcov_fn_info *fn, *tmp;
-
- list_for_each_entry_safe(fn, tmp, &info->functions, head) {
- kfree(fn->function_name);
- vfree(fn->counters);
- list_del(&fn->head);
- kfree(fn);
- }
- kfree(info->filename);
- kfree(info);
-}
-#else

void gcov_info_free(struct gcov_info *info)
{
struct gcov_fn_info *fn, *tmp;

@@ -443,7 +359,6 @@ void gcov_info_free(struct gcov_info *info)
kfree(info->filename);
kfree(info);
}
-#endif

#define ITER_STRIDE PAGE_SIZE

--
2.31.0.rc2.261.g7f71774620-goog

[Peter Oberparleiter]

On 12.03.2021 23:41, Nick Desaulniers wrote:
> LLVM changed the expected function signatures for llvm_gcda_start_file()
> and llvm_gcda_emit_function() in the clang-11 release. Users of clang-11
> or newer may have noticed their kernels failing to boot due to a panic
> when enabling CONFIG_GCOV_KERNEL=y +CONFIG_GCOV_PROFILE_ALL=y. Fix up
> the function signatures so calling these functions doesn't panic the
> kernel.
>
> Link: https://reviews.llvm.org/rGcdd683b516d147925212724b09ec6fb792a40041
> Link: https://reviews.llvm.org/rG13a633b438b6500ecad9e4f936ebadf3411d0f44
> Cc: sta...@vger.kernel.org # 5.4
> Reported-by: Prasad Sodagudi <psod...@quicinc.com>
> Suggested-by: Nathan Chancellor <nat...@kernel.org>
> Reviewed-by: Fangrui Song <mas...@google.com>
> Signed-off-by: Nick Desaulniers <ndesau...@google.com>
> Tested-by: Nathan Chancellor <nat...@kernel.org>

Looks good to me (minus the code duplication - but that's IMO acceptable
since it's cleaned up again with patch 2).

Acked-by: Peter Oberparleiter <obe...@linux.ibm.com>

That said, I'm currently thinking of adding a compile time check that
performs a dry-run gcov_info => gcda conversion in user space to detect
these kind of issues before kernels fail unpredictably [1]. I'm
confident that this could work for the GCC gcov kernel code, not sure
about the Clang version though. But if it's possible I guess it would
make sense to extend this to include the Clang code as well.

Note that this check wouldn't work for cross-compiles since the build
machine must be able to run code for the target machine.

[1]
https://lore.kernel.org/lkml/1c7a49e7-0e27-561b...@linux.ibm.com/


Regards,
Peter

--
Peter Oberparleiter
Linux on Z Development - IBM Germany

[Peter Oberparleiter]

On 12.03.2021 23:41, Nick Desaulniers wrote:

> LLVM changed the expected function signatures for llvm_gcda_start_file()
> and llvm_gcda_emit_function() in the clang-11 release. Drop the older
> implementations and require folks to upgrade their compiler if they're
> interested in GCOV support.
>
> Link: https://reviews.llvm.org/rGcdd683b516d147925212724b09ec6fb792a40041
> Link: https://reviews.llvm.org/rG13a633b438b6500ecad9e4f936ebadf3411d0f44
> Suggested-by: Nathan Chancellor <nat...@kernel.org>
> Signed-off-by: Nick Desaulniers <ndesau...@google.com>

Acked-by: Peter Oberparleiter <obe...@linux.ibm.com>

[Nathan Chancellor]

On Fri, Mar 12, 2021 at 02:41:31PM -0800, Nick Desaulniers wrote:
> LLVM changed the expected function signatures for llvm_gcda_start_file()
> and llvm_gcda_emit_function() in the clang-11 release. Users of clang-11
> or newer may have noticed their kernels failing to boot due to a panic
> when enabling CONFIG_GCOV_KERNEL=y +CONFIG_GCOV_PROFILE_ALL=y. Fix up
> the function signatures so calling these functions doesn't panic the
> kernel.
>
> Link: https://reviews.llvm.org/rGcdd683b516d147925212724b09ec6fb792a40041
> Link: https://reviews.llvm.org/rG13a633b438b6500ecad9e4f936ebadf3411d0f44
> Cc: sta...@vger.kernel.org # 5.4
> Reported-by: Prasad Sodagudi <psod...@quicinc.com>
> Suggested-by: Nathan Chancellor <nat...@kernel.org>
> Reviewed-by: Fangrui Song <mas...@google.com>
> Signed-off-by: Nick Desaulniers <ndesau...@google.com>
> Tested-by: Nathan Chancellor <nat...@kernel.org>

Reviewed-by: Nathan Chancellor <nat...@kernel.org>

[Nathan Chancellor]

On Fri, Mar 12, 2021 at 02:41:32PM -0800, Nick Desaulniers wrote:
> LLVM changed the expected function signatures for llvm_gcda_start_file()
> and llvm_gcda_emit_function() in the clang-11 release. Drop the older
> implementations and require folks to upgrade their compiler if they're
> interested in GCOV support.
>
> Link: https://reviews.llvm.org/rGcdd683b516d147925212724b09ec6fb792a40041
> Link: https://reviews.llvm.org/rG13a633b438b6500ecad9e4f936ebadf3411d0f44
> Suggested-by: Nathan Chancellor <nat...@kernel.org>
> Signed-off-by: Nick Desaulniers <ndesau...@google.com>

Reviewed-by: Nathan Chancellor <nat...@kernel.org>