tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 8096acd7442e613fad0354fc8dfdb2003cceea0b
commit: e927e1e0f0dd3e353d5556503a71484008692c82 v4l: ioctl: Use %p4cc printk modifier to print FourCC codes
date: 5 months ago
config: mips-randconfig-r004-20210714 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 8d69635ed9ecf36fd0ca85906bfde17949671cbe)
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 mips cross compiling tool for clang build
# apt-get install binutils-mips-linux-gnu
# https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e927e1e0f0dd3e353d5556503a71484008692c82
git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git fetch --no-tags linus master
git checkout e927e1e0f0dd3e353d5556503a71484008692c82
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> drivers/media/v4l2-core/v4l2-ioctl.c:303:28: warning: taking address of packed member 'pixelformat' of class or structure 'v4l2_pix_format_mplane' may result in an unaligned pointer value [-Waddress-of-packed-member]
mp->width, mp->height, &mp->pixelformat,
^~~~~~~~~~~~~~~
include/linux/printk.h:385:26: note: expanded from macro 'pr_cont'
printk(KERN_CONT fmt, ##__VA_ARGS__)
^~~~~~~~~~~
>> drivers/media/v4l2-core/v4l2-ioctl.c:347:37: warning: taking address of packed member 'pixelformat' of class or structure 'v4l2_sdr_format' may result in an unaligned pointer value [-Waddress-of-packed-member]
pr_cont(", pixelformat=%p4cc\n", &sdr->pixelformat);
^~~~~~~~~~~~~~~~
include/linux/printk.h:385:26: note: expanded from macro 'pr_cont'
printk(KERN_CONT fmt, ##__VA_ARGS__)
^~~~~~~~~~~
>> drivers/media/v4l2-core/v4l2-ioctl.c:353:5: warning: taking address of packed member 'dataformat' of class or structure 'v4l2_meta_format' may result in an unaligned pointer value [-Waddress-of-packed-member]
&meta->dataformat, meta->buffersize);
^~~~~~~~~~~~~~~~
include/linux/printk.h:385:26: note: expanded from macro 'pr_cont'
printk(KERN_CONT fmt, ##__VA_ARGS__)
^~~~~~~~~~~
3 warnings generated.
vim +303 drivers/media/v4l2-core/v4l2-ioctl.c
273
274 static void v4l_print_format(const void *arg, bool write_only)
275 {
276 const struct v4l2_format *p = arg;
277 const struct v4l2_pix_format *pix;
278 const struct v4l2_pix_format_mplane *mp;
279 const struct v4l2_vbi_format *vbi;
280 const struct v4l2_sliced_vbi_format *sliced;
281 const struct v4l2_window *win;
282 const struct v4l2_sdr_format *sdr;
283 const struct v4l2_meta_format *meta;
284 u32 planes;
285 unsigned i;
286
287 pr_cont("type=%s", prt_names(p->type, v4l2_type_names));
288 switch (p->type) {
289 case V4L2_BUF_TYPE_VIDEO_CAPTURE:
290 case V4L2_BUF_TYPE_VIDEO_OUTPUT:
291 pix = &p->fmt.pix;
292 pr_cont(", width=%u, height=%u, pixelformat=%p4cc, field=%s, bytesperline=%u, sizeimage=%u, colorspace=%d, flags=0x%x, ycbcr_enc=%u, quantization=%u, xfer_func=%u\n",
293 pix->width, pix->height, &pix->pixelformat,
294 prt_names(pix->field, v4l2_field_names),
295 pix->bytesperline, pix->sizeimage,
296 pix->colorspace, pix->flags, pix->ycbcr_enc,
297 pix->quantization, pix->xfer_func);
298 break;
299 case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
300 case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
301 mp = &p->fmt.pix_mp;
302 pr_cont(", width=%u, height=%u, format=%p4cc, field=%s, colorspace=%d, num_planes=%u, flags=0x%x, ycbcr_enc=%u, quantization=%u, xfer_func=%u\n",
> 303 mp->width, mp->height, &mp->pixelformat,
304 prt_names(mp->field, v4l2_field_names),
305 mp->colorspace, mp->num_planes, mp->flags,
306 mp->ycbcr_enc, mp->quantization, mp->xfer_func);
307 planes = min_t(u32, mp->num_planes, VIDEO_MAX_PLANES);
308 for (i = 0; i < planes; i++)
309 printk(KERN_DEBUG "plane %u: bytesperline=%u sizeimage=%u\n", i,
310 mp->plane_fmt[i].bytesperline,
311 mp->plane_fmt[i].sizeimage);
312 break;
313 case V4L2_BUF_TYPE_VIDEO_OVERLAY:
314 case V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY:
315 win = &p->fmt.win;
316 /* Note: we can't print the clip list here since the clips
317 * pointer is a userspace pointer, not a kernelspace
318 * pointer. */
319 pr_cont(", wxh=%dx%d, x,y=%d,%d, field=%s, chromakey=0x%08x, clipcount=%u, clips=%p, bitmap=%p, global_alpha=0x%02x\n",
320 win->w.width, win->w.height, win->w.left, win->w.top,
321 prt_names(win->field, v4l2_field_names),
322 win->chromakey, win->clipcount, win->clips,
323 win->bitmap, win->global_alpha);
324 break;
325 case V4L2_BUF_TYPE_VBI_CAPTURE:
326 case V4L2_BUF_TYPE_VBI_OUTPUT:
327 vbi = &p->fmt.vbi;
328 pr_cont(", sampling_rate=%u, offset=%u, samples_per_line=%u, sample_format=%p4cc, start=%u,%u, count=%u,%u\n",
329 vbi->sampling_rate, vbi->offset,
330 vbi->samples_per_line, &vbi->sample_format,
331 vbi->start[0], vbi->start[1],
332 vbi->count[0], vbi->count[1]);
333 break;
334 case V4L2_BUF_TYPE_SLICED_VBI_CAPTURE:
335 case V4L2_BUF_TYPE_SLICED_VBI_OUTPUT:
336 sliced = &p->fmt.sliced;
337 pr_cont(", service_set=0x%08x, io_size=%d\n",
338 sliced->service_set, sliced->io_size);
339 for (i = 0; i < 24; i++)
340 printk(KERN_DEBUG "line[%02u]=0x%04x, 0x%04x\n", i,
341 sliced->service_lines[0][i],
342 sliced->service_lines[1][i]);
343 break;
344 case V4L2_BUF_TYPE_SDR_CAPTURE:
345 case V4L2_BUF_TYPE_SDR_OUTPUT:
346 sdr = &p->fmt.sdr;
> 347 pr_cont(", pixelformat=%p4cc\n", &sdr->pixelformat);
348 break;
349 case V4L2_BUF_TYPE_META_CAPTURE:
350 case V4L2_BUF_TYPE_META_OUTPUT:
351 meta = &p->fmt.meta;
352 pr_cont(", dataformat=%p4cc, buffersize=%u\n",
> 353 &meta->dataformat, meta->buffersize);
354 break;
355 }
356 }
357
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
+Cc: Nick. Nick, any recommendations on how to fix this in the best
possible way?
On Wed, Jul 14, 2021 at 8:58 PM kernel test robot <[email protected]> wrote:
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 8096acd7442e613fad0354fc8dfdb2003cceea0b
> commit: e927e1e0f0dd3e353d5556503a71484008692c82 v4l: ioctl: Use %p4cc printk modifier to print FourCC codes
> date: 5 months ago
> config: mips-randconfig-r004-20210714 (attached as .config)
> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 8d69635ed9ecf36fd0ca85906bfde17949671cbe)
> 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 mips cross compiling tool for clang build
> # apt-get install binutils-mips-linux-gnu
> # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e927e1e0f0dd3e353d5556503a71484008692c82
> git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git fetch --no-tags linus master
> git checkout e927e1e0f0dd3e353d5556503a71484008692c82
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
>
> All warnings (new ones prefixed by >>):
>
> >> drivers/media/v4l2-core/v4l2-ioctl.c:303:28: warning: taking address of packed member 'pixelformat' of class or structure 'v4l2_pix_format_mplane' may result in an unaligned pointer value [-Waddress-of-packed-member]
> mp->width, mp->height, &mp->pixelformat,
This seems unsolvable without copying a value.
> >> drivers/media/v4l2-core/v4l2-ioctl.c:347:37: warning: taking address of packed member 'pixelformat' of class or structure 'v4l2_sdr_format' may result in an unaligned pointer value [-Waddress-of-packed-member]
Why is it packed in the first place? Is it used on unaligned addresses
in other structures? But even so, why should it matter?
> >> drivers/media/v4l2-core/v4l2-ioctl.c:353:5: warning: taking address of packed member 'dataformat' of class or structure 'v4l2_meta_format' may result in an unaligned pointer value [-Waddress-of-packed-member]
> &meta->dataformat, meta->buffersize);
Ditto.
--
With Best Regards,
Andy Shevchenko
Hi Andy,
On Wed, Jul 14, 2021 at 10:45:26PM +0300, Andy Shevchenko wrote:
> > >> drivers/media/v4l2-core/v4l2-ioctl.c:347:37: warning: taking address of packed member 'pixelformat' of class or structure 'v4l2_sdr_format' may result in an unaligned pointer value [-Waddress-of-packed-member]
>
> Why is it packed in the first place? Is it used on unaligned addresses
> in other structures? But even so, why should it matter?
It's packed since we wanted to avoid having holes in the structs. There are
other ways to do that but it's ABI dependent and is prone to human errors,
too.
--
Sakari Ailus
On Fri, Jul 16, 2021 at 02:41:05PM +0300, Sakari Ailus wrote:
> On Wed, Jul 14, 2021 at 10:45:26PM +0300, Andy Shevchenko wrote:
> > > >> drivers/media/v4l2-core/v4l2-ioctl.c:347:37: warning: taking address of packed member 'pixelformat' of class or structure 'v4l2_sdr_format' may result in an unaligned pointer value [-Waddress-of-packed-member]
> >
> > Why is it packed in the first place? Is it used on unaligned addresses
> > in other structures? But even so, why should it matter?
>
> It's packed since we wanted to avoid having holes in the structs. There are
> other ways to do that but it's ABI dependent and is prone to human errors,
> too.
What holes can you think about in the above mention structure?
In case if you are going to extend it you will need anyway changes somewhere
else as well.
--
With Best Regards,
Andy Shevchenko
On Fri, Jul 16, 2021 at 03:12:11PM +0300, Andy Shevchenko wrote:
> On Fri, Jul 16, 2021 at 02:41:05PM +0300, Sakari Ailus wrote:
> > On Wed, Jul 14, 2021 at 10:45:26PM +0300, Andy Shevchenko wrote:
> > > > >> drivers/media/v4l2-core/v4l2-ioctl.c:347:37: warning: taking address of packed member 'pixelformat' of class or structure 'v4l2_sdr_format' may result in an unaligned pointer value [-Waddress-of-packed-member]
> > >
> > > Why is it packed in the first place? Is it used on unaligned addresses
> > > in other structures? But even so, why should it matter?
> >
> > It's packed since we wanted to avoid having holes in the structs. There are
> > other ways to do that but it's ABI dependent and is prone to human errors,
> > too.
>
> What holes can you think about in the above mention structure?
Probably not that one but it has happened in the past that the struct
memory layout has been unintentionally different in different ABIs and that
has not been the intention, but rather a bug. Packing has been added in
newer structs to avoid that.
--
Sakari Ailus
On Thu 2021-08-19 11:10:53, Sakari Ailus wrote:
> On Fri, Jul 16, 2021 at 03:12:11PM +0300, Andy Shevchenko wrote:
> > On Fri, Jul 16, 2021 at 02:41:05PM +0300, Sakari Ailus wrote:
> > > On Wed, Jul 14, 2021 at 10:45:26PM +0300, Andy Shevchenko wrote:
> > > > > >> drivers/media/v4l2-core/v4l2-ioctl.c:347:37: warning: taking address of packed member 'pixelformat' of class or structure 'v4l2_sdr_format' may result in an unaligned pointer value [-Waddress-of-packed-member]
> > > >
Is there any plan to fix this, please?
> > > > Why is it packed in the first place? Is it used on unaligned addresses
> > > > in other structures? But even so, why should it matter?
> > >
> > > It's packed since we wanted to avoid having holes in the structs. There are
> > > other ways to do that but it's ABI dependent and is prone to human errors,
> > > too.
> > What holes can you think about in the above mention structure?
>
> Probably not that one but it has happened in the past that the struct
> memory layout has been unintentionally different in different ABIs and that
> has not been the intention, but rather a bug.
What kind of bugs did the different ABI caused, please? Incompatibly
between 3rd party drivers that were built with different compilers?
I am not familiar with these problems. I wonder if there is a better
solution. I guess that it might be a common problem affecting most
drivers.
Anyway, the non-aligned struct members might create slower code.
> Packing has been added in newer structs to avoid that.
And this smells with cargo-cult programming. People might make all new
structures packed even when it is not really needed.
Best Regards,
Petr