[Toybox] warning: char *s is never used uninitialized.

[enh]

On Mon, Dec 23, 2019 at 9:32 PM Rob Landley <rob at landley.net> wrote:
>
> Right now toys/pending/sh.c has two legitimate "unused variable" warnings
> (because commented out unfinished code block, it's very much a work in
> progress), but this:
>
> toys/pending/sh.c: In function 'run_command.constprop':
> toys/pending/sh.c:596:5: warning: 's' may be used uninitialized in this function
> [-Wmaybe-uninitialized]
>      syntax_err("bad %s", s);
>      ^~~~~~~~~~~~~~~~~~~~~~~
> toys/pending/sh.c:442:9: note: 's' was declared here
>    char *s, *ss, *sss, *cv = 0;
>
> Ummm... how? The code there boils down to:
>
>   char *s;
>
>   if (envlen>=arg->c) return 0;
>   for (j = envlen; j<arg->c; j++) {
>     s = arg->v[j];
>     stuff();
>   }
>   if (j != arg->c) syntax_err("bad %s", s);
>
> If we didn't go into the loop at least once, we don't call syntax_err(). If it's
> >= the limit we returned early, if it's less we went into the loop at least once.
>
> So no matter what value of envlen and arg->c are passed in, HOW can this be used
> uninitialized? (Anybody else spot it?)

this is quite common that the compiler won't fully understand this
kind of "duplicated condition" stuff. it's a trade off of how much
time you want the compiler to spend analysing your code. the
clang-tidy static analyser might well be able to spot it, but that's
far too slow for you to use day to day. (Java has a very simple
algorithm for this, explicitly stated in the JLS, so ironically Java
is more C-like in its predictability here than C, which leaves so much
to "quality of implementation".)

speaking of which, we're about to turn on
https://reviews.llvm.org/rL349442 for Android to at least remove one
area of undefined-ness. toybox isn't even on the list of code we're
actually worried about (and it's actually information leaks that are
the security folks' concern, not flaky failures caused by
uninitialized memory), but the recent ls(1) issues for example would
have been consistent failures that wouldn't have made it through
presubmit with this feature turned on, rather than "depends what
happened to be in that uninitialized memory". see also
https://outflux.net/slides/2018/lss/danger.pdf for a kernel
perspective.

> Rob
> _______________________________________________
> Toybox mailing list
> Toybox at lists.landley.net
> http://lists.landley.net/listinfo.cgi/toybox-landley.net