harp_str64 cannot handle LONG_MIN

schwehr commented 4 years ago

I observed this at 718225a9c4008f6ec331ad2e37e009cbbd57fdd7 with asan. This should still be present at head.

third_party/stcorp_harp/libharp/harp-utils.c:838:32: runtime error: negation of -9223372036854775808 cannot be represented in type 'int64_t' (aka 'long'); cast to an unsigned type to negate this value to itself
    #0 0x7fabf7c2ed07 in harp_str64 third_party/stcorp_harp/libharp/harp-utils.c:838:32
    #1 0x7fabfcc9c694 in (anonymous namespace)::HarpStandaloneTest_Str64_Test::TestBody() third_party/stcorp_harp/test/harp_test.cc:92:3

Resulting from me working on tests for harp.h:

TEST(HarpStandaloneTest, Str64) {
  char buf[21];
  harp_str64(0, buf);
  EXPECT_STREQ("0", buf);
  harp_str64(1, buf);
  EXPECT_STREQ("1", buf);
  harp_str64(-1, buf);
  EXPECT_STREQ("-1", buf);

  harp_str64(std::numeric_limits<int64_t>::max(), buf);
  EXPECT_STREQ("9223372036854775807", buf);
  harp_str64(std::numeric_limits<int64_t>::min()+1, buf);
  EXPECT_STREQ("-9223372036854775807", buf);

  harp_str64(std::numeric_limits<int64_t>::min(), buf);  // <---- Trouble here
}

See https://en.cppreference.com/w/cpp/types/numeric_limits

svniemeijer commented 4 years ago

Actually, this is a different problem. These functions don't below in the HARP API in the first place (since HARP does not support 64bit integers). Fixed in 92db7ef2db6f6d751c677ec02177cc2e391d5a02.

The problem does apply to the CODA versions of these functions though. However, with the clang C compiler I am getting a correct answer for the value -9223372036854775808. So it seems to be a compiler implementation specific problem. Nevertheless, I implemented a solution for this in https://github.com/stcorp/coda/commit/a5bfc132361c8b1d6e564207fa53078dad0110f4 that should pass through your checks.

schwehr commented 4 years ago

The more important thing for both coda and harp is that singed integer overflow is undefined behavior (UB). Having a specific compiler give the correct value is luck. A different version or different compiler flags may result in a different answer or even an abort. c.f.

stcorp / harp

harp_str64 cannot handle LONG_MIN #215