Bug reports for the GNU m4 macro processor

john | 19 Jan 2006 03:37

dangling pointer bug and proposed patch

Hello,

There is a dangling pointer bug in GNU m4, exposed by this example:

$ m4 -dqeat
define(`f',`one')f(undefine(`f')`two')
m4trace: -1- define(`f', `one')
m4trace: -2- undefine(`f')
m4trace: -1- øò·øò·(`two') -> `øò·øò·H'
øò·øò·H

The problem is that undefining a macro while it is in the "collect
arguments" phase leaves a dangling symbol pointer on the stack.  Bug
aside, the GNU m4 info file doesn't seem to specify what *should*
happen in this situation.  I propose that when a macro whose arguments
have just been expanded discovers that its pre-argument-expansion
definition has been deleted, it should use the "current" definition
(i.e. the same definition a new invocation of the macro would use).
If no current definition exists, the macro should use the expansion
$0($ <at> ), which treats the macro as effectively undefined, except that
leading whitespace is stripped from the arguments.

I've written up the gory details (mostly to help me make sure I
understand them), including justification of the proposed behaviour,
in the attached PDF.

A patch for the proposed fix against m4-1.4.4 is also attached.
(The branch name m4-1.4.4.1 is just for my own convenience.)

Regards,

(Continue reading)

Eric Blake | 10 May 2006 15:23

Re: dangling pointer bug and proposed patch


According to john on 1/18/2006 7:37 PM:
> Hello,
> 
> There is a dangling pointer bug in GNU m4, exposed by this example:
> 
> $ m4 -dqeat
> define(`f',`one')f(undefine(`f')`two')
> m4trace: -1- define(`f', `one')
> m4trace: -2- undefine(`f')
> m4trace: -1- øò·øò·(`two') -> `øò·øò·H'
> øò·øò·H
...
> I've written up the gory details (mostly to help me make sure I
> understand them), including justification of the proposed behaviour,
> in the attached PDF.

I see your copyright assignment went through in March; sorry it has taken
so long, but I am finally looking at this patch.  Thanks for an excellent
bug report!

First, I claim that some of the behavior you call unspecified in section 2
is actually specified.  For example, you mention

