Ticket #847 (new defect)

Opened 5 years ago

Last modified 5 years ago

Dereferencing pointers to pointers on 64 bit machines

Reported by: ak19 Owned by: nobody
Priority: moderate Milestone: Greenstone 2 wishlist
Component: Greenstone2&3 Severity: major
Keywords: Cc:

Description

Dereferencing pointers to pointers on 64 bit machines. If not done correctly, this can result pointer addresses getting truncated to 32 bit values during intermediate conversions.

At least, this is what happened in suffix.cpp (suffix.exe) used by the Phind browser, resulting in the occasional segmentation fault when the bug became fatal. (The code was shared between GS2 and GS3, so both were affected.)

Perhaps we should consider inspecting the code to see if this sort of conversion happens anywhere else.

Change History

Changed 5 years ago by ak19

The particular issue with suffix.cpp is described in http://trac.greenstone.org/changeset/26158

The phind browse classifier used to fail on certain particular combinations of documents on 64 bit linux. This was because of a bug in suffix.exe that caused a seg fault: all pointers are 64 bits on 64 bit machines. However, suffixCompare() and prefixCompare() functions used to try to convert their arguments, which were void* pointers to pointers, by first casting the void* pointers to symbol* and then dereference these to get their values (which are ultimately 32 bit unsigned int) but which are meant to be addresses (pointers themselves) and then converting these back to pointers objects which are 64 bit values. During this process of multiple conversion, 64 bit pointers were truncated to 32 bits before being given 64 bits for storage again. This used to work on 32 bit machines before since pointers there were 32 bits, but showed up here. The solution was a double pointer cast operation: since the data in the void* arguments are actually pointers to pointers, convert them directly back to the specific pointer data types we need.

// WRONG: Cast then dereference pointers to suffix array elements //symbol *pa = (symbol *) cpa; //symbol *pb = (symbol *) cpb; //pa = (symbol *) *pa; //pb = (symbol *) *pb;

// This is the correct way that will work on both 64 bit and 32 bit machines: // convert to double pointer, then dereference that to get its value: symbol *pa = *((symbol **) cpa); symbol *pb = *((symbol **) cpb);

It may be that such code is unique to suffix.cpp, but if it's not, similar code may be lurking elsewhere and could act up at any time with a seg fault too.

Changed 5 years ago by ak19

Formatting code from above for correct code structure:

// WRONG: Cast then dereference pointers to suffix array elements

//symbol *pa = (symbol *) cpa;

//symbol *pb = (symbol *) cpb;

//pa = (symbol *) *pa;

//pb = (symbol *) *pb;

// This is the correct way that will work on both 64 bit and 32 bit machines:

// convert to double pointer, then dereference that to get its value:

symbol *pa = *((symbol **) cpa);

symbol *pb = *((symbol **) cpb);

Note: See TracTickets for help on using tickets.