{"thread":{"id":"49445","subject":"[PATCH] read-cache: fix division by zero core-dump","startedAt":"2018-09-27T22:24:45Z","lastAt":"2018-10-01T12:41:05Z","messageCount":4,"participants":["Ramsay Jones","Ben Peart","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"359153","messageId":"476b5678-41b2-d2f8-1890-ba315354ebc0@ramsayjones.plus.com","threadId":"49445","inReplyTo":null,"subject":"[PATCH] read-cache: fix division by zero core-dump","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-09-27T22:24:39Z","receivedAt":"2018-09-27T22:24:45Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\ncommit 225df8a468 (\"ieot: add Index Entry Offset Table (IEOT)\nextension\", 2018-09-26) added a 'DIV_ROUND_UP(entries, ieot_blocks)\nexpression, where ieot_blocks was set to zero for a single cpu\nplatform. This caused an SIGFPE and a core dump in practically\nevery test in the test-suite, until test t4056-diff-order.sh, which\nthen went into an infinite loop!\n\nSigned-off-by: Ramsay Jones \n---\n\nHi Ben,\n\nCould you please squash this into the relevant commits on your\n'bp/read-cache-parallel' branch. (The first hunk fixes a sparse\nwarning about using an integer as a NULL pointer).\n\nThanks!\n\nATB,\nRamsay Jones\n\n read-cache.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 6755d58877..40f096f70a 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2141,7 +2141,7 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n \tsize_t extension_offset = 0;\n #ifndef NO_PTHREADS\n \tint nr_threads, cpus;\n-\tstruct index_entry_offset_table *ieot = 0;\n+\tstruct index_entry_offset_table *ieot = NULL;\n #endif\n \n \tif (istate->initialized)\n@@ -2771,7 +2771,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n \t\t\tif (ieot_blocks < 1)\n \t\t\t\tieot_blocks = 1;\n \t\t\tcpus = online_cpus();\n-\t\t\tif (ieot_blocks > cpus - 1)\n+\t\t\tif (cpus > 1 && ieot_blocks > cpus - 1)\n \t\t\t\tieot_blocks = cpus - 1;\n \t\t} else {\n \t\t\tieot_blocks = nr;\n-- \n2.19.0\n"},{"id":"359171","messageId":"8a76234e-fcf8-f435-33e9-1846c3b6ad14@gmail.com","threadId":"49445","inReplyTo":"476b5678-41b2-d2f8-1890-ba315354ebc0@ramsayjones.plus.com","subject":"Re: [PATCH] read-cache: fix division by zero core-dump","fromName":"Ben Peart","fromEmail":"peartben@gmail.com","sentAt":"2018-09-28T01:20:32Z","receivedAt":"2018-09-28T01:20:37Z","isPatch":true,"sender":{"key":"benpeart@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/15252029?v=4"},"body":"\n\nOn 9/27/2018 6:24 PM, Ramsay Jones wrote:\n> \n> commit 225df8a468 (\"ieot: add Index Entry Offset Table (IEOT)\n> extension\", 2018-09-26) added a 'DIV_ROUND_UP(entries, ieot_blocks)\n> expression, where ieot_blocks was set to zero for a single cpu\n> platform. This caused an SIGFPE and a core dump in practically\n> every test in the test-suite, until test t4056-diff-order.sh, which\n> then went into an infinite loop!\n> \n> Signed-off-by: Ramsay Jones \n> ---\n> \n> Hi Ben,\n> \n> Could you please squash this into the relevant commits on your\n> 'bp/read-cache-parallel' branch. (The first hunk fixes a sparse\n> warning about using an integer as a NULL pointer).\n> \n\nAbsolutely - thanks for the patch.\n\nI don't know how long it's been since I've been on a single core CPU - \nI'm sad for you. ;-)\n\n> Thanks!\n> \n> ATB,\n> Ramsay Jones\n> \n> read-cache.c | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/read-cache.c b/read-cache.c\n> index 6755d58877..40f096f70a 100644\n> --- a/read-cache.c\n> +++ b/read-cache.c\n> @@ -2141,7 +2141,7 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)\n> \tsize_t extension_offset = 0;\n> #ifndef NO_PTHREADS\n> \tint nr_threads, cpus;\n> -\tstruct index_entry_offset_table *ieot = 0;\n> +\tstruct index_entry_offset_table *ieot = NULL;\n> #endif\n> \n> \tif (istate->initialized)\n> @@ -2771,7 +2771,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,\n> \t\t\tif (ieot_blocks < 1)\n> \t\t\t\tieot_blocks = 1;\n> \t\t\tcpus = online_cpus();\n> -\t\t\tif (ieot_blocks > cpus - 1)\n> +\t\t\tif (cpus > 1 && ieot_blocks > cpus - 1)\n> \t\t\t\tieot_blocks = cpus - 1;\n> \t\t} else {\n> \t\t\tieot_blocks = nr;\n> \n"},{"id":"359206","messageId":"da607bd3-c91b-556d-b342-927cb2f773c7@ramsayjones.plus.com","threadId":"49445","inReplyTo":"8a76234e-fcf8-f435-33e9-1846c3b6ad14@gmail.com","subject":"Re: [PATCH] read-cache: fix division by zero core-dump","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-09-28T15:31:13Z","receivedAt":"2018-09-28T15:31:50Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 28/09/18 02:20, Ben Peart wrote:\n> \n> \n> On 9/27/2018 6:24 PM, Ramsay Jones wrote:\n>>\n>> commit 225df8a468 (\"ieot: add Index Entry Offset Table (IEOT)\n>> extension\", 2018-09-26) added a 'DIV_ROUND_UP(entries, ieot_blocks)\n>> expression, where ieot_blocks was set to zero for a single cpu\n>> platform. This caused an SIGFPE and a core dump in practically\n>> every test in the test-suite, until test t4056-diff-order.sh, which\n>> then went into an infinite loop!\n>>\n>> Signed-off-by: Ramsay Jones \n>> ---\n>>\n>> Hi Ben,\n>>\n>> Could you please squash this into the relevant commits on your\n>> 'bp/read-cache-parallel' branch. (The first hunk fixes a sparse\n>> warning about using an integer as a NULL pointer).\n>>\n> \n> Absolutely - thanks for the patch.\n> \n> I don't know how long it's been since I've been on a single core CPU - I'm sad for you. ;-)\n\nHeh, don't be - whilst I do still have a single cpu laptop about\nthe place _somewhere_, I haven't booted it up in about 15 years! :-D\n\nI used to regularly test git (and other software) on my old 32-bit\nlaptop (windows xp/Linux Mint 17.x, Core2 duo), but just lately\nI have taken to using a 32-bit VM on my current laptop (4th gen i5)\ninstead. (The git test-suite would take approx 50 min to run on\nthe 32-bit hardware, whereas it only takes 8 min on the VM!).\n\nI have configured the 32-bit VM with a single cpu, because when\nthe VM was configured with two cpus the git test-suite would take\nlonger to run (approx. 8 -> 10 min)! Taking more resources from\nthe host, but increasing the running time, didn't seem like a good\nreturn. ;-)\n\nAlso, this is not the first time some multi-threaded code in git\nhas 'failed' by assuming more than one cpu, so ...\n\nATB,\nRamsay Jones\n\n\n"},{"id":"359357","messageId":"b30e106e-b34b-2738-d4a2-001d77d1b08b@gmail.com","threadId":"49445","inReplyTo":"da607bd3-c91b-556d-b342-927cb2f773c7@ramsayjones.plus.com","subject":"Re: [PATCH] read-cache: fix division by zero core-dump","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-10-01T12:41:01Z","receivedAt":"2018-10-01T12:41:05Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 9/28/2018 11:31 AM, Ramsay Jones wrote:\n> Also, this is not the first time some multi-threaded code in git\n> has 'failed' by assuming more than one cpu, so ...\n>\nI wonder if this is a good time to create a GIT_TEST_CPU_COUNT variable \nso we can mock out single-processor environments instead of relying on \nold hardware or custom VMs.\n\nThanks,\n-Stolee\n"}]}