Re: [PATCH] lib/test_stackinit: move a local outside the switch statement

From: Kees Cook
Date: Wed Feb 19 2020 - 16:58:09 EST


On Wed, Feb 19, 2020 at 06:56:38PM +0100, Alexander Potapenko wrote:
> On Wed, Feb 19, 2020 at 6:36 PM Kees Cook <keescook@xxxxxxxxxxxx> wrote:
> Am I understanding right that these warnings only show up in the
> instrumented build?

Correct.

> According to the GCC manual:
>
> -Wswitch-unreachable does not warn if the statement between the
> controlling expression and the first case label is just a declaration

Right, just a declaration is okay. An initializer is not handled:

switch (argc) {
int foo = 0;
case 0:
...

foo.c:6:7: warning: statement will never be executed
[-Wswitch-unreachable]
6 | int foo = 0;
| ^~~

The problem I had with the "simple" stackinit GCC plugin was that it
didn't handle padding. What I don't understand is why structleak (with
seemingly the same initialization) _does_ initialize padding:


structleak:

PASS_INFO(structleak, "early_optimizations", 1, PASS_POS_INSERT_BEFORE);
...

/* build the initializer expression */
type = TREE_TYPE(var);
if (AGGREGATE_TYPE_P(type))
initializer = build_constructor(type, NULL);
else
initializer = fold_convert(type, integer_zero_node);

/* build the initializer stmt */
init_stmt = gimple_build_assign(var, initializer);
gsi = gsi_after_labels(single_succ(ENTRY_BLOCK_PTR_FOR_FN(cfun)));
gsi_insert_before(&gsi, init_stmt, GSI_NEW_STMT);
update_stmt(init_stmt);

vs stackinit:

register_callback(plugin_name, PLUGIN_FINISH_DECL, finish_decl, NULL);
...

type = TREE_TYPE (decl);
if (AGGREGATE_TYPE_P (type))
DECL_INITIAL (decl) = build_constructor (type, NULL);
else
DECL_INITIAL (decl) = fold_convert (type, integer_zero_node);

I assume the difference is due to either pass ordering or the former's
basic block splitting. I haven't had time to dig in and figure it out.

--
Kees Cook