Opened 16 years ago

Last modified 13 years ago

#202 closed defect

Undefined behavior in Coordinate::hashCode — at Version 2

Reported by: mloskot Owned by: geos-devel@…
Priority: major Milestone: 3.2.0
Component: Core Version: main
Severity: Significant Keywords: coordinate hash double
Cc:

Description (last modified by mloskot)

If ./configure fails to detect availability of 64-bit integer, it sets int64 typedef to long int (in file platform.h).

In this case, when in64 is 32-bit wide, undefined behavior occurs in Coordiante::hashDouble() function:

unsigned int Coordinate::hashCode(double d)
{
   int64 f = (int64)(d);
   return (int)(f^(f>>32)); // <--- UB
}

According to the standards C (section 6.5.7/3) and C++ (section 5.8/1):

The behavior is undefined if the right operand is negative, or greater than or equal to the length in bits of the promoted left operand.

This error occur in f>>32, when sizeof(f) == 32.

Simple but not ideal fix could be:

#include <cstring> // std::memcpy

int Coordinate::hashDouble(double d)
{
   unsigned int arr[2] = { 0 };
   std::memcpy(arr, &d, sizeof(double));
   return (arr[0] ^ (((arr[1] >> 16) & 0x000000FF)
           | ((arr[1] >> 8) & 0x0000FF00)
           | ((arr[1] << 8) & 0x00FF0000)
           | ((arr[1] << 16) & 0xFF000000)));
}

Change History (2)

comment:1 by bmharper, 16 years ago

Mateusz - you mentioned that this violates aliasing rules:

unsigned int HashDouble( double d ) {

unsigned int* i = (unsigned int*) &d; return i[0] i[1];

}

Can you explain further, or point me to the relevant C/++ docs?

Thanks, Ben

ps-- Your code references &x. Should this not be &d?

comment:2 by mloskot, 16 years ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.