> On May 13, 2024, at 20:14, Kees Cook <keesc...@chromium.org> wrote: > > On Tue, May 14, 2024 at 01:38:49AM +0200, Andrew Pinski wrote: >> On Mon, May 13, 2024, 11:41 PM Kees Cook <keesc...@chromium.org> wrote: >>> But it makes no sense to warn about: >>> >>> void sparx5_set (int * ptr, struct nums * sg, int index) >>> { >>> if (index >= 4) >>> warn (); >>> *ptr = 0; >>> *val = sg->vals[index]; >>> if (index >= 4) >>> warn (); >>> *ptr = *val; >>> } >>> >>> Because at "*val = sg->vals[index];" the actual value range tracking for >>> index is _still_ [INT_MIN,INT_MAX]. (Only within the "then" side of the >>> "if" statements is the range tracking [4,INT_MAX].) >>> >>> However, in the case where jump threading has split the execution flow >>> and produced a copy of "*val = sg->vals[index];" where the value range >>> tracking for "index" is now [4,INT_MAX], is the warning valid. But it >>> is only for that instance. Reporting it for effectively both (there is >>> only 1 source line for the array indexing) is misleading because there >>> is nothing the user can do about it -- the compiler created the copy and >>> then noticed it had a range it could apply to that array index. >>> >> >> "there is nothing the user can do about it" is very much false. They could >> change warn call into a noreturn function call instead. (In the case of >> the Linux kernel panic). There are things the user can do to fix the >> warning and even get better code generation out of the compilers. > > This isn't about warn() not being noreturn. The warn() could be any > function call; the jump threading still happens.
When the program is executed on the “if (index > = 4)” path, the value of “index” is definitely >= 4, when sg->vals[index] is referenced on this path (the case when the >routine “warn” is NOT noreturn), it’s definitely an out-of-bounds array access. So, the compiler’s warning is correct. And this warning does catch a potential issue in the source code that need to be fixed by either of the following two solutions: 1. Make the routine “warn” as noreturn and mark it noreturn; Or 2. On the path “if (index >= 4)”, make the value of “index” in the bound of the array. With either of the above source code changes, we can fix this potential out-of-bound array access bug in the source code. Qing > > GCC is warning about a compiler-constructed situation that cannot be > reliably fixed on the source side (GCC emitting the warning is highly > unstable in these cases), since the condition is not *always* true for > the given line of code. If it is not useful to warn for "array[index]" > being out of range when "index" is always [INT_MIN,INT_MAX], then it > is not useful to warn when "index" MAY be [INT_MIN,INT_MAX] for a given > line of code. > > -Kees > > -- > Kees Cook