Thread: Signal handler for assert()

I was faced with a situation today where I had a class that was in a library that I could not change that was throwing an assertion under stupid circumstances. Regardless if it is easier to change the base object (it is) I was fooling with trying to trap the assertion with a signal handler using SIGABRT (which did nothing)...then I read someplace that some signals just cannot be trapped but I could not find good information if the signal sent by an assert was one of those or not...if not, am I trapping the wrong one? signal.h was little help.

Tks in advance. I would change the code if I could but politics being what they are...

I take it you mean that a function written in C++ or C is exiting with a failed assertion. In practice, the SIGABRT signal (raised by abort() when an assertion fails) can generally be caught, but it cannot be blocked, which means the signal handler is not allowed to return. Caveats apply: this depends on operating system, but is usually the case for most flavours of unix.

You'd be better off hunting for information about the circumstances that cause the library to abort, and avoiding those circumstances.

Originally Posted by grumpy

I take it you mean that a function written in C++ or C is exiting with a failed assertion. In practice, the SIGABRT signal (raised by abort() when an assertion fails) can generally be caught, but it cannot be blocked, which means the signal handler is not allowed to return. Caveats apply: this depends on operating system, but is usually the case for most flavours of unix.

You'd be better off hunting for information about the circumstances that cause the library to abort, and avoiding those circumstances.

Actually I know why the assertion is happening; I consider it bad design. Essentially it it something like this:

Code:

typedef enum {statusErr, statusOK, statusWarning (etc)} machineStates;

class Status
{
private:
     int *ptrToStatus;
public:
     Status() // def ctor
     {
           ptrToStatus = NULL;
     }
     machineStates getStatus()
     {
           assert(ptrToStatus);
           machineStates state = (*ptrToStatus);
           return state;
     }
     void setState(machineStates newState); // allocates
};

// so in header for a file you see:
Status Stat;

// and later in code you want to check the status
if( Stat.getStatus() == statusOK)
{
     // do something
}

A lot of code has been chopped for brevity but the point is if the Status has not been set to something specific, it will assert when you simply as for the status. Brain-damaged? Of course but this is what I am, faced with and I need to work around this so my thought was to trap the assertion in this case since I KNOW it is the wrong answer and deal with it that way. I have already written up a bug report on this and sent it off to the dept who wrote this monstrosity but was told by others it could take a year to get it fixed.

I mean if it was as simple as that I would not have wasted anyones time with this. At the end of the day however the question becomes: can an assertion be caught and dealt with cleanly via signals or other mechanism. This is the situation on the ground that I am faced with and complaining about the issue doesn't do anything useful.

So does anyone have any useful suggestions? All I want to know is if it is uninitialized or if it blows up for no good reason that I can handle it gracefully at runtime...

Am I missing something here?
Because the assert seems a perfectly reasonable attempt to prevent you from dereferencing a NULL pointer.

Or is the "bad design" the fact that it IS a NULL pointer to begin with, and that a half-decent design would have resulted in an object you could do get before set and actually get a useful "no state" answer out of it.

An assert() is a perfectly valid way of dealing with an uninitialized ptr BUT once you get about two steps beyond this line of code, something like this would actually be useful:

Code:

machineStates Status::getStatus()
{
     if(!ptrToStatus)
    {
          return statusUninitialized;
    }
    else
    {
          // deref the ptr and return the value

    }
}

That way something as simple as:

Code:

Status myStat;
machineStates state = myStat.getStatus();

Doesn't blow up as it does now. The way it is coded, there is no way to interrogate the status without risk (high risk at that) of a needless coredump. If the conditions under which ptrToStatus is NULL were truly exceptional I could almost buy this but the class is designed as I have stated and any attempt to simply look at the status of the variable you have a better than average change of a needless coredump....

Actually just solved it by deriving a class from it like so:

Code:

class FooStatus: public Status
{

public:
   bool mightExplode()
   {
      return (bool)(ptrToStatus == NULL);
   }

};

