Bug 29574 - Gcc gives invalid warning about unitialized variable
Summary: Gcc gives invalid warning about unitialized variable
Status: RESOLVED DUPLICATE of bug 21513
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 4.1.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-10-23 20:28 UTC by Martin J. Bligh
Modified: 2006-10-23 21:50 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin J. Bligh 2006-10-23 20:28:25 UTC
Compile Linux 2.6.18 kernel. Warns that idx may be uninitialized in fs/bio.c:bio_alloc_bioset(), but it clearly *is* being initialized
by bvec_alloc_bs. 

The kernel team will not allow us to pre-initialize these, as it is
a gcc bug. On the other hand, we're drowned in invalid warnings, so
can't see any real problems. Older versions of gcc (3.x) did not
have this problem.

struct bio *bio_alloc_bioset(gfp_t gfp_mask, int nr_iovecs, struct bio_set *bs)
{
        struct bio *bio = mempool_alloc(bs->bio_pool, gfp_mask);

        if (likely(bio)) {
                struct bio_vec *bvl = NULL;

                bio_init(bio);
                if (likely(nr_iovecs)) {
                        unsigned long idx;

                        bvl = bvec_alloc_bs(gfp_mask, nr_iovecs, &idx, bs);
                        if (unlikely(!bvl)) {
                                mempool_free(bio, bs->bio_pool);
                                bio = NULL;
                                goto out;
                        }
                        bio->bi_flags |= idx << BIO_POOL_OFFSET;
                        bio->bi_max_vecs = bvec_slabs[idx].nr_vecs;
                }
                bio->bi_io_vec = bvl;
        }
out:
        return bio;
}

static inline struct bio_vec *bvec_alloc_bs(gfp_t gfp_mask, int nr, unsigned long *idx, struct bio_set *bs)
{
        struct bio_vec *bvl;
        struct biovec_slab *bp;

        /*
         * see comment near bvec_array define!
         */
        switch (nr) {
                case   1        : *idx = 0; break;
                case   2 ...   4: *idx = 1; break;
                case   5 ...  16: *idx = 2; break;
                case  17 ...  64: *idx = 3; break;
                case  65 ... 128: *idx = 4; break;
                case 129 ... BIO_MAX_PAGES: *idx = 5; break;
                default:
                        return NULL;
        }
        /*
         * idx now points to the pool we want to allocate from
         */

        bp = bvec_slabs + *idx;
        bvl = mempool_alloc(bs->bvec_pools[*idx], gfp_mask);
        if (bvl)
                memset(bvl, 0, bp->nr_vecs * sizeof(struct bio_vec));

        return bvl;
}
Comment 1 Andrew Pinski 2006-10-23 20:57:25 UTC
How is unlikely defined?
Comment 2 Andrew Pinski 2006-10-23 20:59:02 UTC
If it is defined using __builtin_expect, then this is most likely PR 21513 which was fixed for 4.2.0, it is hard to fix correctly for 4.1.0 and 4.0.0 without removing all of loop.c.

Can you attach the preprocessed source?
Comment 3 Martin J. Bligh 2006-10-23 21:08:55 UTC
Yeah, is builtin_expect ;-(

#define likely(x)       __builtin_expect(!!(x), 1)
#define unlikely(x)     __builtin_expect(!!(x), 0)
Comment 4 Martin J. Bligh 2006-10-23 21:19:08 UTC
And indeed, if I remove that unlikely(), it does work.

There's another set of these where the called initializer is not inlined,
ie:

int x;
initializer(&x);

Which it seems totally blind to, even if the initializer does "x = 0"
right at the top. Are those fixable?
Comment 5 Andrew Pinski 2006-10-23 21:30:15 UTC
(In reply to comment #4)
> And indeed, if I remove that unlikely(), it does work.
> 
> There's another set of these where the called initializer is not inlined,
> ie:
> 
> int x;
> initializer(&x);
If initializer is not inlined, you will not get a warning.

This is the same as PR 21513 as __builtin_expect is getting in the way.

*** This bug has been marked as a duplicate of 21513 ***
Comment 6 Martin J. Bligh 2006-10-23 21:42:37 UTC
Re the non-inlined functions ...
Have been discussing this with Andrew Morton.

what do we do then? Adding dead code to fix the fact that gcc can't see into other functions is incorrect and inefficient.

Having lots of compiler warnings drowns out real ones, and is not acceptable.

How do we get around this? Can we at least get the option to not warn if it's "blind" to the problem because it goes through another function, if not make that the default?

gcc warnings are an incredibly valuable way to find code bugs if they're real. If some are not real, we lose the benefit to all, as the real ones are drowned in the noise.
Comment 7 Andrew Pinski 2006-10-23 21:45:42 UTC
(In reply to comment #6)
> Re the non-inlined functions ...
> Have been discussing this with Andrew Morton.
> 
> what do we do then? Adding dead code to fix the fact that gcc can't see into
> other functions is incorrect and inefficient.

Initialize the variable and forget about inefficiency.  Again this is fixed for 4.2.0, the warning is only because __builtin_expect gets in the way of figuring out if the variable is used uninitialized, nothing more.
Comment 8 Martin J. Bligh 2006-10-23 21:50:17 UTC
> Initialize the variable and forget about inefficiency.  Again this is fixed for
> 4.2.0, the warning is only because __builtin_expect gets in the way of figuring
> out if the variable is used uninitialized, nothing more.

I understand the __builtin_expect case is fixed in 4.2, and am fine with that, but I was talking about the ones that aren't inlined, and don't use builtin_expect. Does gcc 4.2 walk through functions as well, or will it always produce an invalid warning there?

Thanks, M.
Comment 9 pinskia@earth.geop.uc.edu 2006-10-23 21:52:22 UTC
Subject: Re:  Gcc gives invalid warning about unitialized variable

> 
> 
> 
> ------- Comment #8 from mbligh at mbligh dot org  2006-10-23 21:50 -------
> > Initialize the variable and forget about inefficiency.  Again this is fixed for
> > 4.2.0, the warning is only because __builtin_expect gets in the way of figuring
> > out if the variable is used uninitialized, nothing more.
> 
> I understand the __builtin_expect case is fixed in 4.2, and am fine with that,
> but I was talking about the ones that aren't inlined, and don't use
> builtin_expect. Does gcc 4.2 walk through functions as well, or will it always
> produce an invalid warning there?

No, for ones which are not inlined, GCC does not look through other functions to
check for the warning.

Thanks,
Andrew Pinski