Skip to content

Commit

Permalink
Fix memory handling in strxfrm_l [BZ #16009]
Browse files Browse the repository at this point in the history
[Modified from the original email by Siddhesh Poyarekar]

This patch solves bug #16009 by implementing an additional path in
strxfrm that does not depend on caching the weight and rule indices.

In detail the following changed:

* The old main loop was factored out of strxfrm_l into the function
do_xfrm_cached to be able to alternativly use the non-caching version
do_xfrm.

* strxfrm_l allocates a a fixed size array on the stack. If this is not
sufficiant to store the weight and rule indices, the non-caching path is
taken. As the cache size is not dependent on the input there can be no
problems with integer overflows or stack allocations greater than
__MAX_ALLOCA_CUTOFF. Note that malloc-ing is not possible because the
definition of strxfrm does not allow an oom errorhandling.

* The uncached path determines the weight and rule index for every char
and for every pass again.

* Passing all the locale data array by array resulted in very long
parameter lists, so I introduced a structure that holds them.

* Checking for zero src string has been moved a bit upwards, it is
before the locale data initialization now.

* To verify that the non-caching path works correct I added a test run
to localedata/sort-test.sh & localedata/xfrm-test.c where all strings
are patched up with spaces so that they are too large for the caching path.
  • Loading branch information
Leonhard Holz authored and Siddhesh Poyarekar committed Jan 13, 2015
1 parent c60ec0e commit 0f9e585
Show file tree
Hide file tree
Showing 5 changed files with 471 additions and 108 deletions.
16 changes: 16 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
2015-01-13 Leonhard Holz <leonhard.holz@web.de>

[BZ #16009]
* string/strxfrm_l.c (STRXFRM): Allocate fixed size cache for
weights and rules. Use do_xfrm_cached if data fits in cache,
do_xfrm otherwise. Moved former main loop to...
* (do_xfrm_cached): New function.
* (do_xfrm): Non-caching version of do_xfrm_cached. Uses
find_idx, find_position and stack_push.
* (find_idx): New function.
* (find_position): Likewise.
* localedata/sort-test.sh: Added test run for do_xfrm.
* localedata/xfrm-test.c (main): Added command line option
-nocache to run the test with strings that are too large for
the STRXFRM cache.

2015-01-13 Torvald Riegel <triegel@redhat.com>

* sysdeps/nptl/fork.c (__libc_fork): Provide address of futex
Expand Down
16 changes: 8 additions & 8 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ Version 2.21
* The following bugs are resolved with this release:

6652, 10672, 12847, 12926, 13862, 14132, 14138, 14171, 14498, 15215,
15884, 16191, 16469, 16617, 16619, 16657, 16740, 16857, 17192, 17266,
17273, 17344, 17363, 17370, 17371, 17411, 17460, 17475, 17485, 17501,
17506, 17508, 17522, 17555, 17570, 17571, 17572, 17573, 17574, 17582,
17583, 17584, 17585, 17589, 17594, 17601, 17608, 17616, 17625, 17630,
17633, 17634, 17635, 17647, 17653, 17657, 17658, 17664, 17665, 17668,
17682, 17717, 17719, 17722, 17723, 17724, 17725, 17732, 17733, 17744,
17745, 17746, 17747, 17748, 17775, 17777, 17780, 17781, 17782, 17791,
17793, 17796, 17797, 17803, 17806, 17834
15884, 16009, 16191, 16469, 16617, 16619, 16657, 16740, 16857, 17192,
17266, 17273, 17344, 17363, 17370, 17371, 17411, 17460, 17475, 17485,
17501, 17506, 17508, 17522, 17555, 17570, 17571, 17572, 17573, 17574,
17582, 17583, 17584, 17585, 17589, 17594, 17601, 17608, 17616, 17625,
17630, 17633, 17634, 17635, 17647, 17653, 17657, 17658, 17664, 17665,
17668, 17682, 17717, 17719, 17722, 17723, 17724, 17725, 17732, 17733,
17744, 17745, 17746, 17747, 17748, 17775, 17777, 17780, 17781, 17782,
17791, 17793, 17796, 17797, 17803, 17806, 17834

* Added support for TSX lock elision of pthread mutexes on powerpc32, powerpc64
and powerpc64le. This may improve lock scaling of existing programs on
Expand Down
7 changes: 7 additions & 0 deletions localedata/sort-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,18 @@ for l in $lang; do
${common_objpfx}localedata/xfrm-test $id < $cns.in \
> ${common_objpfx}localedata/$cns.xout || here=1
cmp -s $cns.in ${common_objpfx}localedata/$cns.xout || here=1
${test_program_prefix_before_env} \
${run_program_env} \
LC_ALL=$l ${test_program_prefix_after_env} \
${common_objpfx}localedata/xfrm-test $id -nocache < $cns.in \
> ${common_objpfx}localedata/$cns.nocache.xout || here=1
cmp -s $cns.in ${common_objpfx}localedata/$cns.nocache.xout || here=1
if test $here -eq 0; then
echo "$l xfrm-test OK"
else
echo "$l xfrm-test FAIL"
diff -u $cns.in ${common_objpfx}localedata/$cns.xout | sed 's/^/ /'
diff -u $cns.in ${common_objpfx}localedata/$cns.nocache.xout | sed 's/^/ /'
status=1
fi
done
Expand Down
52 changes: 46 additions & 6 deletions localedata/xfrm-test.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdbool.h>

/* Keep in sync with string/strxfrm_l.c. */
#define SMALL_STR_SIZE 4095

struct lines
{
Expand All @@ -37,14 +40,26 @@ int
main (int argc, char *argv[])
{
int result = 0;
bool nocache = false;
size_t nstrings, nstrings_max;
struct lines *strings;
char *line = NULL;
size_t len = 0;
size_t n;

if (argc < 2)
error (1, 0, "usage: %s <random seed>", argv[0]);
error (1, 0, "usage: %s <random seed> [-nocache]", argv[0]);

if (argc == 3)
{
if (strcmp (argv[2], "-nocache") == 0)
nocache = true;
else
{
printf ("Unknown option %s!\n", argv[2]);
exit (1);
}
}

setlocale (LC_ALL, "");

Expand All @@ -59,9 +74,9 @@ main (int argc, char *argv[])

while (1)
{
char saved, *newp;
int needed;
int l;
char saved, *word, *newp;
size_t l, line_len, needed;

if (getline (&line, &len, stdin) < 0)
break;

Expand All @@ -83,10 +98,35 @@ main (int argc, char *argv[])

saved = line[l];
line[l] = '\0';
needed = strxfrm (NULL, line, 0);

if (nocache)
{
line_len = strlen (line);
word = malloc (line_len + SMALL_STR_SIZE + 1);
if (word == NULL)
{
printf ("malloc failed: %m\n");
exit (1);
}
memset (word, ' ', SMALL_STR_SIZE);
memcpy (word + SMALL_STR_SIZE, line, line_len);
word[line_len + SMALL_STR_SIZE] = '\0';
}
else
word = line;

needed = strxfrm (NULL, word, 0);
newp = malloc (needed + 1);
strxfrm (newp, line, needed + 1);
if (newp == NULL)
{
printf ("malloc failed: %m\n");
exit (1);
}
strxfrm (newp, word, needed + 1);
strings[nstrings].xfrm = newp;

if (nocache)
free (word);
line[l] = saved;
++nstrings;
}
Expand Down
Loading

0 comments on commit 0f9e585

Please sign in to comment.