Code that transform roman numerals into arabic numbers gets RunTime Error on LeetCode but not on Visual Studio

Question

I'm a new coder and I'm learning C, after studying some of the basics I tried doing some LeetCode exercises, and one of them is a Roman to Integer converter, after some trying I moved from the LeetCode site to Visual Studio to try and do a functioning code since for me at least the assisting tools of the VStudio are very helpful, in there I did a code that worked, I think, smoothly, but when I copied it into LeetCode and did some tweaks it returned me an Run Time Error, can anyone help me discovering what is causing the problem?

Sorry for any mistakes, English is not my first Language and it's the first time I'm posting on StackOverflow.

The code that I'm using on Visual Studio with the testcase "MCMXCIV":

#include <stdio.h>

char s[] = {'M','C','M','X','C','I','V'};
int vals[sizeof(s)];

int sum = 0;

int main(void) {
    //Getting the info about how many characters there are in the Roman Numeral
    int nchar = sizeof(s);

    //For loop that assigns each Roman Numeral its correspondant in the Arabic Numbers
    for (int i = 0; i < nchar; i++) {
        if (s[i] == 'I') {
            vals[i] = 1;
        }
        else if (s[i] == 'V') {
            vals[i] = 5;
        }
        else if (s[i] == 'X') {
            vals[i] = 10;
        }
        else if (s[i] == 'L') {
            vals[i] = 50;
        }
        else if (s[i] == 'C') {
            vals[i] = 100;
        }
        else if (s[i] == 'D') {
            vals[i] = 500;
        }
        else {
            vals[i] = 1000;
        }
    }

    //For loop that get the sum in Arabics
    for (int i = 0; i < nchar; i++) {
        if (i != nchar - 1 && vals[i] < vals[i + 1]) {
            sum = sum + vals[i + 1] - vals[i];
            i++;
        }
        else {
            sum = sum + vals[i];
        }
    }
    printf("Your Roman Numer is %i in Arabic Numbers!", sum);
    return 0;
}

The slightly tweaked code that I'm using on LeetCode(with the same testcase):

int romanToInt(char * s){
    //variables used int the loops
    int nchar = sizeof(s);
    int vals[sizeof(s)];
    int sum = 0;

    //for loop that assigns each Roman Number to its Arabic correspondant
    for (int i = 0; i < nchar; i++) {
        if (s[i] == 'I') {
            vals[i] = 1;
        }
        else if (s[i] == 'V') {
            vals[i] = 5;
        }
        else if (s[i] == 'X') {
            vals[i] = 10;
        }
        else if (s[i] == 'L') {
            vals[i] = 50;
        }
        else if (s[i] == 'C') {
            vals[i] = 100;
        }
        else if (s[i] == 'D') {
            vals[i] = 500;
        }
        else {
            vals[i] = 1000;
        }
    }

    //sum algorithm
    for (int i = 0; i < nchar; i++) {
        if (i != nchar - 1 && vals[i] < vals[i + 1]) {
            sum = sum + vals[i + 1] - vals[i];
            i++;
        }
        else {
            sum = sum + vals[i];
        }
    }
    return sum;
}

The Error I'm getting on LeetCode:

=================================================================
==34==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000014 at pc 0x563ee0bf8176 bp 0x7ffd0819f880 sp 0x7ffd0819f870
READ of size 1 at 0x602000000014 thread T0
    #2 0x7f786db5b0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
0x602000000014 is located 0 bytes to the right of 4-byte region [0x602000000010,0x602000000014)
allocated by thread T0 here:
    #0 0x7f786e7a0bc8 in malloc (/lib/x86_64-linux-gnu/libasan.so.5+0x10dbc8)
    #3 0x7f786db5b0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
Shadow bytes around the buggy address:
  0x0c047fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c047fff8000: fa fa[04]fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8010: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==34==ABORTING

Sorry for any mistakes, English is not my first Language and it's the first time I'm posting on StackOverflow.

In the second one for LeetCode the `int nchar = sizeof(s);` is incorrect, because here `s` is a pointer not an array. Instead of `7` if they are using a 64-bit compiler it will be `8`, with a 32-bit compiler that will be `4` and the array will overflow. — Weather Vane, Nov 07 '22 at 15:31
General question: Why do you use different code? If you want to test your code before submitting, write your own test harness where you call your code in the same way it will be called online. Don't expect to get same result if you test different code. — Gerhardh, Nov 07 '22 at 15:35
@WeatherVane That's the year 1994 in roman numerals. It's the input. — Oka, Nov 07 '22 at 15:39
@Oka ooh.. the input is hard coded? How is that going to work when submitted? I thought it was meant to create a lookup table maybe. If that was the test harness Gerhardh is right: test with exactly the same code that will be submitted. — Weather Vane, Nov 07 '22 at 15:42
I guess the LeetCode version that calls `romanToInt()` will be passing a null-terminated string? Use `strlen(s);` to determine its length. Your other, non-LeetCode version with the `char s[]` array doesn't have a null-terminated string, but you can change that easily enough. — Ian Abbott, Nov 07 '22 at 15:43
Build with debug information (add the `-g` flag when building) and the dump should be able to give you more information about the location of the problem in your code. — Some programmer dude, Nov 07 '22 at 15:57
And please learn how to use a [*debugger*](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) to catch problems a run-time. — Some programmer dude, Nov 07 '22 at 15:58
That `if/else` chain can be replaced by a much more readable lookup table. — William Pursell, Nov 07 '22 at 15:59

Oka · Accepted Answer · 2022-11-07T16:19:22.523

The sizeof operator evaluates to the size of its operand's object representation in bytes.

With

char s[] = {'M','C','M','X','C','I','V'};

s has the type char [7], and sizeof s is 7 bytes.

With

int romanToInt(char *s)

s has the type char *, and sizeof s is platform dependent. Common sizes for a pointer are 8 bytes (64-bit) and 4 bytes (32-bit).

In the LeetCode environment, the argument to the function is a properly null-terminated string, and the problem description states

It is guaranteed that s is a valid roman numeral in the range [1, 3999].

Which means the input can be of length [1, 15].

Your loop always iterates sizeof (char *) times, and will surely read past the end of shorter strings, accessing memory it should not. This is Undefined Behaviour, but is caught by ASan.

Use strlen to find the length of the input, or loop until you reach the null-terminating byte ('\0').

for (size_t i = 0; s[i] != '\0'; i++) {
    /* ... */
}

Note that this will not work with your local version, as

char s[] = {'M','C','M','X','C','I','V'};

lacks the null-terminating byte. Initialize your array with a string literal

char s[] = "MCMXCIV";

to include one.

Code that transform roman numerals into arabic numbers gets RunTime Error on LeetCode but not on Visual Studio

1 Answers1