Fix random segfaults in pack-objects.

Junio noticed that 'non-trivial' pushes were failing if executed
using the sliding window mmap changes.  This was somewhat difficult
to track down as the failure was appearing randomly.

It turns out this was a failure caused by the delta base reference
(either ref or offset format) spanning over the end of a mmap window.

The error in pack-objects was we were not recalling use_pack
after the object header was unpacked, and therefore we did not
get the promise of at least 20 bytes in the buffer for the delta
base parsing.  This would case later memcmp() calls to walk into
unassigned address space at the end of the window.

The reason Junio and I had hard time tracking this down in current
Git repositories is we were both probably packing with offset deltas,
which minimized the odds of the delta base reference spanning over
the end of the mmap window.  Stepping back and repacking with
version 1.3.3 (which only supported reference deltas) increased
the likelyhood of seeing the bug.

The correct technique (as used in sha1_file.c) is to invoke
use_pack() after unpack_object_header_gently to ensure we have
enough data available for the delta base decoding.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Junio C Hamano <junkio@cox.net>

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index afb926a34c53a2221fabe38cb5003acdcc9ffee6..e9a1804fa8e44c02fce844c77665eb939b386d19 100644 (file)
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -995,8 +995,6 @@ static void check_object(struct object_entry *entry)
                 */
                used = unpack_object_header_gently(buf, left,
                                                   &entry->in_pack_type, &size);
-               if (!used || left - used <= 20)
-                       die("corrupt pack for %s", sha1_to_hex(entry->sha1));
 
                /* Check if it is delta, and the base is also an object
                 * we are going to pack.  If so we will reuse the existing
@@ -1008,10 +1006,13 @@ static void check_object(struct object_entry *entry)
                        /* there is at least 20 bytes left in the pack */
                        switch (entry->in_pack_type) {
                        case OBJ_REF_DELTA:
-                               base_name = buf + used;
+                               base_name = use_pack(p, &w_curs,
+                                       entry->in_pack_offset + used, NULL);
                                used += 20;
                                break;
                        case OBJ_OFS_DELTA:
+                               buf = use_pack(p, &w_curs,
+                                       entry->in_pack_offset + used, NULL);
                                c = buf[used++];
                                ofs = c & 127;
                                while (c & 128) {