kernel/rcutree.c:2850:13: warning: array subscript is above array bounds

DISCUSSION Markus Trippelsdorf 2 participants 8 messages Nov 29, 2012

Share & tools

Markus Trippelsdorf

author

2012-11-29 13:48 UTC #0 lore raw

With gcc-4.8 I get:

CC kernel/rcutree.o
kernel/rcutree.c: In function ‘rcu_init_one’:
kernel/rcutree.c:2850:13: warning: array subscript is above array bounds [-Warray-bounds]

     rsp->level[i] = rsp->level[i - 1] + rsp->levelcnt[i - 1];
                  ^

2849 for (i = 1; i < rcu_num_lvls; i++)
2850 rsp->level[i] = rsp->level[i - 1] + rsp->levelcnt[i - 1];

At first I thought that the warning was bogus, but rcu_num_lvls isn't static and gets modified prior to the for loop.

Signature · 1 line

Markus

Paul E. McKenney

2012-11-29 17:09 UTC #1 lore raw

Quoted 5 lines · Markus Trippelsdorf

With gcc-4.8 I get:
CC kernel/rcutree.o
kernel/rcutree.c: In function ‘rcu_init_one’:
kernel/rcutree.c:2850:13: warning: array subscript is above array bounds [-Warray-bounds]

Quoted 7 lines

     rsp->level[i] = rsp->level[i - 1] + rsp->levelcnt[i - 1];
                  ^
2849 for (i = 1; i < rcu_num_lvls; i++)
2850 rsp->level[i] = rsp->level[i - 1] + rsp->levelcnt[i - 1];
At first I thought that the warning was bogus, but rcu_num_lvls isn't static and gets modified prior to the for loop.

You are quite correct that rcu_num_lvls does get modified, but there are checks in rcu_init_geometry() to ensure that it does not increase:

	/*
	 * The boot-time rcu_fanout_leaf parameter is only permitted
	 * to increase the leaf-level fanout, not decrease it.  Of course,
	 * the leaf-level fanout cannot exceed the number of bits in
	 * the rcu_node masks.  Finally, the tree must be able to accommodate
	 * the configured number of CPUs.  Complain and fall back to the
	 * compile-time values if these limits are exceeded.
	 */
	if (rcu_fanout_leaf < CONFIG_RCU_FANOUT_LEAF ||
	    rcu_fanout_leaf > sizeof(unsigned long) * 8 ||
	    n > rcu_capacity[MAX_RCU_LVLS]) {
		WARN_ON(1);
		return;
	}

The value of rcu_num_lvls starts out at RCU_NUM_LVLS, the same as the dimension of the ->level[] array. The loop goes only to one less than rcu_num_lvls, as needed, and rcu_num_lvls is never greater than RCU_NUM_LVLS, so this should be safe.

So what am I missing here?

Thanx, Paul

Markus Trippelsdorf

author

2012-11-29 17:44 UTC #2 lore raw

Quoted 32 lines · Paul E. McKenney

Quoted 5 lines · Markus Trippelsdorf
With gcc-4.8 I get:
CC kernel/rcutree.o
kernel/rcutree.c: In function ‘rcu_init_one’:
kernel/rcutree.c:2850:13: warning: array subscript is above array bounds [-Warray-bounds]
Quoted 7 lines
     rsp->level[i] = rsp->level[i - 1] + rsp->levelcnt[i - 1];
                  ^
2849 for (i = 1; i < rcu_num_lvls; i++)
2850 rsp->level[i] = rsp->level[i - 1] + rsp->levelcnt[i - 1];
At first I thought that the warning was bogus, but rcu_num_lvls isn't static and gets modified prior to the for loop.
You are quite correct that rcu_num_lvls does get modified, but there are checks in rcu_init_geometry() to ensure that it does not increase:
	/*
	 * The boot-time rcu_fanout_leaf parameter is only permitted
	 * to increase the leaf-level fanout, not decrease it.  Of course,
	 * the leaf-level fanout cannot exceed the number of bits in
	 * the rcu_node masks.  Finally, the tree must be able to accommodate
	 * the configured number of CPUs.  Complain and fall back to the
	 * compile-time values if these limits are exceeded.
	 */
	if (rcu_fanout_leaf < CONFIG_RCU_FANOUT_LEAF ||
	    rcu_fanout_leaf > sizeof(unsigned long) * 8 ||
	    n > rcu_capacity[MAX_RCU_LVLS]) {
		WARN_ON(1);
		return;
	}

