From fd66eeefaf18f00c5460ab7c329677644c8b7f15 Mon Sep 17 00:00:00 2001
From: Timothy E Baldwin <T.E.Baldwin99@members.leeds.ac.uk>
Date: Mon, 25 Jan 2021 18:19:44 +0000
Subject: [PATCH] Fix OS_EvaluateExpression

OS_EvaluateExpression is documented as applying GSTrans to any strings
within the expression, but did instead GSTrans the entire expression.

This would result in a sequence such as:

    *Set Alias$@RunType_FD1 "Basic -quit ""%0"" %*1"
    *If "<Alias$@RunType_FD1>"="" Then Set Alias$@RunType_FD1 @RunType_FFB %*0

raising a "Unknown operand" error as the quote in the variable
were interpreted as expression syntax. See
https://www.riscosopen.org/forum/forums/5/topics/16127

Unfortunately skipping the initial GSTrans breaks common code such as:

    If "<StrED_cfg$Dir>" = "" AND <Ctrl$Pressed> = 0 Then ...
    If "<Zap$OSVsn>" <> "" Then SetEval ZapFonts$OSVsn <Zap$OSVsn>

Fix by applying a GSTrans transformation over unquoted parts of the
expression, then applying GSTrans over the quoted strings.

Version 6.51. Tagged as 'Kernel-6_51'
---
 VersionASM |  12 +++---
 VersionNum |  20 +++++-----
 s/Arthur2  |   3 ++
 s/Arthur3  | 107 +++++++++++++++++++++++++++++++++++++++--------------
 4 files changed, 99 insertions(+), 43 deletions(-)

diff --git a/VersionASM b/VersionASM
index 06e742a..5f3ed41 100644
--- a/VersionASM
+++ b/VersionASM
@@ -9,12 +9,12 @@
                         GBLS    Module_ApplicationDate
                         GBLS    Module_HelpVersion
                         GBLS    Module_ComponentName
-Module_MajorVersion     SETS    "6.50"
-Module_Version          SETA    650
+Module_MajorVersion     SETS    "6.51"
+Module_Version          SETA    651
 Module_MinorVersion     SETS    ""
-Module_Date             SETS    "30 Jan 2021"
-Module_ApplicationDate  SETS    "30-Jan-21"
+Module_Date             SETS    "10 Feb 2021"
+Module_ApplicationDate  SETS    "10-Feb-21"
 Module_ComponentName    SETS    "Kernel"
-Module_FullVersion      SETS    "6.50"
-Module_HelpVersion      SETS    "6.50 (30 Jan 2021)"
+Module_FullVersion      SETS    "6.51"
+Module_HelpVersion      SETS    "6.51 (10 Feb 2021)"
                         END
diff --git a/VersionNum b/VersionNum
index c0a5771..1e72e5a 100644
--- a/VersionNum
+++ b/VersionNum
@@ -1,21 +1,21 @@
-/* (6.50)
+/* (6.51)
  *
  * This file is automatically maintained by srccommit, do not edit manually.
  *
  */
-#define Module_MajorVersion_CMHG        6.50
+#define Module_MajorVersion_CMHG        6.51
 #define Module_MinorVersion_CMHG
-#define Module_Date_CMHG                30 Jan 2021
+#define Module_Date_CMHG                10 Feb 2021
 
-#define Module_MajorVersion             "6.50"
-#define Module_Version                  650
+#define Module_MajorVersion             "6.51"
+#define Module_Version                  651
 #define Module_MinorVersion             ""
-#define Module_Date                     "30 Jan 2021"
+#define Module_Date                     "10 Feb 2021"
 
-#define Module_ApplicationDate          "30-Jan-21"
+#define Module_ApplicationDate          "10-Feb-21"
 
 #define Module_ComponentName            "Kernel"
 
-#define Module_FullVersion              "6.50"
-#define Module_HelpVersion              "6.50 (30 Jan 2021)"
-#define Module_LibraryVersionInfo       "6:50"
+#define Module_FullVersion              "6.51"
+#define Module_HelpVersion              "6.51 (10 Feb 2021)"
+#define Module_LibraryVersionInfo       "6:51"
diff --git a/s/Arthur2 b/s/Arthur2
index 3d7a062..947a5ee 100644
--- a/s/Arthur2
+++ b/s/Arthur2
@@ -465,6 +465,9 @@ GS_StackLimitPos * 19           ; The bit position of the LSB of the byte
 
 ; After GSINIT, R2 has these flags, and if expanding a count in the low byte
 
+; The flags GS_ReadingString and GS_Macroing returned by GSREAD are used by
+; ReadExpression in Arthur3.
+
 GSINIT  ROUT
 ;  In  : R0 pointer to string to expand
 ;        R2 has flags :
