Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compatibility with gcc 4.9 #46

Closed
patmarion opened this issue Jan 27, 2016 · 7 comments
Closed

Compatibility with gcc 4.9 #46

patmarion opened this issue Jan 27, 2016 · 7 comments

Comments

@patmarion
Copy link
Contributor

@rdeits reports this issue over on the Drake project. Cross posting here for visibility.

RobotLocomotion/drake#1663

Perhaps we can write a minimal c++ program that demonstrates the issue.

@rdeits
Copy link

rdeits commented Jan 27, 2016

Okay, I figured out what's going on. In the generated header for one of our LCM message types, the _computeHash function looks like this:

int64_t hash = 0x4909180abc8e6b92LL +
         drake::lcmt_piecewise_polynomial::_computeHash(&cp);
return (hash<<1) + ((hash>>63)&1);

For reference, the Python version (which matches Java), looks like this:

tmphash = (0x4909180abc8e6b92+ lcmt_piecewise_polynomial.lcmt_piecewise_polynomial._get_hash_recursive(newparents)) & 0xffffffffffffffff
tmphash  = (((tmphash<<1)&0xffffffffffffffff)  + (tmphash>>63)) & 0xffffffffffffffff
return tmphash

The problem is that for my particular message types, it happens that, in C++, the value of hash overflows the maximum value of an int64_t, resulting in a negative hash value. When that value is left-shifted by one, the sign is corrected, and Python and C++ agree:

C++:

hash << 1 = 3051220294024266798

Python:

(tmphash<<1)&0xffffffffffffffff = 3051220294024266798

But when right-shifting, the C++ behavior is different:

C++:

(hash >> 63): 0

Python:

(tmphash>>63) = 1

I believe this is due to the fact that the behavior of a right-shift of a negative integer in C++ is undefined (see page 85 of http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1336.pdf ).

The result of this is that certain LCM message types generate incorrect fingerprints in C++, at least on some platforms and compilers.

@rdeits
Copy link

rdeits commented Jan 27, 2016

Disabling compiler optimizations (with -O0) results in the correct hash value in C++ (that is, the value matches the one from Python and Java). It would appear that newer versions of g++ are relying on the undefined nature of right-shift of negative numbers and producing different answers depending on the optimization level.

@rdeits
Copy link

rdeits commented Jan 27, 2016

It's also difficult to produce a minimal example that demonstrates the problem, because g++ really does produce different hash values depending on what optimizations it chooses to make. The simplest possible code:

  int64_t hash = -7697761889842642409LL;
  std::cout << "hash: " << hash << std::endl;
  std::cout << ((hash >> 63)&1) << std::endl;

always results in the output:

hash: -7697761889842642409
1

regardless of optimization level. But somehow with the -O3 flag present, g++ produces a different result for ((hash >> 63)&1 when used within an LCM type within our library.

@rdeits
Copy link

rdeits commented Jan 27, 2016

Okay, here's the simplest example I can find that demonstrates that the issue is real, and the behavior from g++ 4.8 is different from 4.9:

My code:

#include <iostream>
#include <lcm/lcm_coretypes.h>

int64_t sub_hash() {
  int64_t hash = 0; 
  for (int64_t i=0; i < 5486218109915584133; i++) {
    hash++;
  }
  return hash;
}

int main(int argc, char** argv) {

  int64_t hash = 0x4909180abc8e6b92LL + sub_hash();
  std::cout << "hash: " << hash << std::endl;
  std::cout << "((hash >> 63)&1): " << ((hash >> 63)&1) << std::endl;
  return 0;
}

Note that the code uses a truly insane loop in order to force g++ to do some optimization magic.

Results:

gcc 4.8:

g++-4.8 -O3 -std=c++11 -I../../build/include hashTest.cpp -o hashTest && ./hashTest
hash: -7697761889842642409
((hash >> 63)&1): 1

gcc 4.9

g++-4.9 -O3 -std=c++11 -I../../build/include hashTest.cpp -o hashTest && ./hashTest
hash: -7697761889842642409
((hash >> 63)&1): 0

@ashuang
Copy link
Member

ashuang commented Jan 28, 2016

Hi all,

Thanks for the report. Do you know if this is related to this pull request #31?

@abachrach

Albert

@rdeits
Copy link

rdeits commented Jan 28, 2016

Ah, yes, looks like exactly the same issue. We've been on an older LCM release, so we haven't gotten that fix.

@ashuang
Copy link
Member

ashuang commented Feb 2, 2016

Ok, thanks. Closing this since I believe the issue to be already resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants