Any way to get compiler or code analysis to detect this defect?

Question

I'm porting a large code-base to x64.  Unfortunately this code-base has code like this in places:

// say a function always wrote a pointer-sized value
// as an out-parameter
void get_value(void **pval)
{
    *pval = (void*) 5;  // arbitrary dumb example
}

// note that long is 32 bits on both x86 and x64
long l; 
get_value((void **) &l); 

// the above is "safe" on x86
// but on x64 will corrupt memory

Please don't tell me that casting like this is a bad idea.  The above is a very simplistic example to illustrate the bad thing I'd like the compiler to tell me about.

Casting a pointer to a long (32 bits of data) to a pointer to a pointer (now pointing to 64 bits of data) seems like something the compiler or static analyzer should be able to warn me about.  Is there any way to ask the compiler to help me find these errors?  I've tried the highest warning levels as well as the static analyzer in Visual Studio 2011 Beta on the x64 target but was not able to get the compiler to complain. 

Thanks for any suggestions!

Rob

Answer 1

The compiler won't really be able to catch it, the casts tell the compiler to ignore the type and pretend that it is the type in the cast. I'm not sure about code analysis tools though.

Your best bet is, if you have to clean up code like this, to find the people who wrote it and hit them with a hammer or something to make sure they will never do it again.

---

This is a signature

Any samples given are not meant to have error checking or show best practices. They are meant to just illustrate a point. I may also give inefficient code or introduce some problems to discourage copy/paste coding. This is because the major point of my posts is to aid in the learning process.

Do you want Visual Studio 11 Express to be freely installable on Windows 7 and able to write regular C++ applications? Please vote for this.

Answer 2

VS11 Code analysis tool doesn't catch it either. A cast is a cast...

If I had a hammer
I'd hammer in the morning
I'd hammer in the evening
All over this land
I'd hammer out danger
I'd hammer out a warning

Answer 3

Some static analyzers may give a warning, mainly because
the cast is from/to different types. That is, the size
differences may not be what triggers the warning(s) in
some cases. e.g. -

http://www.gimpel-online.com/OnlineTesting.html

FlexeLint for C/C++ (Unix/386) Vers. 9.00i, Copyright Gimpel Software 1985-2012
--- Module: diy.cpp (C++)
     1  void get_value(void **pval)
     2  {
     3      *pval = (void*) 5;  // arbitrary dumb example
     4  }
     5  
     6  int main()
     7  {
     8  long l; 
                              _
     9  get_value((void **) &l); 
diy.cpp  9  Info 740:  Unusual pointer cast (incompatible indirect types)
diy.cpp  9  Info 740:  Unusual pointer cast (incompatible indirect types)
    10  
    11  return 0;
    12  }
    13  
    14 
    

For an explanation of "Info 740" see:

http://gimpel-online.com/MsgRef.html#740

- Wayne

 

Answer 4

I don't know what the situation is, but your example is one massive big failure. I mean, all is just... Horrid. Use of blind C cast - stupid, double pointer not needed - stupid, even use of pointers is not needed (a reference would work)...

I think that a better bet is to actually fix the code. Find out the meaning of the assignment in get_value. With that knowledge, change get_value code to reflect it.e.g. as it is, get_value should be a mere

void get_value(long& val)
{
  val = 5;
}

// and you call it so:

long l;
get_value(l);

See how all pointers and all casting is gone? No analysis tool will tell you that they should be gone.

Of course, reality will prove to be much harder to work with ;-), but still...

I would start with removing pointers as much as possible first. Function receives a pointer, and dereferences it without a check for null? Change it into a reference, it's not a pointer anyhow!

Next, I would replace blind C castswith correct C++ casts (not reinterpret_cast!). They should never, not once, be seen (well, unless you compile with a C compiler, which you really shouldn't these days).

Finally... When someone says he needs a void*, I usually tell him that he's a liar, an that he, at worst, needs e.g.

union whatever
{
TYPE1* p1;
TYPE2* p2;
...
TYPEn* pn;
}

;-)

Answer 5

This company specializes on 64-bit compatibility analysis tools:

http://www.viva64.com/en/

-- pa

Answer 6

Hi Rob,

Would you mind letting me know the result of the suggestions? If you need further assistance, feel free to let me know. I will be more than happy to be of assistance.

Best regards,
Jesse

---

Jesse Jiang [MSFT]
MSDN Community Support | Feedback to us

Answer 7

Thanks everyone for your responses. 

Both the pc-lint software from Gimpel and the viva64 static analyzer pick up this defect.  The Gimpel analyzer is an order of magnitude less expensive but depending on the complexity of your build setup could be an order of magnitude harder to setup.  

The viva64 analyzer has a generous trial which already pointed out about 10 such defects in my software. 

Thanks again,

Rob