Re: Remaining randconfig objtool warnings, linux-next-20200428

[Arnd Bergmann]

On Thu, Apr 30, 2020 at 4:05 PM Arnd Bergmann <arnd@xxxxxxxx> wrote:
> On Tue, Apr 28, 2020 at 4:49 PM Arnd Bergmann <arnd@xxxxxxxx> wrote:
> drivers/media/dvb-frontends/rtl2832_sdr.o: warning: objtool: .text.unlikely: unexpected end of section
> drivers/media/dvb-frontends/rtl2832_sdr.o: warning: objtool: rtl2832_sdr_try_fmt_sdr_cap() falls through to next function
> rtl2832_sdr_s_fmt_sdr_cap.cold()

I had a look at this one and found this happens when  gcc optimizes this loop

        memset(f->fmt.sdr.reserved, 0, sizeof(f->fmt.sdr.reserved));
        for (i = 0; i < dev->num_formats; i++) {
                if (formats[i].pixelformat == f->fmt.sdr.pixelformat) {
                        f->fmt.sdr.buffersize = formats[i].buffersize;

formats[] is a static array, so if num_formats is larger than
ARRAY_SIZE(formats),
it gets into undefined behavior and stops emitting code after the call to
__sanitizer_cov_trace_pc.
https://godbolt.org/z/h9Gx3S shows a reduced test case:

struct v4l2_sdr_format {
  int pixelformat;
  int buffersize;
};
struct rtl2832_sdr_format {
  int pixelformat;
  int buffersize;
};
static struct rtl2832_sdr_format formats[] = {{}, {}};
struct rtl2832_sdr_dev {
  int num_formats;
};
void rtl2832_sdr_try_fmt_sdr_cap(struct v4l2_sdr_format *f,
                                 struct rtl2832_sdr_dev *dev) {
  int i = 0;
  for (; i < dev->num_formats; i++)
    if (formats[i].pixelformat)
      f->buffersize = 0;
}

With this source change, the warning goes away:

diff --git a/drivers/media/dvb-frontends/rtl2832_sdr.c
b/drivers/media/dvb-frontends/rtl2832_sdr.c
index 60d1e59d2292..faae510985e0 100644
--- a/drivers/media/dvb-frontends/rtl2832_sdr.c
+++ b/drivers/media/dvb-frontends/rtl2832_sdr.c
@@ -1150,7 +1150,7 @@ static int rtl2832_sdr_s_fmt_sdr_cap(struct file
*file, void *priv,
                return -EBUSY;

        memset(f->fmt.sdr.reserved, 0, sizeof(f->fmt.sdr.reserved));
-       for (i = 0; i < dev->num_formats; i++) {
+       for (i = 0; i < min(dev->num_formats, NUM_FORMATS); i++) {
                if (formats[i].pixelformat == f->fmt.sdr.pixelformat) {
                        dev->pixelformat = formats[i].pixelformat;
                        dev->buffersize = formats[i].buffersize;
@@ -1178,7 +1178,7 @@ static int rtl2832_sdr_try_fmt_sdr_cap(struct
file *file, void *priv,
                (char *)&f->fmt.sdr.pixelformat);

        memset(f->fmt.sdr.reserved, 0, sizeof(f->fmt.sdr.reserved));
-       for (i = 0; i < dev->num_formats; i++) {
+       for (i = 0; i < min(dev->num_formats, NUM_FORMATS); i++) {
                if (formats[i].pixelformat == f->fmt.sdr.pixelformat) {
                        f->fmt.sdr.buffersize = formats[i].buffersize;
                        return 0;

Do we consider this expected behavior on gcc's side, or is it something
that should not happen and needs a gcc bug report?

       Arnd