aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFlorian Weimer <fweimer@redhat.com>2019-02-04 10:01:29 +0100
committerFlorian Weimer <fweimer@redhat.com>2019-02-04 10:01:29 +0100
commit221baae0012e56e4043b914fec47340ef3a1e0c8 (patch)
tree3d6e6c931cffecfd10a836bc0acaf51410a3a5cc
parentb8c7238167de4c080b8b0909213bc7b5abef46e3 (diff)
downloadglibc-221baae0012e56e4043b914fec47340ef3a1e0c8.tar
glibc-221baae0012e56e4043b914fec47340ef3a1e0c8.tar.gz
glibc-221baae0012e56e4043b914fec47340ef3a1e0c8.tar.bz2
glibc-221baae0012e56e4043b914fec47340ef3a1e0c8.zip
time: Avoid alignment gaps in __tzfile_read
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. The separate call to __fread_unlocked does not matter from a performance point of view because __tzfile_read is only invoked rarely.
-rw-r--r--ChangeLog5
-rw-r--r--time/tzfile.c57
2 files changed, 32 insertions, 30 deletions
diff --git a/ChangeLog b/ChangeLog
index bc1b17ffa7..0c9a4e885b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2019-02-04 Florian Weimer <fweimer@redhat.com>
+
+ * time/tzfile.c (__tzfile_read): Reorder suballocations to avoid
+ alignment gaps.
+
2019-02-03 Florian Weimer <fweimer@redhat.com>
* time/tzfile.c (__tzfile_read): Use struct alloc_buffer and its
diff --git a/time/tzfile.c b/time/tzfile.c
index e107b33a82..7229ed93b7 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;
@@ -274,35 +281,25 @@ __tzfile_read (const char *file, size_t extra, char **extrap)
/* The address of the first allocation is already stored in the
pointer transitions. */
(void) alloc_buffer_alloc_array (&buf, __time64_t, num_transitions);
- 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 (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 (extra > 0)
+ *extrap = alloc_buffer_alloc_array (&buf, char, extra);
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. */