[v2,5/5] gcc-plugins/stackleak: Add 'verbose' plugin parameter
diff mbox series

Message ID 20200624123330.83226-6-alex.popov@linux.com
State New
Headers show
Series
  • Improvements of the stackleak gcc plugin
Related show

Commit Message

Alexander Popov June 24, 2020, 12:33 p.m. UTC
Add 'verbose' plugin parameter for stackleak gcc plugin.
It can be used for printing additional info about the kernel code
instrumentation.

For using it add the following to scripts/Makefile.gcc-plugins:
  gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STACKLEAK) \
    += -fplugin-arg-stackleak_plugin-verbose

Signed-off-by: Alexander Popov <alex.popov@linux.com>
---
 scripts/gcc-plugins/stackleak_plugin.c | 47 +++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 5 deletions(-)

Comments

Luis Chamberlain June 24, 2020, 12:53 p.m. UTC | #1
On Wed, Jun 24, 2020 at 03:33:30PM +0300, Alexander Popov wrote:
> Add 'verbose' plugin parameter for stackleak gcc plugin.
> It can be used for printing additional info about the kernel code
> instrumentation.
> 
> For using it add the following to scripts/Makefile.gcc-plugins:
>   gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STACKLEAK) \
>     += -fplugin-arg-stackleak_plugin-verbose

Would be nice if we instead could pass an argument to make which lets
us enable this.

  Luis
Alexander Popov June 24, 2020, 1:09 p.m. UTC | #2
On 24.06.2020 15:53, Luis Chamberlain wrote:
> On Wed, Jun 24, 2020 at 03:33:30PM +0300, Alexander Popov wrote:
>> Add 'verbose' plugin parameter for stackleak gcc plugin.
>> It can be used for printing additional info about the kernel code
>> instrumentation.
>>
>> For using it add the following to scripts/Makefile.gcc-plugins:
>>   gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STACKLEAK) \
>>     += -fplugin-arg-stackleak_plugin-verbose
> 
> Would be nice if we instead could pass an argument to make which lets
> us enable this.

This feature is useful only for debugging stackleak gcc plugin.

The cflag that enables it is similar to -fplugin-arg-structleak_plugin-verbose,
which is used for debugging the structleak plugin.

This debugging feature clutters the kernel build output, I don't think that many
people will use it. So IMO creating a separate argument for make is not really
needed.

Thanks!

Best regards,
Alexander
Kees Cook June 24, 2020, 2:41 p.m. UTC | #3
On Wed, Jun 24, 2020 at 04:09:20PM +0300, Alexander Popov wrote:
> On 24.06.2020 15:53, Luis Chamberlain wrote:
> > On Wed, Jun 24, 2020 at 03:33:30PM +0300, Alexander Popov wrote:
> >> Add 'verbose' plugin parameter for stackleak gcc plugin.
> >> It can be used for printing additional info about the kernel code
> >> instrumentation.
> >>
> >> For using it add the following to scripts/Makefile.gcc-plugins:
> >>   gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STACKLEAK) \
> >>     += -fplugin-arg-stackleak_plugin-verbose
> > 
> > Would be nice if we instead could pass an argument to make which lets
> > us enable this.
> 
> This feature is useful only for debugging stackleak gcc plugin.
> 
> The cflag that enables it is similar to -fplugin-arg-structleak_plugin-verbose,
> which is used for debugging the structleak plugin.
> 
> This debugging feature clutters the kernel build output, I don't think that many
> people will use it. So IMO creating a separate argument for make is not really
> needed.

Yup, agreed. The precedent for plugin verbosity is via CONFIGs. They're
not really general purpose enough to justify a "make" argument.
Kees Cook June 24, 2020, 2:53 p.m. UTC | #4
On Wed, Jun 24, 2020 at 03:33:30PM +0300, Alexander Popov wrote:
> Add 'verbose' plugin parameter for stackleak gcc plugin.
> It can be used for printing additional info about the kernel code
> instrumentation.
> 
> For using it add the following to scripts/Makefile.gcc-plugins:
>   gcc-plugin-cflags-$(CONFIG_GCC_PLUGIN_STACKLEAK) \
>     += -fplugin-arg-stackleak_plugin-verbose
> 
> Signed-off-by: Alexander Popov <alex.popov@linux.com>

Applied to for-next/gcc-plugins.

Patch
diff mbox series

