[RFC PATCH 0/2] kunit: Support redirecting function calls

[RFC PATCH 0/2] kunit: Support redirecting function calls

[David Gow]

When writing tests, it'd often be very useful to be able to intercept
calls to a function in the code being tested and replace it with a
test-specific stub. This has always been an obviously missing piece of
KUnit, and the solutions always involve some tradeoffs with cleanliness,
performance, or impact on non-test code. See the folowing document for
some of the challenges:
https://kunit.dev/mocking.html

This series consists of two prototype patches which add support for this
sort of redirection to KUnit tests:

1: static_stub: Any function which might want to be intercepted adds a
call to a macro which checks if a test has redirected calls to it, and
calls the corresponding replacement.

2: ftrace_stub: Functions are intercepted using ftrace and livepatch.
This doesn't require adding a new prologue to each function being
replaced, but does have more dependencies (which restricts it to a small
number of architectures, not including UML), and doesn't work well with
inline functions.

The API for both implementations is very similar, so it should be easy
to migrate from one to the other if necessary.  Both of these
implementations restrict the redirection to the test context: it is
automatically undone after the KUnit test completes, and does not affect
calls in other threads. If CONFIG_KUNIT is not enabled, there should be
no overhead in either implementation.

Does either (or both) of these features sound useful, and is this
sort-of API the right model? (Personally, I think there's a reasonable
scope for both.) Is anything obviously missing or wrong? Do the names,
descriptions etc. make any sense?

Note that these patches are definitely still at the "prototype" level,
and things like error-handling, documentation, and testing are still
pretty sparse. There is also quite a bit of room for optimisation.
These'll all be improved for v1 if the concept seems good.

Cheers,
-- David

Daniel Latypov (1):
  kunit: expose ftrace-based API for stubbing out functions during tests

David Gow (1):
  kunit: Expose 'static stub' API to redirect functions

 include/kunit/ftrace_stub.h         |  84 +++++++++++++++++
 include/kunit/static_stub.h         | 106 +++++++++++++++++++++
 lib/kunit/Kconfig                   |  11 +++
 lib/kunit/Makefile                  |   5 +
 lib/kunit/ftrace_stub.c             | 138 ++++++++++++++++++++++++++++
 lib/kunit/kunit-example-test.c      |  64 +++++++++++++
 lib/kunit/static_stub.c             | 125 +++++++++++++++++++++++++
 lib/kunit/stubs_example.kunitconfig |  11 +++
 8 files changed, 544 insertions(+)
 create mode 100644 include/kunit/ftrace_stub.h
 create mode 100644 include/kunit/static_stub.h
 create mode 100644 lib/kunit/ftrace_stub.c
 create mode 100644 lib/kunit/static_stub.c
 create mode 100644 lib/kunit/stubs_example.kunitconfig

-- 
2.35.1.894.gb6a874cedc-goog

[RFC PATCH 1/2] kunit: Expose 'static stub' API to redirect functions

[David Gow]

Add a simple way of redirecting calls to functions by including a
special prologue in the "real" function which checks to see if the
replacement function should be called (and, if so, calls it).

To redirect calls to a function, make the first (non-declaration) line
of the function:

	KUNIT_STATIC_STUB_REDIRECT(function_name, [function arguments]);

(This will compile away to nothing if KUnit is not enabled, otherwise it
will check if a redirection is active, call the replacement function,
and return.)

Calls to the real function can be redirected to a replacement using:

	kunit_activate_static_stub(test, real_fn, replacement_fn);

The redirection will only affect calls made from within the kthread of
the current test, and will be automatically disabled when the test
completes. It can also be manually disabled with
kunit_deactivate_static_stub().

The 'example' KUnit test suite has a more complete example.

This is intended to be an alternative to ftrace-based stubbing (see the
next patch), with different tradeoffs.

In particular:
- 'static stubs' work on all architectures, and don't have any
  dependencies.
  - There's no separate Kconfig option to enable them, they always
    exist.
- They must be explicitly opted-into by the function being replaced
  (which can be good or bad depending on your POV)
- They have a greater performance penalty when not in active use (as
  every call to the real function must check to see if the stub is
  enabled)
- They could more easily be extended to pass additional context to
  replacement functions.

Co-developed-by: Daniel Latypov <dlatypov@google.com>
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Signed-off-by: David Gow <davidgow@google.com>
---
 include/kunit/static_stub.h    | 106 ++++++++++++++++++++++++++++
 lib/kunit/Makefile             |   1 +
 lib/kunit/kunit-example-test.c |  39 ++++++++++
 lib/kunit/static_stub.c        | 125 +++++++++++++++++++++++++++++++++
 4 files changed, 271 insertions(+)
 create mode 100644 include/kunit/static_stub.h
 create mode 100644 lib/kunit/static_stub.c

diff --git a/include/kunit/static_stub.h b/include/kunit/static_stub.h
new file mode 100644
index 000000000000..020e0e9110f2
--- /dev/null
+++ b/include/kunit/static_stub.h
@@ -0,0 +1,106 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * KUnit function redirection (static stubbing) API.
+ *
+ * Copyright (C) 2022, Google LLC.
+ * Author: David Gow <davidgow@google.com>
+ */
+#ifndef _KUNIT_STATIC_STUB_H
+#define _KUNIT_STATIC_STUB_H
+
+#if !IS_REACHABLE(CONFIG_KUNIT)
+
+/* If CONFIG_KUNIT is not enabled, these stubs quietly disappear. */
+#define KUNIT_TRIGGER_STATIC_STUB(real_fn_name, args...) do {} while (0)
+
+#else
+
+#include <kunit/test.h>
+
+#include <linux/compiler.h> /* for {un,}likely() */
+#include <linux/sched.h> /* for task_struct */
+
+/* Returns the address of the replacement function. */
+void *__kunit_get_static_stub_address(struct kunit *test, void *real_fn_addr);
+
+/**
+ * KUNIT_STATIC_STUB_REDIRECT() - call a replacement 'static stub' if one exists
+ * @real_fn_name: The name of this function (as an identifier, not a string)
+ * @args: All of the arguments passed to this function
+ *
+ * This is a function prologue which is used to allow calls to the current
+ * function to be redirected by a KUnit test. KUnit tests can call
+ * kunit_activate_static_stub() to pass a replacement function in. The
+ * replacement function will be called by KUNIT_TRIGGER_STATIC_STUB(), which
+ * will then return from the function. If the caller is not in a KUnit context,
+ * the function will continue execution as normal.
+ *
+ * Example:
+ *
+ * .. code-block:: c
+ *	int real_func(int n)
+ *	{
+ *		KUNIT_STATIC_STUB_REDIRECT(real_func, n);
+ *		return 0;
+ *	}
+ *
+ *	void replacement_func(int n)
+ *	{
+ *		return 42;
+ *	}
+ *
+ *	void example_test(struct kunit *test)
+ *	{
+ *		kunit_activate_static_stub(test, real_func, replacement_func);
+ *		KUNIT_EXPECT_EQ(test, real_func(1), 42);
+ *	}
+ *
+ */
+#define KUNIT_STATIC_STUB_REDIRECT(real_fn_name, args...)		\
+do {									\
+	typeof(&real_fn_name) replacement;				\
+	struct kunit *current_test = current->kunit_test;		\
+									\
+	if (likely(!current_test))					\
+		break;							\
+									\
+	KUNIT_ASSERT_STREQ_MSG(current_test, __func__, #real_fn_name,	\
+			       "Static stub function name mismatch");	\
+									\
+	replacement = __kunit_get_static_stub_address(current_test,	\
+							&real_fn_name);	\
+									\
+	if (unlikely(replacement))					\
+		return replacement(args);				\
+} while (0)
+
+/* Helper function for kunit_activate_static_stub(). The macro does
+ * typechecking, so use it instead.
+ */
+void __kunit_activate_static_stub(struct kunit *test,
+				  void *real_fn_addr,
+				  void *replacement_addr);
+
+/**
+ * kunit_activate_static_stub() - replace a function using static stubs.
+ * @test: A pointer to the 'struct kunit' test context for the current test.
+ * @real_fn_addr: The address of the function to replace.
+ * @replacement_addr: The address of the function to replace it with.
+ *
+ * When activated, calls to real_fn_addr from within this test (even if called
+ * indirectly) will instead call replacement_addr. The function pointed to by
+ * real_fn_addr must begin with the static stub prologue in
+ * KUNIT_TRIGGER_STATIC_STUB() for this to work. real_fn_addr and
+ * replacement_addr must have the same type.
+ *
+ * The redirection can be disabled again with kunit_deactivate_static_stub().
+ */
+#define kunit_activate_static_stub(test, real_fn_addr, replacement_addr) do {	\
+	typecheck(typeof(real_fn_addr), replacement_addr);			\
+	__kunit_activate_static_stub(test, real_fn_addr, replacement_addr);	\
+} while (0)
+
+void kunit_deactivate_static_stub(struct kunit *test, void *real_fn_addr);
+
+#endif
+#endif
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index c49f4ffb6273..f9e929700782 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -1,6 +1,7 @@
 obj-$(CONFIG_KUNIT) +=			kunit.o
 
 kunit-objs +=				test.o \
+					static_stub.o \
 					string-stream.o \
 					assert.o \
 					try-catch.o \
diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
index 51099b0ca29c..670c21e74446 100644
--- a/lib/kunit/kunit-example-test.c
+++ b/lib/kunit/kunit-example-test.c
@@ -7,6 +7,7 @@
  */
 
 #include <kunit/test.h>
+#include <kunit/static_stub.h>
 
 /*
  * This is the most fundamental element of KUnit, the test case. A test case
@@ -69,6 +70,43 @@ static void example_mark_skipped_test(struct kunit *test)
 	/* This line should run */
 	kunit_info(test, "You should see this line.");
 }
