Undefined Behavior: A checklist for code reviews

Question

Possible Duplicate:
What are all the common undefined behaviour that a C++ programmer should know about?

I am about to prepare a checklist or guideliness for C++ self & peer code reviews, Since there are so many scenarios which may lead to realm of the dreaded Undefined Behavior, I was thinking of coming up with a sort of checklist of Undefined behaviors in most heavily used C++ language constructs.

Ofcourse it is not possible to anticipate the Undefined behaviors which emnate through modifications of variables between Sequence points but I think it is possible to list down scenarios emnating from other scenarios.

If you were performing a code review what commonly Undefined Behavior producing scenarios would you look out for?

It's a half hour job to search the standard for *all* mentions of undefined behaviour and list the potential causes... (I've included such a list in one of my old S.O. answers). If you're serious about this you'll want such an exhaustive list and not an ad-hoc off-the-top-of-our-head collection from S.O. readers. Given such a list, you're in a better position than us to say which ones you think are worth checking for given your coding environment, techniques and robustness needs.... — Tony Delroy, Jun 07 '11 at 04:11
@Tony: I could'nt find any duplicate, Eventhough I explicitly searched for it. Can you post a link to your previous answer? — Alok Save, Jun 07 '11 at 04:13
I couldn't find mine (maybe the question went in S.O. housekeeping), but Greg's link looks spot on. — Tony Delroy, Jun 07 '11 at 04:24

iammilind · Answer 1 · 2011-06-07T04:20:54.910

2

(1) Bad delete

T* p = new T[N];
delete p; // ...1
delete (p+x); //...2

(2) Missing return

int* foo ()
{
  // code
  return p;
  // code
}

(3) Double delete

(4) Reference to temporary

(5) Adding vector element to itself (when size was defined)

  vector<int> vi(1);
  vi.push_back(0);
  vi.push_back(vi[0]); // it can become a use case of pt(4) (sometimes)

edited Jun 07 '11 at 04:20

answered Jun 07 '11 at 04:07

iammilind

68,093
33
169
336

score 1 · Answer 2 · edited Mar 19 '17 at 22:22

1

deleting only pointers that are newed. for example, you can't call p = new int [5]; and then delete p+2; this may cause undefined behavior.

also when trying to use dlls only use primitive types since different compilers create different memory layout and that may cause some problems if you try to exchange classes or structs.

the other thing that I can think of is to watch out for the deleted memory, in some cases, you can write and read from those places without any error or access violation but it's always undefined behavior.

edited Mar 19 '17 at 22:22

curious_beast

85
1
9

answered Jun 07 '11 at 04:07

Ali1S232

3,373
2
27
46

1

You can't even `delete p;` in this case. You must `delete [] p;` or it's undefined behavior. – Fred Larson Jun 07 '11 at 04:27
@fred larson that thing depends on compiler, in some compilers there is no defrence between `delete p` and `delete []p` – Ali1S232 Jun 07 '11 at 04:45
3

One of the amazing things about undefined behavior is that, under certain compilers/circumstances, it might appear well defined. – Dennis Zickefoose Jun 07 '11 at 05:28
@dennis: agreed but the sad point is even on same compiler your code might not work if you just add something in some other file in some other class which is absolutly not related to your ***misbehaved*** code! – Ali1S232 Jun 07 '11 at 05:44

score 1 · Answer 3 · edited May 23 '17 at 12:32

1

Non-Availability of Virtual Destructor in polymorphic base class

Trivial Example

class Base
{
   public:
    Base();
   // some virtual functions     
   // no virtual destructor
   ~Base();
};

class Derived : public Base
{
   public:
   Derived();
   // override the functions here      
   ~Derived();
};
// definitions
int main()
{
   Base *p = new Derived();
   // function calls
   delete p; // UB
}

Here is a comprehensive list of undefined behaviour scenarios in C++.

edited May 23 '17 at 12:32

Community

1
1

answered Jun 07 '11 at 04:08

Prasoon Saurav

91,295
49
239
345

This is not necessarily UB; it can cause bad effects. http://www2.research.att.com/~bs/bs_faq2.html#virtual-dtor – iammilind Jun 07 '11 at 04:16
@iammilind : It is **definitely** UB. $5.3.5/3 In the first alternative (delete object), if the static type of the operand is different from its dynamic type, the static type shall be a base class of the operand’s dynamic type and the **static type shall have a virtual destructor or the behavior is undefined**. – Prasoon Saurav Jun 07 '11 at 04:20
@iammilind: Prof. Stroustrup has a nicely (apparently confusingly) understated way of expressing it, but his answer agrees with the Standard as quoted by Prasoon. – Tony Delroy Jun 07 '11 at 04:27
Yes, standard should be the correct resort. Bjarne has really understated the impact. – iammilind Jun 07 '11 at 04:29
@Parasoon: Note that if the class is not intended to be used polymorphic (e.g. a mixin), then a `protected` destructor is fine too. – Billy ONeal Jun 07 '11 at 04:49
1

Also note that you don't need virtual destructors even for polymorphic use if you use shared_ptr (which you probably should). – ronag Jun 07 '11 at 05:11

score 1 · Answer 4 · answered Jun 07 '11 at 04:48

Less obvious cases than other answers:

The initialization order of nonlocal statics in different translation units is undefined. That is:

a.cpp

struct SomeStatic
{
    SomeStatic()
    {
        // Some init code
    }
    void AMethodUsingTheInitdCode()
    {
        //Blah blah
    }
} static;

b.cpp

struct SomeStatic;
extern SomeStatic static;

struct SomeOtherStatic
{
    SomeOtherStatic()
    {
        static.AMethodUsingTheInitdCode(); //Undefined -- SomeStatic's init code may not have run yet.
    }
} runMe;

Undefined Behavior: A checklist for code reviews

4 Answers4