Caution! No sign ahead!
-
i found this one a couple of days ago. it's the inner loop in a Gaussian blur. it's running a kernel over a rectangular section of pixels...
double v = 0, s = 0;
for (int cc=start;cc < stop;cc++)
{
double k = kernel[center+cc];
v += pArray[cc * uCPP] * k;
s += k;
}uCPP is unsigned. start can be negative. and when start is negative,
cc * uCPPis probably going to give an integer overflow, since cc will be implicitly cast into a REALLY BIG unsigned integer... and then you'll get an access violation, since pArray really isn't that big. oddly, it took an x64 build for this to give an A.V. - it worked without crashing (somehow) on all the Win32 builds we had. the fix, of course, is:cc * (int)uCPPalas. -
i found this one a couple of days ago. it's the inner loop in a Gaussian blur. it's running a kernel over a rectangular section of pixels...
double v = 0, s = 0;
for (int cc=start;cc < stop;cc++)
{
double k = kernel[center+cc];
v += pArray[cc * uCPP] * k;
s += k;
}uCPP is unsigned. start can be negative. and when start is negative,
cc * uCPPis probably going to give an integer overflow, since cc will be implicitly cast into a REALLY BIG unsigned integer... and then you'll get an access violation, since pArray really isn't that big. oddly, it took an x64 build for this to give an A.V. - it worked without crashing (somehow) on all the Win32 builds we had. the fix, of course, is:cc * (int)uCPPalas.Chris Losinger wrote:
since cc will be implicitly cast into a REALLY BIG unsigned integer
hmmm, I thought it would be the other way around: uCPP would be cast to a signed integer. But it would not be the first time I was wrong. I do have a question about the performace of the code you supplied. Are you not taking a major performance hit by declaring
double kinside the loop? Would it not be better to have declared k outside the loop, the same place as you declared v and s?
You may be right
I may be crazy
-- Billy Joel --Within you lies the power for good, use it!!!
-
Chris Losinger wrote:
since cc will be implicitly cast into a REALLY BIG unsigned integer
hmmm, I thought it would be the other way around: uCPP would be cast to a signed integer. But it would not be the first time I was wrong. I do have a question about the performace of the code you supplied. Are you not taking a major performance hit by declaring
double kinside the loop? Would it not be better to have declared k outside the loop, the same place as you declared v and s?
You may be right
I may be crazy
-- Billy Joel --Within you lies the power for good, use it!!!
PJ Arends wrote:
hmmm, I thought it would be the other way around
that's what i thought, too. now i'm trying to find all the other places i might have made a similar mistake.
PJ Arends wrote:
Are you not taking a major performance hit by declaring double k inside the loop?
the compiler is smart enough to optimize that away. in the final _asm, it just grabs the data directly from the kernel array and stuffs it into the FP registers as part of the calculation - there's no storage or initialization for anything like 'k' at all.
-
i found this one a couple of days ago. it's the inner loop in a Gaussian blur. it's running a kernel over a rectangular section of pixels...
double v = 0, s = 0;
for (int cc=start;cc < stop;cc++)
{
double k = kernel[center+cc];
v += pArray[cc * uCPP] * k;
s += k;
}uCPP is unsigned. start can be negative. and when start is negative,
cc * uCPPis probably going to give an integer overflow, since cc will be implicitly cast into a REALLY BIG unsigned integer... and then you'll get an access violation, since pArray really isn't that big. oddly, it took an x64 build for this to give an A.V. - it worked without crashing (somehow) on all the Win32 builds we had. the fix, of course, is:cc * (int)uCPPalas.This is getting into the deepest, darkest depths of the C spec, but doesn't integral promotion go the other way? unsigned is converted to signed when the other operand is signed (and there is no built-in type that can express both operands without data loss)? Even if
cc*uCPPended up being a large unsigned value, an array index can be negative, so the large unsigned value would be interpreted as a small negative value. I bet you saw it crash on x64 due to the quantities being extended to 64 bit longs (the next largest type that can express both operands) and not overflowing, sopArray[cc*uCPP]really was indexing far off the end of the array.--Mike-- Visual C++ MVP :cool: LINKS~! Ericahist | PimpFish | CP SearchBar v3.0 | C++ Forum FAQ
-
This is getting into the deepest, darkest depths of the C spec, but doesn't integral promotion go the other way? unsigned is converted to signed when the other operand is signed (and there is no built-in type that can express both operands without data loss)? Even if
cc*uCPPended up being a large unsigned value, an array index can be negative, so the large unsigned value would be interpreted as a small negative value. I bet you saw it crash on x64 due to the quantities being extended to 64 bit longs (the next largest type that can express both operands) and not overflowing, sopArray[cc*uCPP]really was indexing far off the end of the array.--Mike-- Visual C++ MVP :cool: LINKS~! Ericahist | PimpFish | CP SearchBar v3.0 | C++ Forum FAQ
Michael Dunn wrote:
I bet you saw it crash on x64 due to the quantities being extended to 64 bit longs (the next largest type that can express both operands) and not overflowing
yeah, that makes sense. i can't imagine where else this kind of thing is going to pop up. ugh.
-
i found this one a couple of days ago. it's the inner loop in a Gaussian blur. it's running a kernel over a rectangular section of pixels...
double v = 0, s = 0;
for (int cc=start;cc < stop;cc++)
{
double k = kernel[center+cc];
v += pArray[cc * uCPP] * k;
s += k;
}uCPP is unsigned. start can be negative. and when start is negative,
cc * uCPPis probably going to give an integer overflow, since cc will be implicitly cast into a REALLY BIG unsigned integer... and then you'll get an access violation, since pArray really isn't that big. oddly, it took an x64 build for this to give an A.V. - it worked without crashing (somehow) on all the Win32 builds we had. the fix, of course, is:cc * (int)uCPPalas.For arithmetic expressions of the form: <signed> <op> <unsigned> or <unsigned> <op> <signed> C/C++ will promote the <signed> value to <unsigned>. The resulting expression 'signedness' will be <unsigned>. Details available here[^]. Later, Dan
Be clear about the difference between your role as a programmer and as a tester. The tester in you must be suspicious, uncompromising, hostile, and compulsively obsessed with destroying, utterly destroying, the programmer's software. ----- Boris Beizer
-
For arithmetic expressions of the form: <signed> <op> <unsigned> or <unsigned> <op> <signed> C/C++ will promote the <signed> value to <unsigned>. The resulting expression 'signedness' will be <unsigned>. Details available here[^]. Later, Dan
Be clear about the difference between your role as a programmer and as a tester. The tester in you must be suspicious, uncompromising, hostile, and compulsively obsessed with destroying, utterly destroying, the programmer's software. ----- Boris Beizer
i wish the compiler would warn about this one. PCLint doesn't seem to care about it, either (at least not on their interactive demo page).
-
i wish the compiler would warn about this one. PCLint doesn't seem to care about it, either (at least not on their interactive demo page).
Some of this stuff is caught when you set the detect 64 bit portability issues.
Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -Brian Kernighan
-
Some of this stuff is caught when you set the detect 64 bit portability issues.
Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -Brian Kernighan
yeah, i turned that on, and it found quite a lot of things. not this one, though :(
-
Chris Losinger wrote:
since cc will be implicitly cast into a REALLY BIG unsigned integer
hmmm, I thought it would be the other way around: uCPP would be cast to a signed integer. But it would not be the first time I was wrong. I do have a question about the performace of the code you supplied. Are you not taking a major performance hit by declaring
double kinside the loop? Would it not be better to have declared k outside the loop, the same place as you declared v and s?
You may be right
I may be crazy
-- Billy Joel --Within you lies the power for good, use it!!!
PJ Arends wrote:
I do have a question about the performace of the code you supplied. Are you not taking a major performance hit by declaring double k inside the loop? Would it not be better to have declared k outside the loop, the same place as you declared v and s?
Where you declare a variable only decides it's scope, it doesn't change when it's allocated. All variables that are declared anywhere in a method are allocated when the method is called. The stack pointer is moved to create the neccesary space on the stack for all the variables.
--- b { font-weight: normal; }
