My program runs correctly while debugging, but incorrectly when out of debugging

Question

I'm currently in the middle of writing a simple program in C that takes a command line argument from the user that is a substitution cipher key. Currently, my program is checking for all the possible incorrect arguments the user might enter.

Running this through the debugger results in my program running perfectly every single time, no matter what the input is. However, running my program without the debugger, while also making the argument have at least one lower-case letter (eg. ./substitution VCHPRZGJNTLSKFBDQWAXEuymoi), results in the 'sum' variable having an incorrect value about 50% of the time.

I'll quickly explain what the 'sum' variable is for just so it's easier to understand. 'sum' starts out at 2015, because that's the value of adding up all 26 upper case letters according to the ASCII table. The for loop in my function then subtracts from 2015 each character from the cipher key. Finally, the last if statement checks for whether sum = 0, meaning that only unique characters were entered.

When running this without a debugger, 'sum' is sometimes 0 and sometimes -32 * the number of lower-case letters. For example, VCHPRZGJNTLSKFBDQWAXEuymoi has 4 lower-case letters, and 'sum' is sometimes 0 and sometimes -128.

I've got no idea where this problem is coming from, and I would greatly appreciate any help.

Here is my code:

#include <cs50.h>
#include <stdio.h>
#include <string.h>

string sub(string plaintext,string cipher);

int main(int argc, string argv[])
{
    string cipher = argv[1];
    // checks for whether an argument was provided
    if (argc != 2)
    {
        printf("Usage: ./substitution KEY\n");
        return 1;
    }
    // checks for whether the cipher is 26 characters long
    else if (strlen(cipher) != 26)
    {
        printf("Key must contain 26 characters\n");
        return 1;
    }
    
    // makes sure the cipher contains only non-alphabetical characters
    // also subtracts from 2015 (sum of all unique, upper-case letters) with each iteration to test for whether every character in the argument is unique
    int sum = 2015;
    for (int i = 0; i < strlen(cipher); i++)
    {
        if ((int) cipher[i] > 96 && (int) cipher < 123)
        {
            cipher[i] = (char) ((int) cipher[i] - 32);
        }
        
        if ((int) cipher[i] < 65 || ((int) cipher[i] > 90 && (int) cipher[i] < 97) || (int) cipher[i] > 122)
        {
            printf("Key must only contain alphabetical characters.\n");
            return 1;
            break;
        }
        sum -= ((int) cipher[i]);
    }
    
    // DEBUG: prints 'sum'
    printf("%i\n", sum);
    
    // THIS IS THE PROBLEM STATEMENT
    // determines whether every character in the argument is unique
    if (sum != 0)
    {   
        printf("Key must contain all unique characters.\n");
        return 1;
    }
    
    // gets the plaintext from the user and prints out the cipher text using the 'sub' function
    string plaintext = get_string("plaintext: ");
    printf("ciphertext: %s\n",sub(plaintext,cipher));
}

Answer 1

Your OS is probably using ASLR for preventing exploitation of memory corruption vulnerabilities. This might be the reason why the value of the sum variable has an incorrect value about 50% of the time. As spotted in this comment a pointer is compared to an integer in if ((int) cipher[i] > 96 && (int) cipher < 123), which might be the problem. However, ASLR seems to get disabled by your debugger and the addresses get not randomized. Therefore, the address pointed by the variable cipher is not randomized and you always get the same result.

If you are using gdb you can enable ASLR with the command set disable-randomization off.

Edit As suggested by Eric Postpischil in his comments [1, 2] you should use 'a' instead of constants like 97 and the (int) casts should be entirely removed. The (int) casts are unnecessary, since the char elements of the array will be promoted to int.

You should also see the compiler warnings.

For

if ((int) cipher[i] > 96 && (int) cipher < 123)

I get the following warning:

warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
if ((int) cipher[i] > 96 && (int) cipher < 123) {

On gcc -Wpointer-to-int-cast should be enabled by default. Otherwise, you can also also try the -Wall option.

Answer 2

Unrelated, but for this kind of comparison, there's ctype.h

This changes the loop to

for (int i = 0; i < strlen(cipher); i++)
{
    if (islower(cipher[i]))
    {
        cipher[i] = toupper(cipher[i]);
    }

    if (!isalpha(cipher[i]))
    {
        printf("Key must only contain alphabetical characters.\n");
        return 1;
        /* break; */
    }

    sum -= (int) cipher[i];
}

This would reduce code length, make the code more readable, avoid constants, and avoid some of the pitfalls. Another point, the break isn't needed here, because you already return from the function. So the break is never reached.