3

The following code is executed without any error in debug mode of Visual C++ 2017 (15.2).

#include <iostream>
int main()
{
    int* ptr1 = new int( 2 );
    int* ptr2 = ptr1;
    delete ptr2;
    ptr2 = nullptr;
    int res = 40 + (*ptr1); // invalid
    std::cout << res << std::endl;
}

But I would expect the debugger to throw an error when the invalid pointer is dereferenced. Is there an option to activate the needed checks?

Back story: I just found a bug in some old legacy code (hence the raw pointers) that essentially boils down to the problem shown above. And the search would have taken muss less time if the Debugger could track the validity of pointers. Basic Runtime Checks (/RTC1) and Security Check (/GS) are enabled. Static code analysis did not warn about this problem is the original code, which is a lot more complicated than this distilled minimal example.

Tobias Hermann
  • 9,936
  • 6
  • 61
  • 134
  • Since `delete ptr`, will not do `ptr = NULL`, so after `delete ptr`, you have no way to identify whether the memory pointed by `ptr` is valid or not. – Gaurav Sehgal Aug 18 '17 at 11:20
  • why are you using `new` and `delete` here anyhow? I would write correct code rather than first asking for trouble and then fix it – 463035818_is_not_an_ai Aug 18 '17 at 11:20
  • To "know", you would use a safer construct like unique_ptr or shared_ptr. – Robinson Aug 18 '17 at 11:29
  • 2
    @GauravSehgal: I do not see why a debugger implementation could not track the validity of memory pointed to. – Tobias Hermann Aug 18 '17 at 11:30
  • @tobi303: The code in which I found the problem is very old. – Tobias Hermann Aug 18 '17 at 11:31
  • Turn on `/Wall /WX` – Ben Aug 18 '17 at 11:31
  • @Robinson: Yes, I know. – Tobias Hermann Aug 18 '17 at 11:31
  • @BensaysNotoPoliticsonSO: This perhaps helps for the particular minimal example, but not for the original code base I was working on. – Tobias Hermann Aug 18 '17 at 11:32
  • Fair enough. The only way to know is to test the pointer before use, which I suppose you should do as a precondition to using it. – Robinson Aug 18 '17 at 11:35
  • 1
    @TobiasHermann it's not advice for this particular issue - that's why it is a comment not an answer. It's general advice. Turn it on. Leave it on. Fix them all. Turn on static analysis. Leave it on. Fix all reported issues. – Ben Aug 18 '17 at 11:35
  • Can you alter the code? – user1810087 Aug 18 '17 at 11:40
  • When my kids lose things, I tell them: "Tidy your room, you'll probably find it. Even if you don't, at least your room will be tidy." That applies here. Fix all your warnings. You'll probably fix this bug. If you don't you'll at least have fixed many, many others. – Ben Aug 18 '17 at 11:41
  • @Robinson: The pointer was not set to `nullptr` in the original code after `delete`. As far as I know there is no possibility to test the pointer before use. – Tobias Hermann Aug 18 '17 at 11:43
  • @BensaysNotoPoliticsonSO: I fully agree. :) However in huge real legacy code bases it is not always feasible to do so. – Tobias Hermann Aug 18 '17 at 11:44
  • @user1810087: Yes. I already fixed the issue in the original code. I just wanted to improve the my process of finding such issues for the future. – Tobias Hermann Aug 18 '17 at 11:44
  • To make more such issues visible to code analysis, you need to add SAL annotations so the compiler can better understand the intended semantics. https://learn.microsoft.com/en-gb/visualstudio/code-quality/using-sal-annotations-to-reduce-c-cpp-code-defects – Ben Aug 18 '17 at 11:47
  • AddressSanitizer, UBsanitizer, valgrind and some other tools all at least attempt to detect this. There is no inherent reason why this cannot be done. It is just a matter of tooling and quality of it. – nwp Aug 18 '17 at 11:57

