1

Consider the following pretty simple C++ code: 

#include <algorithm>
#include <iostream>
using namespace std;

int main()
{
  int a[7] = {1, 2, 3, 4, 5, 6, 7};
  int b[7];
  copy(a, a+7, b);
  for (int i=0; i<8; ++i)
    cout << b[i] << endl;
}

Now here's what I get when I load this code in gdb: 

(gdb) b 1
Breakpoint 1 at 0x100000a64: file stdcopy.cpp, line 1.
(gdb) r
Starting program: /Users/Babai/pastebin/a.out 
Reading symbols for shared libraries ++......................... done

Breakpoint 1, main () at stdcopy.cpp:7
7     int a[7] = {1, 2, 3, 4, 5, 6, 7};
(gdb) n
9     copy(a, a+7, b);
(gdb) s
std::copy<int*, int*> (__first=0x7fff5fbffb8c, __last=0x7fff5fbffba8, __result=0x7fff5fbffb70) at stl_algobase.h:398
398        const bool __in = __is_normal_iterator<_InputIterator>::__value;
(gdb) bt
#0  std::copy<int*, int*> (__first=0x7fff5fbffb8c, __last=0x7fff5fbffba8, __result=0x7fff5fbffb70) at stl_algobase.h:398
#1  0x0000000100000acd in main () at stdcopy.cpp:9
(gdb) up
#1  main () at stdcopy.cpp:10
10    for (int i=0; i<8; ++i)
(gdb) p &a
$1 = (int (*)[7]) 0x7fff5fbffb8c
(gdb) p a + 7
$2 = (int *) 0x7fff5fbffba8

I don't see any valgrind errors in this code and I am wondering why. The array a has 7 elements and accessing up to a + 6 is fine, but why is valgrind not showing a + 7 as a valid error?

Fanatic23
  • 3,378
  • 2
  • 28
  • 51
  • Should it report it? I'm not too familiar with valgrind but you may be allowed to write to that memory. – Johan Lundberg Feb 08 '12 at 19:51
  • 3
    Why are you showing us GDB output? Why are you talking about valgrind but don't show any valgrind output? Is this a prank? – Kerrek SB Feb 08 '12 at 19:51
  • 1
    It doesn't look like you stepped through the program far enough for `a[7]` to be run – Seth Carnegie Feb 08 '12 at 19:51
  • @KerrekSB: You might be the kind of a person who plays random pranks on unsuspecting people, but I frankly don't have patience for this kind of frivolous antics. The reason I showed the GDB output since it was accessing 1 + the last known valid array address (i.e. a + 6). – Fanatic23 Feb 08 '12 at 19:57
  • @Fanatic23 don't be disrespectful. Also it's not accessing 1 + the last known valid array address, it's just storing the address of 1 past the end, without accessing it, which is fine. – Seth Carnegie Feb 08 '12 at 20:00

4 Answers4

2

The memcheck tool in Valgrind does not report stack based memory errors (unless you overrun the top of your stack address space). It reports heap based memory errors. Allocate your array on the heap, and Valgrind should report invalid reads (not from the copy, but from the for loop which goes past the end.)

#include <algorithm>
#include <iostream>
#include <cstring>

int main()
{
  int* a = new int[7];
  int* b = new int[7];
  std::memset(a, 0, sizeof(int) * 7);
  std::memset(b, 0, sizeof(int) * 7);

  std::copy(a, a+7, b);

  for (int i=0; i<8; ++i)
    std::cout << b[i] << std::endl;

  delete[] a;
  delete[] b;
}


From the Valgrind manual:

Memcheck is a memory error detector. It can detect the following problems that are common in C and C++ programs.

Accessing memory you shouldn't, e.g. overrunning and underrunning heap blocks, overrunning the top of the stack, and accessing memory after it has been freed. Using undefined values, i.e. values that have not been initialised, or that have been derived from other undefined values.