diff --git a/s/Arthur3 b/s/Arthur3
index 69a2ed0..09f0974 100644
--- a/s/Arthur3
+++ b/s/Arthur3
@@ -267,20 +267,82 @@ ExprBuffOFlo ROUT
         Pull    "R0-R4, lr"
         B       SLVK_SetV
 
+ReadExpression_Error
+        LDR     R5, [stack], #8
+      [ International
+        BL      TranslateError
+      ]
+        Pull    "R1-R4, lr"
+        B       SLVK_SetV
+
 ReadExpression ROUT
         Push    "R0-R4, lr"
         CLRPSR  I_bit, R12    ; interrupts on, ta.
         LDR     R12, =ExprWSpace
         STR     R13, ExprSVCstack
-        LDR     R1, =ExprBuff
-        MOV     R2, #LongCLISize
-        ORR     R2, R2, #(1 :SHL: 30) :OR: (1 :SHL: 31)
-        SWI     XOS_GSTrans   ; No | transformation, no " or space termination.
-                              ; so can never go wrong!
-        BCS     ExprBuffOFlo
+
+        ; Historically in violation of the behaviour documented in the
+        ; Programmer's Reference Manual the expression was passed to
+        ; OS_GSTrans, which would transform a reasonable expression such as
+        ; "<Alias$@RunType_FFB>"="" into "Basic -quit "%0" %*1"="" which
+        ; is not a valid expression.
+        ;
+        ; Unfortunately merely removing this transformation breaks
+        ; software that references system variable using angle brackets
+        ; but not quote marks. eg. SetEval ZapFonts$OSVsn <Zap$OSVsn>
+        ;
+        ; Instead process the unquoted parts of the expression using
+        ; OS_GSRead, using the internal flags to detect when a variable
+        ; is being expanded.
+
+        ; Register allocation for code below:
+        ; R0 = Pointer to expression passed by caller
+        ; R1 = Character read
+        ; R2 = GSRead flags
+        ; R3 = pointer into ExprBuff
+        ; R4 = bytes remaining in output buffer - 1
+        ; R5 = flag to indicate not in quoted string
+
+        Push    "R5"
+        LDR     R3, =ExprBuff
+        MOV     R4, #LongCLISize
+        MOV     R2, #GS_NoQuoteMess | GS_NoVBarStuff
+        SUBS    R4, R4, #2
+        SWI     XOS_GSInit
+        BVS     ReadExpression_Error
+        MOV     R5, #0x100
+
+00      ; If OS_GSRead is reading a system variable, or outside quotes
+        ; then use OS_GSRead, else read directly.
+        TST     R2, #GS_ReadingString | GS_Macroing
+        TSTEQ   R5, #0x100 ; Clears C
+        LDREQB  R1, [R0], #1
+        SWINE   XOS_GSRead
+        BVS     ReadExpression_Error
+
+        ; Check for end of expression
+        BCS     %FT00
+        CMP     R1, #13
+        CMPNE   R1, #10
+        CMPNE   R1, #0
+        BEQ     %FT00
+
+        ; Write character into buffer, check for overflow.
+        STRB    R1, [R3], #1
+        SUBS    R4, R4, #1
+        BMI     ExprBuffOFlo
+
+        ; Toggle quote flag if needed and continue loop
+        TEQ     R1, #""""
+        EOREQ   R5, R5, #0x100
+        B       %BT00
+
+00      ; Write terminator
         MOV     R0, #13
-        STRB    R0, [R1, R2]
+        STRB    R0, [R3]
 
+        LDR     R1, =ExprBuff
+        Pull    "R5"
         LDR     R11, =ExprStackStart
         LDR     R10, =ExprStackLimit
         MOV     R0, #0
@@ -1085,28 +1147,19 @@ DivZeroErr
         B       ExprErrCommon
         MakeErrorBlock DivZero
 
-04
-        LDR     R2, =exprSTRACC
-05
-        LDRB    R0, [R1], #1
-        CMP     R0, #13
-        CMPNE   R0, #10
-        CMPNE   R0, #0
-        BEQ     BadStringErr
-        CMP     R0, #""""
-        BEQ     %FT06
-07
-        STRB    R0, [R2], #1    ; can't overflow - comes from buffer
-        B       %BT05
+05      LDR     R13, ExprSVCstack
+        B       BumNumber2
 
-06
-        LDRB    R0, [R1], #1
-        CMP     R0, #""""
-        BEQ     %BT07
-        SUB     R1, R1, #1
-        LDR     R0, =exprSTRACC
-        SUB     R2, R2, R0      ; length to R2
+04
         Push    "lr"
+        SUB     R0, R1, #1
+        LDR     R1, =exprSTRACC
+        MOV     R2, #LongCLISize
+        ORR     R2, R2, #GS_NoVBarStuff | GS_Spc_term
+        SWI     XOS_GSTrans
+        BCS     ExprBuffOFlo
+        BVS     %BT05
+        MOV     R1, R0
         BL      Push_String
         ePush   "R0, R2"
         Pull    "PC"
-- 
GitLab