4 Answers4

2

This particular code example makes code analysis issues warning:

warning C6001: Using uninitialized memory 'ptr'.

You can launch it using Main menu -> Build -> Run code analysis on Solution.

Since capabilities of static code verification are rather limited you may want to enable additional runtime checks (project properties -> C/C++ -> Code generation -> Basic runtime checks should be set to Both) and tools, such as Application verifier.

user7860670
  • 35,849
  • 4
  • 58
  • 84
  • Thanks, but the runtime check do not help and static code analysis does not help in the original code base I was working on. – Tobias Hermann Aug 18 '17 at 11:34
2

But I would expect the debugger to throw an error when the invalid pointer is dereferenced.

The debugger cannot know whether or not a given pointer is invalid. It can only make a guess. That guess is usually on but there are some cases where it's impossible for the debugger to know for sure.

Puppy
  • 144,682
  • 38
  • 256
  • 465
  • Interesting. Can you elaborate on why the debugger can not track the validity of memory pointed to? – Tobias Hermann Aug 18 '17 at 11:32
  • 1
    @TobiasHermann Not a place for such discussion, but e.g. you may have several copies of the pointer in some data structure; you do `free`/`delete` on one of them; debugger would have to find all variables, which have been assigned that particular pointer to mark them invalid. Or it would have to check the heap structure every time you use a dereference operator (which would fail anyway if you do another `malloc`/`new` in a meantime which would allocate the same area for new purpose). – CiaPan Aug 18 '17 at 11:45
  • The problem is that memory usage is decided in pages, which are typically 4-8KB. The only thing the debugger knows is that you accessed memory in a page allocated to your program. When you call `delete`, you are simply returning it to *another part of your program*, the Standard library, which *may* return it to the OS, or it may keep it around. If it does keep it around, it's still allocated to your process, so it's fine. You're only breaking the rules of the Standard library to access it, not the rules of the system. So the system never alerts the debugger of a problem. – Puppy Aug 18 '17 at 12:20
  • It is possible in theory for the debugger to track whether or not the memory is allocated to the Standard library at the time of access, but this would require an enormous amount of book-keeping and overhead slowing the debugged program to a crawl. – Puppy Aug 18 '17 at 12:25
0

The real solution to this problem is to not manually free your resources, but always use self-releasing classes like unique_ptr which are immune to this problem.

It's a lot easier to simply never have the problem in the first place.

Puppy
  • 144,682
  • 38
  • 256
  • 465
-1

You can replace the builtin pointertype with this pointer-template-class, which detects at runtime; you need to #define DEBUG and use ptr<T> instead of T*:

void warning(std::string description) {
    std::cout << "WARNING: " << description << '\n';
};

    template<typename T>
struct debug_ptr {
     T* ptr;

    debug_ptr(T* ptr): ptr(ptr) {};
    debug_ptr(): ptr(nullptr) {};

    T& operator *() {
        if(!ptr) {
            warning("Access to uninitialized pointer");
        };
        return *ptr;
    };

    operator bool() { return ptr; };

    ~debug_ptr() {
        if(ptr) {
            warning("pointer is not freed");
        };
    };
};

        template<typename T>
#ifdef DEBUG
    using ptr = debug_ptr<T>;
#else
    using ptr = T*;
#endif

     template<T>
void del(T ptr) {
    delete ptr;
};

    template<typename T>
void del(debug_ptr<T>& ptr) {
    delete ptr.ptr;
    ptr.ptr = nullptr;
};

You must replace delete with del() to work, but if you #undef DEBUG, everything is compiled back to normal again. I used del() instead of delete here, because delete cannot change the variable like a funtion with a reference parameter can.

  • If you (want to) keep delete, there are possible ways you insert debug-information after the new/delete-keyword like this post shows.
  • You can use the stl-pointer-types such as unique_ptr or shared_ptr.
cmdLP
  • 1,658
  • 9
  • 19