Quoted 6 lines

The value of rcu_num_lvls starts out at RCU_NUM_LVLS, the same as the dimension of the ->level[] array. The loop goes only to one less than rcu_num_lvls, as needed, and rcu_num_lvls is never greater than RCU_NUM_LVLS, so this should be safe.
So what am I missing here?

rcu_num_lvls does get modified in rcu_init_geometry:

2942 /* Calculate the number of rcu_nodes at each level of the tree. */
2943 for (i = 1; i <= MAX_RCU_LVLS; i++)
2944 if (n <= rcu_capacity[i]) {
2945 for (j = 0; j <= i; j++)
2946 num_rcu_lvl[j] =
2947 DIV_ROUND_UP(n, rcu_capacity[i - j]);
2948 rcu_num_lvls = i;

And rcu_init_geometry gets called before rcu_init_one, so the compiler assumes the worst and issues a warning.
So, in your opinion, what would be the best way to silence this warning?

Signature · 1 line

Markus

Paul E. McKenney

2012-11-29 18:10 UTC #3 lore raw

Quoted 50 lines · Markus Trippelsdorf

Quoted 32 lines · Paul E. McKenney
Quoted 5 lines · Markus Trippelsdorf
With gcc-4.8 I get:
CC kernel/rcutree.o
kernel/rcutree.c: In function ‘rcu_init_one’:
kernel/rcutree.c:2850:13: warning: array subscript is above array bounds [-Warray-bounds]
Quoted 7 lines
     rsp->level[i] = rsp->level[i - 1] + rsp->levelcnt[i - 1];
                  ^
2849 for (i = 1; i < rcu_num_lvls; i++)
2850 rsp->level[i] = rsp->level[i - 1] + rsp->levelcnt[i - 1];
At first I thought that the warning was bogus, but rcu_num_lvls isn't static and gets modified prior to the for loop.
You are quite correct that rcu_num_lvls does get modified, but there are checks in rcu_init_geometry() to ensure that it does not increase:
	/*
	 * The boot-time rcu_fanout_leaf parameter is only permitted
	 * to increase the leaf-level fanout, not decrease it.  Of course,
	 * the leaf-level fanout cannot exceed the number of bits in
	 * the rcu_node masks.  Finally, the tree must be able to accommodate
	 * the configured number of CPUs.  Complain and fall back to the
	 * compile-time values if these limits are exceeded.
	 */
	if (rcu_fanout_leaf < CONFIG_RCU_FANOUT_LEAF ||
	    rcu_fanout_leaf > sizeof(unsigned long) * 8 ||
	    n > rcu_capacity[MAX_RCU_LVLS]) {
		WARN_ON(1);
		return;
	}
Quoted 6 lines
The value of rcu_num_lvls starts out at RCU_NUM_LVLS, the same as the dimension of the ->level[] array. The loop goes only to one less than rcu_num_lvls, as needed, and rcu_num_lvls is never greater than RCU_NUM_LVLS, so this should be safe.
So what am I missing here?
rcu_num_lvls does get modified in rcu_init_geometry:
2942 /* Calculate the number of rcu_nodes at each level of the tree. */
2943 for (i = 1; i <= MAX_RCU_LVLS; i++)
2944 if (n <= rcu_capacity[i]) {
2945 for (j = 0; j <= i; j++)
2946 num_rcu_lvl[j] =
2947 DIV_ROUND_UP(n, rcu_capacity[i - j]);
2948 rcu_num_lvls = i;

Quoted 3 lines

And rcu_init_geometry gets called before rcu_init_one, so the compiler assumes the worst and issues a warning.
So, in your opinion, what would be the best way to silence this warning?

Good question. Are you saying that if the compiler cannot prove that the index is in bounds, it is going to throw a warning? If that is the case, perhaps telling the compiler to cool it via the command line would be best.

Or is this really one of a very few places in the kernel where the compiler is complaining?

Thanx, Paul

Markus Trippelsdorf

author

2012-11-29 18:22 UTC #4 lore raw

Quoted 51 lines · Paul E. McKenney

Quoted 50 lines · Markus Trippelsdorf
Quoted 32 lines · Paul E. McKenney
Quoted 5 lines · Markus Trippelsdorf
With gcc-4.8 I get:
CC kernel/rcutree.o
kernel/rcutree.c: In function ‘rcu_init_one’:
kernel/rcutree.c:2850:13: warning: array subscript is above array bounds [-Warray-bounds]
Quoted 7 lines
     rsp->level[i] = rsp->level[i - 1] + rsp->levelcnt[i - 1];
                  ^
2849 for (i = 1; i < rcu_num_lvls; i++)
2850 rsp->level[i] = rsp->level[i - 1] + rsp->levelcnt[i - 1];
At first I thought that the warning was bogus, but rcu_num_lvls isn't static and gets modified prior to the for loop.
You are quite correct that rcu_num_lvls does get modified, but there are checks in rcu_init_geometry() to ensure that it does not increase:
	/*
	 * The boot-time rcu_fanout_leaf parameter is only permitted
	 * to increase the leaf-level fanout, not decrease it.  Of course,
	 * the leaf-level fanout cannot exceed the number of bits in
	 * the rcu_node masks.  Finally, the tree must be able to accommodate
	 * the configured number of CPUs.  Complain and fall back to the
	 * compile-time values if these limits are exceeded.
	 */
	if (rcu_fanout_leaf < CONFIG_RCU_FANOUT_LEAF ||
	    rcu_fanout_leaf > sizeof(unsigned long) * 8 ||
	    n > rcu_capacity[MAX_RCU_LVLS]) {
		WARN_ON(1);
		return;
	}
Quoted 6 lines
The value of rcu_num_lvls starts out at RCU_NUM_LVLS, the same as the dimension of the ->level[] array. The loop goes only to one less than rcu_num_lvls, as needed, and rcu_num_lvls is never greater than RCU_NUM_LVLS, so this should be safe.
So what am I missing here?
rcu_num_lvls does get modified in rcu_init_geometry:
2942 /* Calculate the number of rcu_nodes at each level of the tree. */
2943 for (i = 1; i <= MAX_RCU_LVLS; i++)
2944 if (n <= rcu_capacity[i]) {
2945 for (j = 0; j <= i; j++)
2946 num_rcu_lvl[j] =
2947 DIV_ROUND_UP(n, rcu_capacity[i - j]);
2948 rcu_num_lvls = i;

Quoted 6 lines

Quoted 3 lines
And rcu_init_geometry gets called before rcu_init_one, so the compiler assumes the worst and issues a warning.
So, in your opinion, what would be the best way to silence this warning?
Good question. Are you saying that if the compiler cannot prove that the index is in bounds, it is going to throw a warning?

Yes, it does seem to be the case. See also my gcc bug report (closed as invalid): http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55529

Quoted 4 lines

If that is the case, perhaps telling the compiler to cool it via the command line would be best.
Or is this really one of a very few places in the kernel where the compiler is complaining?

Yes. With my (admittedly minimal) config this is only place.

Signature · 1 line

Markus

Paul E. McKenney

2012-11-29 19:28 UTC #5 lore raw

Quoted 62 lines · Markus Trippelsdorf

Quoted 51 lines · Paul E. McKenney
Quoted 50 lines · Markus Trippelsdorf
Quoted 32 lines · Paul E. McKenney
Quoted 5 lines · Markus Trippelsdorf
With gcc-4.8 I get:
CC kernel/rcutree.o
kernel/rcutree.c: In function ‘rcu_init_one’:
kernel/rcutree.c:2850:13: warning: array subscript is above array bounds [-Warray-bounds]
Quoted 7 lines
     rsp->level[i] = rsp->level[i - 1] + rsp->levelcnt[i - 1];
                  ^
2849 for (i = 1; i < rcu_num_lvls; i++)
2850 rsp->level[i] = rsp->level[i - 1] + rsp->levelcnt[i - 1];
At first I thought that the warning was bogus, but rcu_num_lvls isn't static and gets modified prior to the for loop.
You are quite correct that rcu_num_lvls does get modified, but there are checks in rcu_init_geometry() to ensure that it does not increase:
	/*
	 * The boot-time rcu_fanout_leaf parameter is only permitted
	 * to increase the leaf-level fanout, not decrease it.  Of course,
	 * the leaf-level fanout cannot exceed the number of bits in
	 * the rcu_node masks.  Finally, the tree must be able to accommodate
	 * the configured number of CPUs.  Complain and fall back to the
	 * compile-time values if these limits are exceeded.
	 */
	if (rcu_fanout_leaf < CONFIG_RCU_FANOUT_LEAF ||
	    rcu_fanout_leaf > sizeof(unsigned long) * 8 ||
	    n > rcu_capacity[MAX_RCU_LVLS]) {
		WARN_ON(1);
		return;
	}
Quoted 6 lines
The value of rcu_num_lvls starts out at RCU_NUM_LVLS, the same as the dimension of the ->level[] array. The loop goes only to one less than rcu_num_lvls, as needed, and rcu_num_lvls is never greater than RCU_NUM_LVLS, so this should be safe.
So what am I missing here?
rcu_num_lvls does get modified in rcu_init_geometry:
2942 /* Calculate the number of rcu_nodes at each level of the tree. */
2943 for (i = 1; i <= MAX_RCU_LVLS; i++)
2944 if (n <= rcu_capacity[i]) {
2945 for (j = 0; j <= i; j++)
2946 num_rcu_lvl[j] =
2947 DIV_ROUND_UP(n, rcu_capacity[i - j]);
2948 rcu_num_lvls = i;
Quoted 6 lines
Quoted 3 lines
And rcu_init_geometry gets called before rcu_init_one, so the compiler assumes the worst and issues a warning.
So, in your opinion, what would be the best way to silence this warning?
Good question. Are you saying that if the compiler cannot prove that the index is in bounds, it is going to throw a warning?
Yes, it does seem to be the case. See also my gcc bug report (closed as invalid): http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55529

Quoted 6 lines

Quoted 4 lines
If that is the case, perhaps telling the compiler to cool it via the command line would be best.
Or is this really one of a very few places in the kernel where the compiler is complaining?
Yes. With my (admittedly minimal) config this is only place.

Hmmmm... In that case...

Given that this is initialization code that is far from any fastpath, could you try putting something like this at the beginning of rcu_init_one()?

	if (rcu_num_lvls > RCU_NUM_LVLS)
		panic("rcu_num_lvls overflow");

If the compiler doesn't know that panic() never returns (despite the __noreturn), you could add a "return" after the panic().

Does that help?

Thanx, Paul

Markus Trippelsdorf

author

2012-11-29 20:03 UTC #6 lore raw

Quoted 39 lines · Paul E. McKenney

Quoted 25 lines · Markus Trippelsdorf
Quoted 13 lines · Paul E. McKenney
Quoted 12 lines · Markus Trippelsdorf
Quoted 11 lines · Paul E. McKenney
Quoted 10 lines · Markus Trippelsdorf
With gcc-4.8 I get:
CC kernel/rcutree.o
kernel/rcutree.c: In function ‘rcu_init_one’:
kernel/rcutree.c:2850:13: warning: array subscript is above array bounds [-Warray-bounds]
     rsp->level[i] = rsp->level[i - 1] + rsp->levelcnt[i - 1];
                  ^
2849 for (i = 1; i < rcu_num_lvls; i++)
2850 rsp->level[i] = rsp->level[i - 1] + rsp->levelcnt[i - 1];
Quoted 7 lines
Quoted 4 lines
Quoted 3 lines
Quoted 2 lines
At first I thought that the warning was bogus, but rcu_num_lvls isn't static and gets modified prior to the for loop.
So, in your opinion, what would be the best way to silence this warning?
Good question. Are you saying that if the compiler cannot prove that the index is in bounds, it is going to throw a warning?
Yes, it does seem to be the case. See also my gcc bug report (closed as invalid): http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55529
Quoted 6 lines
Quoted 4 lines
If that is the case, perhaps telling the compiler to cool it via the command line would be best.
Or is this really one of a very few places in the kernel where the compiler is complaining?
Yes. With my (admittedly minimal) config this is only place.
Hmmmm... In that case...
Given that this is initialization code that is far from any fastpath, could you try putting something like this at the beginning of rcu_init_one()?

Quoted 7 lines

	if (rcu_num_lvls > RCU_NUM_LVLS)
		panic("rcu_num_lvls overflow");
If the compiler doesn't know that panic() never returns (despite the __noreturn), you could add a "return" after the panic().
Does that help?

Yes. This fixes the issue. Many thanks.

(Perhaps not surprisingly the warning never occurred for CONFIG_NR_CPUS>64.

I've also built a allyesconfig config and there was no -Warray-bounds warning at all.)

Signature · 1 line

Markus

Paul E. McKenney

2012-11-29 20:25 UTC #7 lore raw

Quoted 48 lines · Markus Trippelsdorf

Quoted 39 lines · Paul E. McKenney
Quoted 25 lines · Markus Trippelsdorf
Quoted 13 lines · Paul E. McKenney
Quoted 12 lines · Markus Trippelsdorf
Quoted 11 lines · Paul E. McKenney
Quoted 10 lines · Markus Trippelsdorf
With gcc-4.8 I get:
CC kernel/rcutree.o
kernel/rcutree.c: In function ‘rcu_init_one’:
kernel/rcutree.c:2850:13: warning: array subscript is above array bounds [-Warray-bounds]
     rsp->level[i] = rsp->level[i - 1] + rsp->levelcnt[i - 1];
                  ^
2849 for (i = 1; i < rcu_num_lvls; i++)
2850 rsp->level[i] = rsp->level[i - 1] + rsp->levelcnt[i - 1];
Quoted 7 lines
Quoted 4 lines
Quoted 3 lines
Quoted 2 lines
At first I thought that the warning was bogus, but rcu_num_lvls isn't static and gets modified prior to the for loop.
So, in your opinion, what would be the best way to silence this warning?
Good question. Are you saying that if the compiler cannot prove that the index is in bounds, it is going to throw a warning?
Yes, it does seem to be the case. See also my gcc bug report (closed as invalid): http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55529
Quoted 6 lines
Quoted 4 lines
If that is the case, perhaps telling the compiler to cool it via the command line would be best.
Or is this really one of a very few places in the kernel where the compiler is complaining?
Yes. With my (admittedly minimal) config this is only place.
Hmmmm... In that case...
Given that this is initialization code that is far from any fastpath, could you try putting something like this at the beginning of rcu_init_one()?
Quoted 7 lines
	if (rcu_num_lvls > RCU_NUM_LVLS)
		panic("rcu_num_lvls overflow");
If the compiler doesn't know that panic() never returns (despite the __noreturn), you could add a "return" after the panic().
Does that help?

Quoted 7 lines

Yes. This fixes the issue. Many thanks.
(Perhaps not surprisingly the warning never occurred for CONFIG_NR_CPUS>64.
I've also built a allyesconfig config and there was no -Warray-bounds warning at all.)

OK, I am applying this change with your Reported-by. But please understand that if something like this shows up in a fastpath, the compiler will likely need to be told to keep its array-bounds opinions to itself.

Thanx, Paul