On Thu, Jan 24, 2019 at 8:18 AM Matthew Wilcox [off-list ref] wrote:

On Wed, Jan 23, 2019 at 04:17:30PM +0200, Jani Nikula wrote:

quoted

Can't have:

      switch (i) {
              int j;
      case 0:
              /* ... */
      }

because it can't be turned into:

      switch (i) {
              int j = 0; /* not valid C */
      case 0:
              /* ... */
      }

but can have e.g.:

      switch (i) {
      case 0:
              {
                      int j = 0;
                      /* ... */
              }
      }

I think Kees' approach of moving such variable declarations to the
enclosing block scope is better than adding another nesting block.

Another nesting level would be bad, but I think this is OK:

        switch (i) {
        case 0: {
                int j = 0;
                /* ... */
        }
        case 1: {
                void *p = q;
                /* ... */
        }
        }

I can imagine Kees' patch might have a bad effect on stack consumption,
unless GCC can be relied on to be smart enough to notice the
non-overlapping liveness of the vriables and use the same stack slots
for both.

GCC is reasonable at this. The main issue, though, was most of these
places were using the variables in multiple case statements, so they
couldn't be limited to a single block (or they'd need to be manually
repeated in each block, which is even more ugly, IMO).

Whatever the consensus, I'm happy to tweak the patch.

Thanks!

-- 
Kees Cook