+
+/* This is a function we'll replace with static stubs. */
+static int add_one(int i)
+{
+	/* This will trigger the stub if active. */
+	KUNIT_STATIC_STUB_REDIRECT(add_one, i);
+
+	return i + 1;
+}
+
+/* This is used as a replacement for the above function. */
+static int subtract_one(int i)
+{
+	/* We don't need to trigger the stub from the replacement. */
+
+	return i - 1;
+}
+
+/*
+ * This test shows the use of static stubs.
+ */
+static void example_static_stub_test(struct kunit *test)
+{
+	/* By default, function is not stubbed. */
+	KUNIT_EXPECT_EQ(test, add_one(1), 2);
+
+	/* Replace add_one() with subtract_one(). */
+	kunit_activate_static_stub(test, add_one, subtract_one);
+
+	/* add_one() is now replaced. */
+	KUNIT_EXPECT_EQ(test, add_one(1), 0);
+
+	/* Return add_one() to normal. */
+	kunit_deactivate_static_stub(test, add_one);
+	KUNIT_EXPECT_EQ(test, add_one(1), 2);
+}
+
 /*
  * Here we make a list of all the test cases we want to add to the test suite
  * below.
@@ -83,6 +121,7 @@ static struct kunit_case example_test_cases[] = {
 	KUNIT_CASE(example_simple_test),
 	KUNIT_CASE(example_skip_test),
 	KUNIT_CASE(example_mark_skipped_test),
+	KUNIT_CASE(example_static_stub_test),
 	{}
 };
 
diff --git a/lib/kunit/static_stub.c b/lib/kunit/static_stub.c
new file mode 100644
index 000000000000..b06b88f82dd7
--- /dev/null
+++ b/lib/kunit/static_stub.c
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit function redirection (static stubbing) API.
+ *
+ * Copyright (C) 2022, Google LLC.
+ * Author: David Gow <davidgow@google.com>
+ */
+
+#include <kunit/test.h>
+#include <kunit/static_stub.h>
+
+
+/* Context for a static stub. This is stored in the resource data. */
+struct kunit_static_stub_ctx {
+	void *real_fn_addr;
+	void *replacement_addr;
+};
+
+static void __kunit_static_stub_resource_free(struct kunit_resource *res)
+{
+	kfree(res->data);
+}
+
+/* Matching function for kunit_find_resource(). match_data is real_fn_addr. */
+static bool __kunit_static_stub_resource_match(struct kunit *test,
+						struct kunit_resource *res,
+						void *match_real_fn_addr)
+{
+	/* This pointer is only valid if res is a static stub resource. */
+	struct kunit_static_stub_ctx *ctx = res->data;
+
+	/* Make sure the resource is a static stub resource. */
+	if (res->free != &__kunit_static_stub_resource_free)
+		return false;
+
+	return ctx->real_fn_addr == match_real_fn_addr;
+}
+
+/* Returns the address of the replacement function. */
+void *__kunit_get_static_stub_address(struct kunit *test, void *real_fn_addr)
+{
+	struct kunit_resource *res;
+	struct kunit_static_stub_ctx *ctx;
+	void *replacement_addr;
+
+	res = kunit_find_resource(test,
+				  __kunit_static_stub_resource_match,
+				  real_fn_addr);
+
+	if (!res)
+		return NULL;
+
+	ctx = res->data;
+	replacement_addr = ctx->replacement_addr;
+	kunit_put_resource(res);
+	return replacement_addr;
+}
+EXPORT_SYMBOL_GPL(__kunit_get_static_stub_address);
+
+void kunit_deactivate_static_stub(struct kunit *test, void *real_fn_addr)
+{
+	struct kunit_resource *res;
+
+	KUNIT_ASSERT_PTR_NE_MSG(test, real_fn_addr, NULL,
+				"Tried to deactivate a NULL stub.");
+
+	/* Look up the existing stub for this function. */
+	res = kunit_find_resource(test,
+				  __kunit_static_stub_resource_match,
+				  real_fn_addr);
+
+	/* Error out if the stub doesn't exist. */
+	KUNIT_ASSERT_PTR_NE_MSG(test, res, NULL,
+				"Tried to deactivate a nonexistent stub.");
+
+	/* Free the stub. We 'put' twice, as we got a reference
+	 * from kunit_find_resource()
+	 */
+	kunit_remove_resource(test, res);
+	kunit_put_resource(res);
+}
+EXPORT_SYMBOL_GPL(kunit_deactivate_static_stub);
+
+/* Helper function for kunit_activate_static_stub(). The macro does
+ * typechecking, so use it instead.
+ */
+void __kunit_activate_static_stub(struct kunit *test,
+				  void *real_fn_addr,
+				  void *replacement_addr)
+{
+	struct kunit_static_stub_ctx *ctx;
+	struct kunit_resource *res;
+
+	KUNIT_ASSERT_PTR_NE_MSG(test, real_fn_addr, NULL,
+				"Tried to activate a stub for function NULL");
+
+	/* If the replacement address is NULL, deactivate the stub. */
+	if (!replacement_addr) {
+		kunit_deactivate_static_stub(test, replacement_addr);
+		return;
+	}
+
+	/* Look up any existing stubs for this function, and replace them. */
+	res = kunit_find_resource(test,
+				  __kunit_static_stub_resource_match,
+				  real_fn_addr);
+	if (res) {
+		ctx = res->data;
+		ctx->replacement_addr = replacement_addr;
+
+		/* We got an extra reference from find_resource(), so put it. */
+		kunit_put_resource(res);
+	} else {
+		ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
+		KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
+		ctx->real_fn_addr = real_fn_addr;
+		ctx->replacement_addr = replacement_addr;
+		res = kunit_alloc_resource(test, NULL,
+				     &__kunit_static_stub_resource_free,
+				     GFP_KERNEL, ctx);
+	}
+}
+EXPORT_SYMBOL_GPL(__kunit_activate_static_stub);
+
+
-- 
2.35.1.894.gb6a874cedc-goog

[RFC PATCH 2/2] kunit: expose ftrace-based API for stubbing out functions during tests

[David Gow]

From: Daniel Latypov <dlatypov@google.com>

Allow function redirection using ftrace and kernel livepatch. This is
basically equivalent to the static_stub support in the previous patch,
but does not require the function being replaced to be modified (save
for the addition of KUNIT_STUBBABLE/noinline).

This is hidden behind the CONFIG_KUNIT_FTRACE_STUBS option, and has a
number of dependencies, including ftrace, livepatch and
CONFIG_KALLSYMS_ALL. As a result, it only works on architectures where
these are available.

You can run the KUnit example tests with the following:
$ ./tools/testing/kunit/kunit.py run --kunitconfig lib/kunit/stubs_example.kunitconfig --arch=x86_64

To the end user, replacing a function is very simple, e.g.
  KUNIT_STUBBABLE void real_func(int n);
  void replacement_func(int n);

  /* in tests */
  kunit_activate_ftrace_stub(test, real_func, replacement_func);

The implementation is inspired by Steven's snippet here [1].

Some more details:
* stubbing is automatically undone at the end of tests
* it can also be manually undone with kunit_deactive_ftrace_stub()
* stubbing only applies when current->kunit_test == test
  * note: currently can't have more than one test running at a time
* KUNIT_STUBBABLE marks tests as noinline when CONFIG_KUNIT_STUBS is set
  * this ensures we can actually stub all calls
* KUNIT_STUBBABLE_TRAMPOLINE is a version that evaluates to
  __always_inline when stubbing is not enabled
  * This may need to be used with a wrapper function.
  * See the doc comment for more details.

Sharp-edges:
* kernel livepatch only works on some arches (not UML)
* if you don't use noinline/KUNIT_STUBBABLE, functions might be inlined
  and thus none of this works:
  * if it's always inlined, at least the attempt to stub will fail
  * if it's sometimes inlined, then the stub silently won't work

[1] https://lore.kernel.org/lkml/20220224091550.2b7e8784@gandalf.local.home

Co-developed-by: David Gow <davidgow@google.com>
Signed-off-by: David Gow <davidgow@google.com>
Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
 include/kunit/ftrace_stub.h         |  84 +++++++++++++++++
 lib/kunit/Kconfig                   |  11 +++
 lib/kunit/Makefile                  |   4 +
 lib/kunit/ftrace_stub.c             | 138 ++++++++++++++++++++++++++++
 lib/kunit/kunit-example-test.c      |  27 +++++-
 lib/kunit/stubs_example.kunitconfig |  11 +++
 6 files changed, 274 insertions(+), 1 deletion(-)
 create mode 100644 include/kunit/ftrace_stub.h
 create mode 100644 lib/kunit/ftrace_stub.c
 create mode 100644 lib/kunit/stubs_example.kunitconfig

