{"action":"created","issue":{"url":"https://api.github.com/repos/google/googletest/issues/791","repository_url":"https://api.github.com/repos/google/googletest","labels_url":"https://api.github.com/repos/google/googletest/issues/791/labels{/name}","comments_url":"https://api.github.com/repos/google/googletest/issues/791/comments","events_url":"https://api.github.com/repos/google/googletest/issues/791/events","html_url":"https://github.com/google/googletest/issues/791","id":157527235,"number":791,"title":"EXPECT_EQ() crashes in armhf if comparing double vs. int","user":{"login":"jlblancoc","id":5497818,"avatar_url":"https://avatars.githubusercontent.com/u/5497818?v=3","gravatar_id":"","url":"https://api.github.com/users/jlblancoc","html_url":"https://github.com/jlblancoc","followers_url":"https://api.github.com/users/jlblancoc/followers","following_url":"https://api.github.com/users/jlblancoc/following{/other_user}","gists_url":"https://api.github.com/users/jlblancoc/gists{/gist_id}","starred_url":"https://api.github.com/users/jlblancoc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jlblancoc/subscriptions","organizations_url":"https://api.github.com/users/jlblancoc/orgs","repos_url":"https://api.github.com/users/jlblancoc/repos","events_url":"https://api.github.com/users/jlblancoc/events{/privacy}","received_events_url":"https://api.github.com/users/jlblancoc/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2016-05-30T15:58:17Z","updated_at":"2016-05-31T14:34:30Z","closed_at":null,"body":"I had unexpected crashes (\"bus error\") on `armhf` in a code that builds and runs cleanly on other platforms and after many tests I finally nailed it down to this: gtest `EXPECT_EQ()` crashes in armhf if comparing double vs. int, for example:\r\n\r\n```\r\n#pragma pack(push,1)\r\nstruct MyType\r\n{\r\n// other variables\r\ndouble d;\r\n};\r\n#pragma pack(pop)\r\n\r\nMyType *a = new MyType;\r\nint b=2;\r\nEXPECT_EQ(a.d,b);\r\n```\r\n\r\nA sample stack trace according to valgrind is (for a more complex code example than above, but essentially the same situation): \r\n\r\n```\r\n==17257== Process terminating with default action of signal 7 (SIGBUS)\r\n==17257==  Invalid address alignment at address 0xC8E93EA\r\n==17257==    at 0x2AE3E: testing::AssertionResult testing::internal::CmpHelperEQ<double, int>(char const*, char const*, double const&, int const&) (gtest.h:18341)\r\n==17257==    by 0x291DF: Compare<double, int> (gtest.h:18378)\r\n==17257==    by 0x291DF: CGPSInterface_parse_NMEA_ZDA_Test::TestBody() (CGPSInterface_unittest.cpp:116)\r\n==17257==    by 0x3C62F: HandleSehExceptionsInMethodIfSupported<testing::Test, void> (gtest-all.cc:3394)\r\n==17257==    by 0x3C62F: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (gtest-all.cc:3430)\r\n==17257==    by 0x37E87: testing::Test::Run() (gtest-all.cc:3466)\r\n==17257==    by 0x37F93: testing::TestInfo::Run() (gtest-all.cc:3642)\r\n==17257==    by 0x38033: testing::TestCase::Run() (gtest-all.cc:3749)\r\n==17257==    by 0x38217: testing::internal::UnitTestImpl::RunAllTests() (gtest-all.cc:5541)\r\n==17257==    by 0x38489: HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> (gtest-all.cc:3394)\r\n==17257==    by 0x38489: HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> (gtest-all.cc:3430)\r\n==17257==    by 0x38489: testing::UnitTest::Run() (gtest-all.cc:5175)\r\n==17257==    by 0x23793: main (test_main.cpp:25)\r\n```\r\n\r\nI don't know how important is this error, since, after all... who would compare a double vs. an integer and simultaneously using `#pragma pack(...)`!!! Still,  I did it unintentionally, and the tests run OK in amd64, i386, etc. so at least I wanted to leave it written here just in case it's useful for somebody else.\r\n\r\n\r\nOne solution might be allowing `testing::internal::CmpHelperEQ` to receive parameters by value, not reference, at the cost of a tiny performance loss (at least when using PODs...). "},"comment":{"url":"https://api.github.com/repos/google/googletest/issues/comments/222707591","html_url":"https://github.com/google/googletest/issues/791#issuecomment-222707591","issue_url":"https://api.github.com/repos/google/googletest/issues/791","id":222707591,"user":{"login":"sbenzaquen","id":14094653,"avatar_url":"https://avatars.githubusercontent.com/u/14094653?v=3","gravatar_id":"","url":"https://api.github.com/users/sbenzaquen","html_url":"https://github.com/sbenzaquen","followers_url":"https://api.github.com/users/sbenzaquen/followers","following_url":"https://api.github.com/users/sbenzaquen/following{/other_user}","gists_url":"https://api.github.com/users/sbenzaquen/gists{/gist_id}","starred_url":"https://api.github.com/users/sbenzaquen/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sbenzaquen/subscriptions","organizations_url":"https://api.github.com/users/sbenzaquen/orgs","repos_url":"https://api.github.com/users/sbenzaquen/repos","events_url":"https://api.github.com/users/sbenzaquen/events{/privacy}","received_events_url":"https://api.github.com/users/sbenzaquen/received_events","type":"User","site_admin":false},"created_at":"2016-05-31T14:34:30Z","updated_at":"2016-05-31T14:34:30Z","body":"On Mon, May 30, 2016 at 11:58 AM, Jose Luis Blanco-Claraco <\nnotifications@github.com> wrote:\n\n> I had unexpected crashes (\"bus error\") on armhf in a code that builds and\n> runs cleanly on other platforms and after many tests I finally nailed it\n> down to this: gtest EXPECT_EQ() crashes in armhf if comparing double vs.\n> int, for example:\n>\n> #pragma pack(push,1)\n> struct MyType\n> {\n> // other variables\n> double d;\n> };\n> #pragma pack(pop)\n>\n>\nThe problem is that you are packing this struct.\nThis causes the variable 'd' to be misaligned.\nWhen the compiler uses 'my_type.d' directly it knows it has to realign the\nvalue before use, but that goes out the window when you take the address.\nNote that passing by reference is pretty much like taking the address of\nthe object.\n\nWhoever uses that address lost the fact that it might be misaligned and\nwill use it directly.\nReading the misaligned variable has undefined behavior in C++, which is\nexperienced as a SIGBUS in your platform.\n\nThere are two options here:\n 1. remove the 'pack' option. This option is non-standard and has bad side\neffects as you have seen.\nor\n 2. Make sure you never take the address of the misaligned object. This can\nbe done my making it private and adding an accessor that returns by value.\nThe temporary copy will always be properly aligned.\n\nEg:\n\n#pragma pack(push,1)\nclass MyType\n{\n  // other variables\n  double d_;\n public:\n  double d() const { return d_; }\n  void set_d(double d) { d_ = d; }\n};\n#pragma pack(pop)\n\n>\n> MyType *a = new MyType;\n> int b=2;\n>\n>  EXPECT_EQ(a.d(), b);\n\n> A sample stack trace according to valgrind is (for a more complex code\n> example than above, but essentially the same situation):\n>\n> ==17257== Process terminating with default action of signal 7 (SIGBUS)\n> ==17257==  Invalid address alignment at address 0xC8E93EA\n> ==17257==    at 0x2AE3E: testing::AssertionResult testing::internal::CmpHelperEQ<double, int>(char const*, char const*, double const&, int const&) (gtest.h:18341)\n> ==17257==    by 0x291DF: Compare<double, int> (gtest.h:18378)\n> ==17257==    by 0x291DF: CGPSInterface_parse_NMEA_ZDA_Test::TestBody() (CGPSInterface_unittest.cpp:116)\n> ==17257==    by 0x3C62F: HandleSehExceptionsInMethodIfSupported<testing::Test, void> (gtest-all.cc:3394)\n> ==17257==    by 0x3C62F: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (gtest-all.cc:3430)\n> ==17257==    by 0x37E87: testing::Test::Run() (gtest-all.cc:3466)\n> ==17257==    by 0x37F93: testing::TestInfo::Run() (gtest-all.cc:3642)\n> ==17257==    by 0x38033: testing::TestCase::Run() (gtest-all.cc:3749)\n> ==17257==    by 0x38217: testing::internal::UnitTestImpl::RunAllTests() (gtest-all.cc:5541)\n> ==17257==    by 0x38489: HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> (gtest-all.cc:3394)\n> ==17257==    by 0x38489: HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> (gtest-all.cc:3430)\n> ==17257==    by 0x38489: testing::UnitTest::Run() (gtest-all.cc:5175)\n> ==17257==    by 0x23793: main (test_main.cpp:25)\n>\n> I don't know how important is this error, since, after all... who would\n> compare a double vs. an integer and simultaneously using #pragma pack(...)!!!\n> Still, I did it unintentionally, and the tests run OK in amd64, i386, etc.\n> so at least I wanted to leave it written here just in case it's useful for\n> somebody else.\n>\n> One solution might be allowing testing::internal::CmpHelperEQ to receive\n> parameters by value, not reference, at the cost of a tiny performance loss\n> (at least when using PODs...).\n>\n> —\n> You are receiving this because you are subscribed to this thread.\n> Reply to this email directly, view it on GitHub\n> <https://github.com/google/googletest/issues/791>, or mute the thread\n> <https://github.com/notifications/unsubscribe/ANcRPXJDyGXy_0MIfawFJuEUduqBRiCuks5qGwkngaJpZM4Ip5zX>\n> .\n>\n"}}
{"actor":{"display_login":"sbenzaquen"}}