From e62b5ffcb6c4a84449e12d706d990de3f8075882 Mon Sep 17 00:00:00 2001 From: Sei Lisa Date: Mon, 31 Oct 2022 18:23:00 +0100 Subject: [PATCH] Fix exception when a global references another without constfold Fixes #19. Thanks to SaladDais for the report and test case. With constant folding inactive, the dead code removal optimization was removing globals and their symbols when they were e.g. integer constants, without substituting them in other globals. This produced a crash when the output module tried to access said symbols. For example, in this code: ``` integer A = 1; integer B = A; default{timer(){llBreakLink(B);}} ``` the line `integer A = 1` was being removed, as well as the `A` global symbol, but the line `integer B = A` was retained with the missing symbol, and the output module crashed when trying to look up the `A` symbol. Apparently, it wasn't an issue when constant folding was active (which is why it went unnoticed for so long) because the constant folding code sets 'orig' in the symbol table to the original value the variable was being set to, which let the output module know what to output. The fix is to replace the references to deleted symbols with their values in global definitions. In normal code that was already happening. --- lslopt/lsldeadcode.py | 34 +++++++++++++++++++----- unit_tests/regression.suite/issue-19.lsl | 14 ++++++++++ unit_tests/regression.suite/issue-19.out | 11 ++++++++ unit_tests/regression.suite/issue-19.run | 1 + 4 files changed, 53 insertions(+), 7 deletions(-) create mode 100644 unit_tests/regression.suite/issue-19.lsl create mode 100644 unit_tests/regression.suite/issue-19.out create mode 100644 unit_tests/regression.suite/issue-19.run diff --git a/lslopt/lsldeadcode.py b/lslopt/lsldeadcode.py index 0e5c6f0..d93bbfb 100644 --- a/lslopt/lsldeadcode.py +++ b/lslopt/lsldeadcode.py @@ -387,12 +387,32 @@ class deadcode(object): return False + def ReplaceGlobalValues(self, parent, index): + """Recursively check global expressions (incl. lists, vectors...) and + if an identifier is scheduled for removal, replace it with its value. + """ + node = parent[index] + if node.ch is not None: + for i in range(len(node.ch)): + self.ReplaceGlobalValues(node.ch, i) + return # Nodes with children can't be identifiers to replace + + if node.nt == 'IDENT' and node.name in self.GlobalDeletions: + sym = self.symtab[node.scope][node.name] + parent[index] = sym['W'].copy() + + return + def CleanNode(self, curnode, isFnDef = False): """Recursively checks if the children are used, deleting those that are - not. + not. Global declarations are treated specially. """ - if curnode.ch is None or (curnode.nt == 'DECL' - and curnode.scope == 0): + if curnode.ch is None: + return + if curnode.nt == 'DECL' and curnode.scope == 0: + # Global declaration; if it references a removed symbol, replace it + # with its value. + self.ReplaceGlobalValues(curnode.ch, 0) return # NOTE: Should not depend on 'Loc', since the nodes that are the # destination of 'Loc' are renumbered as we delete stuff from globals. @@ -533,7 +553,7 @@ class deadcode(object): # Track removal of global lines, to reasign locations later. LocMap = list(range(len(self.tree))) - GlobalDeletions = [] + self.GlobalDeletions = [] # Perform the removal idx = 0 @@ -554,7 +574,7 @@ class deadcode(object): # that we will remove in CleanNode later, that hold the # original value. if node.nt == 'DECL' or node.nt == 'STDEF': - GlobalDeletions.append(node.name) + self.GlobalDeletions.append(node.name) del self.tree[idx] del LocMap[idx] else: @@ -562,10 +582,10 @@ class deadcode(object): self.CleanNode(node) # Remove the globals now. - for name in GlobalDeletions: + for name in self.GlobalDeletions: del self.symtab[0][name] - del GlobalDeletions + del self.GlobalDeletions # Reassign locations for name in self.symtab[0]: diff --git a/unit_tests/regression.suite/issue-19.lsl b/unit_tests/regression.suite/issue-19.lsl new file mode 100644 index 0000000..34bdf29 --- /dev/null +++ b/unit_tests/regression.suite/issue-19.lsl @@ -0,0 +1,14 @@ +integer FOO=1; +integer BAR=FOO; +integer BAZ=BAR; + +integer FEZ=2; +list L = [1,FEZ]; + +integer FUZ=3; + +default { + state_entry() { + llOwnerSay((string)BAZ+(string)L+(string)FUZ); + } +} diff --git a/unit_tests/regression.suite/issue-19.out b/unit_tests/regression.suite/issue-19.out new file mode 100644 index 0000000..e2ece1b --- /dev/null +++ b/unit_tests/regression.suite/issue-19.out @@ -0,0 +1,11 @@ +integer BAR = 1; +integer BAZ = BAR; +list L = [1, 2]; + +default +{ + state_entry() + { + llOwnerSay((string)BAZ + (string)L + (string)3); + } +} diff --git a/unit_tests/regression.suite/issue-19.run b/unit_tests/regression.suite/issue-19.run new file mode 100644 index 0000000..1a68111 --- /dev/null +++ b/unit_tests/regression.suite/issue-19.run @@ -0,0 +1 @@ +main.py -y -O -constfold -