diff --git a/include/kunit/ftrace_stub.h b/include/kunit/ftrace_stub.h
new file mode 100644
index 000000000000..54c053b7e9c1
--- /dev/null
+++ b/include/kunit/ftrace_stub.h
@@ -0,0 +1,84 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _KUNIT_FTRACE_STUB_H
+#define _KUNIT_FTRACE_STUB_H
+
+/** KUNIT_STUBBABLE - marks a function as stubbable when stubbing support is
+ * enabled.
+ *
+ * Stubbing uses ftrace internally, so we can only stub out functions when they
+ * are not inlined. This macro eavlautes to noinline when stubbing support is
+ * enabled to thus make it safe.
+ *
+ * If you cannot add this annotation to the function, you can instead use
+ * KUNIT_STUBBABLE_TRAMPOLINE, which is the same, but evaluates to
+ * __always_inline when stubbing is not enabled.
+ *
+ * Consider copy_to_user, which is marked as __always_inline:
+ *
+ * .. code-block:: c
+ *	static KUNIT_STUBBABLE_TRAMPOLINE unsigned long
+ *	copy_to_user_trampoline(void __user *to, const void *from, unsigned long n)
+ *	{
+ *		return copy_to_user(to, from, n);
+ *	}
+ *
+ * Then we simply need to update our code to go through this function instead
+ * (in the places where we want to stub it out).
+ */
+#if IS_ENABLED(CONFIG_KUNIT_FTRACE_STUBS)
+#define KUNIT_STUBBABLE noinline
+#define KUNIT_STUBBABLE_TRAMPOLINE noinline
+#else
+#define KUNIT_STUBBABLE
+#define KUNIT_STUBBABLE_TRAMPOLINE __always_inline
+#endif
+
+struct kunit;
+
+/**
+ * kunit_activate_ftrace_stub() - makes all calls to @func go to @replacement during @test.
+ * @test: The test context object.
+ * @func: The function to stub out, must be annotated with KUNIT_STUBBABLE.
+ * @replacement: The function to replace @func with.
+ *
+ * All calls to @func will instead call @replacement for the duration of the
+ * current test. If called from outside the test's thread, the function will
+ * not be redirected.
+ *
+ * The redirection can be disabled again with kunit_deactivate_ftrace_stub().
+ *
+ * Example:
+ *
+ * .. code-block:: c
+ *	KUNIT_STUBBABLE int real_func(int n)
+ *	{
+ *		pr_info("real_func() called with %d", n);
+ *		return 0;
+ *	}
+ *
+ *	void replacement_func(int n)
+ *	{
+ *		pr_info("replacement_func() called with %d", n);
+ *		return 42;
+ *	}
+ *
+ *	void example_test(struct kunit *test)
+ *	{
+ *		kunit_active_ftrace_stub(test, real_func, replacement_func);
+ *		KUNIT_EXPECT_EQ(test, real_func(1), 42);
+ *	}
+ *
+ */
+#define kunit_activate_ftrace_stub(test, func, replacement) do { \
+	typecheck(typeof(func), replacement); \
+	__kunit_activate_ftrace_stub(test, #func, func, replacement); \
+} while (0)
+
+void __kunit_activate_ftrace_stub(struct kunit *test,
+				  const char *name,
+				  void *real_fn_addr,
+				  void *replacement_addr);
+
+
+void kunit_deactivate_ftrace_stub(struct kunit *test, void *real_fn_addr);
+#endif  /* _KUNIT_STUB_H */
diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig
index 0b5dfb001bac..978e4f72bae0 100644
--- a/lib/kunit/Kconfig
+++ b/lib/kunit/Kconfig
@@ -59,4 +59,15 @@ config KUNIT_ALL_TESTS
 
 	  If unsure, say N.
 
+config KUNIT_FTRACE_STUBS
+	bool "Support for stubbing out functions in KUnit tests with ftrace and kernel livepatch"
+	depends on FTRACE=y && FUNCTION_TRACER=y && MODULES=y && DEBUG_KERNEL=y && KALLSYMS_ALL=y && LIVEPATCH=y
+	help
+	  Builds support for stubbing out functions for the duration of KUnit
+	  test cases or suites using ftrace and kernel livepatch.
+	  See KUNIT_EXAMPLE_TEST for an example.
+
+	  NOTE: this does not work on all architectures (like UML, arm64) and
+	  relies on a lot of magic (see the dependencies list).
+
 endif # KUNIT
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index f9e929700782..75092c12c3d0 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -19,3 +19,7 @@ obj-$(CONFIG_KUNIT_TEST) +=		string-stream-test.o
 endif
 
 obj-$(CONFIG_KUNIT_EXAMPLE_TEST) +=	kunit-example-test.o
+
+ifeq ($(CONFIG_KUNIT_FTRACE_STUBS),y)
+kunit-objs +=				ftrace_stub.o
+endif
diff --git a/lib/kunit/ftrace_stub.c b/lib/kunit/ftrace_stub.c
new file mode 100644
index 000000000000..13207e1c7aff
--- /dev/null
+++ b/lib/kunit/ftrace_stub.c
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <kunit/test.h>
+
+#include <linux/typecheck.h>
+
+#include <linux/ftrace.h>
+#include <linux/livepatch.h>
+#include <linux/sched.h>
+
+struct kunit_ftrace_stub_ctx {
+	struct kunit *test;
+	unsigned long real_fn_addr; /* used as a key to lookup the stub */
+	unsigned long replacement_addr;
+	struct ftrace_ops ops; /* a copy of kunit_stub_base_ops with .private set */
+};
+
+static void kunit_stub_trampoline(unsigned long ip, unsigned long parent_ip,
+				  struct ftrace_ops *ops,
+				  struct ftrace_regs *fregs)
+{
+	struct kunit_ftrace_stub_ctx *ctx = ops->private;
+	int lock_bit;
+
+	if (current->kunit_test != ctx->test)
+		return;
+
+	lock_bit = ftrace_test_recursion_trylock(ip, parent_ip);
+	KUNIT_ASSERT_GE(ctx->test, lock_bit, 0);
+
+	klp_arch_set_pc(fregs, ctx->replacement_addr);
+
+	ftrace_test_recursion_unlock(lock_bit);
+}
+
+static struct ftrace_ops kunit_stub_base_ops = {
+	.func = &kunit_stub_trampoline,
+	.flags = FTRACE_OPS_FL_IPMODIFY |
+#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
+		FTRACE_OPS_FL_SAVE_REGS |
+#endif
+		FTRACE_OPS_FL_DYNAMIC
+};
+
+static void __kunit_ftrace_stub_resource_free(struct kunit_resource *res)
+{
+	struct kunit_ftrace_stub_ctx *ctx = res->data;
+
+	unregister_ftrace_function(&ctx->ops);
+	kfree(ctx);
+}
+
+/* Matching function for kunit_find_resource(). match_data is real_fn_addr. */
+static bool __kunit_static_stub_resource_match(struct kunit *test,
+						struct kunit_resource *res,
+						void *match_real_fn_addr)
+{
+	/* This pointer is only valid if res is a static stub resource. */
+	struct kunit_ftrace_stub_ctx *ctx = res->data;
+
+	/* Make sure the resource is a static stub resource. */
+	if (res->free != &__kunit_ftrace_stub_resource_free)
+		return false;
+
+	return ctx->real_fn_addr == (unsigned long)match_real_fn_addr;
+}
+
+void kunit_deactivate_ftrace_stub(struct kunit *test, void *real_fn_addr)
+{
+	struct kunit_resource *res;
+
+	KUNIT_ASSERT_PTR_NE_MSG(test, real_fn_addr, NULL,
+				"Tried to deactivate a NULL stub.");
+
+	/* Look up the existing stub for this function. */
+	res = kunit_find_resource(test,
+				  __kunit_static_stub_resource_match,
+				  real_fn_addr);
+
+	/* Error out if the stub doesn't exist. */
+	KUNIT_ASSERT_PTR_NE_MSG(test, res, NULL,
+				"Tried to deactivate a nonexistent stub.");
+
+	/* Free the stub. We 'put' twice, as we got a reference
+	 * from kunit_find_resource(). The free function will deactivate the
+	 * ftrace stub.
+	 */
+	kunit_remove_resource(test, res);
+	kunit_put_resource(res);
+}
+EXPORT_SYMBOL_GPL(kunit_deactivate_ftrace_stub);
+
+void __kunit_activate_ftrace_stub(struct kunit *test,
+				  const char *name,
+				  void *real_fn_addr,
+				  void *replacement_addr)
+{
+	unsigned long ftrace_ip;
+	struct kunit_ftrace_stub_ctx *ctx;
+	int ret;
+
+	ftrace_ip = ftrace_location((unsigned long)real_fn_addr);
+	if (!ftrace_ip)
+		KUNIT_ASSERT_FAILURE(test, "%s ip is invalid: not a function, or is marked notrace or inline", name);
+
+	/* Allocate the stub context, which contains pointers to the replacement
+	 * function and the test object. It's also registered as a KUnit
+	 * resource which can be looked up by address (to deactivate manually)
+	 * and is destroyed automatically on test exit.
+	 */
+	ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL_MSG(test, ctx, "failed to allocate kunit stub for %s", name);
+
+	ctx->test = test;
+	ctx->ops = kunit_stub_base_ops;
+	ctx->ops.private = ctx;
+	ctx->real_fn_addr = (unsigned long)real_fn_addr;
+	ctx->replacement_addr = (unsigned long)replacement_addr;
+
+	ret = ftrace_set_filter_ip(&ctx->ops, ftrace_ip, 0, 0);
+	if (ret) {
+		kfree(ctx);
+		KUNIT_ASSERT_FAILURE(test, "failed to set filter ip for %s: %d", name, ret);
+	}
+
+	ret = register_ftrace_function(&ctx->ops);
+	if (ret) {
+		kfree(ctx);
+		if (ret == -EBUSY)
+			KUNIT_ASSERT_FAILURE(test, "failed to register stub (-EBUSY) for %s, likely due to already stubbing it?", name);
+		KUNIT_ASSERT_FAILURE(test, "failed to register stub for %s: %d", name, ret);
+	}
+
+	/* Register the stub as a resource with a cleanup function */
+	kunit_alloc_resource(test, NULL,
+			     __kunit_ftrace_stub_resource_free,
+			     GFP_KERNEL, ctx);
+}
+EXPORT_SYMBOL_GPL(__kunit_activate_ftrace_stub);
diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
index 670c21e74446..7f20b132343b 100644
--- a/lib/kunit/kunit-example-test.c
+++ b/lib/kunit/kunit-example-test.c
@@ -8,6 +8,7 @@
 
 #include <kunit/test.h>
 #include <kunit/static_stub.h>
+#include <kunit/ftrace_stub.h>
 
 /*
  * This is the most fundamental element of KUnit, the test case. A test case
@@ -72,7 +73,7 @@ static void example_mark_skipped_test(struct kunit *test)
 }
 
 /* This is a function we'll replace with static stubs. */
-static int add_one(int i)
+static KUNIT_STUBBABLE int add_one(int i)
 {
 	/* This will trigger the stub if active. */
 	KUNIT_STATIC_STUB_REDIRECT(add_one, i);
@@ -107,6 +108,29 @@ static void example_static_stub_test(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, add_one(1), 2);
 }
 