define(`f',`one')f(define(`f',`two'))

According to POSIX, "Macros may have arguments, in which case the
arguments shall be substituted into the defining text before it is
rescanned."  So the argument "define(`f',`two')" is expanded to void, then
plugged into the defining text of f (which is now `two'), resulting in

(Continue reading)

Eric Blake | 11 May 2006 00:03

Re: dangling pointer bug and proposed patch

Eric Blake <ebb9 <at> byu.net> writes:

Answering myself,

> First, I claim that some of the behavior you call unspecified in section 2
> is actually specified.  For example, you mention
> 
> define(`f',`one')f(define(`f',`two'))
> 
> According to POSIX, "Macros may have arguments, in which case the
> arguments shall be substituted into the defining text before it is
> rescanned."  So the argument "define(`f',`two')" is expanded to void, then
> plugged into the defining text of f (which is now `two'), resulting in
> "two" as you note.

I went one step further, and compared against Solaris 8's /usr/
{ccs,xpg4}/bin/m4, with surprising results:

In a slight modification of your third example,

define(`f',1)f(pushdef(`f',2)f(define(`f',3)))f`'popdef(`f')f

GNU m4 gives "131", but Solaris gave "33f".  This seems like a Solaris bug, 
since the pushdef failed to preserve the definition of f being "1".

Then, on your proposals in section 3,

define(`f',1)f(undefine(`f')2)f`'popdef(`f')f

Your proposal would give "f(2)ff", but Solaris gave "1ff".  In other words,

(Continue reading)

Eric Blake | 11 May 2006 00:59

Re: dangling pointer bug and proposed patch

Eric Blake <ebb9 <at> byu.net> writes:

> Answering myself,
yet again,

> I went one step further, and compared against Solaris 8's /usr/
> {ccs,xpg4}/bin/m4, with surprising results:
> 
> In a slight modification of your third example,
> 
> define(`f',1)f(pushdef(`f',2)f(define(`f',3)))f`'popdef(`f')f
> 
> GNU m4 gives "131", but Solaris gave "33f".  This seems like a Solaris bug, 
> since the pushdef failed to preserve the definition of f being "1".

On the other hand, maybe it is what POSIX intended.  The NEWS for CVS head 
claims that "* POSIXLY_CORRECT and `m4 --traditional' now makes the `define' 
builtin replace all `pushdef'ed values of a macro, as POSIX requires."  In 
light of that, Solaris' behavior makes perfect sense - the inner define wipes 
out the stack of definitions, leaving only a single definition for f.  Then the 
outer f uses the (one and only) current definition for f, and the popdef 
removes the last definition.

But trying that on CVS head of GNU m4 shows that we have regressed from 1.4.x:

$ POSIXLY_CORRECT=1 ~/m4-head/tests/m4 -dqeat
define(`f',1)f(pushdef(`f',2)define(`f',3))f
/home/eblake/m4-head/tests/m4: line 7:  6100 Segmentation fault      (core 
dumped) "/home/eblake/m4-head/src/m4" --module-directory="/home/eblake/m4-
head/modules" ${1+"$ <at> "} 2>/tmp/m4-$$

(Continue reading)

Eric Blake | 3 Jun 2006 22:51

Re: dangling pointer bug and proposed patch


According to Eric Blake on 5/10/2006 4:59 PM:
>> In a slight modification of your third example,
>>
>> define(`f',1)f(pushdef(`f',2)f(define(`f',3)))f`'popdef(`f')f
>>
>> GNU m4 gives "131", but Solaris gave "33f".  This seems like a Solaris bug, 
>> since the pushdef failed to preserve the definition of f being "1".
> 
> On the other hand, maybe it is what POSIX intended.  The NEWS for CVS head 
> claims that "* POSIXLY_CORRECT and `m4 --traditional' now makes the `define' 
> builtin replace all `pushdef'ed values of a macro, as POSIX requires."  In 
> light of that, Solaris' behavior makes perfect sense - the inner define wipes 
> out the stack of definitions, leaving only a single definition for f.  Then the 
> outer f uses the (one and only) current definition for f, and the popdef 
> removes the last definition.

Also, notice that m4 1.4.4 does the following inconsistent behavior:
define(`f',`1')f(popdef(`f')pushdef(`f',`2'))
1
define(`g',`1')g(define(`g',`2'))
2

So, the following patch is my proposed solution to the original problem of
core dumps when undefining a symbol while it is still in use by a pending
expansion.  It currently requires that you manually apply the two patches
in this thread first:
http://lists.gnu.org/archive/html/bug-m4/2006-06/msg00002.html

I will apply it in a couple of days if I don't hear any feedback, at which

(Continue reading)

Eric Blake | 6 Jun 2006 14:38

Re: dangling pointer bug and proposed patch


According to Eric Blake on 6/3/2006 2:51 PM:
> I will apply it in a couple of days if I don't hear any feedback, at which
> point all known core dumps in 1.4.4 will be solved, and we can start
> thinking about releasing 1.4.5.
> 
> 2006-06-03  Eric Blake  <ebb9 <at> byu.net>
> 
> 	When changing macro definitions inside the arguments to the macro,
> 	consistently preserve the old definition that was in effect before
> 	argument collection, similar to the C pre-processor.
> 	* NEWS: Document this change.
> 	* doc/m4.texinfo (Macro Arguments, Undefine, Dumpdef): Document
> 	this policy, and add tests that expose core dumps prior to this
> 	patch.
> 	* src/m4.h (struct symbol): New members to track when a symbol is
> 	still in use after removal from the symbol table.
> 	(SYMBOL_PENDING_EXPANSIONS, SYMBOL_DELETED): Define.
> 	(free_symbol): Prototype.
> 	* src/macro.c (expand_macro): Track pending expansions of a
> 	symbol.  On completion, if a symbol is deleted and no longer
> 	pending, free its memory.
> 	* src/symtab.c (free_symbol): Export.  Don't free memory if symbol
> 	is still in use.
> 	(lookup_symbol) <SYMBOL_INSERT>: Create new entry when old entry
> 	is still in use.
> 	(lookup_symbol) <SYMBOL_DELETE, SYMBOL_POPDEF>: Mark entries still
> 	in use as deleted and remove from the table without freeing
> 	memory.
> 	(symtab_debug, symtab_print_list): Fix for debugging.

(Continue reading)

Eric Blake | 5 Sep 2006 15:25

Re: dangling pointer bug and proposed patch


According to Eric Blake on 6/3/2006 2:51 PM:
> Also, notice that m4 1.4.4 does the following inconsistent behavior:
> define(`f',`1')f(popdef(`f')pushdef(`f',`2'))
> 1
> define(`g',`1')g(define(`g',`2'))
> 2
> 
> So, the following patch is my proposed solution to the original problem of
> core dumps when undefining a symbol while it is still in use by a pending
> expansion.
> It will take some work to forward port a similar patch to CVS head.

And it is now finally ported.

2006-09-05  Eric Blake  <ebb9 <at> byu.net>

	* m4/macro.c (expansion_level, macro_call_id): Change to size_t.
	All users updated.
	(expand_token): Avoid assertion just added to docs.
	(expand_macro): Track pending expansions, for when a symbol's
	definition changes during argument collection.
	(m4_macro_call, process_macro): Operate on symbol value, not
	symbol, since symbol may have changed during argument collection.
	* m4/m4private.h (m4_symbol_value): Add pending_expansions member.
	(VALUE_PENDING, SYMBOL_PENDING, VALUE_DELETED_BIT): New defines.
	(m4_get_symbol_value): Add fast macro version.
	* m4/m4module.h (M4_BUILTIN_FLAGS_MASK): New enumerator.
	(m4_macro_call): Adjust prototype.
	* m4/module.c (install_builtin_table): Check that flags are valid

(Continue reading)

Eric Blake | 19 Jan 2006 14:18

Re: dangling pointer bug and proposed patch


[moving discussion to m4-patches]

According to john on 1/18/2006 7:37 PM:
> Hello,
> 
> There is a dangling pointer bug in GNU m4, exposed by this example:
> 
> $ m4 -dqeat
> define(`f',`one')f(undefine(`f')`two')
> m4trace: -1- define(`f', `one')
> m4trace: -2- undefine(`f')
> m4trace: -1- øò·øò·(`two') -> `øò·øò·H'
> øò·øò·H

Thanks for catching that, and a great analysis of the issue!

Also, could you check if the bug is still present on CVS head?

> 
> A patch for the proposed fix against m4-1.4.4 is also attached.
> (The branch name m4-1.4.4.1 is just for my own convenience.)

Unfortunately, the patch is too big to be considered trivial; are you
willing to assign copyright to the FSF?  This is a legal necessity before
your patch can be applied.

--
Life is short - so eat dessert first!

(Continue reading)

john | 21 Jan 2006 22:14

Re: dangling pointer bug and proposed patch

Hello Eric,

 > Unfortunately, the patch is too big to be considered trivial; are
 > you willing to assign copyright to the FSF?  This is a legal
 > necessity before your patch can be applied.
 > 

Of course. I hereby assert authorship of the "dangling pointer bug in
m4" patch, and assign copyright in it to the Free Software Foundation.
I also assert that I am not under any legal obligations which prevent
me from doing this.

Of files affected by this patch, only m4.h has any changes in the CVS
head, so the bug should still be present.  I merged the CVS head into
1.4.4, and the resulting build still has the bug.

(will that do?  I'm new at this.)

Regards,

John Brzustowski

Return

Return to gmane.comp.gnu.m4.bugs.

Search Archive

Language

Change language

Options

Current view: Threads only / Showing only 20 lines / Not hiding cited text.
Change to All messages, whole messages, or hide cited text.

Post a message
NNTP Newsgroup
Classic Gmane web interface
XML

XML

RSS Feed
List Information

About Gmane