From patchwork Sat Feb 2 22:19:57 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 1035525 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=sourceware.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=libc-alpha-return-99706-incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="SlVrMHjb"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 43sT1R1r2Rz9s4V for ; Sun, 3 Feb 2019 09:20:11 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id:mime-version :content-type; q=dns; s=default; b=n7vDSChO8ZAfpHoxHEVa5BCOL5jtF 1TorXIOEe4JMVIAxnVR5JW3jOAHMK8s/bBXEU1klufbi6iBMjH55Y+pMU915o6jH Vtk1BPqStEDj3O+W+IuMyPq6uFyip7rzXqFeG3o5AW143yx3TkpvRW44TmIQqjvc c55HLUXQXBfjRg= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id:mime-version :content-type; s=default; bh=9OqY1Dkq46COd/iBnlpaSFgH+HU=; b=SlV rMHjblfJqI8Om7tnT0FH/s1Mb6rUbgBSLZgaw03x3MfJxu2x8bfM02cSCvnsSuyJ 2ICnPEyCKL9vNOrChZo1XhZlIj/dh0hid9m/KREEdqcWiQrMjPfHXdO0VUEKjLht mw19vZhvqVGCjJLpbGIAxs5daOJEzetB6vBgIR34= Received: (qmail 71111 invoked by alias); 2 Feb 2019 22:20:04 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 70943 invoked by uid 89); 2 Feb 2019 22:20:02 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Hx-languages-length:4026, avoided X-HELO: mx1.redhat.com From: Florian Weimer To: libc-alpha@sourceware.org Subject: [PATCH] time: Avoid alignment gaps in __tzfile_read Date: Sat, 02 Feb 2019 23:19:57 +0100 Message-ID: <87o97t7rzm.fsf@oldenburg2.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 By ordering the suballocations by decreasing alignment, alignment gaps can be avoided. Also use __glibc_unlikely for reading the transitions and type indexes. In the 8-byte case, two reads are now needed because the transitions and type indexes are no longer adjacent. 2019-02-02 Florian Weimer * time/tzfile.c (__tzfile_read): Reorder suballocations to avoid alignment gaps. diff --git a/time/tzfile.c b/time/tzfile.c index fb1202b80c..d0054459fe 100644 --- a/time/tzfile.c +++ b/time/tzfile.c @@ -246,25 +246,32 @@ __tzfile_read (const char *file, size_t extra, char **extrap) the following arrays: __time64_t transitions[num_transitions]; + struct leap leaps[num_leaps]; struct ttinfo types[num_types]; unsigned char type_idxs[num_types]; char zone_names[chars]; - struct leap leaps[num_leaps]; - char extra_array[extra]; // Stored into *pextras if requested. char tzspec[tzspec_len]; + char extra_array[extra]; // Stored into *pextras if requested. The piece-wise allocations from buf below verify that no - overflow/wraparound occurred in these computations. */ + overflow/wraparound occurred in these computations. + + The order of the suballocations is important for alignment + purposes. __time64_t outside a struct may require more alignment + then inside a struct on some architectures, so it must come + first. */ + _Static_assert (__alignof (__time64_t) >= __alignof (struct leap), + "alignment of __time64_t"); + _Static_assert (__alignof (struct leap) >= __alignof (struct ttinfo), + "alignment of struct leap"); struct alloc_buffer buf; { - size_t total_size = num_transitions * (sizeof (__time64_t) + 1); - total_size = ((total_size + __alignof__ (struct ttinfo) - 1) - & ~(__alignof__ (struct ttinfo) - 1)); - total_size += num_types * sizeof (struct ttinfo) + chars; - total_size = ((total_size + __alignof__ (struct leap) - 1) - & ~(__alignof__ (struct leap) - 1)); - total_size += num_leaps * sizeof (struct leap) + tzspec_len + extra; - + size_t total_size = (num_transitions * sizeof (__time64_t) + + num_leaps * sizeof (struct leap) + + num_types * sizeof (struct ttinfo) + + num_transitions /* type_idxs */ + + chars /* zone_names */ + + tzspec_len + extra); transitions = malloc (total_size); if (transitions == NULL) goto lose; @@ -277,35 +284,25 @@ __tzfile_read (const char *file, size_t extra, char **extrap) (misaligned heap pointer, should not happen), or we had an overflow. */ goto lose; - type_idxs = alloc_buffer_alloc_array (&buf, unsigned char, num_transitions); + leaps = alloc_buffer_alloc_array (&buf, struct leap, num_leaps); types = alloc_buffer_alloc_array (&buf, struct ttinfo, num_types); + type_idxs = alloc_buffer_alloc_array (&buf, unsigned char, num_transitions); zone_names = alloc_buffer_alloc_array (&buf, char, chars); - leaps = alloc_buffer_alloc_array (&buf, struct leap, num_leaps); + if (trans_width == 8) + tzspec = alloc_buffer_alloc_array (&buf, char, tzspec_len); + else + tzspec = NULL; if (extra > 0) *extrap = alloc_buffer_alloc_array (&buf, char, extra); - if (trans_width == 8) - tzspec = alloc_buffer_alloc_array (&buf, char, tzspec_len); - else - tzspec = NULL; if (alloc_buffer_has_failed (&buf)) goto lose; - if (__builtin_expect (trans_width == 8, 1)) - { - if (__builtin_expect (__fread_unlocked (transitions, trans_width + 1, - num_transitions, f) - != num_transitions, 0)) + if (__glibc_unlikely (__fread_unlocked (transitions, trans_width, + num_transitions, f) + != num_transitions) + || __glibc_unlikely (__fread_unlocked (type_idxs, 1, num_transitions, f) + != num_transitions)) goto lose; - } - else - { - if (__builtin_expect (__fread_unlocked (transitions, 4, - num_transitions, f) - != num_transitions, 0) - || __builtin_expect (__fread_unlocked (type_idxs, 1, num_transitions, - f) != num_transitions, 0)) - goto lose; - } /* Check for bogus indices in the data file, so we can hereafter safely use type_idxs[T] as indices into `types' and never crash. */