Jump to content

  • Log In with Google      Sign In   
  • Create Account

Bugs you expected to throw a compiler error ...


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
24 replies to this topic

#1 Endar   Members   -  Reputation: 668

Like
0Likes
Like

Posted 22 May 2011 - 01:15 AM

I just made a mistake and saw something in C, using Visual Studio 2008 C++ Express, which was so dumb (at least to my mind), that I thought it would throw a compiler error, or at least a warning, but doesn't (at least at the standard error/warning level).

 // set the free block mem to a released value
 memset(    ((u8*)pFree) + sizeof(MemHeapFree), 
            MEMHEAP_RELEASED_MEM, 
            pFree->uSize - sizeof(sizeof(MemHeapFree)) );

See it? See the sizeof(sizeof(expression))? I just had a massive WTF moment when I saw that while stepping through the code.

So, this thread is for any other things in C, C++ or any other languages that you think other people would find strange, amusing or unbelievable.

Oh, and for anyone that's interested, the sizeof(sizeof(x)) comes out to 4, which I guess is expected once you know the compiler actually accepts it.
Mort, Duke of Sto Helit: NON TIMETIS MESSOR -- Don't Fear The Reaper

Sponsor:

#2 SymLinked   Members   -  Reputation: 887

Like
0Likes
Like

Posted 22 May 2011 - 02:56 AM

Okay, I understand your point and I see the mistake, but IMO letting the compiler flag this as an error would be stupid.
What if I want to know the size of whatever the type is that sizeof () returns?

That's not a common thing to do, but letting the compiler parse for mistakes like that would be horribly slow - and even then, the compiler shouldn't error on something that's perfectly valid. I could accept a warning though, but it still comes down to the same problem - parsing all sorts of potential mistakes like this would only make the compiler slower.

#3 Rattenhirn   Crossbones+   -  Reputation: 1808

Like
0Likes
Like

Posted 22 May 2011 - 03:51 AM

The compiler can and should only check for lexical, syntactic or semantic errors that cannot be translated. Your example is perfectly valid source code.

I guess what you're looking for is a static code analyzer.
Here's a list of tools that do that, but I'm not sure any of these will the detect the mistake in your example...
http://en.wikipedia.org/wiki/List_of_tools_for_static_code_analysis

#4 Hodgman   Moderators   -  Reputation: 31799

Like
0Likes
Like

Posted 22 May 2011 - 04:16 AM

I think you two missed the point of the thread...

#5 braindigitalis   Members   -  Reputation: 591

Like
0Likes
Like

Posted 22 May 2011 - 06:08 AM

Sizeof(sizeof(x)) only evaluates to 4 on a 32-bit system. This looks like it is part of a memory management system and a pointer to the data is stored in a data structure. Doing it this way would ensure there is always enough memory allocated to store the pointer without hard-coding it. Does it make a bit more sense now?

Games Currently In Development:

Currently rewriting Firework Factory - Casual Puzzler for PC in Direct3D 11. Latest Journal Entry.

 

 


#6 Antheus   Members   -  Reputation: 2397

Like
0Likes
Like

Posted 22 May 2011 - 07:24 AM

There is nothing wrong in that code.

size_t is defined as result of sizeof operation. While somewhat unusual, it's perfectly legit to query the sizeof(sizeof(x)) to determine its size. While not strictly defined, size_t is commonly size of register on that machine or size of addressable space.

Above code says:

Set pFree-uSize bytes at a specific offset to specific value, but leave last sizeof(size_t) bytes intact. perhaps the last 4 are used for some specific purpose.

In C++, the above code might look like std::fill(pFree.begin() + sizeof(MemFree), pFree.end() - sizeof(FreeContainerType::size_t), X);


Bugs like this are annoying, but causes are quite complex. Pointer arithmetic is idiomatic to C and trying to report every such slightly uncommon use of language features would result in a flood of false positives.


I'm not really sure if a C idiom exists to avoid mistakes like that. Especially since it's really not a mistake.

#7 Hodgman   Moderators   -  Reputation: 31799

Like
0Likes
Like

Posted 22 May 2011 - 08:21 AM

There is nothing wrong in that code.

The author knows it's valid. It's not wrong, except for the part where it didn't do what the author intended, meaning it caused a bug (and in a memory allocator a bug is quite a wrong thing).

So, this thread is for any other things in C, C++ or any other languages that you think other people would find strange, amusing or unbelievable.

