diff --git a/ChangeLog b/ChangeLog index 84ba7a28..09f657ad 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2011-08-29 Paul Smith + + * expand.c (variable_expand_string): Always allocate a new buffer + for a string we're expanding. The string we're working on can get + freed while we work on it (for example if it's the value of a + variable which modifies itself using an eval operation). + See Savannah patch #7534 for the original report by Lubomir Rintel. + 2011-06-12 Paul Smith * read.c (parse_file_seq): Move the check for empty members out of diff --git a/expand.c b/expand.c index d1404b94..f5b6b996 100644 --- a/expand.c +++ b/expand.c @@ -197,7 +197,7 @@ variable_expand_string (char *line, const char *string, long length) { struct variable *v; const char *p, *p1; - char *abuf = NULL; + char *save; char *o; unsigned int line_offset; @@ -212,16 +212,11 @@ variable_expand_string (char *line, const char *string, long length) return (variable_buffer); } - /* If we want a subset of the string, allocate a temporary buffer for it. - Most of the functions we use here don't work with length limits. */ - if (length > 0 && string[length] != '\0') - { - abuf = xmalloc(length+1); - memcpy(abuf, string, length); - abuf[length] = '\0'; - string = abuf; - } - p = string; + /* We need a copy of STRING: due to eval, it's possible that it will get + freed as we process it (it might be the value of a variable that's reset + for example). Also having a nil-terminated string is handy. */ + save = length < 0 ? xstrdup (string) : xstrndup (string, length); + p = save; while (1) { @@ -411,8 +406,7 @@ variable_expand_string (char *line, const char *string, long length) ++p; } - if (abuf) - free (abuf); + free (save); variable_buffer_output (o, "", 1); return (variable_buffer + line_offset); diff --git a/tests/ChangeLog b/tests/ChangeLog index d800b499..32213bbe 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,8 @@ +2011-08-29 Paul Smith + + * scripts/features/varnesting: Test resetting of variables while + expanding them. See Savannah patch #7534 + 2011-06-12 Paul Smith * scripts/features/archives: Check archives with whitespace at the diff --git a/tests/scripts/features/varnesting b/tests/scripts/features/varnesting index 15d50711..d8f3ffbb 100644 --- a/tests/scripts/features/varnesting +++ b/tests/scripts/features/varnesting @@ -1,29 +1,30 @@ -$description = "The following test creates a makefile to ..."; +# -*-perl-*- +$description = "Test recursive variables"; $details = ""; -open(MAKEFILE,"> $makefile"); +run_make_test(' +x = variable1 +variable2 := Hello +y = $(subst 1,2,$(x)) +z = y +a := $($($(z))) +all: + @echo $(a) +', + '', "Hello\n"); -# The Contents of the MAKEFILE ... +# This tests resetting the value of a variable while expanding it. +# You may only see problems with this if you're using valgrind or +# some other memory checker that poisons freed memory. +# See Savannah patch #7534 -print MAKEFILE "x = variable1\n" - ."variable2 := Hello\n" - ."y = \$(subst 1,2,\$(x))\n" - ."z = y\n" - ."a := \$(\$(\$(z)))\n" - ."all: \n" - ."\t\@echo \$(a)\n"; - -# END of Contents of MAKEFILE - -close(MAKEFILE); - -&run_make_with_options($makefile,"",&get_logfile); - -# Create the answer to what should be produced by this Makefile -$answer = "Hello\n"; - -&compare_output($answer,&get_logfile(1)); +run_make_test(' +VARIABLE = $(eval VARIABLE := echo hi)$(VARIABLE) +wololo: + @$(VARIABLE) +', + '', "hi\n"); 1;