CREXX icon indicating copy to clipboard operation
CREXX copied to clipboard

Value function fails to distinguish variables in different levels

Open Peter-Jacob opened this issue 11 months ago • 7 comments

I have reviewed @mikebeer's request regarding the variable scopes of nested calls. Assume we have the following rexx:

/* Stress test of Value function */
options levelb
import rxfnsb
## Main Procedutre
  city='Munich'
  country='Germany'
  database='None'
  call level2
exit

## level 2 procedure  
level2: procedure = .string
  decade=100
  pi=3.1415926
  mypi=pi
  call level3
  database='SQL'
return ""
## level 3 procedure  
level3: procedure = .string
  database='DB2'
  access1='VSAM'
  access2='BDAM'
  database='Oracle'
  say 999 value("database")
return ""

VALUE is called in the level 3 proc and doesn't find any value for "database". The reason is caused the following clause:

if pos("."reg"@",symbol"@") > 0 then do / TODO - Rough and ready find /

do i = 1 to meta_array
        assembler linkattr1 meta_entry,meta_array,i
        if meta_entry = ".meta_clear" then do /* Object type */
           assembler linkattr1 symbol,meta_entry,1
           if pos("."reg"@",symbol"@") > 0 then do /* TODO - Rough and ready find */**
              leave a
           end
        end 
...

If the field database is defined and set at a higher level, clearing that definition will terminate the search, even if a new setting is specified at a lower level. I then removed the leave a statement, allowing the search to proceed. Instead of retrieving the latest database value of "Oracle," the value "SQL" is returned, which hasn't been set at this point, as it will be assigned only after returning from the level 3 procedure.

I assumed that the code sequence:

  assembler metaloadcalleraddr address_object /* Address from where are we called */
  assembler linkattr1 module,address_object,1  /* 1 = Module number */
  assembler linkattr1 addr,address_object,2 /* 2 = Address in module */

  /* Read the addresses backwards */
  do a = addr to 0 by -1
     /* Get the metadata for that address */
     assembler metaloaddata meta_array,module,a
     do i = 1 to meta_array
        assembler linkattr1 meta_entry,meta_array,i

runs through the active procedure frames in reverse order and can access the variable contents of each frame, though this may be a misinterpretation. Could you please clarify this for me?

Peter-Jacob avatar Jan 20 '25 17:01 Peter-Jacob

Very interesting - what does the rxas file look like? That shows the ,meta statements

On Mon, 20 Jan 2025 at 17:07, Peter-Jacob @.***> wrote:

I have reviewed Mike's request regarding the variable scopes of nested calls. Assume we have the following rexx:

/* Stress test of Value function */ options levelb import rxfnsb

Main Procedutre

city='Munich' country='Germany' database='None' call level2 exit

level 2 procedure

level2: procedure = .string decade=100 pi=3.1415926 mypi=pi call level3 database='SQL' return ""

level 3 procedure

level3: procedure = .string database='DB2' access1='VSAM' access2='BDAM' database='Oracle' say 999 value("database") return ""

VALUE is called in the level 3 proc and doesn't find any value for "database". The reason is caused the following clause:

if pos("."reg"@",symbol"@") > 0 then do / TODO - Rough and ready find /

do i = 1 to meta_array assembler linkattr1 meta_entry,meta_array,i if meta_entry = ".meta_clear" then do /* Object type / assembler linkattr1 symbol,meta_entry,1 if pos("."reg"@",symbol"@") > 0 then do / TODO - Rough and ready find /* leave a end end ...

If the field database is defined and set at a higher level, clearing that definition will terminate the search, even if a new setting is specified at a lower level. I then removed the leave a statement, allowing the search to proceed. Instead of retrieving the latest database value of "Oracle," the value "SQL" is returned. I assumed that the code sequence:

assembler metaloadcalleraddr address_object /* Address from where are we called / assembler linkattr1 module,address_object,1 / 1 = Module number / assembler linkattr1 addr,address_object,2 / 2 = Address in module */

/* Read the addresses backwards / do a = addr to 0 by -1 / Get the metadata for that address */ assembler metaloaddata meta_array,module,a do i = 1 to meta_array assembler linkattr1 meta_entry,meta_array,i

I assumed it runs through the active procedure frames in reverse order and can access the variable contents of each frame, though this may be a misinterpretation. Could you please clarify this for me?

— Reply to this email directly, view it on GitHub https://github.com/adesutherland/CREXX/issues/425, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABBKI6UXQLLPAYLHGTEJKND2LUULVAVCNFSM6AAAAABVQZ2UBGVHI2DSMVQWIX3LMV43ASLTON2WKOZSG44TSOJTGE2DCNQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

adesutherland avatar Jan 20 '25 17:01 adesutherland

