RFA: Fix sim scache handling to use unsigned values.
Commit Message
Hi Mike,
A recent bug report on the binutils list pointed out that a construct
like this:
for (i = 1; i; i <<= 1)
can have undefined behaviour if the loop variable is signed and the
shift operation moves a 1 into the sign bit:
lists.gnu.org/archive/html/bug-binutils/2016-02/msg00006.html
A quick scan of the simulator sources found one place where this
happens, in common/cgen-scache.c:scache_option_handler(). Looking at
the code here it seemed to me that the simplest fix is to switch to
using an unsigned int for the parsed value of the scache size. (A
negative scache size does not make any sense right ?) I wondered if
it would be worth using "unsigned long" instead of "unsigned int",
since there is no strtou() function, but this seemed like overkill.
So I created the patch below. It builds OK, but I am not sure how to
test it. Any suggestions ?
Cheers
Nick
Comments
On 03 Feb 2016 16:25, Nick Clifton wrote:
> A recent bug report on the binutils list pointed out that a construct
> like this:
>
> for (i = 1; i; i <<= 1)
>
> can have undefined behaviour if the loop variable is signed and the
> shift operation moves a 1 into the sign bit:
>
> lists.gnu.org/archive/html/bug-binutils/2016-02/msg00006.html
>
> A quick scan of the simulator sources found one place where this
> happens, in common/cgen-scache.c:scache_option_handler(). Looking at
> the code here it seemed to me that the simplest fix is to switch to
> using an unsigned int for the parsed value of the scache size. (A
> negative scache size does not make any sense right ?) I wondered if
> it would be worth using "unsigned long" instead of "unsigned int",
> since there is no strtou() function, but this seemed like overkill.
>
> So I created the patch below. It builds OK, but I am not sure how to
> test it. Any suggestions ?
seems fine
-mike
@@ -121,24 +121,26 @@ scache_option_handler (SIM_DESC sd, sim_cpu *cpu, int opt,
{
if (arg != NULL)
{
- int n = strtol (arg, NULL, 0);
+ unsigned int n = (unsigned int) strtoul (arg, NULL, 0);
if (n < MIN_SCACHE_SIZE)
{
- sim_io_eprintf (sd, "invalid scache size `%d', must be at least 4", n);
+ sim_io_eprintf (sd, "invalid scache size `%u', must be at least %u",
+ n, MIN_SCACHE_SIZE);
return SIM_RC_FAIL;
}
/* Ensure it's a multiple of 2. */
if ((n & (n - 1)) != 0)
{
- sim_io_eprintf (sd, "scache size `%d' not a multiple of 2\n", n);
- {
- /* round up to nearest multiple of 2 */
- int i;
- for (i = 1; i < n; i <<= 1)
- continue;
- n = i;
- }
- sim_io_eprintf (sd, "rounding scache size up to %d\n", n);
+ unsigned int i;
+ sim_io_eprintf (sd, "scache size `%u' not a multiple of 2\n", n);
+ /* Round up to nearest multiple of 2. */
+ for (i = 1; i && i < n; i <<= 1)
+ continue;
+ if (i)
+ {
+ n = i;
+ sim_io_eprintf (sd, "rounding scache size up to %u\n", n);
+ }
}
if (cpu == NULL)
STATE_SCACHE_SIZE (sd) = n;