| Message ID | 20190709230552.61842-1-natechancellor@gmail.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 4d2b8517ba1f3aba9a952ebf153ec972a127c80c |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | [v2] IB/rdmavt: Fix variable shadowing issue in rvt_create_cq | expand |
On Tue, Jul 9, 2019 at 4:06 PM Nathan Chancellor <natechancellor@gmail.com> wrote: > The function scoped err variable is uninitialized when the flow jumps > into the if statement. The if scoped err variable shadows the function > scoped err variable, preventing the err assignments within the if > statement to be reflected at the function level, which will cause > uninitialized use when the goto statements are taken. > > Just remove the if scoped err declaration so that there is only one > copy of the err variable for this function. > > Fixes: 239b0e52d8aa ("IB/hfi1: Move rvt_cq_wc struct into uapi directory") > Link: https://github.com/ClangBuiltLinux/linux/issues/594 > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > --- > > v1 -> v2: > > * Updated the wording of the commit message to use proper terms like > scoping and shadowing, thanks to review from Nick (let me know if the > wording isn't up to snuff). LGTM thanks for following up w/ v2.
On Tue, Jul 09, 2019 at 04:05:53PM -0700, Nathan Chancellor wrote: > clang warns: > > drivers/infiniband/sw/rdmavt/cq.c:260:7: warning: variable 'err' is used > uninitialized whenever 'if' condition is true > [-Wsometimes-uninitialized] > if (err) > ^~~ > drivers/infiniband/sw/rdmavt/cq.c:310:9: note: uninitialized use occurs > here > return err; > ^~~ > drivers/infiniband/sw/rdmavt/cq.c:260:3: note: remove the 'if' if its > condition is always false > if (err) > ^~~~~~~~ > drivers/infiniband/sw/rdmavt/cq.c:253:7: warning: variable 'err' is used > uninitialized whenever 'if' condition is true > [-Wsometimes-uninitialized] > if (!cq->ip) { > ^~~~~~~ > drivers/infiniband/sw/rdmavt/cq.c:310:9: note: uninitialized use occurs > here > return err; > ^~~ > drivers/infiniband/sw/rdmavt/cq.c:253:3: note: remove the 'if' if its > condition is always false > if (!cq->ip) { > ^~~~~~~~~~~~~~ > drivers/infiniband/sw/rdmavt/cq.c:211:9: note: initialize the variable > 'err' to silence this warning > int err; > ^ > = 0 > 2 warnings generated. > > The function scoped err variable is uninitialized when the flow jumps > into the if statement. The if scoped err variable shadows the function > scoped err variable, preventing the err assignments within the if > statement to be reflected at the function level, which will cause > uninitialized use when the goto statements are taken. > > Just remove the if scoped err declaration so that there is only one > copy of the err variable for this function. > > Fixes: 239b0e52d8aa ("IB/hfi1: Move rvt_cq_wc struct into uapi directory") > Link: https://github.com/ClangBuiltLinux/linux/issues/594 > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> Applied to for-next with Mike's ack Thanks, Jason
On Tue, Jul 09, 2019 at 04:05:53PM -0700, Nathan Chancellor wrote: > clang warns: > > drivers/infiniband/sw/rdmavt/cq.c:260:7: warning: variable 'err' is used > uninitialized whenever 'if' condition is true > [-Wsometimes-uninitialized] > if (err) > ^~~ > drivers/infiniband/sw/rdmavt/cq.c:310:9: note: uninitialized use occurs > here > return err; > ^~~ > drivers/infiniband/sw/rdmavt/cq.c:260:3: note: remove the 'if' if its > condition is always false > if (err) > ^~~~~~~~ > drivers/infiniband/sw/rdmavt/cq.c:253:7: warning: variable 'err' is used > uninitialized whenever 'if' condition is true > [-Wsometimes-uninitialized] > if (!cq->ip) { > ^~~~~~~ > drivers/infiniband/sw/rdmavt/cq.c:310:9: note: uninitialized use occurs > here > return err; > ^~~ > drivers/infiniband/sw/rdmavt/cq.c:253:3: note: remove the 'if' if its > condition is always false > if (!cq->ip) { > ^~~~~~~~~~~~~~ > drivers/infiniband/sw/rdmavt/cq.c:211:9: note: initialize the variable > 'err' to silence this warning > int err; > ^ > = 0 > 2 warnings generated. What version of the kernel was this found on? I don't see the problem with 5.2. AFAICS there is no 'err' in the function scope and the if scoped 'err' is initialized properly on line 239. Ira > > The function scoped err variable is uninitialized when the flow jumps > into the if statement. The if scoped err variable shadows the function > scoped err variable, preventing the err assignments within the if > statement to be reflected at the function level, which will cause > uninitialized use when the goto statements are taken. > > Just remove the if scoped err declaration so that there is only one > copy of the err variable for this function. > > Fixes: 239b0e52d8aa ("IB/hfi1: Move rvt_cq_wc struct into uapi directory") > Link: https://github.com/ClangBuiltLinux/linux/issues/594 > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > --- > > v1 -> v2: > > * Updated the wording of the commit message to use proper terms like > scoping and shadowing, thanks to review from Nick (let me know if the > wording isn't up to snuff). > > drivers/infiniband/sw/rdmavt/cq.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/infiniband/sw/rdmavt/cq.c b/drivers/infiniband/sw/rdmavt/cq.c > index fac87b13329d..a85571a4cf57 100644 > --- a/drivers/infiniband/sw/rdmavt/cq.c > +++ b/drivers/infiniband/sw/rdmavt/cq.c > @@ -247,8 +247,6 @@ int rvt_create_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr, > * See rvt_mmap() for details. > */ > if (udata && udata->outlen >= sizeof(__u64)) { > - int err; > - > cq->ip = rvt_create_mmap_info(rdi, sz, udata, u_wc); > if (!cq->ip) { > err = -ENOMEM; > -- > 2.22.0 >
On Wed, Jul 10, 2019 at 10:03:23AM -0700, Ira Weiny wrote: > What version of the kernel was this found on? > > I don't see the problem with 5.2. AFAICS there is no 'err' in the function > scope and the if scoped 'err' is initialized properly on line 239. > > Ira > $ git describe --contains 239b0e52d8aa next-20190709~84^2~57 I should probably be better about adding 'PATCH -next' to my patches, sorry for the confusion! Cheers, Nathan
On Wed, Jul 10, 2019 at 02:02:16PM -0300, Jason Gunthorpe wrote: > > Applied to for-next with Mike's ack > > Thanks, > Jason Thank you Jason, much appreciated! Nathan
On Wed, Jul 10, 2019 at 10:07:11AM -0700, Nathan Chancellor wrote: > On Wed, Jul 10, 2019 at 10:03:23AM -0700, Ira Weiny wrote: > > What version of the kernel was this found on? > > > > I don't see the problem with 5.2. AFAICS there is no 'err' in the function > > scope and the if scoped 'err' is initialized properly on line 239. > > > > Ira > > > > $ git describe --contains 239b0e52d8aa > next-20190709~84^2~57 > > I should probably be better about adding 'PATCH -next' to my patches, > sorry for the confusion! Ah I see it now... Sorry I was just not up to date on the rdma tree. Sorry about the noise. Thanks, Ira > > Cheers, > Nathan
diff --git a/drivers/infiniband/sw/rdmavt/cq.c b/drivers/infiniband/sw/rdmavt/cq.c index fac87b13329d..a85571a4cf57 100644 --- a/drivers/infiniband/sw/rdmavt/cq.c +++ b/drivers/infiniband/sw/rdmavt/cq.c @@ -247,8 +247,6 @@ int rvt_create_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr, * See rvt_mmap() for details. */ if (udata && udata->outlen >= sizeof(__u64)) { - int err; - cq->ip = rvt_create_mmap_info(rdi, sz, udata, u_wc); if (!cq->ip) { err = -ENOMEM;
clang warns: drivers/infiniband/sw/rdmavt/cq.c:260:7: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized] if (err) ^~~ drivers/infiniband/sw/rdmavt/cq.c:310:9: note: uninitialized use occurs here return err; ^~~ drivers/infiniband/sw/rdmavt/cq.c:260:3: note: remove the 'if' if its condition is always false if (err) ^~~~~~~~ drivers/infiniband/sw/rdmavt/cq.c:253:7: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized] if (!cq->ip) { ^~~~~~~ drivers/infiniband/sw/rdmavt/cq.c:310:9: note: uninitialized use occurs here return err; ^~~ drivers/infiniband/sw/rdmavt/cq.c:253:3: note: remove the 'if' if its condition is always false if (!cq->ip) { ^~~~~~~~~~~~~~ drivers/infiniband/sw/rdmavt/cq.c:211:9: note: initialize the variable 'err' to silence this warning int err; ^ = 0 2 warnings generated. The function scoped err variable is uninitialized when the flow jumps into the if statement. The if scoped err variable shadows the function scoped err variable, preventing the err assignments within the if statement to be reflected at the function level, which will cause uninitialized use when the goto statements are taken. Just remove the if scoped err declaration so that there is only one copy of the err variable for this function. Fixes: 239b0e52d8aa ("IB/hfi1: Move rvt_cq_wc struct into uapi directory") Link: https://github.com/ClangBuiltLinux/linux/issues/594 Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> --- v1 -> v2: * Updated the wording of the commit message to use proper terms like scoping and shadowing, thanks to review from Nick (let me know if the wording isn't up to snuff). drivers/infiniband/sw/rdmavt/cq.c | 2 -- 1 file changed, 2 deletions(-)