Incorrect freeing of heap memory, such as double-freeing heap blocks, or mismatched use of malloc/new/new[] versus free/delete/delete[]

Overlapping src and dst pointers in memcpy and related functions.

Memory leaks.

Charles Salvia
  • 52,325
  • 13
  • 128
  • 140
1

Going past the end of the array results in Undefined Behaviour, meaning that anything could happen. However, your pointer into the array, is allowed to go one past the array, if the pointer is not dereferenced.

This is allowed so that you can check for end of an array, and it is used in containers with iterators. The end() function for a container iterator points to one past the end. This however never get's dereferenced, else you're in Undefined Behaviour land.

Tony The Lion
  • 61,704
  • 67
  • 242
  • 415
  • This is what I needed to know. Could you please point me to corresponding section of the standard which mentions this. – Fanatic23 Feb 08 '12 at 19:55
  • I think he's talking about `for (int i=0; i<8; ++i)` which does cause the pointer to the element past the end to be dereferenced (I didn't downvote) – Seth Carnegie Feb 08 '12 at 19:55
  • @SethCarnegie look at what OP says above your comment. I know 8 goes one past the end to be dereferenced, he's talking about a + 7 in his question however. – Tony The Lion Feb 08 '12 at 19:57
  • 1
    -1, wrong answer. Of course this is undefined behaviour. The question was why is Valgrind, a error detecting tool, is not detecting the error. – cha0site Feb 08 '12 at 19:58
  • @Fanatic23 I'm not familiar enough with the standard to point you to the section where it says this. I'm sure someone else is though?? Anyone? – Tony The Lion Feb 08 '12 at 19:59
  • @Tony: Huh. No, I missed it, sorry. – cha0site Feb 08 '12 at 20:00
  • 1
    §5.7.5 is relevant I think: _Moreover, if the expression P points to the last element of an array object, the expression (P)+1 points one past the last element of the array object, and if the expression Q points one past the last element of an array object, the expression (Q)-1 points to the last element of the array object. If both the pointer operand and the result point to elements of the same array object, or one past the last element of the array object, the evaluation shall not produce an overflow; otherwise, the behavior is undefined._ – Seth Carnegie Feb 08 '12 at 20:01
0

The a+7 expression isn't an error - that's the end indicator for the copy and is never accessed. The copy is performed up to, but not including a+7.

However, the cout << b[i] << endl; being executed when i == 7 is another story - that is an error (though I'm not sure if valgrind is designed to catch that kind of error on a non-heap allocated array).

Michael Burr
  • 333,147
  • 50
  • 533
  • 760
0

I think I got it. What basically is happening is that there's no access of *(a + 7) anywhere. The call to std::copy ultimately boils down to memmove, and couple with what Seth posted in section 5.7.5 I think we are good here: 

(gdb) down
#0  std::__copy_aux<int*, int*> (__first=0x7fff5fbffb8c, __last=0x7fff5fbffba8, __result=0x7fff5fbffb70) at stl_algobase.h:313
313                  && __are_same<_ValueTypeI, _ValueTypeO>::__value);
(gdb) n
315       return std::__copy<__simple, _Category>::copy(__first, __last, __result);
(gdb) s
std::__copy<true, std::random_access_iterator_tag>::copy<int> (__first=0x7fff5fbffb8c, __last=0x7fff5fbffba8, __result=0x7fff5fbffb70) at stl_algobase.h:298
298       std::memmove(__result, __first, sizeof(_Tp) * (__last - __first));
(gdb) list
293     {
294       template<typename _Tp>
295         static _Tp*
296         copy(const _Tp* __first, const _Tp* __last, _Tp* __result)
297         { 
298       std::memmove(__result, __first, sizeof(_Tp) * (__last - __first));
299       return __result + (__last - __first);
300     }
301     };
302
Fanatic23
  • 3,378
  • 2
  • 28
  • 51