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

[Kees Cook <keescook@chromium.org>]

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@chromium.org> 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