Skip to content

Commit

Permalink
Fix some Squeak3D UB: shifting left some negative int
Browse files Browse the repository at this point in the history
A reproducible case of crash provided by Stephane Rollandin gives the following warning with clang `-fsanitize=undefined`:

>../../platforms/Cross/plugins/Squeak3D/b3dMain.c:1252:29: runtime error: left shift of negative value -760
>../../platforms/Cross/plugins/Squeak3D/b3dMain.c:1254:25: runtime error: left shift of negative value -751
>../../platforms/Cross/plugins/Squeak3D/b3dDraw.c:317:33: runtime error: left shift of negative value -802
>../../platforms/Cross/plugins/Squeak3D/b3dDraw.c:318:33: runtime error: left shift of negative value -802
>../../platforms/Cross/plugins/Squeak3D/b3dDraw.c:316:33: runtime error: left shift of negative value -114
>../../platforms/Cross/plugins/Squeak3D/b3dMain.c:829:61: runtime error: left shift of negative value -2

On OSX optimized VM, a crash happens in b3dMain.c, in function b3dAddBackFill at line 994 soon after those warnings
By protecting the shift with (unsigned) cast, this particular crash disappear.

There is still other crash happening related to bad fill list, but one thing at a time...
  • Loading branch information
nicolas-cellier-aka-nice committed Feb 8, 2020
1 parent 0f974af commit 015d381
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 9 deletions.
12 changes: 6 additions & 6 deletions platforms/Cross/plugins/Squeak3D/b3dDraw.c
Original file line number Diff line number Diff line change
Expand Up @@ -313,9 +313,9 @@ void b3dDrawRGB(int leftX, int rightX, int yValue, B3DPrimitiveFace *face)
we have dealt with huge polys above */
while(deltaX >= nPixels) {
{ /* Compute right most values of color interpolation */
int maxR = rValue + (deltaR << pixelShift);
int maxG = gValue + (deltaG << pixelShift);
int maxB = bValue + (deltaB << pixelShift);
int maxR = rValue + ((unsigned)deltaR << pixelShift);
int maxG = gValue + ((unsigned)deltaG << pixelShift);
int maxB = bValue + ((unsigned)deltaB << pixelShift);
/* Clamp those guys */
CLAMP_RGB(maxR, maxG, maxB);
/* And compute the actual delta */
Expand Down Expand Up @@ -401,9 +401,9 @@ void b3dDrawSTWRGB(int leftX, int rightX, int yValue, B3DPrimitiveFace *face)
int nPixels = 1 << pixelShift;
while(deltaX >= nPixels) {
{ /* Compute right most values of color interpolation */
int maxR = rValue + (deltaR << pixelShift);
int maxG = gValue + (deltaG << pixelShift);
int maxB = bValue + (deltaB << pixelShift);
int maxR = rValue + ((unsigned)deltaR << pixelShift);
int maxG = gValue + ((unsigned)deltaG << pixelShift);
int maxB = bValue + ((unsigned)deltaB << pixelShift);
/* Clamp those guys */
CLAMP_RGB(maxR, maxG, maxB);
/* And compute the actual delta */
Expand Down
6 changes: 3 additions & 3 deletions platforms/Cross/plugins/Squeak3D/b3dMain.c
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,7 @@ int b3dCheckIntersectionOfFaces(B3DPrimitiveFace *frontFace,
if(xValue > rightX) xValue = rightX;
/* Ignore intersections at or before the leftEdge's x value. Important. */
if((xValue >> B3D_FixedToIntShift) <= (leftEdge->xValue >> B3D_FixedToIntShift))
xValue = ((leftEdge->xValue >> B3D_FixedToIntShift) + 1) << B3D_IntToFixedShift;
xValue = (unsigned)((leftEdge->xValue >> B3D_FixedToIntShift) + 1) << B3D_IntToFixedShift;
if(xValue < nextIntersection->xValue) {
nextIntersection->xValue = xValue;
nextIntersection->leftFace = frontFace;
Expand Down Expand Up @@ -1249,9 +1249,9 @@ int b3dMainLoop(B3DRasterizerState *state, int stopReason)
/* STEP 2: Add new edges if necessary */
if(yValue == nextEdgeY) {
B3DPrimitiveObject *obj = activeStart;
int scaledY = (yValue+1) << B3D_IntToFixedShift;
int scaledY = (unsigned)(yValue+1) << B3D_IntToFixedShift;

nextEdgeY = nextObjY << B3D_IntToFixedShift;
nextEdgeY = (unsigned)nextObjY << B3D_IntToFixedShift;
while(obj != passiveStart) {
B3DInputFace *objFaces = obj->faces;
B3DPrimitiveVertex *objVtx = obj->vertices;
Expand Down

0 comments on commit 015d381

Please sign in to comment.