* Re: mainline build failure due to 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()")
@ 2022-10-06 19:39 ` Linus Torvalds
0 siblings, 0 replies; 21+ messages in thread
From: Linus Torvalds @ 2022-10-06 19:39 UTC (permalink / raw)
To: Sudip Mukherjee (Codethink), Kees Cook, Nathan Chancellor
Cc: Hamza Mahfooz, Alex Deucher, Harry Wentland, Leo Li,
Rodrigo Siqueira, Christian König, Pan, Xinhui, David Airlie,
Daniel Vetter, amd-gfx, dri-devel, linux-kernel
On Thu, Oct 6, 2022 at 1:51 AM Sudip Mukherjee (Codethink)
<sudipm.mukherjee@gmail.com> wrote:
>
> This is only seen with gcc-11, gcc-12 builds are ok.
Hmm. This seems to be some odd gcc issue.
I *think* that what is going on is that the test
j = 0 ; j < MAX_DWB_PIPES
makes gcc decide that "hey, j is in the range [0,MAX_DWB_PIPES[", and
then since MAX_DWB_PIPES is 1, that simplifies to "j must be zero".
Good range analysis so far.
But for 'i', we start off with that lower bound of 0, but the upper
bound is not fixed (the test for "i" is: "i < stream->num_wb_info").
And then that "if (i != j)", so now gcc decides that it can simplify
that to "if (i != 0)", and then simplifies *that* to "oh, the lower
bound of 'i' in that code is actually 1.
So then it decides that "stream->writeback_info[i]" must be out of bounds.
Of course, the *reality* is that stream->num_wb_info should be <=
MAX_DWB_PIPES, and as such (with the current MAX_DWB_PIPES value of 1)
it's not that 'i' can be 1, it's that the code in question cannot be
reached at all.
What confuses me is that error message ("array subscript [0, 0] is
outside array bounds of 'struct dc_writeback_info[1]') which seems to
be aware that the value is actually 0.
So this seems to be some gcc-11 range analysis bug, but I don't know
what the fix is. I suspect some random code change would magically
just make gcc realize it's ok after all, but since it all depends on
random gcc confusion, I don't know what the random code change would
be.
The fix *MAY* be to just add a '&& i < MAX_DWB_PIPES' to that loop
too, and then gcc will see that both i and j are always 0, and that
the code is unreachable and not warn about it. Hmm? Can you test that?
And the reason gcc-12 builds are ok probably isn't that gcc-12 gets
this right, it's simply that gcc-12 gets so many *opther* things wrong
that we already disabled -Warray-bounds with gcc-12 entirely.
If somebody cannot come up with a fix, I suspect the solution is "gcc
array bounds analysis is terminally buggy" and we just need to disable
it for gcc-11 too.
Kees, any idea? Who else might be interested in fixing a -Warray-bounds issue?
Linus
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: mainline build failure due to 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()")
@ 2022-10-06 19:39 ` Linus Torvalds
0 siblings, 0 replies; 21+ messages in thread
From: Linus Torvalds @ 2022-10-06 19:39 UTC (permalink / raw)
To: Sudip Mukherjee (Codethink), Kees Cook, Nathan Chancellor
Cc: Leo Li, dri-devel, Pan, Xinhui, Rodrigo Siqueira, linux-kernel,
amd-gfx, Hamza Mahfooz, Alex Deucher, Christian König
On Thu, Oct 6, 2022 at 1:51 AM Sudip Mukherjee (Codethink)
<sudipm.mukherjee@gmail.com> wrote:
>
> This is only seen with gcc-11, gcc-12 builds are ok.
Hmm. This seems to be some odd gcc issue.
I *think* that what is going on is that the test
j = 0 ; j < MAX_DWB_PIPES
makes gcc decide that "hey, j is in the range [0,MAX_DWB_PIPES[", and
then since MAX_DWB_PIPES is 1, that simplifies to "j must be zero".
Good range analysis so far.
But for 'i', we start off with that lower bound of 0, but the upper
bound is not fixed (the test for "i" is: "i < stream->num_wb_info").
And then that "if (i != j)", so now gcc decides that it can simplify
that to "if (i != 0)", and then simplifies *that* to "oh, the lower
bound of 'i' in that code is actually 1.
So then it decides that "stream->writeback_info[i]" must be out of bounds.
Of course, the *reality* is that stream->num_wb_info should be <=
MAX_DWB_PIPES, and as such (with the current MAX_DWB_PIPES value of 1)
it's not that 'i' can be 1, it's that the code in question cannot be
reached at all.
What confuses me is that error message ("array subscript [0, 0] is
outside array bounds of 'struct dc_writeback_info[1]') which seems to
be aware that the value is actually 0.
So this seems to be some gcc-11 range analysis bug, but I don't know
what the fix is. I suspect some random code change would magically
just make gcc realize it's ok after all, but since it all depends on
random gcc confusion, I don't know what the random code change would
be.
The fix *MAY* be to just add a '&& i < MAX_DWB_PIPES' to that loop
too, and then gcc will see that both i and j are always 0, and that
the code is unreachable and not warn about it. Hmm? Can you test that?
And the reason gcc-12 builds are ok probably isn't that gcc-12 gets
this right, it's simply that gcc-12 gets so many *opther* things wrong
that we already disabled -Warray-bounds with gcc-12 entirely.
If somebody cannot come up with a fix, I suspect the solution is "gcc
array bounds analysis is terminally buggy" and we just need to disable
it for gcc-11 too.
Kees, any idea? Who else might be interested in fixing a -Warray-bounds issue?
Linus
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: mainline build failure due to 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()")
2022-10-06 19:39 ` Linus Torvalds
(?)
@ 2022-10-06 19:51 ` Hamza Mahfooz
-1 siblings, 0 replies; 21+ messages in thread
From: Hamza Mahfooz @ 2022-10-06 19:51 UTC (permalink / raw)
To: Linus Torvalds, Sudip Mukherjee (Codethink), Kees Cook,
Nathan Chancellor
Cc: Leo Li, David Airlie, Pan, Xinhui, Rodrigo Siqueira, linux-kernel,
amd-gfx, dri-devel, Daniel Vetter, Alex Deucher, Harry Wentland,
Christian König
Hey Linus,
On 2022-10-06 15:39, Linus Torvalds wrote:
> On Thu, Oct 6, 2022 at 1:51 AM Sudip Mukherjee (Codethink)
> <sudipm.mukherjee@gmail.com> wrote:
>>
>> This is only seen with gcc-11, gcc-12 builds are ok.
>
> Hmm. This seems to be some odd gcc issue.
>
> I *think* that what is going on is that the test
>
> j = 0 ; j < MAX_DWB_PIPES
>
> makes gcc decide that "hey, j is in the range [0,MAX_DWB_PIPES[", and
> then since MAX_DWB_PIPES is 1, that simplifies to "j must be zero".
> Good range analysis so far.
>
> But for 'i', we start off with that lower bound of 0, but the upper
> bound is not fixed (the test for "i" is: "i < stream->num_wb_info").
>
> And then that "if (i != j)", so now gcc decides that it can simplify
> that to "if (i != 0)", and then simplifies *that* to "oh, the lower
> bound of 'i' in that code is actually 1.
>
> So then it decides that "stream->writeback_info[i]" must be out of bounds.
>
> Of course, the *reality* is that stream->num_wb_info should be <=
> MAX_DWB_PIPES, and as such (with the current MAX_DWB_PIPES value of 1)
> it's not that 'i' can be 1, it's that the code in question cannot be
> reached at all.
>
> What confuses me is that error message ("array subscript [0, 0] is
> outside array bounds of 'struct dc_writeback_info[1]') which seems to
> be aware that the value is actually 0.
>
> So this seems to be some gcc-11 range analysis bug, but I don't know
> what the fix is. I suspect some random code change would magically
> just make gcc realize it's ok after all, but since it all depends on
> random gcc confusion, I don't know what the random code change would
> be.
>
> The fix *MAY* be to just add a '&& i < MAX_DWB_PIPES' to that loop
> too, and then gcc will see that both i and j are always 0, and that
> the code is unreachable and not warn about it. Hmm? Can you test that?
>
> And the reason gcc-12 builds are ok probably isn't that gcc-12 gets
> this right, it's simply that gcc-12 gets so many *opther* things wrong
> that we already disabled -Warray-bounds with gcc-12 entirely.
>
> If somebody cannot come up with a fix, I suspect the solution is "gcc
> array bounds analysis is terminally buggy" and we just need to disable
> it for gcc-11 too.
It seems that Stephen has a fix for this that works for multiple
versions of GCC, see:
https://lore.kernel.org/all/20221006191245.11bb0e2c@canb.auug.org.au/
>
> Kees, any idea? Who else might be interested in fixing a -Warray-bounds issue?
>
> Linus
--
Hamza
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: mainline build failure due to 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()")
@ 2022-10-06 19:51 ` Hamza Mahfooz
0 siblings, 0 replies; 21+ messages in thread
From: Hamza Mahfooz @ 2022-10-06 19:51 UTC (permalink / raw)
To: Linus Torvalds, Sudip Mukherjee (Codethink), Kees Cook,
Nathan Chancellor
Cc: Alex Deucher, Harry Wentland, Leo Li, Rodrigo Siqueira,
Christian König, Pan, Xinhui, David Airlie, Daniel Vetter,
amd-gfx, dri-devel, linux-kernel
Hey Linus,
On 2022-10-06 15:39, Linus Torvalds wrote:
> On Thu, Oct 6, 2022 at 1:51 AM Sudip Mukherjee (Codethink)
> <sudipm.mukherjee@gmail.com> wrote:
>>
>> This is only seen with gcc-11, gcc-12 builds are ok.
>
> Hmm. This seems to be some odd gcc issue.
>
> I *think* that what is going on is that the test
>
> j = 0 ; j < MAX_DWB_PIPES
>
> makes gcc decide that "hey, j is in the range [0,MAX_DWB_PIPES[", and
> then since MAX_DWB_PIPES is 1, that simplifies to "j must be zero".
> Good range analysis so far.
>
> But for 'i', we start off with that lower bound of 0, but the upper
> bound is not fixed (the test for "i" is: "i < stream->num_wb_info").
>
> And then that "if (i != j)", so now gcc decides that it can simplify
> that to "if (i != 0)", and then simplifies *that* to "oh, the lower
> bound of 'i' in that code is actually 1.
>
> So then it decides that "stream->writeback_info[i]" must be out of bounds.
>
> Of course, the *reality* is that stream->num_wb_info should be <=
> MAX_DWB_PIPES, and as such (with the current MAX_DWB_PIPES value of 1)
> it's not that 'i' can be 1, it's that the code in question cannot be
> reached at all.
>
> What confuses me is that error message ("array subscript [0, 0] is
> outside array bounds of 'struct dc_writeback_info[1]') which seems to
> be aware that the value is actually 0.
>
> So this seems to be some gcc-11 range analysis bug, but I don't know
> what the fix is. I suspect some random code change would magically
> just make gcc realize it's ok after all, but since it all depends on
> random gcc confusion, I don't know what the random code change would
> be.
>
> The fix *MAY* be to just add a '&& i < MAX_DWB_PIPES' to that loop
> too, and then gcc will see that both i and j are always 0, and that
> the code is unreachable and not warn about it. Hmm? Can you test that?
>
> And the reason gcc-12 builds are ok probably isn't that gcc-12 gets
> this right, it's simply that gcc-12 gets so many *opther* things wrong
> that we already disabled -Warray-bounds with gcc-12 entirely.
>
> If somebody cannot come up with a fix, I suspect the solution is "gcc
> array bounds analysis is terminally buggy" and we just need to disable
> it for gcc-11 too.
It seems that Stephen has a fix for this that works for multiple
versions of GCC, see:
https://lore.kernel.org/all/20221006191245.11bb0e2c@canb.auug.org.au/
>
> Kees, any idea? Who else might be interested in fixing a -Warray-bounds issue?
>
> Linus
--
Hamza
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: mainline build failure due to 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()")
@ 2022-10-06 19:51 ` Hamza Mahfooz
0 siblings, 0 replies; 21+ messages in thread
From: Hamza Mahfooz @ 2022-10-06 19:51 UTC (permalink / raw)
To: Linus Torvalds, Sudip Mukherjee (Codethink), Kees Cook,
Nathan Chancellor
Cc: Leo Li, Pan, Xinhui, Rodrigo Siqueira, linux-kernel, amd-gfx,
dri-devel, Alex Deucher, Christian König
Hey Linus,
On 2022-10-06 15:39, Linus Torvalds wrote:
> On Thu, Oct 6, 2022 at 1:51 AM Sudip Mukherjee (Codethink)
> <sudipm.mukherjee@gmail.com> wrote:
>>
>> This is only seen with gcc-11, gcc-12 builds are ok.
>
> Hmm. This seems to be some odd gcc issue.
>
> I *think* that what is going on is that the test
>
> j = 0 ; j < MAX_DWB_PIPES
>
> makes gcc decide that "hey, j is in the range [0,MAX_DWB_PIPES[", and
> then since MAX_DWB_PIPES is 1, that simplifies to "j must be zero".
> Good range analysis so far.
>
> But for 'i', we start off with that lower bound of 0, but the upper
> bound is not fixed (the test for "i" is: "i < stream->num_wb_info").
>
> And then that "if (i != j)", so now gcc decides that it can simplify
> that to "if (i != 0)", and then simplifies *that* to "oh, the lower
> bound of 'i' in that code is actually 1.
>
> So then it decides that "stream->writeback_info[i]" must be out of bounds.
>
> Of course, the *reality* is that stream->num_wb_info should be <=
> MAX_DWB_PIPES, and as such (with the current MAX_DWB_PIPES value of 1)
> it's not that 'i' can be 1, it's that the code in question cannot be
> reached at all.
>
> What confuses me is that error message ("array subscript [0, 0] is
> outside array bounds of 'struct dc_writeback_info[1]') which seems to
> be aware that the value is actually 0.
>
> So this seems to be some gcc-11 range analysis bug, but I don't know
> what the fix is. I suspect some random code change would magically
> just make gcc realize it's ok after all, but since it all depends on
> random gcc confusion, I don't know what the random code change would
> be.
>
> The fix *MAY* be to just add a '&& i < MAX_DWB_PIPES' to that loop
> too, and then gcc will see that both i and j are always 0, and that
> the code is unreachable and not warn about it. Hmm? Can you test that?
>
> And the reason gcc-12 builds are ok probably isn't that gcc-12 gets
> this right, it's simply that gcc-12 gets so many *opther* things wrong
> that we already disabled -Warray-bounds with gcc-12 entirely.
>
> If somebody cannot come up with a fix, I suspect the solution is "gcc
> array bounds analysis is terminally buggy" and we just need to disable
> it for gcc-11 too.
It seems that Stephen has a fix for this that works for multiple
versions of GCC, see:
https://lore.kernel.org/all/20221006191245.11bb0e2c@canb.auug.org.au/
>
> Kees, any idea? Who else might be interested in fixing a -Warray-bounds issue?
>
> Linus
--
Hamza
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: mainline build failure due to 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()")
2022-10-06 19:39 ` Linus Torvalds
(?)
@ 2022-10-06 19:55 ` Sudip Mukherjee
-1 siblings, 0 replies; 21+ messages in thread
From: Sudip Mukherjee @ 2022-10-06 19:55 UTC (permalink / raw)
To: Linus Torvalds
Cc: Kees Cook, Leo Li, David Airlie, dri-devel, Pan, Xinhui,
Rodrigo Siqueira, linux-kernel, amd-gfx, Nathan Chancellor,
Hamza Mahfooz, Daniel Vetter, Alex Deucher, Harry Wentland,
Christian König
On Thu, Oct 6, 2022 at 8:39 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Thu, Oct 6, 2022 at 1:51 AM Sudip Mukherjee (Codethink)
> <sudipm.mukherjee@gmail.com> wrote:
> >
> > This is only seen with gcc-11, gcc-12 builds are ok.
>
> Hmm. This seems to be some odd gcc issue.
>
<snip>
>
> The fix *MAY* be to just add a '&& i < MAX_DWB_PIPES' to that loop
> too, and then gcc will see that both i and j are always 0, and that
> the code is unreachable and not warn about it. Hmm? Can you test that?
Builds fine with the change you suggested. Here is the diff.
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
index ae13887756bf..51571245c49a 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
@@ -520,7 +520,7 @@ bool dc_stream_remove_writeback(struct dc *dc,
}
/* remove writeback info for disabled writeback pipes from stream */
- for (i = 0, j = 0; i < stream->num_wb_info && j < MAX_DWB_PIPES; i++) {
+ for (i = 0, j = 0; i < stream->num_wb_info && j <
MAX_DWB_PIPES && i < MAX_DWB_PIPES; i++) {
if (stream->writeback_info[i].wb_enabled) {
if (i != j)
/* trim the array */
--
Regards
Sudip
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: mainline build failure due to 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()")
@ 2022-10-06 19:55 ` Sudip Mukherjee
0 siblings, 0 replies; 21+ messages in thread
From: Sudip Mukherjee @ 2022-10-06 19:55 UTC (permalink / raw)
To: Linus Torvalds
Cc: Kees Cook, Nathan Chancellor, Hamza Mahfooz, Alex Deucher,
Harry Wentland, Leo Li, Rodrigo Siqueira, Christian König,
Pan, Xinhui, David Airlie, Daniel Vetter, amd-gfx, dri-devel,
linux-kernel
On Thu, Oct 6, 2022 at 8:39 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Thu, Oct 6, 2022 at 1:51 AM Sudip Mukherjee (Codethink)
> <sudipm.mukherjee@gmail.com> wrote:
> >
> > This is only seen with gcc-11, gcc-12 builds are ok.
>
> Hmm. This seems to be some odd gcc issue.
>
<snip>
>
> The fix *MAY* be to just add a '&& i < MAX_DWB_PIPES' to that loop
> too, and then gcc will see that both i and j are always 0, and that
> the code is unreachable and not warn about it. Hmm? Can you test that?
Builds fine with the change you suggested. Here is the diff.
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
index ae13887756bf..51571245c49a 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
@@ -520,7 +520,7 @@ bool dc_stream_remove_writeback(struct dc *dc,
}
/* remove writeback info for disabled writeback pipes from stream */
- for (i = 0, j = 0; i < stream->num_wb_info && j < MAX_DWB_PIPES; i++) {
+ for (i = 0, j = 0; i < stream->num_wb_info && j <
MAX_DWB_PIPES && i < MAX_DWB_PIPES; i++) {
if (stream->writeback_info[i].wb_enabled) {
if (i != j)
/* trim the array */
--
Regards
Sudip
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: mainline build failure due to 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()")
@ 2022-10-06 19:55 ` Sudip Mukherjee
0 siblings, 0 replies; 21+ messages in thread
From: Sudip Mukherjee @ 2022-10-06 19:55 UTC (permalink / raw)
To: Linus Torvalds
Cc: Kees Cook, Leo Li, dri-devel, Pan, Xinhui, Rodrigo Siqueira,
linux-kernel, amd-gfx, Nathan Chancellor, Hamza Mahfooz,
Alex Deucher, Christian König
On Thu, Oct 6, 2022 at 8:39 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Thu, Oct 6, 2022 at 1:51 AM Sudip Mukherjee (Codethink)
> <sudipm.mukherjee@gmail.com> wrote:
> >
> > This is only seen with gcc-11, gcc-12 builds are ok.
>
> Hmm. This seems to be some odd gcc issue.
>
<snip>
>
> The fix *MAY* be to just add a '&& i < MAX_DWB_PIPES' to that loop
> too, and then gcc will see that both i and j are always 0, and that
> the code is unreachable and not warn about it. Hmm? Can you test that?
Builds fine with the change you suggested. Here is the diff.
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
index ae13887756bf..51571245c49a 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
@@ -520,7 +520,7 @@ bool dc_stream_remove_writeback(struct dc *dc,
}
/* remove writeback info for disabled writeback pipes from stream */
- for (i = 0, j = 0; i < stream->num_wb_info && j < MAX_DWB_PIPES; i++) {
+ for (i = 0, j = 0; i < stream->num_wb_info && j <
MAX_DWB_PIPES && i < MAX_DWB_PIPES; i++) {
if (stream->writeback_info[i].wb_enabled) {
if (i != j)
/* trim the array */
--
Regards
Sudip
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: mainline build failure due to 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()")
2022-10-06 19:39 ` Linus Torvalds
(?)
@ 2022-10-06 20:37 ` Kees Cook
-1 siblings, 0 replies; 21+ messages in thread
From: Kees Cook @ 2022-10-06 20:37 UTC (permalink / raw)
To: Linus Torvalds
Cc: Leo Li, David Airlie, dri-devel, Pan, Xinhui, Rodrigo Siqueira,
linux-kernel, amd-gfx, Christian König, Nathan Chancellor,
Hamza Mahfooz, Daniel Vetter, Alex Deucher, Harry Wentland,
Sudip Mukherjee (Codethink)
On Thu, Oct 06, 2022 at 12:39:40PM -0700, Linus Torvalds wrote:
> What confuses me is that error message ("array subscript [0, 0] is
> outside array bounds of 'struct dc_writeback_info[1]') which seems to
> be aware that the value is actually 0.
I've seen bugs in the tracker where the reporting is broken but the
range checker is working "correctly", which seems to be the case here.
> If somebody cannot come up with a fix, I suspect the solution is "gcc
> array bounds analysis is terminally buggy" and we just need to disable
> it for gcc-11 too.
It does continue to find bugs, so I'd rather keep it on. GCC has fixed
all the issues we've run into so far (though not all have been back
ported to GCC 12 yet, so yes, let's keep -Warray-bounds disabled there).
Specifically, I've been tracking:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105679 Fixed 13+
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578 Fixed 12+, 11.3
And it looks like Sudip's proposed fix for this particular code is
additionally fixing unsigned vs signed as well. I think -Warray-bounds
did its job (though, with quite a confusing index range in the report).
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: mainline build failure due to 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()")
@ 2022-10-06 20:37 ` Kees Cook
0 siblings, 0 replies; 21+ messages in thread
From: Kees Cook @ 2022-10-06 20:37 UTC (permalink / raw)
To: Linus Torvalds
Cc: Sudip Mukherjee (Codethink), Nathan Chancellor, Hamza Mahfooz,
Alex Deucher, Harry Wentland, Leo Li, Rodrigo Siqueira,
Christian König, Pan, Xinhui, David Airlie, Daniel Vetter,
amd-gfx, dri-devel, linux-kernel
On Thu, Oct 06, 2022 at 12:39:40PM -0700, Linus Torvalds wrote:
> What confuses me is that error message ("array subscript [0, 0] is
> outside array bounds of 'struct dc_writeback_info[1]') which seems to
> be aware that the value is actually 0.
I've seen bugs in the tracker where the reporting is broken but the
range checker is working "correctly", which seems to be the case here.
> If somebody cannot come up with a fix, I suspect the solution is "gcc
> array bounds analysis is terminally buggy" and we just need to disable
> it for gcc-11 too.
It does continue to find bugs, so I'd rather keep it on. GCC has fixed
all the issues we've run into so far (though not all have been back
ported to GCC 12 yet, so yes, let's keep -Warray-bounds disabled there).
Specifically, I've been tracking:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105679 Fixed 13+
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578 Fixed 12+, 11.3
And it looks like Sudip's proposed fix for this particular code is
additionally fixing unsigned vs signed as well. I think -Warray-bounds
did its job (though, with quite a confusing index range in the report).
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: mainline build failure due to 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()")
@ 2022-10-06 20:37 ` Kees Cook
0 siblings, 0 replies; 21+ messages in thread
From: Kees Cook @ 2022-10-06 20:37 UTC (permalink / raw)
To: Linus Torvalds
Cc: Leo Li, dri-devel, Pan, Xinhui, Rodrigo Siqueira, linux-kernel,
amd-gfx, Christian König, Nathan Chancellor, Hamza Mahfooz,
Alex Deucher, Sudip Mukherjee (Codethink)
On Thu, Oct 06, 2022 at 12:39:40PM -0700, Linus Torvalds wrote:
> What confuses me is that error message ("array subscript [0, 0] is
> outside array bounds of 'struct dc_writeback_info[1]') which seems to
> be aware that the value is actually 0.
I've seen bugs in the tracker where the reporting is broken but the
range checker is working "correctly", which seems to be the case here.
> If somebody cannot come up with a fix, I suspect the solution is "gcc
> array bounds analysis is terminally buggy" and we just need to disable
> it for gcc-11 too.
It does continue to find bugs, so I'd rather keep it on. GCC has fixed
all the issues we've run into so far (though not all have been back
ported to GCC 12 yet, so yes, let's keep -Warray-bounds disabled there).
Specifically, I've been tracking:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105679 Fixed 13+
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578 Fixed 12+, 11.3
And it looks like Sudip's proposed fix for this particular code is
additionally fixing unsigned vs signed as well. I think -Warray-bounds
did its job (though, with quite a confusing index range in the report).
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: mainline build failure due to 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()")
2022-10-06 20:37 ` Kees Cook
(?)
@ 2022-10-06 20:49 ` Sudip Mukherjee
-1 siblings, 0 replies; 21+ messages in thread
From: Sudip Mukherjee @ 2022-10-06 20:49 UTC (permalink / raw)
To: Kees Cook
Cc: Leo Li, Harry Wentland, dri-devel, Pan, Xinhui, Rodrigo Siqueira,
linux-kernel, amd-gfx, Nathan Chancellor, Hamza Mahfooz,
Daniel Vetter, Alex Deucher, David Airlie, Linus Torvalds,
Christian König
On Thu, Oct 6, 2022 at 9:37 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Oct 06, 2022 at 12:39:40PM -0700, Linus Torvalds wrote:
> > What confuses me is that error message ("array subscript [0, 0] is
> > outside array bounds of 'struct dc_writeback_info[1]') which seems to
> > be aware that the value is actually 0.
>
> I've seen bugs in the tracker where the reporting is broken but the
> range checker is working "correctly", which seems to be the case here.
>
> > If somebody cannot come up with a fix, I suspect the solution is "gcc
> > array bounds analysis is terminally buggy" and we just need to disable
> > it for gcc-11 too.
>
> It does continue to find bugs, so I'd rather keep it on. GCC has fixed
> all the issues we've run into so far (though not all have been back
> ported to GCC 12 yet, so yes, let's keep -Warray-bounds disabled there).
>
> Specifically, I've been tracking:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105679 Fixed 13+
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578 Fixed 12+, 11.3
Thats odd, the bug report says its fixed but I am using:
gcc version 11.3.1 20220925 (GCC)
>
> And it looks like Sudip's proposed fix for this particular code is
> additionally fixing unsigned vs signed as well. I think -Warray-bounds
> did its job (though, with quite a confusing index range in the report).
Not my. Linus's. I just tested. :)
--
Regards
Sudip
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: mainline build failure due to 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()")
@ 2022-10-06 20:49 ` Sudip Mukherjee
0 siblings, 0 replies; 21+ messages in thread
From: Sudip Mukherjee @ 2022-10-06 20:49 UTC (permalink / raw)
To: Kees Cook
Cc: Linus Torvalds, Nathan Chancellor, Hamza Mahfooz, Alex Deucher,
Harry Wentland, Leo Li, Rodrigo Siqueira, Christian König,
Pan, Xinhui, David Airlie, Daniel Vetter, amd-gfx, dri-devel,
linux-kernel
On Thu, Oct 6, 2022 at 9:37 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Oct 06, 2022 at 12:39:40PM -0700, Linus Torvalds wrote:
> > What confuses me is that error message ("array subscript [0, 0] is
> > outside array bounds of 'struct dc_writeback_info[1]') which seems to
> > be aware that the value is actually 0.
>
> I've seen bugs in the tracker where the reporting is broken but the
> range checker is working "correctly", which seems to be the case here.
>
> > If somebody cannot come up with a fix, I suspect the solution is "gcc
> > array bounds analysis is terminally buggy" and we just need to disable
> > it for gcc-11 too.
>
> It does continue to find bugs, so I'd rather keep it on. GCC has fixed
> all the issues we've run into so far (though not all have been back
> ported to GCC 12 yet, so yes, let's keep -Warray-bounds disabled there).
>
> Specifically, I've been tracking:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105679 Fixed 13+
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578 Fixed 12+, 11.3
Thats odd, the bug report says its fixed but I am using:
gcc version 11.3.1 20220925 (GCC)
>
> And it looks like Sudip's proposed fix for this particular code is
> additionally fixing unsigned vs signed as well. I think -Warray-bounds
> did its job (though, with quite a confusing index range in the report).
Not my. Linus's. I just tested. :)
--
Regards
Sudip
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: mainline build failure due to 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()")
@ 2022-10-06 20:49 ` Sudip Mukherjee
0 siblings, 0 replies; 21+ messages in thread
From: Sudip Mukherjee @ 2022-10-06 20:49 UTC (permalink / raw)
To: Kees Cook
Cc: Leo Li, dri-devel, Pan, Xinhui, Rodrigo Siqueira, linux-kernel,
amd-gfx, Nathan Chancellor, Hamza Mahfooz, Alex Deucher,
Linus Torvalds, Christian König
On Thu, Oct 6, 2022 at 9:37 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Oct 06, 2022 at 12:39:40PM -0700, Linus Torvalds wrote:
> > What confuses me is that error message ("array subscript [0, 0] is
> > outside array bounds of 'struct dc_writeback_info[1]') which seems to
> > be aware that the value is actually 0.
>
> I've seen bugs in the tracker where the reporting is broken but the
> range checker is working "correctly", which seems to be the case here.
>
> > If somebody cannot come up with a fix, I suspect the solution is "gcc
> > array bounds analysis is terminally buggy" and we just need to disable
> > it for gcc-11 too.
>
> It does continue to find bugs, so I'd rather keep it on. GCC has fixed
> all the issues we've run into so far (though not all have been back
> ported to GCC 12 yet, so yes, let's keep -Warray-bounds disabled there).
>
> Specifically, I've been tracking:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105679 Fixed 13+
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578 Fixed 12+, 11.3
Thats odd, the bug report says its fixed but I am using:
gcc version 11.3.1 20220925 (GCC)
>
> And it looks like Sudip's proposed fix for this particular code is
> additionally fixing unsigned vs signed as well. I think -Warray-bounds
> did its job (though, with quite a confusing index range in the report).
Not my. Linus's. I just tested. :)
--
Regards
Sudip
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: mainline build failure due to 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()")
2022-10-06 20:49 ` Sudip Mukherjee
(?)
@ 2022-10-06 23:48 ` Linus Torvalds
-1 siblings, 0 replies; 21+ messages in thread
From: Linus Torvalds @ 2022-10-06 23:48 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Kees Cook, Leo Li, David Airlie, dri-devel, Pan, Xinhui,
Rodrigo Siqueira, linux-kernel, amd-gfx, Nathan Chancellor,
Hamza Mahfooz, Daniel Vetter, Alex Deucher, Harry Wentland,
Christian König
On Thu, Oct 6, 2022 at 1:50 PM Sudip Mukherjee
<sudipm.mukherjee@gmail.com> wrote:
>
> > And it looks like Sudip's proposed fix for this particular code is
> > additionally fixing unsigned vs signed as well. I think -Warray-bounds
> > did its job (though, with quite a confusing index range in the report).
>
> Not my. Linus's. I just tested. :)
I suspect Kees meant Stephen's other patch that Hamza pointed at, and
that is perhaps the cleaner version.
That said, I hate how this forces us to write random code changes just
to make a compiler just randomly _happen_ to not complain about it.
Linus
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: mainline build failure due to 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()")
@ 2022-10-06 23:48 ` Linus Torvalds
0 siblings, 0 replies; 21+ messages in thread
From: Linus Torvalds @ 2022-10-06 23:48 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Kees Cook, Nathan Chancellor, Hamza Mahfooz, Alex Deucher,
Harry Wentland, Leo Li, Rodrigo Siqueira, Christian König,
Pan, Xinhui, David Airlie, Daniel Vetter, amd-gfx, dri-devel,
linux-kernel
On Thu, Oct 6, 2022 at 1:50 PM Sudip Mukherjee
<sudipm.mukherjee@gmail.com> wrote:
>
> > And it looks like Sudip's proposed fix for this particular code is
> > additionally fixing unsigned vs signed as well. I think -Warray-bounds
> > did its job (though, with quite a confusing index range in the report).
>
> Not my. Linus's. I just tested. :)
I suspect Kees meant Stephen's other patch that Hamza pointed at, and
that is perhaps the cleaner version.
That said, I hate how this forces us to write random code changes just
to make a compiler just randomly _happen_ to not complain about it.
Linus
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: mainline build failure due to 5d8c3e836fc2 ("drm/amd/display: fix array-bounds error in dc_stream_remove_writeback()")
@ 2022-10-06 23:48 ` Linus Torvalds
0 siblings, 0 replies; 21+ messages in thread
From: Linus Torvalds @ 2022-10-06 23:48 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Kees Cook, Leo Li, dri-devel, Pan, Xinhui, Rodrigo Siqueira,
linux-kernel, amd-gfx, Nathan Chancellor, Hamza Mahfooz,
Alex Deucher, Christian König
On Thu, Oct 6, 2022 at 1:50 PM Sudip Mukherjee
<sudipm.mukherjee@gmail.com> wrote:
>
> > And it looks like Sudip's proposed fix for this particular code is
> > additionally fixing unsigned vs signed as well. I think -Warray-bounds
> > did its job (though, with quite a confusing index range in the report).
>
> Not my. Linus's. I just tested. :)
I suspect Kees meant Stephen's other patch that Hamza pointed at, and
that is perhaps the cleaner version.
That said, I hate how this forces us to write random code changes just
to make a compiler just randomly _happen_ to not complain about it.
Linus
^ permalink raw reply [flat|nested] 21+ messages in thread