+/*
+ * This test shows the use of static stubs.
+ */
+static void example_ftrace_stub_test(struct kunit *test)
+{
+#if !IS_ENABLED(CONFIG_KUNIT_FTRACE_STUBS)
+	kunit_skip(test, "KUNIT_FTRACE_STUBS not enabled");
+#else
+	/* By default, function is not stubbed. */
+	KUNIT_EXPECT_EQ(test, add_one(1), 2);
+
+	/* Replace add_one() with subtract_one(). */
+	kunit_activate_ftrace_stub(test, add_one, subtract_one);
+
+	/* add_one() is now replaced. */
+	KUNIT_EXPECT_EQ(test, add_one(1), 0);
+
+	/* Return add_one() to normal. */
+	kunit_deactivate_ftrace_stub(test, add_one);
+	KUNIT_EXPECT_EQ(test, add_one(1), 2);
+#endif
+}
+
 /*
  * Here we make a list of all the test cases we want to add to the test suite
  * below.
@@ -122,6 +146,7 @@ static struct kunit_case example_test_cases[] = {
 	KUNIT_CASE(example_skip_test),
 	KUNIT_CASE(example_mark_skipped_test),
 	KUNIT_CASE(example_static_stub_test),
+	KUNIT_CASE(example_ftrace_stub_test),
 	{}
 };
 
diff --git a/lib/kunit/stubs_example.kunitconfig b/lib/kunit/stubs_example.kunitconfig
new file mode 100644
index 000000000000..a47369199fb9
--- /dev/null
+++ b/lib/kunit/stubs_example.kunitconfig
@@ -0,0 +1,11 @@
+CONFIG_KUNIT=y
+CONFIG_KUNIT_FTRACE_STUBS=y
+CONFIG_KUNIT_EXAMPLE_TEST=y
+
+# Depedencies
+CONFIG_FTRACE=y
+CONFIG_FUNCTION_TRACER=y
+CONFIG_MODULES=y
+CONFIG_DEBUG_KERNEL=y
+CONFIG_KALLSYMS_ALL=y
+CONFIG_LIVEPATCH=y
-- 
2.35.1.894.gb6a874cedc-goog

Re: [RFC PATCH 0/2] kunit: Support redirecting function calls

[Steven Rostedt]

On Fri, 18 Mar 2022 10:13:12 +0800
David Gow <davidgow@google.com> wrote:

> Does either (or both) of these features sound useful, and is this
> sort-of API the right model? (Personally, I think there's a reasonable
> scope for both.) Is anything obviously missing or wrong? Do the names,
> descriptions etc. make any sense?

Obviously I'm biased toward the ftrace solution ;-)

-- Steve

Re: [RFC PATCH 0/2] kunit: Support redirecting function calls

[Brendan Higgins]

On Fri, Mar 18, 2022 at 9:22 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Fri, 18 Mar 2022 10:13:12 +0800
> David Gow <davidgow@google.com> wrote:
>
> > Does either (or both) of these features sound useful, and is this
> > sort-of API the right model? (Personally, I think there's a reasonable
> > scope for both.) Is anything obviously missing or wrong? Do the names,
> > descriptions etc. make any sense?
>
> Obviously I'm biased toward the ftrace solution ;-)

Personally, I like providing both - as long as we can keep the
interface the same.

Ftrace is less visually invasive, but it is also less flexible in
capabilities, and requires substantial work to support on new
architectures.

Re: [RFC PATCH 0/2] kunit: Support redirecting function calls

[Brendan Higgins]

+Steve Muckle - since I think this might affect things he is working on.

On Thu, Mar 17, 2022 at 10:13 PM David Gow <davidgow@google.com> wrote:
>
> When writing tests, it'd often be very useful to be able to intercept
> calls to a function in the code being tested and replace it with a
> test-specific stub. This has always been an obviously missing piece of
> KUnit, and the solutions always involve some tradeoffs with cleanliness,
> performance, or impact on non-test code. See the folowing document for
> some of the challenges:
> https://kunit.dev/mocking.html
>
> This series consists of two prototype patches which add support for this
> sort of redirection to KUnit tests:
>
> 1: static_stub: Any function which might want to be intercepted adds a
> call to a macro which checks if a test has redirected calls to it, and
> calls the corresponding replacement.
>
> 2: ftrace_stub: Functions are intercepted using ftrace and livepatch.
> This doesn't require adding a new prologue to each function being
> replaced, but does have more dependencies (which restricts it to a small
> number of architectures, not including UML), and doesn't work well with
> inline functions.
>
> The API for both implementations is very similar, so it should be easy
> to migrate from one to the other if necessary.  Both of these
> implementations restrict the redirection to the test context: it is
> automatically undone after the KUnit test completes, and does not affect
> calls in other threads. If CONFIG_KUNIT is not enabled, there should be
> no overhead in either implementation.
>
> Does either (or both) of these features sound useful, and is this
> sort-of API the right model? (Personally, I think there's a reasonable
> scope for both.) Is anything obviously missing or wrong? Do the names,
> descriptions etc. make any sense?
>
> Note that these patches are definitely still at the "prototype" level,
> and things like error-handling, documentation, and testing are still
> pretty sparse. There is also quite a bit of room for optimisation.
> These'll all be improved for v1 if the concept seems good.
>
> Cheers,
> -- David
>
> Daniel Latypov (1):
>   kunit: expose ftrace-based API for stubbing out functions during tests
>
> David Gow (1):
>   kunit: Expose 'static stub' API to redirect functions
>
>  include/kunit/ftrace_stub.h         |  84 +++++++++++++++++
>  include/kunit/static_stub.h         | 106 +++++++++++++++++++++
>  lib/kunit/Kconfig                   |  11 +++
>  lib/kunit/Makefile                  |   5 +
>  lib/kunit/ftrace_stub.c             | 138 ++++++++++++++++++++++++++++
>  lib/kunit/kunit-example-test.c      |  64 +++++++++++++
>  lib/kunit/static_stub.c             | 125 +++++++++++++++++++++++++
>  lib/kunit/stubs_example.kunitconfig |  11 +++
>  8 files changed, 544 insertions(+)
>  create mode 100644 include/kunit/ftrace_stub.h
>  create mode 100644 include/kunit/static_stub.h
>  create mode 100644 lib/kunit/ftrace_stub.c
>  create mode 100644 lib/kunit/static_stub.c
>  create mode 100644 lib/kunit/stubs_example.kunitconfig
>
> --
> 2.35.1.894.gb6a874cedc-goog
>

Re: [RFC PATCH 1/2] kunit: Expose 'static stub' API to redirect functions

[Brendan Higgins]

+Steve Muckle

On Thu, Mar 17, 2022 at 10:13 PM David Gow <davidgow@google.com> wrote:
>
> Add a simple way of redirecting calls to functions by including a
> special prologue in the "real" function which checks to see if the
> replacement function should be called (and, if so, calls it).
>
> To redirect calls to a function, make the first (non-declaration) line
> of the function:
>
>         KUNIT_STATIC_STUB_REDIRECT(function_name, [function arguments]);
>
> (This will compile away to nothing if KUnit is not enabled, otherwise it
> will check if a redirection is active, call the replacement function,
> and return.)
>
> Calls to the real function can be redirected to a replacement using:
>
>         kunit_activate_static_stub(test, real_fn, replacement_fn);
>
> The redirection will only affect calls made from within the kthread of
> the current test, and will be automatically disabled when the test
> completes. It can also be manually disabled with
> kunit_deactivate_static_stub().
>
> The 'example' KUnit test suite has a more complete example.
>
> This is intended to be an alternative to ftrace-based stubbing (see the
> next patch), with different tradeoffs.
>
> In particular:
> - 'static stubs' work on all architectures, and don't have any
>   dependencies.
>   - There's no separate Kconfig option to enable them, they always
>     exist.
> - They must be explicitly opted-into by the function being replaced
>   (which can be good or bad depending on your POV)
> - They have a greater performance penalty when not in active use (as
>   every call to the real function must check to see if the stub is
>   enabled)
> - They could more easily be extended to pass additional context to
>   replacement functions.
>
> Co-developed-by: Daniel Latypov <dlatypov@google.com>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
> Signed-off-by: David Gow <davidgow@google.com>
> ---
>  include/kunit/static_stub.h    | 106 ++++++++++++++++++++++++++++
>  lib/kunit/Makefile             |   1 +
>  lib/kunit/kunit-example-test.c |  39 ++++++++++
>  lib/kunit/static_stub.c        | 125 +++++++++++++++++++++++++++++++++
>  4 files changed, 271 insertions(+)
>  create mode 100644 include/kunit/static_stub.h
>  create mode 100644 lib/kunit/static_stub.c
>
> diff --git a/include/kunit/static_stub.h b/include/kunit/static_stub.h
> new file mode 100644
> index 000000000000..020e0e9110f2
> --- /dev/null
> +++ b/include/kunit/static_stub.h
> @@ -0,0 +1,106 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * KUnit function redirection (static stubbing) API.
> + *
> + * Copyright (C) 2022, Google LLC.
> + * Author: David Gow <davidgow@google.com>
> + */
> +#ifndef _KUNIT_STATIC_STUB_H
> +#define _KUNIT_STATIC_STUB_H
> +
> +#if !IS_REACHABLE(CONFIG_KUNIT)
> +
> +/* If CONFIG_KUNIT is not enabled, these stubs quietly disappear. */
> +#define KUNIT_TRIGGER_STATIC_STUB(real_fn_name, args...) do {} while (0)
> +
> +#else
> +
> +#include <kunit/test.h>
> +
> +#include <linux/compiler.h> /* for {un,}likely() */
> +#include <linux/sched.h> /* for task_struct */
> +
> +/* Returns the address of the replacement function. */
> +void *__kunit_get_static_stub_address(struct kunit *test, void *real_fn_addr);
> +
> +/**
> + * KUNIT_STATIC_STUB_REDIRECT() - call a replacement 'static stub' if one exists
> + * @real_fn_name: The name of this function (as an identifier, not a string)
> + * @args: All of the arguments passed to this function
> + *
> + * This is a function prologue which is used to allow calls to the current
> + * function to be redirected by a KUnit test. KUnit tests can call
> + * kunit_activate_static_stub() to pass a replacement function in. The
> + * replacement function will be called by KUNIT_TRIGGER_STATIC_STUB(), which
> + * will then return from the function. If the caller is not in a KUnit context,
> + * the function will continue execution as normal.
> + *
> + * Example:
> + *
> + * .. code-block:: c
> + *     int real_func(int n)
> + *     {
> + *             KUNIT_STATIC_STUB_REDIRECT(real_func, n);
> + *             return 0;
> + *     }
> + *
> + *     void replacement_func(int n)
> + *     {
> + *             return 42;
> + *     }
> + *
> + *     void example_test(struct kunit *test)
> + *     {
> + *             kunit_activate_static_stub(test, real_func, replacement_func);
> + *             KUNIT_EXPECT_EQ(test, real_func(1), 42);
> + *     }
> + *
> + */
> +#define KUNIT_STATIC_STUB_REDIRECT(real_fn_name, args...)              \
> +do {                                                                   \
> +       typeof(&real_fn_name) replacement;                              \
> +       struct kunit *current_test = current->kunit_test;               \
> +                                                                       \
> +       if (likely(!current_test))                                      \
> +               break;                                                  \
> +                                                                       \
> +       KUNIT_ASSERT_STREQ_MSG(current_test, __func__, #real_fn_name,   \
> +                              "Static stub function name mismatch");   \
> +                                                                       \
> +       replacement = __kunit_get_static_stub_address(current_test,     \
> +                                                       &real_fn_name); \
> +                                                                       \
> +       if (unlikely(replacement))                                      \
> +               return replacement(args);                               \
> +} while (0)
> +
> +/* Helper function for kunit_activate_static_stub(). The macro does
> + * typechecking, so use it instead.
> + */
> +void __kunit_activate_static_stub(struct kunit *test,
> +                                 void *real_fn_addr,
> +                                 void *replacement_addr);
> +
> +/**
> + * kunit_activate_static_stub() - replace a function using static stubs.
> + * @test: A pointer to the 'struct kunit' test context for the current test.
> + * @real_fn_addr: The address of the function to replace.
> + * @replacement_addr: The address of the function to replace it with.
> + *
> + * When activated, calls to real_fn_addr from within this test (even if called
> + * indirectly) will instead call replacement_addr. The function pointed to by
> + * real_fn_addr must begin with the static stub prologue in
> + * KUNIT_TRIGGER_STATIC_STUB() for this to work. real_fn_addr and
> + * replacement_addr must have the same type.
> + *
> + * The redirection can be disabled again with kunit_deactivate_static_stub().
> + */
> +#define kunit_activate_static_stub(test, real_fn_addr, replacement_addr) do {  \
> +       typecheck(typeof(real_fn_addr), replacement_addr);                      \
> +       __kunit_activate_static_stub(test, real_fn_addr, replacement_addr);     \
> +} while (0)
> +
> +void kunit_deactivate_static_stub(struct kunit *test, void *real_fn_addr);
> +
> +#endif
> +#endif
> diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
> index c49f4ffb6273..f9e929700782 100644
> --- a/lib/kunit/Makefile
> +++ b/lib/kunit/Makefile
> @@ -1,6 +1,7 @@
>  obj-$(CONFIG_KUNIT) +=                 kunit.o
>
>  kunit-objs +=                          test.o \
> +                                       static_stub.o \
>                                         string-stream.o \
>                                         assert.o \
>                                         try-catch.o \
> diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
> index 51099b0ca29c..670c21e74446 100644
> --- a/lib/kunit/kunit-example-test.c
> +++ b/lib/kunit/kunit-example-test.c
> @@ -7,6 +7,7 @@
>   */
>
>  #include <kunit/test.h>
> +#include <kunit/static_stub.h>
>
>  /*
>   * This is the most fundamental element of KUnit, the test case. A test case
> @@ -69,6 +70,43 @@ static void example_mark_skipped_test(struct kunit *test)
>         /* This line should run */
>         kunit_info(test, "You should see this line.");
>  }
> +
> +/* This is a function we'll replace with static stubs. */
> +static int add_one(int i)
> +{
> +       /* This will trigger the stub if active. */
> +       KUNIT_STATIC_STUB_REDIRECT(add_one, i);
> +
> +       return i + 1;
> +}
> +
> +/* This is used as a replacement for the above function. */
> +static int subtract_one(int i)
> +{
> +       /* We don't need to trigger the stub from the replacement. */
> +
> +       return i - 1;
> +}
> +
> +/*
> + * This test shows the use of static stubs.
> + */
> +static void example_static_stub_test(struct kunit *test)
> +{
> +       /* By default, function is not stubbed. */
> +       KUNIT_EXPECT_EQ(test, add_one(1), 2);
> +
> +       /* Replace add_one() with subtract_one(). */
> +       kunit_activate_static_stub(test, add_one, subtract_one);
> +
> +       /* add_one() is now replaced. */
> +       KUNIT_EXPECT_EQ(test, add_one(1), 0);
> +
> +       /* Return add_one() to normal. */
> +       kunit_deactivate_static_stub(test, add_one);
> +       KUNIT_EXPECT_EQ(test, add_one(1), 2);
> +}
> +
>  /*
>   * Here we make a list of all the test cases we want to add to the test suite
>   * below.
> @@ -83,6 +121,7 @@ static struct kunit_case example_test_cases[] = {
>         KUNIT_CASE(example_simple_test),
>         KUNIT_CASE(example_skip_test),
>         KUNIT_CASE(example_mark_skipped_test),
> +       KUNIT_CASE(example_static_stub_test),
>         {}
>  };
>
> diff --git a/lib/kunit/static_stub.c b/lib/kunit/static_stub.c
> new file mode 100644
> index 000000000000..b06b88f82dd7
> --- /dev/null
> +++ b/lib/kunit/static_stub.c
> @@ -0,0 +1,125 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit function redirection (static stubbing) API.
> + *
> + * Copyright (C) 2022, Google LLC.
> + * Author: David Gow <davidgow@google.com>
> + */
> +
> +#include <kunit/test.h>
> +#include <kunit/static_stub.h>
> +
> +
> +/* Context for a static stub. This is stored in the resource data. */
> +struct kunit_static_stub_ctx {
> +       void *real_fn_addr;
> +       void *replacement_addr;
> +};
> +
> +static void __kunit_static_stub_resource_free(struct kunit_resource *res)
> +{
> +       kfree(res->data);
> +}
> +
> +/* Matching function for kunit_find_resource(). match_data is real_fn_addr. */
> +static bool __kunit_static_stub_resource_match(struct kunit *test,
> +                                               struct kunit_resource *res,
> +                                               void *match_real_fn_addr)
> +{
> +       /* This pointer is only valid if res is a static stub resource. */
> +       struct kunit_static_stub_ctx *ctx = res->data;
> +
> +       /* Make sure the resource is a static stub resource. */
> +       if (res->free != &__kunit_static_stub_resource_free)
> +               return false;
> +
> +       return ctx->real_fn_addr == match_real_fn_addr;
> +}
> +
> +/* Returns the address of the replacement function. */
> +void *__kunit_get_static_stub_address(struct kunit *test, void *real_fn_addr)
> +{
> +       struct kunit_resource *res;
> +       struct kunit_static_stub_ctx *ctx;
> +       void *replacement_addr;
> +
> +       res = kunit_find_resource(test,
> +                                 __kunit_static_stub_resource_match,
> +                                 real_fn_addr);
> +
> +       if (!res)
> +               return NULL;
> +
> +       ctx = res->data;
> +       replacement_addr = ctx->replacement_addr;
> +       kunit_put_resource(res);
> +       return replacement_addr;
> +}
> +EXPORT_SYMBOL_GPL(__kunit_get_static_stub_address);
> +
> +void kunit_deactivate_static_stub(struct kunit *test, void *real_fn_addr)
> +{
> +       struct kunit_resource *res;
> +
> +       KUNIT_ASSERT_PTR_NE_MSG(test, real_fn_addr, NULL,
> +                               "Tried to deactivate a NULL stub.");
> +
> +       /* Look up the existing stub for this function. */
> +       res = kunit_find_resource(test,
> +                                 __kunit_static_stub_resource_match,
> +                                 real_fn_addr);
> +
> +       /* Error out if the stub doesn't exist. */
> +       KUNIT_ASSERT_PTR_NE_MSG(test, res, NULL,
> +                               "Tried to deactivate a nonexistent stub.");
> +
> +       /* Free the stub. We 'put' twice, as we got a reference
> +        * from kunit_find_resource()
> +        */
> +       kunit_remove_resource(test, res);
> +       kunit_put_resource(res);
> +}
> +EXPORT_SYMBOL_GPL(kunit_deactivate_static_stub);
> +
> +/* Helper function for kunit_activate_static_stub(). The macro does
> + * typechecking, so use it instead.
> + */
> +void __kunit_activate_static_stub(struct kunit *test,
> +                                 void *real_fn_addr,
> +                                 void *replacement_addr)
> +{
> +       struct kunit_static_stub_ctx *ctx;
> +       struct kunit_resource *res;
> +
> +       KUNIT_ASSERT_PTR_NE_MSG(test, real_fn_addr, NULL,
> +                               "Tried to activate a stub for function NULL");
> +
> +       /* If the replacement address is NULL, deactivate the stub. */
> +       if (!replacement_addr) {
> +               kunit_deactivate_static_stub(test, replacement_addr);
> +               return;
> +       }
> +
> +       /* Look up any existing stubs for this function, and replace them. */
> +       res = kunit_find_resource(test,
> +                                 __kunit_static_stub_resource_match,
> +                                 real_fn_addr);
> +       if (res) {
> +               ctx = res->data;
> +               ctx->replacement_addr = replacement_addr;
> +
> +               /* We got an extra reference from find_resource(), so put it. */
> +               kunit_put_resource(res);
> +       } else {
> +               ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
> +               KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +               ctx->real_fn_addr = real_fn_addr;
> +               ctx->replacement_addr = replacement_addr;
> +               res = kunit_alloc_resource(test, NULL,
> +                                    &__kunit_static_stub_resource_free,
> +                                    GFP_KERNEL, ctx);
> +       }
> +}
> +EXPORT_SYMBOL_GPL(__kunit_activate_static_stub);
> +
> +
> --
> 2.35.1.894.gb6a874cedc-goog
>

