On Fri, Jun 18, 2021 at 10:04:58AM -0700, Nathan Chancellor wrote:
> On 6/18/2021 1:44 AM, Sean Young wrote:
> > On Fri, Jun 18, 2021 at 06:18:06AM +0800, kernel test robot wrote:
> > > Hi Sean,
> > > 
> > > I love your patch! Perhaps something to improve:
> > > 
> > > [auto build test WARNING on linuxtv-media/master]
> > > [also build test WARNING on usb-serial/usb-next usb/usb-testing peter.chen-usb/for-usb-next v5.13-rc6 next-20210617]
> > > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > > And when submitting patch, we suggest to use '--base' as documented in
> > > https://git-scm.com/docs/git-format-patch]
> > > 
> > > url:    https://github.com/0day-ci/linux/commits/Sean-Young/IR-driver-for-USB-UIRT-device/20210616-182135
> > > base:   git://linuxtv.org/media_tree.git master
> > > config: powerpc64-randconfig-r012-20210617 (attached as .config)
> > > compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 64720f57bea6a6bf033feef4a5751ab9c0c3b401)
> > > reproduce (this is a W=1 build):
> > >          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > >          chmod +x ~/bin/make.cross
> > >          # install powerpc64 cross compiling tool for clang build
> > >          # apt-get install binutils-powerpc64-linux-gnu
> > >          # https://github.com/0day-ci/linux/commit/17d3a0332baecb0359e05e8ae755478c7a1a4468
> > >          git remote add linux-review https://github.com/0day-ci/linux
> > >          git fetch --no-tags linux-review Sean-Young/IR-driver-for-USB-UIRT-device/20210616-182135
> > >          git checkout 17d3a0332baecb0359e05e8ae755478c7a1a4468
> > >          # save the attached .config to linux build tree
> > >          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64
> > > 
> > > If you fix the issue, kindly add following tag as appropriate
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > 
> > > All warnings (new ones prefixed by >>):
> > > 
> > >     In file included from drivers/media/rc/uirt.c:11:
> > >     In file included from include/linux/completion.h:12:
> > >     In file included from include/linux/swait.h:5:
> > >     In file included from include/linux/list.h:9:
> > >     In file included from include/linux/kernel.h:12:
> > >     In file included from include/linux/bitops.h:32:
> > >     In file included from arch/powerpc/include/asm/bitops.h:62:
> > >     arch/powerpc/include/asm/barrier.h:49:9: warning: '__lwsync' macro redefined [-Wmacro-redefined]
> > >     #define __lwsync()      __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
> > >             ^
> > >     <built-in>:310:9: note: previous definition is here
> > >     #define __lwsync __builtin_ppc_lwsync
> > >             ^
> > > > > drivers/media/rc/uirt.c:639:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
> > >             if (!urb)
> > >                 ^~~~
> > >     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
> > >             return err;
> > 
> > This is interesting. clang is right here, there are error paths where err is
> > not initialized. gcc-11.1 does not pick this up for some reason. The error path
> > should be an immediate dominator so it shouldn't be complicated to detect.
> 
> The reason GCC does not warn about this is due to commit 78a5255ffb6a ("Stop
> the ad-hoc games with -Wno-maybe-initialized"), which disables the GCC
> version of this warning except with W=2, which very few people use. You
> could use 'KCFLAGS=-Wmaybe-uninitialized' to try and see the same warning.

You're right, this is exactly the issue here.  When running with W=2 or
KCFLAGS=-Wmaybe-uninitialized you do get the warning.

There are paths where err are initialized, so gcc correctly says:

warning: ‘err’ may be used uninitialized in this function [-Wmaybe-uninitialized]


Thanks,

Sean

> 
> Cheers,
> Nathan
> 
> > I'll send out a v5 with this issue fixed.
> > 
> > Sean
> > 
> > >                    ^~~
> > >     drivers/media/rc/uirt.c:639:2: note: remove the 'if' if its condition is always false
> > >             if (!urb)
> > >             ^~~~~~~~~
> > >     drivers/media/rc/uirt.c:630:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
> > >             if (!urb)
> > >                 ^~~~
> > >     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
> > >             return err;
> > >                    ^~~
> > >     drivers/media/rc/uirt.c:630:2: note: remove the 'if' if its condition is always false
> > >             if (!urb)
> > >             ^~~~~~~~~
> > >     drivers/media/rc/uirt.c:626:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
> > >             if (!rc)
> > >                 ^~~
> > >     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
> > >             return err;
> > >                    ^~~
> > >     drivers/media/rc/uirt.c:626:2: note: remove the 'if' if its condition is always false
> > >             if (!rc)
> > >             ^~~~~~~~
> > >     drivers/media/rc/uirt.c:622:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
> > >             if (!uirt->out)
> > >                 ^~~~~~~~~~
> > >     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
> > >             return err;
> > >                    ^~~
> > >     drivers/media/rc/uirt.c:622:2: note: remove the 'if' if its condition is always false
> > >             if (!uirt->out)
> > >             ^~~~~~~~~~~~~~~
> > >     drivers/media/rc/uirt.c:618:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
> > >             if (!uirt->in)
> > >                 ^~~~~~~~~
> > >     drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
> > >             return err;
> > >                    ^~~
> > >     drivers/media/rc/uirt.c:618:2: note: remove the 'if' if its condition is always false
> > >             if (!uirt->in)
> > >             ^~~~~~~~~~~~~~
> > >     drivers/media/rc/uirt.c:604:15: note: initialize the variable 'err' to silence this warning
> > >             int pipe, err;
> > >                          ^
> > >                           = 0
> > >     6 warnings generated.
> > > 
> > > 
> > > vim +639 drivers/media/rc/uirt.c
> > > 
> > >     594	
> > >     595	static int uirt_probe(struct usb_interface *intf,
> > >     596			      const struct usb_device_id *id)
> > >     597	{
> > >     598		struct usb_device *usbdev = interface_to_usbdev(intf);
> > >     599		struct usb_endpoint_descriptor *ep_in;
> > >     600		struct usb_endpoint_descriptor *ep_out;
> > >     601		struct uirt *uirt;
> > >     602		struct rc_dev *rc;
> > >     603		struct urb *urb;
> > >     604		int pipe, err;
> > >     605	
> > >     606		if (usb_find_common_endpoints(intf->cur_altsetting, &ep_in, &ep_out, NULL, NULL) ||
> > >     607		    usb_endpoint_maxp(ep_in) != MAX_PACKET ||
> > >     608		    usb_endpoint_maxp(ep_out) != MAX_PACKET) {
> > >     609			dev_err(&intf->dev, "required endpoints not found\n");
> > >     610			return -ENODEV;
> > >     611		}
> > >     612	
> > >     613		uirt = kzalloc(sizeof(*uirt), GFP_KERNEL);
> > >     614		if (!uirt)
> > >     615			return -ENOMEM;
> > >     616	
> > >     617		uirt->in = kmalloc(MAX_PACKET, GFP_KERNEL);
> > >     618		if (!uirt->in)
> > >     619			goto free_uirt;
> > >     620	
> > >     621		uirt->out = kmalloc(MAX_PACKET, GFP_KERNEL);
> > >     622		if (!uirt->out)
> > >     623			goto free_uirt;
> > >     624	
> > >     625		rc = rc_allocate_device(RC_DRIVER_IR_RAW);
> > >     626		if (!rc)
> > >     627			goto free_uirt;
> > >     628	
> > >     629		urb = usb_alloc_urb(0, GFP_KERNEL);
> > >     630		if (!urb)
> > >     631			goto free_rcdev;
> > >     632	
> > >     633		pipe = usb_rcvbulkpipe(usbdev, ep_in->bEndpointAddress);
> > >     634		usb_fill_bulk_urb(urb, usbdev, pipe, uirt->in, MAX_PACKET,
> > >     635				  uirt_in_callback, uirt);
> > >     636		uirt->urb_in = urb;
> > >     637	
> > >     638		urb = usb_alloc_urb(0, GFP_KERNEL);
> > >   > 639		if (!urb)
> > >     640			goto free_rcdev;
> > >     641	
> > >     642		pipe = usb_sndbulkpipe(usbdev, ep_out->bEndpointAddress);
> > >     643		usb_fill_bulk_urb(urb, usbdev, pipe, uirt->out, MAX_PACKET,
> > >     644				  uirt_out_callback, uirt);
> > >     645	
> > >     646		uirt->dev = &intf->dev;
> > >     647		uirt->usbdev = usbdev;
> > >     648		uirt->rc = rc;
> > >     649		uirt->urb_out = urb;
> > >     650		uirt->rx_state = RX_STATE_INTERSPACE_HIGH;
> > >     651	
> > >     652		err = usb_submit_urb(uirt->urb_in, GFP_KERNEL);
> > >     653		if (err != 0) {
> > >     654			dev_err(uirt->dev, "failed to submit read urb: %d\n", err);
> > >     655			goto free_rcdev;
> > >     656		}
> > >     657	
> > >     658		err = init_ftdi(usbdev);
> > >     659		if (err) {
> > >     660			dev_err(uirt->dev, "failed to setup ftdi: %d\n", err);
> > >     661			goto kill_urbs;
> > >     662		}
> > >     663	
> > >     664		err = uirt_setup(uirt);
> > >     665		if (err)
> > >     666			goto kill_urbs;
> > >     667	
> > >     668		usb_make_path(usbdev, uirt->phys, sizeof(uirt->phys));
> > >     669	
> > >     670		rc->device_name = "USB-UIRT";
> > >     671		rc->driver_name = KBUILD_MODNAME;
> > >     672		rc->input_phys = uirt->phys;
> > >     673		usb_to_input_id(usbdev, &rc->input_id);
> > >     674		rc->dev.parent = &intf->dev;
> > >     675		rc->priv = uirt;
> > >     676		rc->tx_ir = uirt_tx;
> > >     677		rc->s_tx_carrier = uirt_set_tx_carrier;
> > >     678		rc->s_learning_mode = uirt_set_rx_wideband;
> > >     679		rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
> > >     680		rc->map_name = RC_MAP_RC6_MCE;
> > >     681		rc->rx_resolution = UNIT_US;
> > >     682		rc->timeout = IR_TIMEOUT;
> > >     683	
> > >     684		uirt_set_tx_carrier(rc, 38000);
> > >     685	
> > >     686		err = rc_register_device(rc);
> > >     687		if (err)
> > >     688			goto kill_urbs;
> > >     689	
> > >     690		usb_set_intfdata(intf, uirt);
> > >     691	
> > >     692		return 0;
> > >     693	
> > >     694	kill_urbs:
> > >     695		usb_kill_urb(uirt->urb_in);
> > >     696		usb_kill_urb(uirt->urb_out);
> > >     697	free_rcdev:
> > >     698		usb_free_urb(uirt->urb_in);
> > >     699		usb_free_urb(uirt->urb_out);
> > >     700		rc_free_device(rc);
> > >     701	free_uirt:
> > >     702		kfree(uirt->in);
> > >     703		kfree(uirt->out);
> > >     704		kfree(uirt);
> > >     705		return err;
> > >     706	}
> > >     707	
> > > 
> > > ---
> > > 0-DAY CI Kernel Test Service, Intel Corporation
> > > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> > 
> >