Then before calling the base-class to get the status I check:

Code:

   Status Stat;
   // if(Stat.getStatus() == statusOK) <<== original boom point
   // {
   //    cout << "Status is OK. " << endl;
   // }
   // else
   // {
   //    cout << "Status is not OK. " << endl;
   // }
   FooStatus *ptrFoo = static_cast<FooStatus*>(&Stat);
   if(!ptrFoo->mightExplode())
   {
      // we are cool, call as usual
      if(Stat.getStatus() == statusOK)
      {
	 cout << "Status is OK. " << endl;
      }
      else
      {
	 cout << "Status is not OK. " << endl;
      }
   }
   else
   {
      cout << "FooStatus thinks this will explode:" << endl;
      Stat.getStatus(); <<== in actual code this would not be there but for demo...
   }

Sometimes the simplest answer is the best...

Originally Posted by jeffcobb

Sometimes the simplest answer is the best...

In your case, since a Status object is not necessarily a FooStatus, your code gives undefined behaviour when calling ptrFoo->mightExplode(). It might work with your compiler today, but is not guaranteed to work in general (eg with other compilers, after an update of your current compiler, etc). Plus the fact it is relying on FooStatus to access a private member of Status, which is not allowed (unless you invoke undefined behaviour in another way).

I'd suggest the more reliable method is, when creating the Status object, immediately do some operation that sets ptrToStatus to be a non-NULL pointer. Since you apparently have the source code for the class, you will be able to find such an operation if it exists. If it doesn't exist, then politics be damned: lobby for fixing the class.

Grumpy:
Sorry but in a place where different depts are treated like fiefdoms, this layer of source is treated much like the vaunted "share or don't look but don't touch" license from Microsoft. You can see the source (and hence the bugs) and even take portions of it and rebuild it with other functionality but that functionality only can only exist on your dev-station and since the shipping product will not contain your changes, it is of little more than debugging value to change this in the first place.

Please explain what you think is undefined behavior? The one thing that this fix (which still is not an answer to the question I originally asked) relies on is the fact that the internal ptr in question was actually protected, not private. My mistake for posting from memory when I was tired. I didn't want (as did happen) the conversation to get bogged down in the why of the assertion and more of "is there a way to deal with this?". Alas, that was not to be; everyone said just to fix the base-class implementation (which I cannot do, no matter how brain-damaged the thing is) and one person argued *for* the assertion.

If any event, the derived class has access to protected members of the base class and unless the C++ spec gets changed, this will be fine and work. It didn't go the signal route as more reading was reflecting that for some signals (SIGABRT being one) you could trap it but not halt the assertion process. I still think there is a way to do this but ATM it eludes me. The derived class trick works but if I could get the signal approach to work I could begin to get some try/catch logic going in the code. Now if I could steer the conversation into that direction I think something good could come from this.

Jeff

Originally Posted by jeffcobb

Please explain what you think is undefined behavior?

Originally Posted by C++ Standard Section 5.2.9 para 8

An rvalue of type “pointer to cv1 B”, where B is a class type, can be converted to an rvalue of type “pointer to cv2 D”, where D is a class derived (clause 10) from B, if a valid standard conversion from “pointer to D” to “pointer to B” exists (4.10), cv2 is the same cv-qualification as, or greater cv-qualification than, cv1, and B is not a virtual base class of D. The null pointer value (4.10) is converted to the null pointer value of the destination type. If the rvalue of type “pointer to cv1 B” points to a B that is actually a sub-object of an object of type D, the resulting pointer points to the enclosing object of type D. Otherwise, the result of the cast is undefined.

Your example relies on converting a pointer to Status (B in the para above) into a pointer to FooStatus (D in the para above). Your Status object is not a sub-object of a FooStatus, so the result of the static_cast you are performing is undefined. The result of dereferencing such a pointer is also undefined.