diff --git a/scripts/gcc-plugins/stackleak_plugin.c b/scripts/gcc-plugins/stackleak_plugin.c
index a18b0d4af456..48e141e07956 100644
--- a/scripts/gcc-plugins/stackleak_plugin.c
+++ b/scripts/gcc-plugins/stackleak_plugin.c
@@ -34,6 +34,8 @@  __visible int plugin_is_GPL_compatible;
 static int track_frame_size = -1;
 static bool build_for_x86 = false;
 static const char track_function[] = "stackleak_track_stack";
+static bool disable = false;
+static bool verbose = false;
 
 /*
  * Mark these global variables (roots) for gcc garbage collector since
@@ -46,6 +48,7 @@  static struct plugin_info stackleak_plugin_info = {
 	.help = "track-min-size=nn\ttrack stack for functions with a stack frame size >= nn bytes\n"
 		"arch=target_arch\tspecify target build arch\n"
 		"disable\t\tdo not activate the plugin\n"
+		"verbose\t\tprint info about the instrumentation\n"
 };
 
 static void add_stack_tracking_gcall(gimple_stmt_iterator *gsi, bool after)
@@ -102,6 +105,10 @@  static tree get_current_stack_pointer_decl(void)
 		return var;
 	}
 
+	if (verbose) {
+		fprintf(stderr, "stackleak: missing current_stack_pointer in %s()\n",
+			DECL_NAME_POINTER(current_function_decl));
+	}
 	return NULL_TREE;
 }
 
@@ -195,6 +202,11 @@  static unsigned int stackleak_instrument_execute(void)
 			if (!is_alloca(stmt))
 				continue;
 
+			if (verbose) {
+				fprintf(stderr, "stackleak: be careful, alloca() in %s()\n",
+					DECL_NAME_POINTER(current_function_decl));
+			}
+
 			/* Insert stackleak_track_stack() call after alloca() */
 			add_stack_tracking(&gsi, true);
 			if (bb == entry_bb)
@@ -384,13 +396,31 @@  static bool remove_stack_tracking_gasm(void)
  */
 static unsigned int stackleak_cleanup_execute(void)
 {
+	const char *fn = DECL_NAME_POINTER(current_function_decl);
 	bool removed = false;
 
-	if (cfun->calls_alloca)
+	/*
+	 * Leave stack tracking in functions that call alloca().
+	 * Additional case:
+	 *   gcc before version 7 called allocate_dynamic_stack_space() from
+	 *   expand_stack_vars() for runtime alignment of constant-sized stack
+	 *   variables. That caused cfun->calls_alloca to be set for functions
+	 *   that in fact don't use alloca().
+	 *   For more info see gcc commit 7072df0aae0c59ae437e.
+	 *   Let's leave such functions instrumented as well.
+	 */
+	if (cfun->calls_alloca) {
+		if (verbose)
+			fprintf(stderr, "stackleak: instrument %s(): calls_alloca\n", fn);
 		return 0;
+	}
 
-	if (large_stack_frame())
+	/* Leave stack tracking in functions with large stack frame */
+	if (large_stack_frame()) {
+		if (verbose)
+			fprintf(stderr, "stackleak: instrument %s()\n", fn);
 		return 0;
+	}
 
 	if (lookup_attribute_spec(get_identifier("no_caller_saved_registers")))
 		removed = remove_stack_tracking_gasm();
@@ -516,9 +546,6 @@  __visible int plugin_init(struct plugin_name_args *plugin_info,
 
 	/* Parse the plugin arguments */
 	for (i = 0; i < argc; i++) {
-		if (!strcmp(argv[i].key, "disable"))
-			return 0;
-
 		if (!strcmp(argv[i].key, "track-min-size")) {
 			if (!argv[i].value) {
 				error(G_("no value supplied for option '-fplugin-arg-%s-%s'"),
@@ -541,6 +568,10 @@  __visible int plugin_init(struct plugin_name_args *plugin_info,
 
 			if (!strcmp(argv[i].value, "x86"))
 				build_for_x86 = true;
+		} else if (!strcmp(argv[i].key, "disable")) {
+			disable = true;
+		} else if (!strcmp(argv[i].key, "verbose")) {
+			verbose = true;
 		} else {
 			error(G_("unknown option '-fplugin-arg-%s-%s'"),
 					plugin_name, argv[i].key);
@@ -548,6 +579,12 @@  __visible int plugin_init(struct plugin_name_args *plugin_info,
 		}
 	}
 
+	if (disable) {
+		if (verbose)
+			fprintf(stderr, "stackleak: disabled for this translation unit\n");
+		return 0;
+	}
+
 	/* Give the information about the plugin */
 	register_callback(plugin_name, PLUGIN_INFO, NULL,
 						&stackleak_plugin_info);