Skip to content

Commit 24e44fd

Browse files
committed
[SYSTEMDS-3885] Fix robustness live variable analysis
This patch adds a new function potpourri test case which produced incorrect results in the past. Although the test itself ran fine, adding a test condition revealed a hidden robustness issue of live variable analysis where parameter builtin functions such as toString when used in predicates add 'null' objects (because they are not bound to variables) to the set of updated variables which in turn then crashes many things in the inter-procedural analysis.
1 parent 4d51d98 commit 24e44fd

File tree

3 files changed

+63
-1
lines changed

3 files changed

+63
-1
lines changed

src/main/java/org/apache/sysds/parser/VariableSet.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ public VariableSet( VariableSet vs ) {
3838
}
3939

4040
public void addVariable(String name, DataIdentifier id) {
41-
_variables.put(name,id);
41+
if( name != null ) // for robustness
42+
_variables.put(name,id);
4243
}
4344

4445
public void addVariables(VariableSet vs) {

src/test/java/org/apache/sysds/test/functions/misc/FunctionPotpourriTest.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ public class FunctionPotpourriTest extends AutomatedTestBase
6464
"FunPotpourriParforEvalSpark",
6565
"FunPotpourriEvalNamespace3",
6666
"FunPotpourriDefaultParams",
67+
"FunPotpourriListHandling"
6768
};
6869

6970
private final static String TEST_DIR = "functions/misc/";
@@ -283,6 +284,11 @@ public void testFunctionDefaultParams() {
283284
runFunctionTest( TEST_NAMES[30], null, false );
284285
}
285286

287+
@Test
288+
public void testFunctionListHandling() {
289+
runFunctionTest( TEST_NAMES[31], null, false );
290+
}
291+
286292
private void runFunctionTest(String testName, Class<?> error) {
287293
runFunctionTest(testName, error, false);
288294
}
@@ -306,6 +312,8 @@ private void runFunctionTest(String testName, Class<?> error, boolean evalRewrit
306312
Assert.assertTrue(heavyHittersContainsString(Opcodes.PRINT.toString()));
307313
if( evalRewrite && !testName.equals(TEST_NAMES[28]) )
308314
Assert.assertTrue(!heavyHittersContainsString(Opcodes.EVAL.toString()));
315+
if( testName.equals(TEST_NAMES[31]) ) //print used for error
316+
Assert.assertFalse(heavyHittersContainsString(Opcodes.PRINT.toString()));
309317
}
310318
finally {
311319
OptimizerUtils.ALLOW_EVAL_FCALL_REPLACEMENT = oldFlag;
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
#-------------------------------------------------------------
2+
#
3+
# Licensed to the Apache Software Foundation (ASF) under one
4+
# or more contributor license agreements. See the NOTICE file
5+
# distributed with this work for additional information
6+
# regarding copyright ownership. The ASF licenses this file
7+
# to you under the Apache License, Version 2.0 (the
8+
# "License"); you may not use this file except in compliance
9+
# with the License. You may obtain a copy of the License at
10+
#
11+
# http://www.apache.org/licenses/LICENSE-2.0
12+
#
13+
# Unless required by applicable law or agreed to in writing,
14+
# software distributed under the License is distributed on an
15+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
16+
# KIND, either express or implied. See the License for the
17+
# specific language governing permissions and limitations
18+
# under the License.
19+
#
20+
#-------------------------------------------------------------
21+
22+
add_and_return = function(Int i_old, List[String] l_old)
23+
return(Int i, List[String] l)
24+
{
25+
i = i_old + 1
26+
if (TRUE)
27+
l = append(l_old, toString(i))
28+
else
29+
l = append(l_old, toString(i))
30+
}
31+
32+
# Works!
33+
init = function() return(Int i, List[String] l) {i=0; l=list();}
34+
[test_i, test_l] = init()
35+
36+
[test_i, test_l] = add_and_return(test_i, test_l)
37+
[test_i, test_l] = add_and_return(test_i, test_l)
38+
[test_i, test_l] = add_and_return(test_i, test_l)
39+
40+
#while(FALSE){} # cut
41+
42+
# Does not work!
43+
test_fail_i = 0
44+
test_fail_l = list()
45+
[test_fail_i, test_fail_l] = add_and_return(test_fail_i, test_fail_l)
46+
[test_fail_i, test_fail_l] = add_and_return(test_fail_i, test_fail_l)
47+
[test_fail_i, test_fail_l] = add_and_return(test_fail_i, test_fail_l)
48+
49+
if( test_i != test_fail_i )
50+
print("test_i discrepancy: "+test_i+" vs "+test_fail_i);
51+
52+
if( toString(test_l) != toString(test_fail_l) )
53+
print("test_l discrepancy: "+toString(test_l)+" vs "+toString(test_fail_l));

0 commit comments

Comments
 (0)