views:

119

answers:

6

Interesting issue we came across here this week.

We are working in C on a Harvard Architecture embedded platform, which has 16-bit data addresses and 32-bit code addresses.

The issue occurs when you are working with function pointers. If you have code like

if (fp) fp();

or

if (fp != 0) fp();

everything is fine.

However if you have code like

if (fp != NULL) fp();

then, because NULL is defined as (void *) 0, the compiler (gcc in this case) a) does not warn and b) does a 16-bit comparison against your function pointer instead of a 32-bit comparison. Fine as long as your function pointer doesn't happen to lie on a 64k boundary so all bottom 16 bits are 0.

At the moment we have large swathes of code which contain explicit checks against NULL. Most of them will be data pointers, but some of them will be function pointers. A quick grep for != NULL or == NULL revealed over 3000 results, to many to go through manually to check.

So, what we would like now would be either

  1. a way to find all the cases where function pointers (but not data pointers) are compared (so we can instead have them compare against FP_NULL which we would define as a 32-bit 0), or

  2. to redefine NULL in such a way that it would do the right thing.

  3. (Or, I suppose, to update our gcc port to detect and correctly handle this case).

I can't think of any approach that works for 1. The only approach I can think of for 2 is to redefine NULL as a 0 function pointer, which would be very wasteful for the vast majority of comparisons which are against data pointers. (A 32-bit compare is 4 instructions, a 16-bit compare is 1 instruction).

Any thoughts or suggestions?

+13  A: 

It seems to me the easiest way is replace all occurrences of NULL by 0. This works for function pointer (as you say) and object pointers.

This is a variant of (2) Redefine NULL to plain 0.

But the fact that you cannot compare function pointers with NULL is a bug in your implementation. C99 states that comparison of the null pointer constant is possible with both object and function pointers, and that NULL should expand to this constant.

Small addition from the C-FAQ question 5.8:

Q: Is NULL valid for pointers to functions?
A: Yes (but see question 4.13)

Mixing function pointers with (void *) 0

(A reply to R..'s comment). I believe using function pointers and (void *) 0 together is well-defined. In my reasoning I will refer to sections of the C99 draft 1256, but will not quote large parts to keep it readable. It should also be applicable to C89.

  • 6.3.2.3 (3) defines the integer constant expression 0 and such an expressions cast to (void *) as null pointer constant. And: "If a null pointer constant is converted to a pointer type, the resulting pointer, called a null pointer, is guaranteed to compare unequal to a pointer to any object or function."
  • 6.8.9 defines the == and != operands for (among other) a pointer operand and a null pointer constant. For these: "If one operand is a pointer and the other is a null pointer constant, the null pointer constant is converted to the type of the pointer."

Conclusion: In fp == (void *) 0, the null pointer constant is converted to the type of fp. This null pointer can be compared to fp and is guaranteed to be unequal to fp if it points to a function. Assignment (=) has a similar clause, so fp = (void *) 0; is also well-defined C.

schot
@schot, my immediate thought was this is GCCs problem, so I upvoted you. Redefine (or even ugly-hack GCC to do the same) NULL to 0 is also OK, I think.
Amigable Clark Kant
Note that C allows `NULL` to be defined as just plain `0` rather than `(void *)0`. A nasty implementation like this should probably make use of that option and `#define NULL 0`, and I would argue that you're justified in **fixing** its broken standard header files in this case so that you can use `NULL` as specified.
R..
@R..: I fully agree. That was the (unstated) rationale for my answer.
schot
Now here's a question. Using `NULL` with function pointers is well-defined by the standard, but is using `(void *)0` with function pointers well-defined? I doubt it.
R..
@R..: See my update.
schot
+2  A: 

You could try this:

#ifdef NULL
  #undef NULL
#endif
#define NULL 0
jv42
+2  A: 

The way you describe should work:

6.3.2.3/3 An integer constant expression with the value 0, or such an expression cast to type void *, is called a null pointer constant. If a null pointer constant is converted to a pointer type, the resulting pointer, called a null pointer, is guaranteed to compare unequal to a pointer to any object or function.

So, either NULL has been redefined to something not 0 or (void*)0 (or equivalent?) or your compiler is non-conformant.

Try redefining NULL yourself (to plain 0) after all the #includes in all files :-)

just for kicks: try gcc -E (to output the preprocessed source) on a problem file and check the expansion of NULL

pmg
NULL is definitely (void *) 0 at present. So yes, a compiler bug seems most likely.
Vicky
Do you get the bad behaviour with and without optimizations? ( http://gcc.gnu.org/bugzilla/show_bug.cgi?id=22051 )
pmg
+1  A: 

Here are a few suggestions:

  1. temporarily change NULL to (char*)0 or something else that is not implicitly convertible to a function pointer. This should give a warning about every comparison with a non-matching pointer. You could then run the produced compiler output through a tool like grep and look for a typical pattern for function pointers, like (*)(.

  2. redefine NULL as 0 (without the cast to void*). This is another valid definition for NULL and might do the right thing for you, but no guarantees.

Bart van Ingen Schenau
+2  A: 

You could try a segment hack (really only a hack), so you can use the fast 16bit comparision, without any risk. Create segements at each n*0x10000 boundary with size of 4 (or even smaller), so there never can't exists a real function.

It depends on your embedded device memory space, if this is a good or a really bad solution. It could work if you have 1MB normal Flash, which will never change. It will be painfull if you have 64MB Nand Flash.

jeb
Interesting approach! +1 for originality if not pleasantness :-)
Vicky
A: 

Edit the implemention's system headers to replace all occurrances of

#define NULL ((void *)0)

with

#define NULL 0

Then file a bug report with the vendor. You should not have to modify your (perfectly correct, albeit ugly style) code because of a bug in the vendor's compiler.

R..