Did you all miss this part? ^^^
Too busy bikeshedding in Parkinson's committee? I heard you like bike sheds (any colour will do)...

#8 Endar   Members   -  Reputation: 668

Like
1Likes
Like

Posted 22 May 2011 - 09:44 AM

There is nothing wrong in that code.

The author knows it's valid. It's not wrong, except for the part where it didn't do what the author intended, meaning it caused a bug (and in a memory allocator a bug is quite a wrong thing).

So, this thread is for any other things in C, C++ or any other languages that you think other people would find strange, amusing or unbelievable.

Did you all miss this part? ^^^
Too busy bikeshedding in Parkinson's committee? I heard you like bike sheds (any colour will do)...


Yes yes yes! :)

Guys, you are great :D I do appreciate the objective analysis of the code, but yes, I know it is part of a memory management system. In fact is it my own memory management system that I was fixing, in part because of a bug (not the one I posted) that was in that very function call. And I'm not looking for a static code analyzer, although I do appreciate the offer. I also completely understand why that expression returns 4 on a 32-bit system. I'm also not suggesting that this and the other million similar kinds of bugs that people write into their code every day should be picked up by the compiler, and although I immediately wondered how it had gotten past the compiler, after a half second of looking at it, I understood why it had.

As Hodgman pointed out, after the initial "What the heck?" moment, I just found this funny and was looking to add a bit more humor to the forum. So, please, whatever you've got, I'd definitely like to see, in whatever language or form that you feel like sharing it in. :D
Mort, Duke of Sto Helit: NON TIMETIS MESSOR -- Don't Fear The Reaper

#9 braindigitalis   Members   -  Reputation: 591

Like
0Likes
Like

Posted 22 May 2011 - 09:53 AM

In that case, I consider the functions sprintf, strcpy, strcmp etc without a length parameter to be some of the silliest features of C :-) the fact that nobody considered it to ever be a security problem, and that so many programs use these functions in network code and other security sensitive code without a second thought... Now that is daft indeed :-) (yeah, modern compilers sometimes warn about unsafe functions such as these but jees, it took them how many decades to realise?)

Edited by braindigitalis, 22 May 2011 - 09:55 AM.

Games Currently In Development:

Currently rewriting Firework Factory - Casual Puzzler for PC in Direct3D 11. Latest Journal Entry.

 

 


#10 Endar   Members   -  Reputation: 668

Like
0Likes
Like

Posted 23 May 2011 - 04:49 AM

In that case, I consider the functions sprintf, strcpy, strcmp etc without a length parameter to be some of the silliest features of C :-) the fact that nobody considered it to ever be a security problem, and that so many programs use these functions in network code and other security sensitive code without a second thought... Now that is daft indeed :-) (yeah, modern compilers sometimes warn about unsafe functions such as these but jees, it took them how many decades to realise?)



I agree, that was kinda strange.

Come on guys and gals, surely more people have something they wish to share with the rest of the class?
Mort, Duke of Sto Helit: NON TIMETIS MESSOR -- Don't Fear The Reaper

#11 forsandifs   Members   -  Reputation: 154

Like
1Likes
Like

Posted 23 May 2011 - 05:33 AM

I'm sorry, when I tried to compile this thread I got an "error C2065: 'humor' : undeclared identifier". :/

#12 Álvaro   Crossbones+   -  Reputation: 13905

Like
0Likes
Like

Posted 23 May 2011 - 10:19 AM

#include <iostream>

struct Price {
  double x;

  explicit Price(double x) : x(x) {
  }

  operator double() {
	return x;
  }
};

Price get_price() {
  return Price(5.2);
}

int main() {
  bool some_condition = false;
  double price = some_condition ? 0 : get_price();
  std::cout << price << '\n';
}

With g++-2.95.2 you used to get "5.2". Then after g++-3 you would get "5". It turns out "5" is the correct answer according to the standard, but it would have been nice to get a warning about loss of precision. This actually created a problem at work that took a lot of effort to fix.

#13 mozie   GDNet+   -  Reputation: 194

Like
0Likes
Like

Posted 24 May 2011 - 04:21 PM

It turns out "5" is the correct answer according



Huh, I guess I don't see why. Cool example though!

#14 Antheus   Members   -  Reputation: 2397

Like
1Likes
Like

Posted 24 May 2011 - 04:33 PM

It turns out "5" is the correct answer according



Huh, I guess I don't see why. Cool example though!


Covariance and auto type conversion.

