Messages in this thread | | | From | Nick Desaulniers <> | Date | Tue, 14 Jun 2022 10:11:37 -0700 | Subject | Re: [PATCH] [RFC] Initialization of unused function parameters |
| |
On Tue, Jun 14, 2022 at 9:48 AM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Tue, Jun 14, 2022 at 7:49 AM Alexander Potapenko <glider@google.com> wrote: > > > > The bigger question I want to raise here is whether we want to > > discourage passing uninitialized variables to functions in the kernel > > altogether. > > I'm assuming you mean pass by reference. > > Some functions are really fundamentally about initializing things, and > expect uninitialized allocations. > > Obviously the traditional example of this is "memset()", and that one > can be special-cased as obvious, but we probably have a ton of wrapper > things like that. > > IOW, things like just "snprintf()" etc is fundamentally passed an > uninitialized buffer, because the whole point is that it will write to > that buffer. > > And no, we don't want to initialize it, since the buffer may be big > (on purpose). > > Now, for *small* things (like that "pointer to inode") that aren't > some kind of array or big structure, I think it might be good to > perhaps be stricter. But even there we do have cases where we pass > things by reference because the function is explicitly designed to > initialize the value: the argument isn't really "an argument", it's a > "second return value". > > But always initializing in the caller sounds stupid and > counter-productive, since the point is to initialize by calling the > helper function (think things like returning a "cookie" or similar: > initializing the cookie to NULL in the caller is just plain _wrong_. > > What I think might be a good model is to be able to mark such > arguments as "must be initialized by callee".
Yeah, being able to enforce that would be nice.
Now that we have clang's static analyzer wired up (commit 6ad7cbc01527 ("Makefile: Add clang-tidy and static analyzer support to makefile")), Intel's 0day bot has been reporting cases it finds for some classes of warnings. There's been a few interesting (to me) cases where these "init" routines would conditionally initialize a "second return value" but the caller either did not do return value checking or the callee was not marked __must_check (or both).
As with -Wsometimes-uninitialized, my experience has been that folks consistently get error handling/exceptional cases wrong in so far as passing unitialized values later. Clang's -Wsometimes-uninitialized is intra-proceedural, so doesn't catch the problems with "init" routines. Clang's static analyzer is interproceedural; the trade off being the time the analysis takes.
Maybe a new function parameter attribute would be nice?
#define __must_init __attribute__((must_init)) int init (int * __must_init x) { // ^ warning: function parameter x marked '__attribute__((must_init))' not unconditionally initialized if (stars_dont_align) { return -42; } *x = 42; return 0; } void foo (void) { int x; init(&x); /* use of x without fear */ }
> > So then the rule could be that small arguments passed by reference > have to be either initialized by the caller, or the argument must have > that "initialized by callee" attribute, and then the initialization > would be enforced in the callee instead. > > But making the rule be that the caller *always* has to initialize > sounds really wrong to me. > > Linus
-- Thanks, ~Nick Desaulniers
| |