[Bug report] 64bit issues in AdvApp2Var

AdvApp2Var contains numerous mixes of casts from pointers to long int's. This seems to derive from f2c (Fortran-to-C) conversion, which happened years ago. This had been working for 32bit, where both pointer and long are both 32 bits. With migration to 64 bit these casts no longer work leading to loss of data and eventually wrong memory accesses and crashes.

AdvApp2Var_SysBase.cxx excerpt:
...
mcrlocv_((long int)&t[1], (long int *)&loc);
...

int mcrlocv_(long int t,
long int *l)

{
*l = t;
return 0 ;
}

DRAW reproducer:
> pload MODELING
> explode c
> gplate res 5 0 c_1 0 c_2 0 c_3 0 c_4 0 c_5 0

This fails in 6.5.1 ros 64 bit and works fine on 32 bit.

I'm now reworking AdvApp2Var to fix these issues...

Hope this helps.
Roman

P.S. I wonder how these issues have not been caught by the OCC team earlier when testing Modeling Algorithms on 64 bit platforms. I came across them the first day when built on 64bit and launched my test suite :-(.

Roman Lygin's picture

Now with an attachment.

Attachments: 
Binary Data c
Thomas Paviot's picture

Hi Roman,

What do you mean with "This fails in 6.5.1 ros 64 and works fine on 32 bit"? What kind of failure is it? segfault? unexpected result? Did you reproduce this issue on Linux and Windows?

Regards,

Thomas

Roman Lygin's picture

Hi Thomas,

Windows 7 64bit.
There is an access violation (as some array indices get incorrectly computed due to wrongly processed pointers), it's converted to OSD_Exception_ACCESS_VIOLATION and is output in DRAW.

Sorry for not being that specific upfront. Thanks for asking.
Roman

Markus Rhein's picture

There are (were?) similar bad type-casts in NCollection_IncAllocator.
(Bugreport here: http://www.opencascade.org/org/forum/thread_19966/)
And probably more in other places.

Linux and Mac 64 bit should work fine, however.

The access violations on Windows 64 bit only showed up in my case (meshing) when the memory consumption of the application was high (>4GB). Since the truncation of the pointer didn´t matter if the allocation address was smaller that 4 byte. I would recommend using the "Virtual Memory" API of Windows to reserve virtual address space in a testing environment. Thus every call to malloc results in a potentially large memory address.

Best regards
Markus

Forum supervisor's picture

Dear Roman,
I would like to inform you that the posted problem is checked and reproduced.
The corresponding issue with ID = 22786 has been registered.
Later you can know if the issue is resolved by checking references to the specified ID in OCCT Release Notes. The analysis of the issue will take some time depending on our technical capability and availability of resources.
Thanks for your contribution.
Regards.

Roman Lygin's picture

OK. Here is the fix for this issue.

As expected, the root-cause is in use of long to store a pointer and then dereferencing it. On Linux/MacOS/gcc this works as long is 8 bytes, but does not on Windows/VisualC++ where it's 4 bytes.

The patch for 6.5.1 is attached. A few notes:
1. the patch (internal number fix490) was done over a previous patch (fix385), so this is a cumulative patch fix385+fix490. fix385 was sent to OCC a few months ago but is not integrated yet
2. for convenience, to easily see the diff between fix385 and fix490, they are put into subdirs and diff is provided. So feel free to apply just this delta to a baseline 6.5.1.
3. fix490 also includes a few other improvements in AdvApp2Var to increase robustness (avoiding unnecessary casts, use of Standard::Allocate(), ::Free(),...)
4. see also occ-fix-readme-excerpts.txt for details

The fix was tested on Visual C++ 2010 SP1 and gcc 4.1.2 64 bit.

Code review is welcomed. Please let me know if you have any comments. Of course, integration into OCC will be appreciated ;-)

Thanks,
Roman

Roman Lygin's picture

Attached

Forum supervisor's picture

Dear Roman,
For your information:
fix385 (joint with fixes fix325,fix350, fix265,fix335) is still under processing. Unfortunately it caused big amount of regressions during testing procedure. As result it was rolled back.
Regards

Roman Lygin's picture

Forum supervisor,

Could you please send me details to the email ? This is the first time I hear of this.
Hundreds/thousands of test cases in my test system pass just fine.

The fix385 enclosed above (without those other you mention) should be very harmless but again I'd like to know details if you found otherwise.
If possible, sharing the test system with failed cases would be helpful to prevent bugging you in the future ;-).

If you want me to isolate fix490 and prepare it for plain 6.5.1, let me know too.

Thanks,
Roman

Mauro Mariotti's picture

Thanks for the fix, I will try it.

I wonder what the status is now, of fixes 385 and 490.

Mauro

Mauro Mariotti's picture

Help, Roman!

the compilation fails, I am afraid some source files are missing from your zip file.
You have converted AdvApp2Var_EvaluatorFunc2Var from a C function pointer to a virtual class, but its users have not changed:
GeomConvert\GeomConvert_ApproxSurface.cxx
GeomPlate_MakeApprox.cxx

Please send them.
Thanks a lot.
Mauro

Roman Lygin's picture

Mauro,

Sorry for delays. As explained in the posts above, this was originally a cumulative patch (fix385 + 490), as there was a previous patch (fix385) months before the latter.
You need to apply only the latter and therefore have to carefully use fix385-vs-fix490.diff file. Sorry I do not have a direct delta of 490-vs-ros.

HTH,
Roman

Mauro Mariotti's picture

Roman,

I have fixed the two source files where compilation failed:
...\ros\src\GeomConvert\GeomConvert_ApproxSurface.cxx
...\ros\src\GeomPlate\GeomPlate_MakeApprox.cxx
In each of them I derived a class from AdvApp2Var_EvaluatorFunc2Var, instead of defining a static function.
Please check the attachment.

Forum supervisor,

I have applied both fix385 and fix490.
Have you tested fix385 on its own?
I remember you tested it together with other ones, which maybe caused the regressions you had.

Thanks everybody.
Mauro

Attachments: 
Forum supervisor's picture

Dear Mauro,
Unfortunately still not.
You may attach your patch directly to the issue with ID=22786 in Mantis BugTracker available
via the Collaborative portal - http://dev.opencascade.org/index.php?q=home/get_involved.
Regards