Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

Re: [BUG & tentative PATCH] Invalid call to upscope in createparam



The second part of the test whose expected result your patch changed:
  () { typeset -n foo; foo=zz; local zz; foo=zz; print $bar $zz }
should in fact assign local zz=zz via the foo reference, because foo
is not an "upper" reference there.  Why did you think otherwise?

Here is the test example with comments showing what I think should happen:

bar=xx
typeset -n foo=bar
() {
  typeset -n foo;   # Creates a local "foo" placeholder ref.
  foo=zz;           # Initializes the local "foo" placeholder ref with "zz" (the name of a not-yet-defined variable).
  foo=zz || print -u2 foo: assignment failed;   # Creates the global variable "zz" and initializes it with "zz".
  print $bar $zz;   # Prints the content of the global variable "bar" and "zz", which results in "xx zz".
}
() {
  typeset -n foo;   # 
Creates a local "foo" placeholder ref.
  foo=zz;           # Initializes 
the local "foo" placeholder ref with a reference to the global "zz" variable.
  local zz;         # Creates a local "zz" variable initialized with "".
  foo=zz;           # Assigns "zz" to the global variable "zz".
  print $bar $zz;   # Prints the content of the global variable "bar" and the local variable "zz", which results in "xx".
}

Note that the global nameref "foo" isn't involved at all. Commenting it out produces the same result. The only effect of the global "foo" is to confuse the current implementation and make it output an incorrect result. If it's commented out, the current implementation produces the same result as my patch.

Here is what happens. The second "foo=zz" in the first function triggers a call to createparam with name="foo" and flags=0 (because the code for the assignment figured that "foo" didn't refer to any variable, it proceeds to first create the global variable referred to by "foo" to then assign "zz" to that variable). The call to createparam first looks up "foo" and finds the local "foo" defined in the function, which it stores in oldpm. The current code then updates oldpm with upscope(oldpm, oldpm->base), which returns the global "foo". Next, createparam checks whether oldpm refers to a nameref that refers to a not-yet-defined variable. It finds none (lastpm = NULL) and thus proceeds to create a global variable named "foo", which fails because there is already one.

With my patch, the initial oldpm is not updated. Thus, lastpm gets initialized with the local nameref "foo" and this leads createparam to update the name of the variable to create with "zz". Since there is no "zz" variable, createparam successfully creates one and the assignment then initializes it with "zz".

Here is a commented version of my example that lead to the discovery of the bug:

typeset -n ref=var1
() {
  typeset -n ref=var2;   # Creates a local "foo" ref.
  ref=RESET              # Creates a global variable "var2" and initializes it with "RESET".
  typeset -p ref var2
}
typeset -p ref var2

Here, the problem is caused by the call to createparam(name="ref", flags=0) triggered by the statement "ref=RESET". Like above, the current code updates oldpm from the local "ref" to the global "ref". This leads createparam to then create a global variable "var1". Then, the assignment fails to assign "RESET" to any variable because the local "ref" still refers to no variable. With my patch, createparam instead creates the global variable "var2".

More later today or tomorrow...

Philippe


 

On Sun, Jun 1, 2025 at 4:39 AM Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
On Sat, May 24, 2025 at 3:30 PM Philippe Altherr
<philippe.altherr@xxxxxxxxx> wrote:
>
> While I was looking into implementing the solution for hidden references, I found this call to upscope, which had to be wrong because it applies "base", a property of the referent, to the reference.

That particular call to upscope() is in fact looking for a property of
the reference.  The second parameter to upscope() says where to start
searching.  We're being asked to create a new non-reference parameter
that has the same name as an existing reference.  That means deleting
the reference and making the new assignment.  But if there's yet
another reference at a surrounding scope with the same name, then if
we delete + assign we'll instead assign through the outer reference,
which may be bad.  Instead the parameter at local scope has to be
re-created.

The first part of the test you changed isn't testing that behavior,
rather it's confirming than when the  local reference is a placeholder
hiding a global reference with a valid referent, that doesn't induce
an infinite loop.

The second part of the test whose expected result your patch changed:
  () { typeset -n foo; foo=zz; local zz; foo=zz; print $bar $zz }
should in fact assign local zz=zz via the foo reference, because foo
is not an "upper" reference there.  Why did you think otherwise?

> I don't fully understand how the stop parameter works, so better double-check this.

The stop parameter conveys three pieces of information:  (1) the name
of the reference that began the resolution (stop.name); (2) the
referent name where resolve_nameref should stop searching through a
reference chain (stop.value.scalar); and (3) whether the referent is
itself expected to be another named reference and/or a local in some
scope (stop->flags).  In several tries at this I can't find an example
in the context of your patch where the values in "stop" matter;
"&stop" just must be non-NULL.

I've added a bunch of comments to explain the above and also made the
failed assignment in the first case an explicit warning rather than
just setting $? = 1.  Patch in a separate message.


Messages sorted by: Reverse Date, Date, Thread, Author