Re: [Mesa3d-dev] Mesa (master): r300/compiler: Assert that array index is not negative.

[Corbin S. <mos...@gm...>]

On Fri, Feb 26, 2010 at 1:38 AM, José Fonseca <jfo...@vm...> wrote:
> On Fri, 2010-02-26 at 01:18 -0800, Corbin Simpson wrote:
>> > Module: Mesa
>> > Branch: master
>> > Commit: e5c691f445e1c02e6e2f75b817b13d7024f7a3a6
>> > URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=e5c691f445e1c02e6e2f75b817b13d7024f7a3a6
>> >
>> > Author: Vinson Lee <vlee at vmware.com>
>> > Date:   Fri Feb 26 00:17:03 2010 -0800
>> >
>> > r300/compiler: Assert that array index is not negative.
>> >
>> > ---
>> >
>> > .../drivers/dri/r300/compiler/r500_fragprog_emit.c |    2 ++
>> > 1 files changed, 2 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/src/mesa/drivers/dri/r300/compiler/r500_fragprog_emit.c b/src/mesa/drivers/dri/r300/compiler/r500_fragprog_emit.c
>> > index 829f028..710cae7 100644
>> > --- a/src/mesa/drivers/dri/r300/compiler/r500_fragprog_emit.c
>> > +++ b/src/mesa/drivers/dri/r300/compiler/r500_fragprog_emit.c
>> > @@ -469,6 +469,8 @@ void r500BuildFragmentProgramHwCode(struct r300_fragment_program_compiler *compi
>> > if (compiler->Base.Error)
>> > return;
>> >
>> > + assert(code->inst_end >= 0);
>> > +
>> > if ((code->inst[code->inst_end].inst0 & R500_INST_TYPE_MASK) != R500_INST_TYPE_OUT) {
>> > /* This may happen when dead-code elimination is disabled or
>> > * when most of the fragment program logic is leading to a KIL */
>>
>> Sorry, is this actually a problem? If this assertion is actually being
>> hit, it sure would be nice to hear about it since it. Empty shaders
>> shouldn't just be handled with debugging code.
>
> Vinson has been cleaning up code base based on the results of coverity
> static code analysis.
>
> Coverity generates thousands of errors/warnings for Mesa source code.
> Not all errors/warnings reported by coverity are actually hit in
> practice, but putting asserts and/or error handling code will at least
> inform coverity that that can't ever happen, and allow the real errors
> to stand out.
>
> I imagine this is the case here.
>
> Jose
>
>

Alright, thanks for the heads-up. I was not sure if this was static
analysis or not. I know that some of the r300 code is not so hot WRT
signed vs. unsigned comps and related things, so these kinds of
commits tend to get extra scrutiny from Nicolai and I.

Thanks for the explanation and hard work!

~ C.

-- 
Only fools are easily impressed by what is only
barely beyond their reach. ~ Unknown

Corbin Simpson
<Mos...@gm...>