Actually it is "as designed" because I think it was only intended to get the level below. You would need to make it loop to a lower level first (if that makes sense) (which you can't unless /until - we change metaloadcalleraddr)

The logic is this: when "call level3" is done "database" is not defined (it is defined in the next line) and "database" in level1 is not in scope in level 2 (in crexx level b). So what the code is doing is working backwards through the addresses - each address can have more than one metadata line (that is what the meta_array is handling). The clear is the clear at the end of the previous procedure. But let's look at the rxas just to make sure I remember this right!

A

So

A

On Mon, 20 Jan 2025 at 17:40, Adrian Sutherland < @.***> wrote:

Very interesting - what does the rxas file look like? That shows the ,meta statements

On Mon, 20 Jan 2025 at 17:07, Peter-Jacob @.***> wrote:

I have reviewed Mike's request regarding the variable scopes of nested calls. Assume we have the following rexx:

/* Stress test of Value function */ options levelb import rxfnsb

Main Procedutre

city='Munich' country='Germany' database='None' call level2 exit

level 2 procedure

level2: procedure = .string decade=100 pi=3.1415926 mypi=pi call level3 database='SQL' return ""

level 3 procedure

level3: procedure = .string database='DB2' access1='VSAM' access2='BDAM' database='Oracle' say 999 value("database") return ""

VALUE is called in the level 3 proc and doesn't find any value for "database". The reason is caused the following clause:

if pos("."reg"@",symbol"@") > 0 then do / TODO - Rough and ready find /

do i = 1 to meta_array assembler linkattr1 meta_entry,meta_array,i if meta_entry = ".meta_clear" then do /* Object type / assembler linkattr1 symbol,meta_entry,1 if pos("."reg"@",symbol"@") > 0 then do / TODO - Rough and ready find /* leave a end end ...

If the field database is defined and set at a higher level, clearing that definition will terminate the search, even if a new setting is specified at a lower level. I then removed the leave a statement, allowing the search to proceed. Instead of retrieving the latest database value of "Oracle," the value "SQL" is returned. I assumed that the code sequence:

assembler metaloadcalleraddr address_object /* Address from where are we called / assembler linkattr1 module,address_object,1 / 1 = Module number / assembler linkattr1 addr,address_object,2 / 2 = Address in module */

/* Read the addresses backwards / do a = addr to 0 by -1 / Get the metadata for that address */ assembler metaloaddata meta_array,module,a do i = 1 to meta_array assembler linkattr1 meta_entry,meta_array,i

I assumed it runs through the active procedure frames in reverse order and can access the variable contents of each frame, though this may be a misinterpretation. Could you please clarify this for me?

— Reply to this email directly, view it on GitHub https://github.com/adesutherland/CREXX/issues/425, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABBKI6UXQLLPAYLHGTEJKND2LUULVAVCNFSM6AAAAABVQZ2UBGVHI2DSMVQWIX3LMV43ASLTON2WKOZSG44TSOJTGE2DCNQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

adesutherland avatar Jan 20 '25 18:01 adesutherland

Here is the assembly: I removed the value function (which is internally used to reflect changes in meta_clear). If I understand correctly, no procedure frame is used to extract the existing variables. I assume that by deleting the meta_clear branch, it resulted in retrieving the database field from the level 2 procedure. So, while my change fixed one issue, it introduced another.

I have a feeling the problem becomes even more complex when considering recursive calls, where the module appears multiple times in the stack.

You would need to make it loop to a lower level first (if that makes sense) (which you can't unless /until - we change metaloadcalleraddr) Yes, the logic is clear to me. The question is, does it fully represent a caller->procedure frame, also considering recursion and the variables involved? My

/*
 * cREXX COMPILER VERSION : crexx-F0049
 * SOURCE                 : vpool_test.rexx
 * BUILT                  : 2025-01-20 17:55:15
 */

.srcfile="vpool_test.rexx"
.globals=0

main() .locals=5
   .meta "vpool_test.main"="b" ".void" main() "" ""
   .src 9:3="call level2"
   .meta "vpool_test.main.city"="b" ".string" "Munich"
   .meta "vpool_test.main.country"="b" ".string" "Germany"
   .meta "vpool_test.main.database"="b" ".string" "None"
   load r1,0
   call r2,level2(),r1
   .src 10:1="exit"
   load r3,1
   settp r4,2
   call r1,_exit(),r3
   .src 10:5=""
   ret
   .meta "vpool_test.main.city"
   .meta "vpool_test.main.country"
   .meta "vpool_test.main.database"

level2() .locals=3
   .meta "vpool_test.level2"="b" ".string" level2() "" ""
   .src 13:1="level2: procedure = .string"
   .src 17:3="call level3"
   .meta "vpool_test.level2.decade"="b" ".int" "100"
   .meta "vpool_test.level2.mypi"="b" ".float" "3.1415926"
   .meta "vpool_test.level2.pi"="b" ".float" "3.1415926"
   load r1,0
   call r2,level3(),r1
   .src 19:1="return \"\""
   .meta "vpool_test.level2.database"="b" ".string" "SQL"
   ret ""
   .meta "vpool_test.level2.database"
   .meta "vpool_test.level2.decade"
   .meta "vpool_test.level2.mypi"
   .meta "vpool_test.level2.pi"

level3() .locals=5
   .meta "vpool_test.level3"="b" ".string" level3() "" ""
   .src 21:1="level3: procedure = .string"
   .src 22:3="database='DB2'"
   .meta "vpool_test.level3.database"="b" ".string" r1
   load r1,"DB2"
   .src 25:3="database='Oracle'"
   .meta "vpool_test.level3.access1"="b" ".string" "VSAM"
   .meta "vpool_test.level3.access2"="b" ".string" "BDAM"
   load r1,"Oracle"
   .src 26:3="say 999 value(\"database\")"
   load r2,1
   load r3,"database"
   settp r3,2
   call r4,value(),r2
   sconcat r4,"999",r4
   say r4
   .src 27:1="return \"\""
   ret ""
   .meta "vpool_test.level3.access1"
   .meta "vpool_test.level3.access2"
   .meta "vpool_test.level3.database"

Peter-Jacob avatar Jan 21 '25 06:01 Peter-Jacob

Please help me understand this code. From what I gather, the metalloadcalleraddr and linkattr instructions address the caller's procedure frame. I initially assumed that iterating through all existing addresses in reverse would correspond to traversing all procedure frames. However, I suspect my assumption might be incorrect.

assembler metaloadcalleraddr address_object /* Address from where are we called */
assembler linkattr1 module,address_object,1  /* 1 = Module number */
assembler linkattr1 addr,address_object,2 /* 2 = Address in module */
/* Read the addresses backwards */
   do level = addr to 0 by -1
  /* Get the metadata for that address */
      assembler metaloaddata meta_array,module,level
...

In my opinion, we need a method to:

  1. Address the caller's address frame.
  2. Link the registers and constants to the metadata.
  3. Retrieve the variable names and their contents.
  4. Access the caller’s caller address (if applicable).
  5. Repeat from step 1.

I could be mistaken, but I believe we are already close to this solution.

Peter-Jacob avatar Jan 21 '25 17:01 Peter-Jacob

I initially assumed that iterating through all existing addresses in reverse would correspond to traversing all procedure frames

You are right that this is not the solution because procedures can call procedures in any order in the source code, indeed across modules/files.

*IF *we want to do this we would need an enhanced version of metaloadcalleraddr to go back more than one stack frame. The problem I have is I think it is a super unsafe instruction. Notwithstanding what you can do in REXXSAA you don't really want a function (and especially an external module) to be able to inspect (or worse update) any of a program's variables. Anyway, I think that is a discussion we should have in our next session - I am open to the debate for sure, just explaining my reticence ...

A

On Tue, 21 Jan 2025 at 17:23, Peter-Jacob @.***> wrote:

Please help me understand this code. From what I gather, the metalloadcalleraddr and linkattr instructions address the caller's procedure frame. I initially assumed that iterating through all existing addresses in reverse would correspond to traversing all procedure frames. However, I suspect my assumption might be incorrect.

assembler metaloadcalleraddr address_object /* Address from where are we called / assembler linkattr1 module,address_object,1 / 1 = Module number / assembler linkattr1 addr,address_object,2 / 2 = Address in module / / Read the addresses backwards / do level = addr to 0 by -1 / Get the metadata for that address */ assembler metaloaddata meta_array,module,level ...

In my opinion, we need a method to:

  1. Address the caller's address frame.
  2. Link the registers and constants to the metadata.
  3. Retrieve the variable names and their contents.
  4. Access the caller’s caller address (if applicable).
  5. Repeat from step 1.

I could be mistaken, but I believe we are already close to this solution.

— Reply to this email directly, view it on GitHub https://github.com/adesutherland/CREXX/issues/425#issuecomment-2605330052, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABBKI6UVLKIIDWE5SN6LX232LZ67ZAVCNFSM6AAAAABVQZ2UBGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMBVGMZTAMBVGI . You are receiving this because you commented.Message ID: @.***>

adesutherland avatar Jan 21 '25 21:01 adesutherland

First, we need to distinguish the standard VALUE function from Regina's version. In the standard implementation, VALUE retrieves the content of a variable provided as a parameter. Your initial version achieves this functionality after some minor adjustments.

Regina's version, however, includes the ability to access variable scopes of any calling procedures, which is a useful but relatively uncommon feature. By contrast, BREXX does not support this option in its VALUE function. However, we implemented this capability as a separate function. My approach would be to retain the minimal VALUE function as it is now and only branch to the extended version, scanning all call levels, only if necessary. This ensures the standard functionality remains lightweight while providing additional flexibility when required. Certainly, we still need to decide how thread-safe and data-safe the extended version needs to be and whether we allow its use in a REXXSAA call. These considerations will help determine the scope and limitations of the extended functionality. However, considering the implications of updating variables in a scope other than the actual one, I would disallow this behaviour altogether.

Peter-Jacob avatar Jan 21 '25 22:01 Peter-Jacob

I'll draft you a version of metaloadcalleraddr with an extra argument to specify the depth if you like. Then we can at least see it in action

adesutherland avatar Jan 21 '25 22:01 adesutherland

This will be needed for level c but not level b.

adesutherland avatar Jun 20 '25 20:06 adesutherland

Fully agree, let's postpone it for level c

Peter-Jacob avatar Jun 23 '25 06:06 Peter-Jacob