3

There is an array out of bounds issue and a missing delete [] in the code below.

Compiling code with the following options does not trigger any errors:

g++ -std=c++2a -Wall -pedantic -fstack-protector-all test.cpp

#include <iostream>

class Base {
public:
    Base() { std::cout << "Base()\n";  }
    ~Base() { std::cout << "~Base()\n"; }
   int m_counter;
};


class Derived: public Base {
public:
    Derived() { std::cout << "Derived()\n"; }
    ~Derived() { std::cout << "~Derived()\n"; }
};


int main() {
    Base* b = new Derived[10];
    std::cout << b[10].m_counter << '\n';
    delete b;
    return 0;
}

Any pointers as to what I could be missing here?

RAR
  • 83
  • 13
inout
  • 65
  • 6
  • I assume `-fsanitize=address -fsanitize=leak` would work. – HolyBlackCat Sep 22 '19 at 18:35
  • See https://stackoverflow.com/questions/1239938/accessing-an-array-out-of-bounds-gives-no-error-why and maybe https://stackoverflow.com/questions/17308104/static-bound-checking-for-array-in-c – B. Go Sep 22 '19 at 18:36
  • See also https://stackoverflow.com/questions/2397984/undefined-unspecified-and-implementation-defined-behavior . Undefined Behavior is Undefined. "It works fine (so far)" is one possible result of UB, since any result is possible. – aschepler Sep 22 '19 at 18:51
  • @HolyBlackCat compiling with the options you suggest, did not trigger any compile time error. However, at run time, the following response was seen ```text SUMMARY: AddressSanitizer: heap-buffer-overflow (/home/saifi/a.out+0x1315) in main Shadow bytes around the buggy address: 0x0c087fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c087fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c087fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c087fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ``` – inout Sep 22 '19 at 18:57
  • @inout It's hard to detect such problems at compile-time, but those flags make it possible to detect them at runtime. – HolyBlackCat Sep 22 '19 at 19:01
  • 1
    I'd recommend using valgrind, it catches more – Wolfgang Brehm Sep 22 '19 at 19:01
  • @B.Go thanks for the links. it seems to suggest that "Bounds checking is not a part of raw arrays (or even std::vector)." Is that correct interpretation ? – inout Sep 22 '19 at 19:04
  • @aschepler the link you highlight, discusses 'undefined behaviour' in depth. What i'm wondering is, we are close to C++20 release and we still unable to address this issue. – inout Sep 22 '19 at 19:10
  • Using vector::at will perform bounds checking, operator[] does not need to (though vendors often provide a debug version of the library that does perform bounds checking on operator []). – SoronelHaetir Sep 22 '19 at 19:11
  • @Lykos thanks. let me give 'valgrind' a try. – inout Sep 22 '19 at 19:12
  • @SoronelHaetir does that mean 'standard library' container(s) are the way to go ? – inout Sep 22 '19 at 19:13
  • compile with debug symbols ( -Og -g ) and then just call your executable with valgrind as you would normally do. valgrind is a bit slower than AddressSanitizer but that should not matter when debugging. – Wolfgang Brehm Sep 22 '19 at 19:16
  • 1
    @inout I think C++ (and C) will always have undefined behavior. Requiring an implementation to do something about many categories of issue would either be flatly impossible or would require sacrificing the "close-to-the-machine" efficiency that some real-world C++ uses rely on. – aschepler Sep 22 '19 at 19:24
  • @Lykos ```text Invalid read of size 4 at 0x1092DA: main (err_four.cpp:23) Address 0x4d8dcb0 is 0 bytes after a block of size 48 alloc'd at 0x483950F: operator new[](unsigned long) (vg_replace_malloc.c:423) ~Base() Invalid free() / delete / delete[] / realloc() at 0x483A08B: operator delete(void*, unsigned long) (vg_replace_malloc.c:585) by 0x109324: main (err_four.cpp:24) Address 0x4d8dc88 is 8 bytes inside a block of size 48 alloc'd at 0x483950F: operator new[](unsigned long) (vg_replace_malloc.c:423) by 0x109229: main (err_four.cpp:22) ``` – inout Sep 22 '19 at 19:24
  • @aschepler what you have highlighted is very logical. Thanks for the insight. Perhaps it's the expectation that with two quick revisions to C++ namely C++17 and now C++20, array out of bound errors will be history. Guess, i was wrong. – inout Sep 22 '19 at 19:26
  • @inout this means you err_four.cpp has probable memory access violation in line 24 – Wolfgang Brehm Sep 22 '19 at 20:04
  • If it was a local array (`Derived b[10];`), enabling optimizations (at least `-O2`) would give you a warning at compile time. There does not seem to be any equivalent with dynamic allocation, you could always try filing a feature request... – Marc Glisse Sep 22 '19 at 20:15
  • 1
    It's not that we're unable to address this issue. C++ has its own goals that may be different from other languages and it has the concept of undefined behavior for a reason. Adding bound checks to every single array access takes up valuable processor time which isn't ideal in many cases. If the people writing the C++ standard wanted to eliminate out-of-bounds errors it would have been done way earlier. – eesiraed Sep 22 '19 at 21:38
  • @AlexanderZhang thanks for sharing your perspective. it helps me see where the focus should be. Thanks once again ! – inout Sep 23 '19 at 02:06
  • @MarcGlisse i really didn't know about the connection between local array and optimization (-O2). This is an area i need to catch up. Thanks. – inout Sep 23 '19 at 02:08
  • @Lykos Yes, line 24 is where i should be looking at. Thanks for the idea about using valgrind. – inout Sep 23 '19 at 02:09

1 Answers1

1

I think you're looking for -fsanitize=bounds

Ripi2
  • 7,031
  • 1
  • 17
  • 33
  • Ripi2 thanks for the suggestion. on executing the binary, the output seen is as follows: ```text Base() Derived() ... 10 times 0 ~Base() munmap_chunk(): invalid_pointer Aborted (core dumped) ``` when the binary is compiled with -fsanitize=address -fsanitize=leak there is a more detailed description of the situation. – inout Sep 23 '19 at 01:58