0 is integer.
function returns a double.

Ternary operator can only work on same type: int = cond ? int : int, for example, or more generally, T = cond ? T : T. Ternary operator should throw an error for: X = cond ? T : U. Imagine T = string and U = vector<int>. There is no possible conversion and no type that could hold both.

I know that in Java this is an error. It should be in C/C++ as well, but it may go unreported since with primitive types casting and default construction/copy construction converts the types on the fly. Since Price has a () operator defined, the returned Price is evaluated, it returns double, which is cast to int. Or the int is converted to double.


This example points more to dangers of operator().

#15 Álvaro   Crossbones+   -  Reputation: 13905

Like
1Likes
Like

Posted 24 May 2011 - 05:57 PM

The bizarre thing about this example is that get_price() used to return a double, and in that case the answer is 5.2, but then someone changed it to return a type that has an implicit conversion to double, and the behavior changed completely.

The standard says that if the two types to the sides of the `:' are different, the compiler will try to find both conversions of one type into the other. If only one succeeds, that's what will happen.

In the case of, (bool ? int : double), it is somehow determined that the promotion from int to double is preferable. When you change it to (bool ? int : Price), an int cannot be converted to a Price, but a Price can be converted to an int through double.

One last thing that makes this example horrible is that g++ will warn you that you may lose precision if you convert a double to an int, but in this complicated case it doesn't give you any warnings.

#16 mozie   GDNet+   -  Reputation: 194

Like
0Likes
Like

Posted 24 May 2011 - 06:18 PM


It turns out "5" is the correct answer according



Huh, I guess I don't see why. Cool example though!


Covariance and auto type conversion.

0 is integer.
function returns a double.

Ternary operator can only work on same type: int = cond ? int : int, for example, or more generally, T = cond ? T : T. Ternary operator should throw an error for: X = cond ? T : U. Imagine T = string and U = vector<int>. There is no possible conversion and no type that could hold both.

I know that in Java this is an error. It should be in C/C++ as well, but it may go unreported since with primitive types casting and default construction/copy construction converts the types on the fly. Since Price has a () operator defined, the returned Price is evaluated, it returns double, which is cast to int. Or the int is converted to double.


This example points more to dangers of operator().


Thanks! So, would changing the 0 to a 0.0 resolve it? Because then it would result in double = (cond) ? double : double.

#17 Álvaro   Crossbones+   -  Reputation: 13905

Like
0Likes
Like

Posted 24 May 2011 - 06:34 PM

Thanks! So, would changing the 0 to a 0.0 resolve it? Because then it would result in double = (cond) ? double : double.


Yes, changing the 0 to 0.0 solves the problem. I now always make my constants the type that I intend them to be (0 for int, 0u for unsigned, 0.0 for double, 0.0f for float, T(0) in a template).

#18 Hodgman   Moderators   -  Reputation: 31799

Like
0Likes
Like

Posted 24 May 2011 - 07:09 PM

Yes, changing the 0 to 0.0 solves the problem. I now always make my constants the type that I intend them to be (0 for int, 0u for unsigned, 0.0 for double, 0.0f for float, T(0) in a template).

I had a similar bug recently with bit-shifting:
u64 mask;
mask = 1 << 30;
assert( mask == 0x40000000 );
mask = 1 << 31;
assert( mask == 0x80000000 );
mask = 1 << 32;
assert( mask == 0x100000000 );//fail
This was failing because the literal '1' is a 32-bit int, so you can't get a 64-bit result from the shift operation. The solution is to use the correct type of literal '1' as above.
The bug snuck in because mask was originally a u32 and was later upgraded to a u64 without scrutinising all the code that used the variable...

#19 rip-off   Moderators   -  Reputation: 8721

Like
0Likes
Like

Posted 25 May 2011 - 02:57 AM

#include <iostream>

class Example
{
public:
    Example(int va1ue)
    :
        value(value)
    {
    }
    
    int value;
};

int main()
{
    Example e(42);
    std::cout << "What? " << e.value << '\n';
}
I had some code similar to above and it drove me demented looking for the problem. Like my example, the type involved was so trivial that I didn't really read it I kept assuming the problem was elsewhere.

#20 Álvaro   Crossbones+   -  Reputation: 13905

Like
0Likes
Like

Posted 25 May 2011 - 09:58 AM

It took me a while to see the problem with rip-off's code. I don't know why g++ doesn't complain about use of uninitialized variables unless you compile with -O.




Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS