Null-pointer exception in btVector3.h - Windows

[GoogleCodeExporter]

What steps will reproduce the problem?
1.Enable the SIMD code path.
2.Run any code that calls btVector3::dot(const btVector3& v) const
3.Watch as it crashes

What is the expected output? What do you see instead?
My app will not even load the simulation world, I simply get the standard "app 
has encountered a problem..." dialog.

What version of the product are you using? On what operating system?
Most recent trunk r2631 on Windows 7 x64

Please provide any additional information below.
I believe it may have been the change to line 232 in btVector.h, previously the 
code would execute the standard sequential code at the bottom of the 
if-then-else block, but now it attempts to execute the line 233, which is 
exactly where the exception is generated. 

Original issue reported on code.google.com by tamaynar...@gmail.com on 17 Dec 2012 at 6:12

[GoogleCodeExporter]

What compiler version are you using?

Can you help reproducing this? The regular Bullet demos work fine under Windows 
using Visual Studio 2010, and SIMD is enabled by default.

Are you passing your own data into btVector3::dot? Make sure the data is 
16-byte aligned. We could add an unalignedDot that runs the old sequential 
data, if needed.

Original comment by erwin.coumans on 17 Dec 2012 at 8:08

[GoogleCodeExporter]

We're using VS2010 as well. When it crashes, the dot function is actually 
indirectly called through an attempt to setOrigin on a btTransform created on 
the stack, and the input vector is also on the stack, but it is cast from a 
custom vector type into a btVector3 in the argument list, as in:

void load(ObjectData* data)
{
   btTransform localTrans;
   localTrans.setRotation( btQuaternion(data->axis, data->angle) );
   localTrans.setOrigin( data->pos );
.
.
.
}
where data->pos is an instance of a custom vector class that has an overridden 
casting operator, like so

MyVector3::operator btVector3() const
{
   return btVector3( x, y, z );
}

So maybe it is an alignment issue, but I thought that the 
ATTRIBUTE_ALIGNED16(class) btVector3
declaration would force alignment even in a implicitly created object. I 
reverted that preprocessor statement on line 232 and it loads fine for me now, 
but I'll need to dig deeper into this tomorrow to truly say for sure what the 
problem was.

Original comment by tamaynar...@gmail.com on 17 Dec 2012 at 8:34

[GoogleCodeExporter]

Just to test, I replaced the casted vector with one created normally, as in
localTrans.setOrigin( btVector3(1, 1, 1) );
And it still crashes when using the unmodified btVector3.h

Original comment by tamaynar...@gmail.com on 17 Dec 2012 at 8:42

[GoogleCodeExporter]

Can you check if the vector is actually 16-byte aligned, at the time of the 
crash?
If not, try to backtrack where the unaligned vector is allocated.

Also, a full callstack would be useful. I don't see a dot product in your load 
function.

Original comment by erwin.coumans on 17 Dec 2012 at 8:47

[GoogleCodeExporter]

One option is to only disable the following define in btScalar.h

#define BT_USE_SIMD_VECTOR3

This way you don't need to disable ALL SIMD, but only the specific btVector3 
case.

Still, it is better if you fix the btVector3 16-byte alignment. Can you try the 
following?

 btTransform localTrans;
  localTrans.setRotation( btQuaternion(data->axis, data->angle) );
  ATTRIBUTE_ALIGNED16(btVector3 pos(data->pos.x,data->pos.y,data->pos.z));;
  localTrans.setOrigin( data->pos );

Original comment by erwin.coumans on 17 Dec 2012 at 8:52

[GoogleCodeExporter]

Visual Studio has problems aligning inline data members.

Try this instead:

btVector3 pos(1, 1, 1);
localTrans.setOrigin( pos);

or
ATTRIBUTE_ALIGNED16(btVector3 pos(1,1,1));
localTrans.setOrigin( pos);

The first should be fine, because ATTRIBUTE_ALIGNED16 is part of the class.

Original comment by erwin.coumans on 17 Dec 2012 at 8:56

[GoogleCodeExporter]

Good guess on the inline declaration, but your suggestion had no effect. It's 
not really a big deal as I can just change the defines as you said, but like 
yourself, I don't like sweeping things under the rug! I'll post the relevant 
part of my stack here, this is everything above the call to setOrigin, and I 
guess I'll attempt to recreate this in a demo to make sure it's not some other 
error on my part.

Original comment by tamaynar...@gmail.com on 18 Dec 2012 at 1:55

Attachments:

• stack.png

[GoogleCodeExporter]

Here's a better picture showing my whole IDE window and the output window. I'm 
still investigating this, I'll post any progress here.

Original comment by tamaynar...@gmail.com on 18 Dec 2012 at 7:31

Attachments:

• stack.png
• output.png

[GoogleCodeExporter]

I've been able to recreate this in BasicDemo.cpp. One mistake I made in my 
initial report was that the exception is actually generated in a call to 
btTransform::operator* not setOrigin (stupid Visual Studio using the same icon 
for 'this is the next line to execute' vs 'this line is currently executing'). 
Either way it must be related to the comment in btScalar.h:

//BT_USE_SSE_IN_API is disabled under Windows by default, because 
//it makes it harder to integrate Bullet into your application under Windows 
//(structured embedding Bullet structs/classes need to be 16-byte aligned)
//with relatively little performance gain
//If you are not embedded Bullet data in your classes, or make sure that you 
align those classes on 16-byte boundaries
//you can manually enable this line or set it in the build system for a bit of 
performance gain (a few percent, dependent on usage)

But that macro is still commented out, so I guess the recent change to 
btVector3.h must have had the same effect as enabling BT_USE_SSE_IN_API. Too 
bad, because any performance gain, no matter how small, would be useful in our 
application

Original comment by tamaynar...@gmail.com on 18 Dec 2012 at 8:01

Attachments:

• BasicDemo.cpp

[GoogleCodeExporter]

This issue was closed by revision r2634.

Original comment by erwin.coumans on 20 Dec 2012 at 10:10

• Changed state: Fixed

[GoogleCodeExporter]

It should be fixed in latest trunk. Indeed, under Windows SSE/requirement of 
16-byte alignment is disabled by default. That was broken in latest trunk.

You can enable BT_USE_SSE_IN_API in btScalar.h

It is very easy to fix your class:
Just change the class or struct definitions that require 16-byte alignedment

ATTRIBUTE_ALIGNED16(class) or
ATTRIBUTE_ALIGNED16(struct)

In case you are dynamically allocating the data, add the 
BT_DECLARE_ALIGNED_ALLOCATOR();
(just check btCollisionObject or so)

Original comment by erwin.coumans on 20 Dec 2012 at 10:15

[GoogleCodeExporter]

BT_USE_SSE_IN_API  is disabled by default to avoid support issues like this one 
:-)

Original comment by erwin.coumans on 20 Dec 2012 at 10:15

[GoogleCodeExporter]

Looks good, thanks again for all the help!

Original comment by tamaynar...@gmail.com on 21 Dec 2012 at 1:01