{"thread":{"id":"51326","subject":"[PATCH] wrapper: avoid UB in macOS","startedAt":"2019-06-16T18:40:28Z","lastAt":"2019-06-19T14:40:35Z","messageCount":2,"participants":["Carlo Marcelo Arenas Belón","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"377320","messageId":"20190616184003.17236-1-carenas@gmail.com","threadId":"51326","inReplyTo":null,"subject":"[PATCH] wrapper: avoid UB in macOS","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2019-06-16T18:40:03Z","receivedAt":"2019-06-16T18:40:28Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"0620b39b3b (\"compat: add a mkstemps() compatibility function\", 2009-05-31)\nincluded a function based on code from libiberty which would result in\nundefined behaviour in platforms where timeval's tv_usec is a 32-bit signed\ntype as shown by:\n\nwrapper.c:505:31: runtime error: left shift of 594546 by 16 places cannot be represented in type '__darwin_suseconds_t' (aka 'int')\n\ninterestingly the version of this code from gcc never had this bug and the\ncode had a cast that would had prevented the issue (at least in 64-bit\nplatforms) but was misapplied.\n\nchange the cast to uint64_t so it also works in 32-bit platforms.\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\n wrapper.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/wrapper.c b/wrapper.c\nindex ea3cf64d4c..1e45ab7b92 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -502,7 +502,7 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int mode)\n \t * Try TMP_MAX different filenames.\n \t */\n \tgettimeofday(&tv, NULL);\n-\tvalue = ((size_t)(tv.tv_usec << 16)) ^ tv.tv_sec ^ getpid();\n+\tvalue = ((uint64_t)tv.tv_usec << 16) ^ tv.tv_sec ^ getpid();\n \tfilename_template = &pattern[len - 6 - suffix_len];\n \tfor (count = 0; count < TMP_MAX; ++count) {\n \t\tuint64_t v = value;\n-- \n2.22.0\n\n"},{"id":"377526","messageId":"xmqqk1dhr63n.fsf@gitster-ct.c.googlers.com","threadId":"51326","inReplyTo":"20190616184003.17236-1-carenas@gmail.com","subject":"Re: [PATCH] wrapper: avoid UB in macOS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-06-19T14:40:28Z","receivedAt":"2019-06-19T14:40:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Marcelo Arenas Belón  <carenas@gmail.com> writes:\n\n> 0620b39b3b (\"compat: add a mkstemps() compatibility function\", 2009-05-31)\n> included a function based on code from libiberty which would result in\n> undefined behaviour in platforms where timeval's tv_usec is a 32-bit signed\n> type as shown by:\n>\n> wrapper.c:505:31: runtime error: left shift of 594546 by 16 places cannot be represented in type '__darwin_suseconds_t' (aka 'int')\n\nI had to scratch my head wondering what an UB was (spell it out as\n\"undefined behaviour\" if that is what you wanted to say).\n\n> interestingly the version of this code from gcc never had this bug and the\n> code had a cast that would had prevented the issue (at least in 64-bit\n> platforms) but was misapplied.\n>\n> change the cast to uint64_t so it also works in 32-bit platforms.\n\nYup, regardless of the platform, that's the \"right\" type to use,\ncertainly more correct than \"size_t\", as value is declared to be u64\nin this function.\n\n>  \tgettimeofday(&tv, NULL);\n> -\tvalue = ((size_t)(tv.tv_usec << 16)) ^ tv.tv_sec ^ getpid();\n> +\tvalue = ((uint64_t)tv.tv_usec << 16) ^ tv.tv_sec ^ getpid();\n>  \tfilename_template = &pattern[len - 6 - suffix_len];\n>  \tfor (count = 0; count < TMP_MAX; ++count) {\n>  \t\tuint64_t v = value;\n\nThanks.\n\n"}]}