gbdev/rgbds
1
0
Fork 0
mirror of https://github.com/gbdev/rgbds.git synced 2025-11-21 10:42:07 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #473 from ISSOtm/shift_ub

Browse Source 
Remove undefined behavior from shifts
This commit is contained in:
Eldred Habert
2020-02-03 03:49:38 +01:00
committed by GitHub
parent b16ec83a33 b11d121c48
commit ac6232bc87
10 changed files with 244 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

62
src/asm/constexpr.c
View File

@@ -205,28 +205,52 @@ void constexpr_BinaryOp(struct ConstExpression *expr,
result = value1 & value2;
break;
case T_OP_SHL:
if (value1 < 0)
warning(WARNING_SHIFT, "Left shift of negative value: %d",
value1);
if (value2 < 0)
fatalerror("Shift by negative value: %d",
value2);
else if (value2 >= 32)
fatalerror("Shift by too big value: %d",
value2);
result = (uint32_t)value1 << value2;
break;
case T_OP_SHR:
if (value2 < 0)
fatalerror("Shift by negative value: %d",
value2);
else if (value2 >= 32)
fatalerror("Shift by too big value: %d",
value2);
warning(WARNING_SHIFT_AMOUNT, "Shifting %s by negative amount %d",
op == T_OP_SHL ? "left" : "right",
value2);
if (op == T_OP_SHR) {
value2 = -value2; /* Right shift == neg left */
result = value1 >> value2;
if (value1 < 0)
warning(WARNING_SHIFT, "Shifting negative value %d",
value1);
}
if (value2 >= 0) {
// Shift left
if (value2 >= 32) {
warning(WARNING_SHIFT_AMOUNT, "Shifting left by large amount %d",
value2);
result = 0;
} else {
/*
* Use unsigned to force a bitwise shift
* Casting back is OK because the types
* implement two's complement behavior
*/
result = (uint32_t)value1 << value2;
}
} else {
// Shift right
value2 = -value2;
if (value2 >= 32) {
warning(WARNING_SHIFT_AMOUNT, "Shifting right by large amount %d",
value2);
result = value1 < 0 ? -1 : 0;
} else if (value1 >= 0) {
result = value1 >> value2;
} else {
/*
* The C standard leaves shifting right
* negative values undefined, so use a
* left shift manually sign-extended
*/
result = (uint32_t)value1 >> value2 |
-((uint32_t)1 << (32 - value2));
}
}
break;
case T_OP_MUL:
result = (uint32_t)value1 * (uint32_t)value2;

60
src/asm/rpn.c
View File

@@ -409,22 +409,59 @@ void rpn_AND(struct Expression *expr, const struct Expression *src1,
expr->nRPNPatchSize++;
}
static int32_t shift(int32_t shiftee, int32_t amount)
{
if (shiftee < 0)
warning(WARNING_SHIFT, "Shifting negative value %d", shiftee);
if (amount >= 0) {
// Left shift
if (amount >= 32) {
warning(WARNING_SHIFT_AMOUNT, "Shifting left by large amount %d",
amount);
return 0;
} else {
/*
* Use unsigned to force a bitwise shift
* Casting back is OK because the types implement two's
* complement behavior
*/
return (uint32_t)shiftee << amount;
}
} else {
// Right shift
amount = -amount;
if (amount >= 32) {
warning(WARNING_SHIFT_AMOUNT, "Shifting right by large amount %d",
amount);
return shiftee < 0 ? -1 : 0;
} else if (shiftee >= 0) {
return shiftee >> amount;
} else {
/*
* The C standard leaves shifting right negative values
* undefined, so use a left shift manually sign-extended
*/
return (uint32_t)shiftee >> amount
| -((uint32_t)1 << (32 - amount));
}
}
}
void rpn_SHL(struct Expression *expr, const struct Expression *src1,
const struct Expression *src2)
{
mergetwoexpressions(expr, src1, src2);
if (!expr->isReloc) {
if (src1->nVal < 0)
warning(WARNING_SHIFT, "Left shift of negative value: %d",
src1->nVal);
if (src2->nVal < 0)
fatalerror("Shift by negative value: %d", src2->nVal);
else if (src2->nVal >= 32)
fatalerror("Shift by too big value: %d", src2->nVal);
warning(WARNING_SHIFT_AMOUNT, "Shifting left by negative value: %d",
src2->nVal);
expr->nVal = ((uint32_t)src1->nVal << src2->nVal);
expr->nVal = shift(src1->nVal, src2->nVal);
}
pushbyte(expr, RPN_SHL);
@@ -438,11 +475,10 @@ void rpn_SHR(struct Expression *expr, const struct Expression *src1,
if (!expr->isReloc) {
if (src2->nVal < 0)
fatalerror("Shift by negative value: %d", src2->nVal);
else if (src2->nVal >= 32)
fatalerror("Shift by too big value: %d", src2->nVal);
warning(WARNING_SHIFT_AMOUNT, "Shifting right by negative value: %d",
src2->nVal);
expr->nVal = (src1->nVal >> src2->nVal);
expr->nVal = shift(src1->nVal, -src2->nVal);
}
pushbyte(expr, RPN_SHR);

3
src/asm/warning.c
View File

@@ -36,6 +36,7 @@ static enum WarningState const defaultWarnings[NB_WARNINGS] = {
WARNING_DISABLED, /* Obsolete things */
WARNING_DISABLED, /* Shifting undefined behavior */
WARNING_ENABLED, /* User warnings */
WARNING_DISABLED, /* Strange shift amount */
};
static enum WarningState warningStates[NB_WARNINGS];
@@ -70,6 +71,7 @@ static char const *warningFlags[NB_WARNINGS_ALL] = {
"obsolete",
"shift",
"user",
"shift-amount",
/* Meta warnings */
"all",
@@ -106,6 +108,7 @@ static uint8_t const _weverythingCommands[] = {
WARNING_OBSOLETE,
WARNING_SHIFT,
WARNING_USER,
WARNING_SHIFT_AMOUNT,
META_WARNING_DONE
};