https://bugs.freedesktop.org/show_bug.cgi?id=54867
Bug #: 54867 Summary: bug in r300 compiler Classification: Unclassified Product: Mesa Version: git Platform: All OS/Version: Linux (All) Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/r300 AssignedTo: dri-devel@lists.freedesktop.org ReportedBy: son_of_the_osiris@interia.pl
Orginally posted on mailing list:
In playing with Coccinelle, I discovered a signed/unsigned bug in radeon_rename_regs.c:rc_rename_regs.
unsigned new_index; unsigned writemask; struct rc_variable * var = var_ptr->Item;
if (var->Inst->U.I.DstReg.File != RC_FILE_TEMPORARY) { continue; }
new_index = rc_find_free_temporary_list(c, used, used_length, RC_MASK_XYZW); if (new_index < 0) { rc_error(c, "Ran out of temporary registers\n"); return; }
unsigned new_index is compared with < 0.
I don't know the code, but I can't imagine that you'd need an unsigned to represent a register index value.
Matt Turner
https://bugs.freedesktop.org/show_bug.cgi?id=54867
--- Comment #1 from Chistopher Krakowiak krzysztof.krakowiak@gmail.com --- Created attachment 80074 --> https://bugs.freedesktop.org/attachment.cgi?id=80074&action=edit s/signed/int/
https://bugs.freedesktop.org/show_bug.cgi?id=54867
--- Comment #2 from Tom Stellard tstellar@gmail.com --- Comment on attachment 80074 --> https://bugs.freedesktop.org/attachment.cgi?id=80074 s/signed/int/
Review of attachment 80074: -----------------------------------------------------------------
This patch looks good to me, but for the commit message, you need to wrap long lines to 80 or fewer characters (I actually wrap to 75, but I'm not sure what the standard convention is) and re-organize the commit message in the form of:
Code area: Brief description
Long description (if necessary)
Link to fixed bugs
For this patch, it should be something like:
r300g/compiler: Fix unsigned comparison with less than zero
rc_find_free_temporary_list() returns signed integer (in case of lack of free temporary registersreturns -1), so new_index in radeon_rename_regs() should be signed.
https://bugs.freedesktop.org/show_bug.cgi?id=54867
https://bugs.freedesktop.org/show_bug.cgi?id=54867
--- Comment #3 from David "okias" Heidelberger david.heidelberger@ixit.cz --- So, still not pushed in today git, can someone push this small fix?
https://bugs.freedesktop.org/show_bug.cgi?id=54867
--- Comment #4 from Tom Stellard tstellar@gmail.com --- (In reply to comment #3)
So, still not pushed in today git, can someone push this small fix?
I can push it if you provide an updated patch with a proper commit message.
https://bugs.freedesktop.org/show_bug.cgi?id=54867
David "okias" Heidelberger david.heidelberger@ixit.cz changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|dri-devel@lists.freedesktop |david.heidelberger@ixit.cz |.org | Attachment #80074|0 |1 is obsolete| |
--- Comment #5 from David "okias" Heidelberger david.heidelberger@ixit.cz --- Created attachment 87247 --> https://bugs.freedesktop.org/attachment.cgi?id=87247&action=edit 0001-r300g-compiler-Fix-unsigned-comparison-with-less-tha.patch
Hi Tom,
sending correctly formated commit message.
David
dri-devel@lists.freedesktop.org