Its actually no less correct or wrong than the %^&*( base class and if the base class had tried just a little harder to be a type instead of a hand-grenade, none of this would have been necessary. If there had been a way to check the state before explosion or, failing that satisfying the initial question (a way to capture the abort signal and deal with it gracefully) none of this would be been needed. However, since that is the reality on the ground and we needed to move on and work around the stupidity of the base class, this is what happened. Since no new members were introduced and this tested out fine on GCC (what I need for work), until the other group fixes the base-class this will do.

As I said in a previous post, an alternative is - whenever you create a Status object - perform some operation that ensures ptrToStatus is not NULL.

Just bear in mind that your "fix" may fail when the compiler gets upgraded. If it's the only option available to you (which I sort of doubt, but you seem to be disinterested in looking further) check the compiler version in your code, and generate a compilation error if the code is fed through a version you haven't checked. With GNU compilers, the relevant macros to check are __GNUG__, __GNUC__, __GNUC_MINOR__, and __GNUC_PATCHLEVEL__).

Or you could try pointing out to management that a poor implementation of the class requires you to use a fragile workaround (a workaround that relies on undefined behaviour that can change with compiler versions is definitely fragile) that compromises programmer productivity. Your manager - or whoever created the class - can't be held responsible if you don't make a case for fixing the problem properly. However, you can be.

And perchance you can bear in mind that since I cannot edit the base class my options were limited and I only considered the stated "fix" or patch when I could not get an answer to my original question which should also kinda point out that I was looking for options. The only option I get here is the one that is closed to me: edit the base class or strong-arm mgmt into changing stuff. I can and already have asked, put in defect tickets to get it changed but for now am faced with very few options. Originally all I wanted to was to stop the brain-damaged assertion from screwing things up unnecessarily.

You're missing my point. I'm not criticising you for having to work around a problem.

The authors of the Status class have unnecessarily caused you a problem to work around. You have decided to work around it using undefined behaviour. That undefined behaviour may cause your code to break under simple circumstances (eg if your compiler gets updated, or if the code ever gets ported to another compiler). When it breaks, the code will compile, but the error will occur at run-time i.e. in the hands of a customer. So, take some measure to ensure the code will not compile if it is fed to a compiler without confirmation your hack works. Give the programmers maintaining your code in future a chance.

Incidentally, thinking about it now, there is an alternative that does not rely on undefined behaviour. You could do this;

Code:

   FooStatus Stat;                //   note change of type of Stat.
   if(!Stat.mightExplode())
   {
      // we are cool, call as usual
      if(Stat.getStatus() == statusOK)
      {
	 cout << "Status is OK. " << endl;
      }
      else
      {
	 cout << "Status is not OK. " << endl;
      }
   }
   else
   {
      cout << "FooStatus thinks this will explode:" << endl;
      Stat.getStatus(); <<== in actual code this would not be there but for demo...
   }

This gives the same checks you are seeking, but without reliance on undefined behaviour.

The only things to remember are to employ a FooStatus wherever you would use a Status and also to ensure FooStatus provides constructors with the same signature as constructors of Status.

Actually I am being handed (programmatically, by the "OS" layer, not a real OS) an instance of Status so its not like I can make my own (and it has no copy ctor). I am stuck with this. If you thought that hack was nasty, the first thing I tried (because I could think of no other way to tell if the ptr was initialized) was to make an instance of Status (which I knew was not) and then memcmp(&myInstance, &orgInstance, sizeof(Status)). Like I said, nasty. This version is sweetness and light by comparison. I still would rather just catch the assertion and deal with it that way until the base class gets fixed...

Or... extend the enum with a new "no status" entry. In addition to everything the original enum provides.

That way, you can do away with the whole "mightExplode()" stuff, and extend State publicly. And basically mediate access to State... Don't ever degrade to hacking, it doesn't do anyone favours. You could even name the new class State (assuming the inherited State is under a different namespace), that way when the base class is "fixed", the changes would be tiny.