Joe Dog Software

The Underhanded C Contest challenges participants to write straightforward and clearly written code which doesn’t perform its intended purpose. Winning entries should easily pass inspection by other programmers so they can be added to the code base in order to execute their intended purpose which is to miscount votes, shave money from transactions or pass along information to another party, etc.

Some of the techniques used in this year’s contest include the use of K&R style function declarations to circumvent type checks, #include statements that change the package structure, swapping user space #define with system ones and a misleadingly long loop execution.

The winning entry leveraged the __isleap() function in time.h. Because that function is actually a macro it expands into an expression when a user defined macro is invoked multiple times. The winning author placed a subtle bug in that macro which plausibly turns the year into a 0 and writes past a buffer thereby performs the author’s intended purpose: to leak information to the outside world.

[Karen Pease: The Underhanded C Contest Winner]

Your JoeDog loves C but he hates memory management. That’s why he writes objects in C. They make managing memory easier. Still, he’s been know to cause a memory leak or two … okay, maybe a few more than that.

Valgrind is supposed to help identify those fsckers. You run a program inside valgrind and it checks your memory utilization. Awesome! Except when it’s not.

Your JoeDog has had zero point zero luck with valgrind. He’s been working on a snippet that will be incorporated into Your Fido. Your JoeDog loves snippets. By reducing complexity they allow him to focus on the integrity of the new code. In this particular snippet, he placed the new functionality inside a loop to check its long-running performance. The damn thing leaked!

“How did you know it was leaking?”

“I watched its memory usage in top and it continued to expand.”

“Well that sucks.”

“Indeed.”

So Your JoeDog ran the program in valgrind like this:

valgrind -v --leak-check=full --tool=memcheck ./haha

After it completed its iterations, valgrind had this to say:

==21112== HEAP SUMMARY:
==21112== in use at exit: 0 bytes in 0 blocks
==21112== total heap usage: 0 allocs, 0 frees, 0 bytes allocated
==21112==
==21112== All heap blocks were freed -- no leaks are possible

What the hell? This thing was leaking all over the place. Now Your JoeDog may have done something stupid. Instead of memory leaks, he might have created memory hogs. “What are memory hogs?” They are allocated resources that are no longer used but with references that are still maintained. Valgrind won’t identify a memory hog as a memory leak. But this was a snippet and an audit was easy. There were no hogs; something was leaking.

After an extensive audit, Your JoeDog identified the leak. It wasn’t in the snippet. He was using a dynamic array from array.c Here’s the offending code:

ARRAY
array_destroy(ARRAY this) 
{
  int i;

  for (i = 0; i < this->length; i++) {
    xfree(this->data[i]);  
  } 
  xfree(this->data);
  this = NULL;
  return this; 
}

Your JoeDog freed everything but the array object itself. Here’s the fix:

ARRAY
array_destroy(ARRAY this) 
{
  int i;

  for (i = 0; i < this->length; i++) {
    xfree(this->data[i]);  
  } 
  xfree(this->data);
  xfree(this);
  this = NULL;
  return this; 
}

It’s not clear why valgrind didn’t identify that. Any thoughts?

Your JoeDog uses gcc v4.8.2 on his snazzy System 76 laptop. By default, that version uses this flag: -Wunused-but-set-variable If you’d like to see a lot of compiler warnings, then I suggest you use it, too.

Inside fido’s src directory you can find GNUs regex. The file names were changed to ereg.c, ereg.h and  ereg2.h to avoid potential naming collisions. Well, sir, that .c file cries like a whiney two-year-old when it’s compiled with -Wunused-but-set-variable. Here’s what you’ll see in the console:

gcc -DHAVE_CONFIG_H -I. -I. -I../include -W -Wall -D_REENTRANT -D_POSIX_PTHREAD_SEMANTICS -g -O2 -c ereg.c
In file included from ereg.c:640:0:
ereg.c: In function ‘byte_re_match_2_internal’:
ereg.c:7145:27: warning: variable ‘sdummy’ set but not used [-Wunused-but-set-variable]
const CHAR_T *sdummy = NULL;
^
ereg.c:7144:22: warning: variable ‘pdummy’ set but not used [-Wunused-but-set-variable]
UCHAR_T *pdummy = NULL;

Let’s see the offending code, shall we?

UCHAR_T *pdummy = NULL;
const CHAR_T *sdummy = NULL;

POP_FAILURE_POINT (sdummy, pdummy, dummy_low_reg, dummy_high_reg, 
                   reg_dummy, reg_dummy, reg_info_dummy);

I don’t know about you, but Your JoeDog thinks those variables are pretty much used. POP_FAILURE_POINT is a macro but that shouldn’t matter. Since he doesn’t maintain this code, Your Nerdblogger decided to put a bandaid on the wound rather than stop its bleeding.

The Internets are loaded with bandaids! Here’s one from a discussion on this exact topic:

#define UNUSED(expr) do { (void)(expr); } while (0)

To silence that warning, pass the offending variables to the macro and “Presto!” no more bitching….

UNUSED(sdummy);
UNUSED(pdummy)