Converting Complex PHP Rotate Functions to Work in 64-bit

Question

A number of years back, my webhost changed from 32-bit to 64-bit, and a critical PHP script stopped working. It was a due to the << and >> (bit shift) operations having changed. I was able to fix my problem by replacing the rotateleft and rotateright routines with rotateleft32 and rotateright32 like this:

function rotateleft($value, $numleft) {
  return (($value << $numleft) | ($value >> (32-$numleft)));
}
function rotateleft32($value, $numleft) {
  return ((($value << $numleft) | ($value >> (32-$numleft))) & 0xFFFFFFFF);
}

function rotateright($value, $numright) {
  return (($value >> $numright) | ($value << (32-$numright)));
}
function rotateright32($value, $numright) {
  return ((($value >> $numright) | ($value << (32-$numright))) & 0xFFFFFFFF);
}

I have now come across a new set of code that seems to be exactly the same issue, but it is more complicated:

function ECC_RotateLeft($a)
{
    $copya = makecopy($a);
    $bit = ($copya->e[0] & ECC_UPRBIT) ? 1 : 0;

    /* looped
    for ($i = 0; $i < ECC_MAXLONG - 1; $i++)
    $copya->e[$i] = ($copya->e[$i] << 1) | (($copya->e[$i + 1] & ECC_MSB) ? 1 : 0);
    $copya->e[0] &= ECC_UPRMASK;
    looped */

    /* unlooped */
    // These lines are optimized for ECC_MAXLONG==4 only!
    $bit = ($copya->e[0] & ECC_UPRBIT) ? 1 : 0;
    $copya->e[0] = (($copya->e[0] << 1) & ECC_UPRMASK) | (($copya->e[1] & ECC_MSB) ? 1 : 0);
    $copya->e[1] = ($copya->e[1] << 1) | (($copya->e[2] & ECC_MSB) ? 1 : 0);
    $copya->e[2] = ($copya->e[2] << 1) | (($copya->e[3] & ECC_MSB) ? 1 : 0);
    /* unlooped */

    $copya->e[3] = ($copya->e[3] << 1) | $bit;
    return $copya;
}

function ECC_RotateRight($a)
{
    $copya = makecopy($a);
    $bit = ($copya->e[ECC_NUMWORD] & 1) ? ECC_UPRBIT : 0;

    /* looped
    for ($i = ECC_MAXLONG - 1; $i > 0; $i--)
    $copya->e[$i] = (($copya->e[$i] >> 1) & 0x7FFFFFFF) | (($copya->e[$i - 1] & 1) ? ECC_MSB : 0);
    looped */

    /* unlooped */
    // Thes lines are optimized for ECC_MAXLONG==4 only!
    $copya->e[3] = (($copya->e[3] >> 1) & 0x7FFFFFFF) | (($copya->e[2] & 1) ? ECC_MSB : 0);
    $copya->e[2] = (($copya->e[2] >> 1) & 0x7FFFFFFF) | (($copya->e[1] & 1) ? ECC_MSB : 0);
    $copya->e[1] = (($copya->e[1] >> 1) & 0x7FFFFFFF) | (($copya->e[0] & 1) ? ECC_MSB : 0);
    /* unlooped */

    $copya->e[0] = (($copya->e[0] >> 1) & 0x7FFFFFFF) | $bit;
    return $copya;
}

I have three problems in trying to fix this myself:

1. It is not my code, so I am not familiar with what it's trying to do.
2. I no longer have a 32-bit server to test it against
3. I am adequate, but not an expert in PHP.

I'd like to know if anyone does see a simple fix to allow this code to work on a 64-bit server and give the same result as it would have on a 32-bit server.

If not, how would you recommend I debug this given that I don't have a 32-bit result to compare against?

---

Here is some discussion regarding this problem, and an attempt to get the developer to fix it: How to get the outdated 32bit keymaker.php Script Working on 64 bit

Answer 1

Answering all four of your questions:

 1. It is not my code, so I am not familiar with what it is trying to do.

While I could go into tracing and debugging procedures in detail, I will instead recommend the classics. I highly recommend picking this up if this is your day job or you have more than a passing interest in refactoring code in the future.

 2. I no longer have a 32-bit server to test it against

As mentioned by Oli in the errata, you'll want to set up a 32-bit VM or chroot, dependent upon the OS your server is running.

 3. I am adequate, but not an expert in PHP.

As above, if this is more than just a spot issue, I recommend the classics.

 4. (Fixing the actual code)

First off, eww. No documentation, comment cruft, duplicate logic, and unexpressive variable names that fail to encapsulate their logic effectively. It's not the worst code I've seen, but I empathize with you here.

Still, the result isn't necessarily wrong. If you don't have a series of unit tests for it in your code base, I recommend adding them.

If you wish to benchmark the efficiency of this function, I highly recommend comparing it to the results of the algorithms defined in the notes here. Ideally, you'll want one of the implementations closest to this reference implementation.

Stealing one off the top of that thread and repurposing it to your API:

function ECC_RotateLeft($value,$amount) {
    if ($amount>0) {
        $amount %= 32;
        $value = ($value<<$amount) | ($value>>(32-$amount));
    }
    return $value;
}

function ECC_RotateRight($value,$amount) {
    if ($amount>0) {
        $amount %= 32;
        $value = ($value>>$amount) | ($value<<(32-$amount));
    }
    return $value;
}

(No surprise that this looks similar to the implementation you initially provided.)

Why am I including $amount as part of the specification? Simple: it doesn't violate encapsulation like the code you're refactoring. It looks like this can be set to ($copya->e[0] & ECC_UPRBIT) ? 1 : 0 as needed.

In short: the easiest way to refactor code isn't necessarily to look at its contained logic. Sometimes, determining intent and finding a good reference implementation is all that's needed.