-
Notifications
You must be signed in to change notification settings - Fork 273
[TG-8293] Constant propagation for empty strings and string concatenation [blocks: #4941] #4885
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e05c57d
36b60ef
f3c5c37
b0ec24e
b9d5f4b
aff73bd
6a26848
312bba8
d9f75eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
public class Main { | ||
public void test() { | ||
String s1 = "abc"; | ||
String s2 = "xyz"; | ||
String s3 = s1 + s2; | ||
assert s3.equals("abcxyz"); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
CORE | ||
Main.class | ||
--function Main.test --property "java::Main.test:()V.assertion.1" --cp `../../../../scripts/format_classpath.sh . ../../../lib/java-models-library/target/core-models.jar` | ||
^Generated [0-9]+ VCC\(s\), 0 remaining after simplification$ | ||
^EXIT=0$ | ||
^SIGNAL=0$ | ||
^VERIFICATION SUCCESSFUL$ | ||
-- | ||
-- |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
public class Main { | ||
public void test1() { | ||
String s1 = "abc"; | ||
String s2 = "xyz"; | ||
String s3 = s1 + s2; | ||
assert s3.length() == 7; | ||
} | ||
|
||
public void test2() { | ||
String s1 = "abc"; | ||
String s2 = "xyz"; | ||
String s3 = s1 + s2; | ||
assert s3.startsWith("abcdefg"); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
CORE | ||
Main.class | ||
--function Main.test1 | ||
^EXIT=10$ | ||
^SIGNAL=0$ | ||
^VERIFICATION FAILED$ | ||
-- | ||
-- |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
CORE | ||
Main.class | ||
--function Main.test2 | ||
^EXIT=10$ | ||
^SIGNAL=0$ | ||
^VERIFICATION FAILED$ | ||
-- | ||
-- |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
public class Main { | ||
public void test() { | ||
String s1 = "ディフ"; | ||
String s2 = "ブルー"; | ||
String s3 = s1 + s2; | ||
assert s3.length() == 6; | ||
assert s3.startsWith("ディフブルー"); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
CORE | ||
Main.class | ||
--function Main.test --property "java::Main.test:()V.assertion.1" --property "java::Main.test:()V.assertion.2" | ||
^Generated [0-9]+ VCC\(s\), 0 remaining after simplification$ | ||
^EXIT=0$ | ||
^SIGNAL=0$ | ||
^VERIFICATION SUCCESSFUL$ | ||
-- | ||
-- |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
public class Main { | ||
public void test() { | ||
String s1 = "\t"; | ||
String s2 = "\\"; | ||
String s3 = s1 + s2; | ||
assert s3.length() == 2; | ||
assert s3.startsWith("\t\\"); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
CORE | ||
Main.class | ||
--function Main.test --property "java::Main.test:()V.assertion.1" --property "java::Main.test:()V.assertion.2" | ||
^Generated [0-9]+ VCC\(s\), 0 remaining after simplification$ | ||
^EXIT=0$ | ||
^SIGNAL=0$ | ||
^VERIFICATION SUCCESSFUL$ | ||
-- | ||
-- |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
public class Main { | ||
public void test() { | ||
StringBuilder sb = new StringBuilder("abc"); | ||
sb.append("xyz"); | ||
String s = sb.toString(); | ||
assert s.length() == 6; | ||
assert s.startsWith("abcxyz"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Surely the tests should check There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test doesn't use the models. I've not included them for tests where they are not needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚫 I think it's not sufficient to have tests for the successful case. We should have tests that check some assertion is violated. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test added by @yumibagge |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
CORE | ||
Main.class | ||
--function Main.test --property "java::Main.test:()V.assertion.1" --property "java::Main.test:()V.assertion.2" | ||
^Generated [0-9]+ VCC\(s\), 0 remaining after simplification$ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be exactly There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So it turns out that
So if |
||
^EXIT=0$ | ||
^SIGNAL=0$ | ||
^VERIFICATION SUCCESSFUL$ | ||
-- | ||
-- |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
public class Main { | ||
public void test1() { | ||
StringBuilder sb1 = new StringBuilder("abc"); | ||
String s2 = ""; | ||
sb1.append(s2); | ||
assert sb1.length() == 3; | ||
assert sb1.toString().startsWith("abc"); | ||
} | ||
|
||
public void test2() { | ||
StringBuilder sb1 = new StringBuilder(); | ||
String s2 = "abc"; | ||
sb1.append(s2); | ||
assert sb1.length() == 3; | ||
assert sb1.toString().startsWith("abc"); | ||
} | ||
|
||
public void test3() { | ||
StringBuilder sb1 = new StringBuilder(); | ||
String s2 = ""; | ||
sb1.append(s2); | ||
assert sb1.length() == 0; | ||
assert sb1.toString().startsWith(""); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
CORE | ||
Main.class | ||
--function Main.test1 --property "java::Main.test1:()V.assertion.1" --property "java::Main.test1:()V.assertion.2" | ||
^Generated [0-9]+ VCC\(s\), 0 remaining after simplification$ | ||
^EXIT=0$ | ||
^SIGNAL=0$ | ||
^VERIFICATION SUCCESSFUL$ | ||
-- | ||
-- |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
CORE | ||
Main.class | ||
--function Main.test2 --property "java::Main.test2:()V.assertion.1" --property "java::Main.test2:()V.assertion.2" | ||
^Generated [0-9]+ VCC\(s\), 0 remaining after simplification$ | ||
^EXIT=0$ | ||
^SIGNAL=0$ | ||
^VERIFICATION SUCCESSFUL$ | ||
-- | ||
-- |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
CORE | ||
Main.class | ||
--function Main.test3 --property "java::Main.test3:()V.assertion.1" --property "java::Main.test3:()V.assertion.2" | ||
^Generated [0-9]+ VCC\(s\), 0 remaining after simplification$ | ||
^EXIT=0$ | ||
^SIGNAL=0$ | ||
^VERIFICATION SUCCESSFUL$ | ||
-- | ||
-- |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
public class Main { | ||
public void test() { | ||
StringBuilder sb = new StringBuilder(); | ||
String s = sb.toString(); | ||
assert s.isEmpty(); | ||
assert s.length() == 0; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
CORE | ||
Main.class | ||
--function Main.test | ||
^Generated [0-9]+ VCC\(s\), 0 remaining after simplification$ | ||
^EXIT=0$ | ||
^SIGNAL=0$ | ||
^VERIFICATION SUCCESSFUL$ | ||
-- | ||
-- |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
public class Main { | ||
public void test() { | ||
String s1 = "abc"; | ||
String s2 = "xyz"; | ||
String s3 = s1 + s2; | ||
assert s3.length() == 6; | ||
assert s3.startsWith("abcxyz"); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
CORE | ||
Main.class | ||
--function Main.test --property "java::Main.test:()V.assertion.1" --property "java::Main.test:()V.assertion.2" | ||
^Generated [0-9]+ VCC\(s\), 0 remaining after simplification$ | ||
^EXIT=0$ | ||
^SIGNAL=0$ | ||
^VERIFICATION SUCCESSFUL$ | ||
-- | ||
-- |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
public class Main { | ||
public void test1() { | ||
String s1 = "abc"; | ||
String s2 = ""; | ||
String s3 = s1 + s2; | ||
assert s3.length() == 3; | ||
assert s3.startsWith("abc"); | ||
} | ||
|
||
public void test2() { | ||
String s1 = ""; | ||
String s2 = "abc"; | ||
String s3 = s1 + s2; | ||
assert s3.length() == 3; | ||
assert s3.startsWith("abc"); | ||
} | ||
|
||
public void test3() { | ||
String s1 = ""; | ||
String s2 = ""; | ||
String s3 = s1 + s2; | ||
assert s3.length() == 0; | ||
assert s3.startsWith(""); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
CORE | ||
Main.class | ||
--function Main.test1 --property "java::Main.test1:()V.assertion.1" --property "java::Main.test1:()V.assertion.2" | ||
^Generated [0-9]+ VCC\(s\), 0 remaining after simplification$ | ||
^EXIT=0$ | ||
^SIGNAL=0$ | ||
^VERIFICATION SUCCESSFUL$ | ||
-- | ||
-- |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
CORE | ||
Main.class | ||
--function Main.test2 --property "java::Main.test2:()V.assertion.1" --property "java::Main.test2:()V.assertion.2" | ||
^Generated [0-9]+ VCC\(s\), 0 remaining after simplification$ | ||
^EXIT=0$ | ||
^SIGNAL=0$ | ||
^VERIFICATION SUCCESSFUL$ | ||
-- | ||
-- |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
CORE | ||
Main.class | ||
--function Main.test3 --property "java::Main.test3:()V.assertion.1" --property "java::Main.test3:()V.assertion.2" | ||
^Generated [0-9]+ VCC\(s\), 0 remaining after simplification$ | ||
^EXIT=0$ | ||
^SIGNAL=0$ | ||
^VERIFICATION SUCCESSFUL$ | ||
-- | ||
-- |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
public class Main { | ||
public void test1(String s1) { | ||
String s2 = "abc"; | ||
assert (s1 + s2).startsWith("abc"); | ||
danpoe marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
public void test2(String s) { assert ("" + s).startsWith("abc"); } | ||
|
||
public void test3(String s) { assert (s + s).startsWith("abc"); } | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
CORE symex-driven-lazy-loading-expected-failure | ||
Main.class | ||
--function Main.test1 --property "java::Main.test1:(Ljava/lang/String;)V.assertion.1" | ||
^Generated [0-9]+ VCC\(s\), 1 remaining after simplification$ | ||
^EXIT=10$ | ||
^SIGNAL=0$ | ||
danpoe marked this conversation as resolved.
Show resolved
Hide resolved
|
||
^VERIFICATION FAILED$ | ||
-- | ||
-- | ||
This test checks that constant propagation does not happen, since a constant | ||
result cannot be determined from the arguments to `+`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
CORE symex-driven-lazy-loading-expected-failure | ||
Main.class | ||
--function Main.test2 --property "java::Main.test2:(Ljava/lang/String;)V.assertion.1" | ||
^Generated [0-9]+ VCC\(s\), 1 remaining after simplification$ | ||
^EXIT=10$ | ||
^SIGNAL=0$ | ||
danpoe marked this conversation as resolved.
Show resolved
Hide resolved
|
||
^VERIFICATION FAILED$ | ||
-- | ||
-- | ||
This test checks that constant propagation does not happen, since a constant | ||
result cannot be determined from the arguments to `+`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
CORE symex-driven-lazy-loading-expected-failure | ||
Main.class | ||
--function Main.test3 --property "java::Main.test3:(Ljava/lang/String;)V.assertion.1" | ||
^Generated [0-9]+ VCC\(s\), 1 remaining after simplification$ | ||
^EXIT=10$ | ||
^SIGNAL=0$ | ||
danpoe marked this conversation as resolved.
Show resolved
Hide resolved
|
||
^VERIFICATION FAILED$ | ||
-- | ||
-- | ||
This test checks that constant propagation does not happen, since a constant | ||
result cannot be determined from the arguments to `+`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
public class Main { | ||
public void test() { | ||
String s = new String(); | ||
assert s.isEmpty(); | ||
assert s.startsWith(""); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
CORE | ||
Main.class | ||
--function Main.test | ||
^Generated [0-9]+ VCC\(s\), 0 remaining after simplification$ | ||
^EXIT=0$ | ||
^SIGNAL=0$ | ||
^VERIFICATION SUCCESSFUL$ | ||
-- | ||
-- |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,34 +15,12 @@ Author: Chris Smowton, [email protected] | |
#include <util/arith_tools.h> | ||
#include <util/expr_initializer.h> | ||
#include <util/namespace.h> | ||
#include <util/string_utils.h> | ||
#include <util/unicode.h> | ||
|
||
#include <iomanip> | ||
#include <sstream> | ||
|
||
/// Replace non-alphanumeric characters with `_xx` escapes, where xx are hex | ||
/// digits. Underscores are replaced by `__`. | ||
/// \param to_escape: string to escape | ||
/// \return string with non-alphanumeric characters escaped | ||
static std::string escape_non_alnum(const std::string &to_escape) | ||
{ | ||
std::ostringstream escaped; | ||
for(auto &ch : to_escape) | ||
{ | ||
if(ch=='_') | ||
escaped << "__"; | ||
else if(isalnum(ch)) | ||
escaped << ch; | ||
else | ||
escaped << '_' | ||
<< std::hex | ||
<< std::setfill('0') | ||
<< std::setw(2) | ||
<< (unsigned int)ch; | ||
} | ||
return escaped.str(); | ||
} | ||
|
||
/// Convert UCS-2 or UTF-16 to an array expression. | ||
/// \par parameters: `in`: wide string to convert | ||
/// \return Returns a Java char array containing the same wchars. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for being a bore, but regression tests that check for success only verify that cbmc doesn't do anything. They should always be paired with a test checking for failure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After discussion with @danpoe, it seems to be unnecessary to "assertion failure" tests for each case, as not many bad things can happen.
We do want to check that we're not completely discarding the trace, so a few tests for assertion failure makes sense. The tests currently included now look sufficient to me.