Re: [RFC PATCH 2/2] kunit: expose ftrace-based API for stubbing out functions during tests

[Brendan Higgins]

+Steve Muckle

On Thu, Mar 17, 2022 at 10:13 PM David Gow <davidgow@google.com> wrote:
>
> From: Daniel Latypov <dlatypov@google.com>
>
> Allow function redirection using ftrace and kernel livepatch. This is
> basically equivalent to the static_stub support in the previous patch,
> but does not require the function being replaced to be modified (save
> for the addition of KUNIT_STUBBABLE/noinline).
>
> This is hidden behind the CONFIG_KUNIT_FTRACE_STUBS option, and has a
> number of dependencies, including ftrace, livepatch and
> CONFIG_KALLSYMS_ALL. As a result, it only works on architectures where
> these are available.
>
> You can run the KUnit example tests with the following:
> $ ./tools/testing/kunit/kunit.py run --kunitconfig lib/kunit/stubs_example.kunitconfig --arch=x86_64
>
> To the end user, replacing a function is very simple, e.g.
>   KUNIT_STUBBABLE void real_func(int n);
>   void replacement_func(int n);
>
>   /* in tests */
>   kunit_activate_ftrace_stub(test, real_func, replacement_func);
>
> The implementation is inspired by Steven's snippet here [1].
>
> Some more details:
> * stubbing is automatically undone at the end of tests
> * it can also be manually undone with kunit_deactive_ftrace_stub()
> * stubbing only applies when current->kunit_test == test
>   * note: currently can't have more than one test running at a time
> * KUNIT_STUBBABLE marks tests as noinline when CONFIG_KUNIT_STUBS is set
>   * this ensures we can actually stub all calls
> * KUNIT_STUBBABLE_TRAMPOLINE is a version that evaluates to
>   __always_inline when stubbing is not enabled
>   * This may need to be used with a wrapper function.
>   * See the doc comment for more details.
>
> Sharp-edges:
> * kernel livepatch only works on some arches (not UML)
> * if you don't use noinline/KUNIT_STUBBABLE, functions might be inlined
>   and thus none of this works:
>   * if it's always inlined, at least the attempt to stub will fail
>   * if it's sometimes inlined, then the stub silently won't work
>
> [1] https://lore.kernel.org/lkml/20220224091550.2b7e8784@gandalf.local.home
>
> Co-developed-by: David Gow <davidgow@google.com>
> Signed-off-by: David Gow <davidgow@google.com>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
> ---
>  include/kunit/ftrace_stub.h         |  84 +++++++++++++++++
>  lib/kunit/Kconfig                   |  11 +++
>  lib/kunit/Makefile                  |   4 +
>  lib/kunit/ftrace_stub.c             | 138 ++++++++++++++++++++++++++++
>  lib/kunit/kunit-example-test.c      |  27 +++++-
>  lib/kunit/stubs_example.kunitconfig |  11 +++
>  6 files changed, 274 insertions(+), 1 deletion(-)
>  create mode 100644 include/kunit/ftrace_stub.h
>  create mode 100644 lib/kunit/ftrace_stub.c
>  create mode 100644 lib/kunit/stubs_example.kunitconfig
>
> diff --git a/include/kunit/ftrace_stub.h b/include/kunit/ftrace_stub.h
> new file mode 100644
> index 000000000000..54c053b7e9c1
> --- /dev/null
> +++ b/include/kunit/ftrace_stub.h
> @@ -0,0 +1,84 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _KUNIT_FTRACE_STUB_H
> +#define _KUNIT_FTRACE_STUB_H
> +
> +/** KUNIT_STUBBABLE - marks a function as stubbable when stubbing support is
> + * enabled.
> + *
> + * Stubbing uses ftrace internally, so we can only stub out functions when they
> + * are not inlined. This macro eavlautes to noinline when stubbing support is
> + * enabled to thus make it safe.
> + *
> + * If you cannot add this annotation to the function, you can instead use
> + * KUNIT_STUBBABLE_TRAMPOLINE, which is the same, but evaluates to
> + * __always_inline when stubbing is not enabled.
> + *
> + * Consider copy_to_user, which is marked as __always_inline:
> + *
> + * .. code-block:: c
> + *     static KUNIT_STUBBABLE_TRAMPOLINE unsigned long
> + *     copy_to_user_trampoline(void __user *to, const void *from, unsigned long n)
> + *     {
> + *             return copy_to_user(to, from, n);
> + *     }
> + *
> + * Then we simply need to update our code to go through this function instead
> + * (in the places where we want to stub it out).
> + */
> +#if IS_ENABLED(CONFIG_KUNIT_FTRACE_STUBS)
> +#define KUNIT_STUBBABLE noinline
> +#define KUNIT_STUBBABLE_TRAMPOLINE noinline
> +#else
> +#define KUNIT_STUBBABLE
> +#define KUNIT_STUBBABLE_TRAMPOLINE __always_inline
> +#endif
> +
> +struct kunit;
> +
> +/**
> + * kunit_activate_ftrace_stub() - makes all calls to @func go to @replacement during @test.
> + * @test: The test context object.
> + * @func: The function to stub out, must be annotated with KUNIT_STUBBABLE.
> + * @replacement: The function to replace @func with.
> + *
> + * All calls to @func will instead call @replacement for the duration of the
> + * current test. If called from outside the test's thread, the function will
> + * not be redirected.
> + *
> + * The redirection can be disabled again with kunit_deactivate_ftrace_stub().
> + *
> + * Example:
> + *
> + * .. code-block:: c
> + *     KUNIT_STUBBABLE int real_func(int n)
> + *     {
> + *             pr_info("real_func() called with %d", n);
> + *             return 0;
> + *     }
> + *
> + *     void replacement_func(int n)
> + *     {
> + *             pr_info("replacement_func() called with %d", n);
> + *             return 42;
> + *     }
> + *
> + *     void example_test(struct kunit *test)
> + *     {
> + *             kunit_active_ftrace_stub(test, real_func, replacement_func);
> + *             KUNIT_EXPECT_EQ(test, real_func(1), 42);
> + *     }
> + *
> + */
> +#define kunit_activate_ftrace_stub(test, func, replacement) do { \
> +       typecheck(typeof(func), replacement); \
> +       __kunit_activate_ftrace_stub(test, #func, func, replacement); \
> +} while (0)
> +
> +void __kunit_activate_ftrace_stub(struct kunit *test,
> +                                 const char *name,
> +                                 void *real_fn_addr,
> +                                 void *replacement_addr);
> +
> +
> +void kunit_deactivate_ftrace_stub(struct kunit *test, void *real_fn_addr);
> +#endif  /* _KUNIT_STUB_H */
> diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig
> index 0b5dfb001bac..978e4f72bae0 100644
> --- a/lib/kunit/Kconfig
> +++ b/lib/kunit/Kconfig
> @@ -59,4 +59,15 @@ config KUNIT_ALL_TESTS
>
>           If unsure, say N.
>
> +config KUNIT_FTRACE_STUBS
> +       bool "Support for stubbing out functions in KUnit tests with ftrace and kernel livepatch"
> +       depends on FTRACE=y && FUNCTION_TRACER=y && MODULES=y && DEBUG_KERNEL=y && KALLSYMS_ALL=y && LIVEPATCH=y
> +       help
> +         Builds support for stubbing out functions for the duration of KUnit
> +         test cases or suites using ftrace and kernel livepatch.
> +         See KUNIT_EXAMPLE_TEST for an example.
> +
> +         NOTE: this does not work on all architectures (like UML, arm64) and
> +         relies on a lot of magic (see the dependencies list).
> +
>  endif # KUNIT
> diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
> index f9e929700782..75092c12c3d0 100644
> --- a/lib/kunit/Makefile
> +++ b/lib/kunit/Makefile
> @@ -19,3 +19,7 @@ obj-$(CONFIG_KUNIT_TEST) +=           string-stream-test.o
>  endif
>
>  obj-$(CONFIG_KUNIT_EXAMPLE_TEST) +=    kunit-example-test.o
> +
> +ifeq ($(CONFIG_KUNIT_FTRACE_STUBS),y)
> +kunit-objs +=                          ftrace_stub.o
> +endif
> diff --git a/lib/kunit/ftrace_stub.c b/lib/kunit/ftrace_stub.c
> new file mode 100644
> index 000000000000..13207e1c7aff
> --- /dev/null
> +++ b/lib/kunit/ftrace_stub.c
> @@ -0,0 +1,138 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <kunit/test.h>
> +
> +#include <linux/typecheck.h>
> +
> +#include <linux/ftrace.h>
> +#include <linux/livepatch.h>
> +#include <linux/sched.h>
> +
> +struct kunit_ftrace_stub_ctx {
> +       struct kunit *test;
> +       unsigned long real_fn_addr; /* used as a key to lookup the stub */
> +       unsigned long replacement_addr;
> +       struct ftrace_ops ops; /* a copy of kunit_stub_base_ops with .private set */
> +};
> +
> +static void kunit_stub_trampoline(unsigned long ip, unsigned long parent_ip,
> +                                 struct ftrace_ops *ops,
> +                                 struct ftrace_regs *fregs)
> +{
> +       struct kunit_ftrace_stub_ctx *ctx = ops->private;
> +       int lock_bit;
> +
> +       if (current->kunit_test != ctx->test)
> +               return;
> +
> +       lock_bit = ftrace_test_recursion_trylock(ip, parent_ip);
> +       KUNIT_ASSERT_GE(ctx->test, lock_bit, 0);
> +
> +       klp_arch_set_pc(fregs, ctx->replacement_addr);
> +
> +       ftrace_test_recursion_unlock(lock_bit);
> +}
> +
> +static struct ftrace_ops kunit_stub_base_ops = {
> +       .func = &kunit_stub_trampoline,
> +       .flags = FTRACE_OPS_FL_IPMODIFY |
> +#ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
> +               FTRACE_OPS_FL_SAVE_REGS |
> +#endif
> +               FTRACE_OPS_FL_DYNAMIC
> +};
> +
> +static void __kunit_ftrace_stub_resource_free(struct kunit_resource *res)
> +{
> +       struct kunit_ftrace_stub_ctx *ctx = res->data;
> +
> +       unregister_ftrace_function(&ctx->ops);
> +       kfree(ctx);
> +}
> +
> +/* Matching function for kunit_find_resource(). match_data is real_fn_addr. */
> +static bool __kunit_static_stub_resource_match(struct kunit *test,
> +                                               struct kunit_resource *res,
> +                                               void *match_real_fn_addr)
> +{
> +       /* This pointer is only valid if res is a static stub resource. */
> +       struct kunit_ftrace_stub_ctx *ctx = res->data;
> +
> +       /* Make sure the resource is a static stub resource. */
> +       if (res->free != &__kunit_ftrace_stub_resource_free)
> +               return false;
> +
> +       return ctx->real_fn_addr == (unsigned long)match_real_fn_addr;
> +}
> +
> +void kunit_deactivate_ftrace_stub(struct kunit *test, void *real_fn_addr)
> +{
> +       struct kunit_resource *res;
> +
> +       KUNIT_ASSERT_PTR_NE_MSG(test, real_fn_addr, NULL,
> +                               "Tried to deactivate a NULL stub.");
> +
> +       /* Look up the existing stub for this function. */
> +       res = kunit_find_resource(test,
> +                                 __kunit_static_stub_resource_match,
> +                                 real_fn_addr);
> +
> +       /* Error out if the stub doesn't exist. */
> +       KUNIT_ASSERT_PTR_NE_MSG(test, res, NULL,
> +                               "Tried to deactivate a nonexistent stub.");
> +
> +       /* Free the stub. We 'put' twice, as we got a reference
> +        * from kunit_find_resource(). The free function will deactivate the
> +        * ftrace stub.
> +        */
> +       kunit_remove_resource(test, res);
> +       kunit_put_resource(res);
> +}
> +EXPORT_SYMBOL_GPL(kunit_deactivate_ftrace_stub);
> +
> +void __kunit_activate_ftrace_stub(struct kunit *test,
> +                                 const char *name,
> +                                 void *real_fn_addr,
> +                                 void *replacement_addr)
> +{
> +       unsigned long ftrace_ip;
> +       struct kunit_ftrace_stub_ctx *ctx;
> +       int ret;
> +
> +       ftrace_ip = ftrace_location((unsigned long)real_fn_addr);
> +       if (!ftrace_ip)
> +               KUNIT_ASSERT_FAILURE(test, "%s ip is invalid: not a function, or is marked notrace or inline", name);
> +
> +       /* Allocate the stub context, which contains pointers to the replacement
> +        * function and the test object. It's also registered as a KUnit
> +        * resource which can be looked up by address (to deactivate manually)
> +        * and is destroyed automatically on test exit.
> +        */
> +       ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
> +       KUNIT_ASSERT_NOT_ERR_OR_NULL_MSG(test, ctx, "failed to allocate kunit stub for %s", name);
> +
> +       ctx->test = test;
> +       ctx->ops = kunit_stub_base_ops;
> +       ctx->ops.private = ctx;
> +       ctx->real_fn_addr = (unsigned long)real_fn_addr;
> +       ctx->replacement_addr = (unsigned long)replacement_addr;
> +
> +       ret = ftrace_set_filter_ip(&ctx->ops, ftrace_ip, 0, 0);
> +       if (ret) {
> +               kfree(ctx);
> +               KUNIT_ASSERT_FAILURE(test, "failed to set filter ip for %s: %d", name, ret);
> +       }
> +
> +       ret = register_ftrace_function(&ctx->ops);
> +       if (ret) {
> +               kfree(ctx);
> +               if (ret == -EBUSY)
> +                       KUNIT_ASSERT_FAILURE(test, "failed to register stub (-EBUSY) for %s, likely due to already stubbing it?", name);
> +               KUNIT_ASSERT_FAILURE(test, "failed to register stub for %s: %d", name, ret);
> +       }
> +
> +       /* Register the stub as a resource with a cleanup function */
> +       kunit_alloc_resource(test, NULL,
> +                            __kunit_ftrace_stub_resource_free,
> +                            GFP_KERNEL, ctx);
> +}
> +EXPORT_SYMBOL_GPL(__kunit_activate_ftrace_stub);
> diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
> index 670c21e74446..7f20b132343b 100644
> --- a/lib/kunit/kunit-example-test.c
> +++ b/lib/kunit/kunit-example-test.c
> @@ -8,6 +8,7 @@
>
>  #include <kunit/test.h>
>  #include <kunit/static_stub.h>
> +#include <kunit/ftrace_stub.h>
>
>  /*
>   * This is the most fundamental element of KUnit, the test case. A test case
> @@ -72,7 +73,7 @@ static void example_mark_skipped_test(struct kunit *test)
>  }
>
>  /* This is a function we'll replace with static stubs. */
> -static int add_one(int i)
> +static KUNIT_STUBBABLE int add_one(int i)
>  {
>         /* This will trigger the stub if active. */
>         KUNIT_STATIC_STUB_REDIRECT(add_one, i);
> @@ -107,6 +108,29 @@ static void example_static_stub_test(struct kunit *test)
>         KUNIT_EXPECT_EQ(test, add_one(1), 2);
>  }
>
> +/*
> + * This test shows the use of static stubs.
> + */
> +static void example_ftrace_stub_test(struct kunit *test)
> +{
> +#if !IS_ENABLED(CONFIG_KUNIT_FTRACE_STUBS)
> +       kunit_skip(test, "KUNIT_FTRACE_STUBS not enabled");
> +#else
> +       /* By default, function is not stubbed. */
> +       KUNIT_EXPECT_EQ(test, add_one(1), 2);
> +
> +       /* Replace add_one() with subtract_one(). */
> +       kunit_activate_ftrace_stub(test, add_one, subtract_one);
> +
> +       /* add_one() is now replaced. */
> +       KUNIT_EXPECT_EQ(test, add_one(1), 0);
> +
> +       /* Return add_one() to normal. */
> +       kunit_deactivate_ftrace_stub(test, add_one);
> +       KUNIT_EXPECT_EQ(test, add_one(1), 2);
> +#endif
> +}
> +
>  /*
>   * Here we make a list of all the test cases we want to add to the test suite
>   * below.
> @@ -122,6 +146,7 @@ static struct kunit_case example_test_cases[] = {
>         KUNIT_CASE(example_skip_test),
>         KUNIT_CASE(example_mark_skipped_test),
>         KUNIT_CASE(example_static_stub_test),
> +       KUNIT_CASE(example_ftrace_stub_test),
>         {}
>  };
>
> diff --git a/lib/kunit/stubs_example.kunitconfig b/lib/kunit/stubs_example.kunitconfig
> new file mode 100644
> index 000000000000..a47369199fb9
> --- /dev/null
> +++ b/lib/kunit/stubs_example.kunitconfig
> @@ -0,0 +1,11 @@
> +CONFIG_KUNIT=y
> +CONFIG_KUNIT_FTRACE_STUBS=y
> +CONFIG_KUNIT_EXAMPLE_TEST=y
> +
> +# Depedencies
> +CONFIG_FTRACE=y
> +CONFIG_FUNCTION_TRACER=y
> +CONFIG_MODULES=y
> +CONFIG_DEBUG_KERNEL=y
> +CONFIG_KALLSYMS_ALL=y
> +CONFIG_LIVEPATCH=y
> --
> 2.35.1.894.gb6a874cedc-goog
>

Re: [RFC PATCH 2/2] kunit: expose ftrace-based API for stubbing out functions during tests

[Daniel Latypov]

On Thu, Mar 17, 2022 at 9:13 PM David Gow <davidgow@google.com> wrote:
> +
> +/** KUNIT_STUBBABLE - marks a function as stubbable when stubbing support is
> + * enabled.
> + *
> + * Stubbing uses ftrace internally, so we can only stub out functions when they
> + * are not inlined. This macro eavlautes to noinline when stubbing support is

Just noting a couple typos lest they be forgotten:
*evaluates

> + * enabled to thus make it safe.
> + *

<snip>

> + *
> + *     void replacement_func(int n)
> + *     {
> + *             pr_info("replacement_func() called with %d", n);
> + *             return 42;
> + *     }
> + *
> + *     void example_test(struct kunit *test)
> + *     {
> + *             kunit_active_ftrace_stub(test, real_func, replacement_func);

*activate

Re: [RFC PATCH 0/2] kunit: Support redirecting function calls

[Steve Muckle]

On 4/4/22 13:13, Brendan Higgins wrote:
> On Fri, Mar 18, 2022 at 9:22 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>>
>> On Fri, 18 Mar 2022 10:13:12 +0800
>> David Gow <davidgow@google.com> wrote:
>>
>>> Does either (or both) of these features sound useful, and is this
>>> sort-of API the right model? (Personally, I think there's a reasonable
>>> scope for both.) Is anything obviously missing or wrong? Do the names,
>>> descriptions etc. make any sense?
>>
>> Obviously I'm biased toward the ftrace solution ;-)
> 
> Personally, I like providing both - as long as we can keep the
> interface the same.
> 
> Ftrace is less visually invasive, but it is also less flexible in
> capabilities, and requires substantial work to support on new
> architectures.

The general feature looks useful to me. I'm not sure the ftrace based 
API is worth it given it is only offering a visual improvement and has 
some drawbacks compared to the other implementation (won't work with 
inline functions, dependencies on other features). Livepatch is absent 
on arm64 which mostly rules it out for my purposes (Android Generic 
Kernel Image testing).

cheers,
Steve

Re: [RFC PATCH 0/2] kunit: Support redirecting function calls

[Steve Muckle]

+Joe Fradley who is also looking at KUnit with Android.

On 4/15/22 14:43, Steve Muckle wrote:
> On 4/4/22 13:13, Brendan Higgins wrote:
>> On Fri, Mar 18, 2022 at 9:22 AM Steven Rostedt <rostedt@goodmis.org> 
>> wrote:
>>>
>>> On Fri, 18 Mar 2022 10:13:12 +0800
>>> David Gow <davidgow@google.com> wrote:
>>>
>>>> Does either (or both) of these features sound useful, and is this
>>>> sort-of API the right model? (Personally, I think there's a reasonable
>>>> scope for both.) Is anything obviously missing or wrong? Do the names,
>>>> descriptions etc. make any sense?
>>>
>>> Obviously I'm biased toward the ftrace solution ;-)
>>
>> Personally, I like providing both - as long as we can keep the
>> interface the same.
>>
>> Ftrace is less visually invasive, but it is also less flexible in
>> capabilities, and requires substantial work to support on new
>> architectures.
> 
> The general feature looks useful to me. I'm not sure the ftrace based 
> API is worth it given it is only offering a visual improvement and has 
> some drawbacks compared to the other implementation (won't work with 
> inline functions, dependencies on other features). Livepatch is absent 
> on arm64 which mostly rules it out for my purposes (Android Generic 
> Kernel Image testing).
> 
> cheers,
> Steve

Re: [RFC PATCH 1/2] kunit: Expose 'static stub' API to redirect functions

[Steve Muckle]

On 4/4/22 13:16, Brendan Higgins wrote:
> On Thu, Mar 17, 2022 at 10:13 PM David Gow <davidgow@google.com> wrote:
>> +/**
>> + * KUNIT_STATIC_STUB_REDIRECT() - call a replacement 'static stub' if one exists
>> + * @real_fn_name: The name of this function (as an identifier, not a string)
>> + * @args: All of the arguments passed to this function
>> + *
>> + * This is a function prologue which is used to allow calls to the current
>> + * function to be redirected by a KUnit test. KUnit tests can call
>> + * kunit_activate_static_stub() to pass a replacement function in. The
>> + * replacement function will be called by KUNIT_TRIGGER_STATIC_STUB(), which
>> + * will then return from the function. If the caller is not in a KUnit context,
>> + * the function will continue execution as normal.
>> + *
>> + * Example:
>> + *
>> + * .. code-block:: c
>> + *     int real_func(int n)
>> + *     {
>> + *             KUNIT_STATIC_STUB_REDIRECT(real_func, n);

I wish there was a way to avoid having to repeat the function's name and 
arguments here...

Re: [RFC PATCH 1/2] kunit: Expose 'static stub' API to redirect functions

[Daniel Latypov]

On Thu, Mar 17, 2022 at 9:13 PM David Gow <davidgow@google.com> wrote:
> +#define kunit_activate_static_stub(test, real_fn_addr, replacement_addr) do {  \
> +       typecheck(typeof(real_fn_addr), replacement_addr);                      \

We can't call this macro in the same scope for functions w/ different
signatures.

E.g. if we add this func to the example test
  static void other_func(void) {}
then trying to call kunit_activate_static_stub() on it in the same
test case, we get

./include/linux/typecheck.h:10:14: error: conflicting types for
‘__dummy’; have ‘void(void)’
   10 | ({      type __dummy; \
      |              ^~~~~~~
./include/kunit/static_stub.h:99:9: note: in expansion of macro ‘typecheck’
   99 |         typecheck(typeof(real_fn_addr), replacement_addr);
                 \
      |         ^~~~~~~~~
lib/kunit/example-test.c:64:9: note: in expansion of macro
‘kunit_activate_static_stub’
   64 |         kunit_activate_static_stub(test, other_func, other_func);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/typecheck.h:10:14: note: previous declaration of
‘__dummy’ with type ‘int(int)’
   10 | ({      type __dummy; \
      |              ^~~~~~~
./include/kunit/static_stub.h:99:9: note: in expansion of macro ‘typecheck’
   99 |         typecheck(typeof(real_fn_addr), replacement_addr);
                 \
      |         ^~~~~~~~~
lib/kunit/example-test.c:62:9: note: in expansion of macro
‘kunit_activate_static_stub’
   62 |         kunit_activate_static_stub(test, add_one, subtract_one);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~

Afaict, the problem is that GCC thinks we're declaring a *function*
called __dummy, not a variable.
So it bleeds across the scope boundary of do-while unlike normal variables.

There's the typecheck_fn macro, but it doesn't work either.

Re: [RFC PATCH 1/2] kunit: Expose 'static stub' API to redirect functions

[Brendan Higgins]

On Wed, May 4, 2022 at 4:35 PM Daniel Latypov <dlatypov@google.com> wrote:
>
> On Thu, Mar 17, 2022 at 9:13 PM David Gow <davidgow@google.com> wrote:
> > +#define kunit_activate_static_stub(test, real_fn_addr, replacement_addr) do {  \
> > +       typecheck(typeof(real_fn_addr), replacement_addr);                      \
>
> We can't call this macro in the same scope for functions w/ different
> signatures.
>
> E.g. if we add this func to the example test
>   static void other_func(void) {}
> then trying to call kunit_activate_static_stub() on it in the same
> test case, we get
>
> ./include/linux/typecheck.h:10:14: error: conflicting types for
> ‘__dummy’; have ‘void(void)’
>    10 | ({      type __dummy; \
>       |              ^~~~~~~
> ./include/kunit/static_stub.h:99:9: note: in expansion of macro ‘typecheck’
>    99 |         typecheck(typeof(real_fn_addr), replacement_addr);
>                  \
>       |         ^~~~~~~~~
> lib/kunit/example-test.c:64:9: note: in expansion of macro
> ‘kunit_activate_static_stub’
>    64 |         kunit_activate_static_stub(test, other_func, other_func);
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
> ./include/linux/typecheck.h:10:14: note: previous declaration of
> ‘__dummy’ with type ‘int(int)’
>    10 | ({      type __dummy; \
>       |              ^~~~~~~
> ./include/kunit/static_stub.h:99:9: note: in expansion of macro ‘typecheck’
>    99 |         typecheck(typeof(real_fn_addr), replacement_addr);
>                  \
>       |         ^~~~~~~~~
> lib/kunit/example-test.c:62:9: note: in expansion of macro
> ‘kunit_activate_static_stub’
>    62 |         kunit_activate_static_stub(test, add_one, subtract_one);
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Afaict, the problem is that GCC thinks we're declaring a *function*
> called __dummy, not a variable.
> So it bleeds across the scope boundary of do-while unlike normal variables.

Yeah, I ran into that problem too. I posted a fix to gerrit. I have
been meaning to share it here.

> There's the typecheck_fn macro, but it doesn't work either.

That's weird. It worked for me.

Re: [RFC PATCH 1/2] kunit: Expose 'static stub' API to redirect functions

[Daniel Latypov]

On Wed, May 4, 2022 at 3:42 PM 'Brendan Higgins' via KUnit Development
<kunit-dev@googlegroups.com> wrote:
>
> On Wed, May 4, 2022 at 4:35 PM Daniel Latypov <dlatypov@google.com> wrote:
> >
> > On Thu, Mar 17, 2022 at 9:13 PM David Gow <davidgow@google.com> wrote:
> > > +#define kunit_activate_static_stub(test, real_fn_addr, replacement_addr) do {  \
> > > +       typecheck(typeof(real_fn_addr), replacement_addr);                      \
> >
> > We can't call this macro in the same scope for functions w/ different
> > signatures.
> >
> > E.g. if we add this func to the example test
> >   static void other_func(void) {}
> > then trying to call kunit_activate_static_stub() on it in the same
> > test case, we get
> >
> > ./include/linux/typecheck.h:10:14: error: conflicting types for
> > ‘__dummy’; have ‘void(void)’
> >    10 | ({      type __dummy; \
> >       |              ^~~~~~~
> > ./include/kunit/static_stub.h:99:9: note: in expansion of macro ‘typecheck’
> >    99 |         typecheck(typeof(real_fn_addr), replacement_addr);
> >                  \
> >       |         ^~~~~~~~~
> > lib/kunit/example-test.c:64:9: note: in expansion of macro
> > ‘kunit_activate_static_stub’
> >    64 |         kunit_activate_static_stub(test, other_func, other_func);
> >       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
> > ./include/linux/typecheck.h:10:14: note: previous declaration of
> > ‘__dummy’ with type ‘int(int)’
> >    10 | ({      type __dummy; \
> >       |              ^~~~~~~
> > ./include/kunit/static_stub.h:99:9: note: in expansion of macro ‘typecheck’
> >    99 |         typecheck(typeof(real_fn_addr), replacement_addr);
> >                  \
> >       |         ^~~~~~~~~
> > lib/kunit/example-test.c:62:9: note: in expansion of macro
> > ‘kunit_activate_static_stub’
> >    62 |         kunit_activate_static_stub(test, add_one, subtract_one);
> >       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Afaict, the problem is that GCC thinks we're declaring a *function*
> > called __dummy, not a variable.
> > So it bleeds across the scope boundary of do-while unlike normal variables.
>
> Yeah, I ran into that problem too. I posted a fix to gerrit. I have
> been meaning to share it here.

For others, gerrit == https://kunit-review.googlesource.com/c/linux/+/5129

>
> > There's the typecheck_fn macro, but it doesn't work either.
>
> That's weird. It worked for me.

I'm running on top of 5.5.
I tried reproducing w/ a stripped down version on 5.18 and saw the same issues.

Huh, I'm trying with
 #define kunit_activate_static_stub(test, real_fn_addr,
replacement_addr) do {  \
-       typecheck(typeof(real_fn_addr), replacement_addr);
         \
+       typecheck_fn(typeof(real_fn_addr), replacement_addr); \
        __kunit_activate_static_stub(test, real_fn_addr,
replacement_addr);     \

This gives me
lib/kunit/example-test.c:62:9: error: function ‘__tmp’ is initialized
like a variable
   62 |         kunit_activate_static_stub(test, add_one, subtract_one);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
lib/kunit/example-test.c:64:9: error: function ‘__tmp’ is initialized
like a variable
   64 |         kunit_activate_static_stub(test, other_func, other_func);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~

Perhaps I'm missing something silly.

Can you post your fix and I can try it out?