VS2015: [C6386] Buffer Overrun while writing (even for same index value)

Question

I'm trying to implement merge sort in C when I came across something interesting raised by [Analyze -> Run Code Analysis] in Visual Studio 2015.

The code is as follows:

void MergeSort_r(int A[], int n)
{
    // A = {1, 3, 2}
    // n = 3
    int rightCount;
    int* R;

    if ( n < 2 ) return;

    // version 1: rightCount = 2
    rightCount = n - (n/2);

    // version 2: rightCount = 2
    rightCount = n - 1;

    R = ( int* ) malloc( rightCount * sizeof( int ) );

    if ( R ) {
        for ( int i = 0; i < rightCount; i++ ) {
            R[i] = A[i];
        }

    free( R );
    }

}

Even though both version of rightCount essentially evaluates to 2, in the first version, I get the warning:

"Buffer overrun while writing to 'R': the writable size is '(unsigned int)rightCount*sizeof(int)' bytes, but '8' bytes might be written."

Any idea why this is the case? Looking forward to hear your answers.

[Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). — Sourav Ghosh, May 13 '16 at 08:40
@MikeMB Isn't it like we have C compilers to compile C code? We should be using that, anyway. BTW, that was just an informative comment. — Sourav Ghosh, May 13 '16 at 08:44
@SouravGhosh, your responses doesn't solve the problem in hand. `malloc` may also be used in C++. Mostly C++ is used, not C. — Ajay, May 13 '16 at 08:45
@Ajay C++ has `new`, better use that. You _can_ shoot your foot, but that does not mean you _have to_. :) — Sourav Ghosh, May 13 '16 at 08:46
@SouravGhosh I get `error: invalid conversion from 'void*' to 'int*' [-fpermissive]`, if I don't cast. — Shreevardhan, May 13 '16 at 08:48
@SouravGhosh, yes ` new` is definitely the way for C++. I advised the same in answer I provided. — Ajay, May 13 '16 at 08:48
I was following this guideline when i cast the return value (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rcpl-subset), so it is not a good idea? — adsisco, May 13 '16 at 08:51
@adsisco :Let's not start a discussion on this here, please visit the linked post and decide for yourself. :) — Sourav Ghosh, May 13 '16 at 08:52
@SouravGhosh To `free` a `NULL` pointer. If you check that malloc succeeded you should `free` it inside the `if` scope. — LPs, May 13 '16 at 08:54
@LPs, that's just more code, in reality. `free` itself checks it. — Ajay, May 13 '16 at 08:55
@LPs In that case, you might want to re-write the ___have to be___ part. Your opinion is private, what spec allows is different. :) — Sourav Ghosh, May 13 '16 at 08:55
`free( R );` should to be inside `if ( R )`. My personal opinion. — LPs, May 13 '16 at 08:56
Moved it inside, but really doesn't explain the discrepancy in warnings =P — adsisco, May 13 '16 at 08:57
Probably @Ajay gave you the right answer. Is a VS s... stuff. ;) — LPs, May 13 '16 at 08:58

Ajay · Accepted Answer · 2020-10-18T13:17:46.403

Visual C++ Code Analysis toolset may not always offer the best warnings. It tries to give you the best set of warnings to fix some potential issues/errors that may creep in at runtime. You have a few options:

Disable the given warning around the code using #pragma directive.
Use C++ constructs: new, make_unique etc.
(Not recommended) is to ignore the warning altogether and move on.

You should ideally always user newer smart pointers primitives like unique_ptr, shared_ptr etc. They not only allocate memory for you but deallocate on any exception thrown across the call stack. You don't need to type * at all!

auto buffer = make_unique<int[]>(10); // 10 integers

Maybe you could explicitly state, that smart pointers should be used for owning pointers. There is nothing wrong with non-owning raw pointers. — MikeMB, May 15 '16 at 20:30

score 3 · Answer 2 · answered May 13 '16 at 08:58

3

Your code is fine and tools(especially analyzers) have their drawbacks — sometimes they generate false-positives. That's one of it. BTW, I checked your code on MSVS2015 and it gives me no warnings.

answered May 13 '16 at 08:58

ixSci

13,100
5
45
79

you need to enable Code-Analysis – Ajay May 13 '16 at 08:59
@Ajay, I ran the code analysis tool against this code: *Running Code Analysis for C/C++...* – ixSci May 13 '16 at 09:01
You are right. Even I didn't get it (both 32-bit, 64-bit compilation). May be OP can shed more light on it. Is this exact code? Which VS version? (I have VS2015 U2) – Ajay May 13 '16 at 09:07

VS2015: [C6386] Buffer Overrun while writing (even for same index value)

2 Answers2

Linked