Discussion:
[PATCH 3/3] Fix NotExpr::computeHash() shadowing hashValue
Jonathan Neuschäfer
2013-07-01 21:25:35 UTC
Permalink
Without this patch NotExpr::computeHash() will have a local
valiable with the name "hashValue" and assign the newly computed
hash to that instead of the member variable with the same name
that should be set by the computeHash method of every Expr subclass.
---
lib/Expr/Expr.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/Expr/Expr.cpp b/lib/Expr/Expr.cpp
index d177eca..1187573 100644
--- a/lib/Expr/Expr.cpp
+++ b/lib/Expr/Expr.cpp
@@ -206,7 +206,7 @@ unsigned ReadExpr::computeHash() {
}

unsigned NotExpr::computeHash() {
- unsigned hashValue = expr->hash() * Expr::MAGIC_HASH_CONSTANT * Expr::Not;
+ hashValue = expr->hash() * Expr::MAGIC_HASH_CONSTANT * Expr::Not;
return hashValue;
}
--
1.8.3.1
Jonathan Neuschäfer
2013-07-01 21:25:34 UTC
Permalink
STP has been removed from the KLEE repository in r161056.
---
LICENSE.TXT | 1 -
1 file changed, 1 deletion(-)

diff --git a/LICENSE.TXT b/LICENSE.TXT
index ce3677c..e2fb630 100644
--- a/LICENSE.TXT
+++ b/LICENSE.TXT
@@ -58,6 +58,5 @@ licenses, and/or restrictions:

Program Directory
------- ---------
-STP klee/stp
klee-libc runtime/klee-libc
--
1.8.3.1
Jonathan Neuschäfer
2013-07-02 09:25:20 UTC
Permalink
Hi Jonathan,
May I know why STP has been removed from the KLEE repo?
Daniel Liew wrote the patch to do that. I think the motivation was to
prevent KLEE's internal copy and the upstream STP from diverging too
much. STP is still used, you'll just need to download and build it
separately.

I've added Daniel, Christian Cadar (who applied the patch), and the
klee-dev mailing list to the CC list, so they participate in the
discussion.


Hope that helps,
Jonathan Neuschäfer
-Vijay.
On Mon, Jul 1, 2013 at 5:25 PM, Jonathan Neuschäfer
Post by Jonathan Neuschäfer
STP has been removed from the KLEE repository in r161056.
---
LICENSE.TXT | 1 -
1 file changed, 1 deletion(-)
diff --git a/LICENSE.TXT b/LICENSE.TXT
index ce3677c..e2fb630 100644
--- a/LICENSE.TXT
+++ b/LICENSE.TXT
Program Directory
------- ---------
-STP klee/stp
klee-libc runtime/klee-libc
--
1.8.3.1
Daniel Liew
2013-07-02 10:01:21 UTC
Permalink
Hi,

The origin of this change is from a message on this mailing list (
http://www.mail-archive.com/klee-***@imperial.ac.uk/msg00856.html ).
Someone asked about which version of STP was used and when I looked at
it I saw that KLEE had it's own internal copy of STP which could be
used instead of an external copy which seemed pointless to me as our
build instructions say to build a particular revision of STP anyway.
So I wrote a small patch to remove the internal copy of STP.

Thanks,
Dan.
Post by Jonathan Neuschäfer
Hi Jonathan,
May I know why STP has been removed from the KLEE repo?
Daniel Liew wrote the patch to do that. I think the motivation was to
prevent KLEE's internal copy and the upstream STP from diverging too
much. STP is still used, you'll just need to download and build it
separately.
I've added Daniel, Christian Cadar (who applied the patch), and the
klee-dev mailing list to the CC list, so they participate in the
discussion.
Hope that helps,
Jonathan Neuschäfer
-Vijay.
On Mon, Jul 1, 2013 at 5:25 PM, Jonathan Neuschäfer
Post by Jonathan Neuschäfer
STP has been removed from the KLEE repository in r161056.
---
LICENSE.TXT | 1 -
1 file changed, 1 deletion(-)
diff --git a/LICENSE.TXT b/LICENSE.TXT
index ce3677c..e2fb630 100644
--- a/LICENSE.TXT
+++ b/LICENSE.TXT
Program Directory
------- ---------
-STP klee/stp
klee-libc runtime/klee-libc
--
1.8.3.1
Cristian Cadar
2013-07-11 16:45:58 UTC
Permalink
Hi Jonathan,

Thanks for these three patches; nice catch with the last one.

There were again no commit messages going to klee-commits about your
patches; after some googling, I found out that commit messages
containing non-ASCII characters are discarded, see the thread below:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080428/061915.html

So from now on I will unfortunately have to misspell your last name as
Neuschafer, sorry!

Finally, I would prefer to use klee-commits for patches, unless they are
likely to be of interest to the wider klee-dev audience.

Best,
Cristian
Post by Jonathan Neuschäfer
Without this patch NotExpr::computeHash() will have a local
valiable with the name "hashValue" and assign the newly computed
hash to that instead of the member variable with the same name
that should be set by the computeHash method of every Expr subclass.
---
lib/Expr/Expr.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/Expr/Expr.cpp b/lib/Expr/Expr.cpp
index d177eca..1187573 100644
--- a/lib/Expr/Expr.cpp
+++ b/lib/Expr/Expr.cpp
@@ -206,7 +206,7 @@ unsigned ReadExpr::computeHash() {
}
unsigned NotExpr::computeHash() {
- unsigned hashValue = expr->hash() * Expr::MAGIC_HASH_CONSTANT * Expr::Not;
+ hashValue = expr->hash() * Expr::MAGIC_HASH_CONSTANT * Expr::Not;
return hashValue;
}
Jonathan Neuschäfer
2013-07-12 13:02:18 UTC
Permalink
Hi Jonathan,
Thanks for these three patches; nice catch with the last one.
There were again no commit messages going to klee-commits about your
patches; after some googling, I found out that commit messages
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080428/061915.html
That's interesting.
So from now on I will unfortunately have to misspell your last name
as Neuschafer, sorry!
"Neuschaefer" would be the more common spelling in cases where the A
"umlaut" isn't available, such as in my email address. I'll leave up it
to you which spelling you'll use. :-)
Finally, I would prefer to use klee-commits for patches, unless they
are likely to be of interest to the wider klee-dev audience.
Ok, I'll remember that.


--
Jonathan
Best,
Cristian
Post by Jonathan Neuschäfer
Without this patch NotExpr::computeHash() will have a local
valiable with the name "hashValue" and assign the newly computed
hash to that instead of the member variable with the same name
that should be set by the computeHash method of every Expr subclass.
---
lib/Expr/Expr.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/Expr/Expr.cpp b/lib/Expr/Expr.cpp
index d177eca..1187573 100644
--- a/lib/Expr/Expr.cpp
+++ b/lib/Expr/Expr.cpp
@@ -206,7 +206,7 @@ unsigned ReadExpr::computeHash() {
}
unsigned NotExpr::computeHash() {
- unsigned hashValue = expr->hash() * Expr::MAGIC_HASH_CONSTANT * Expr::Not;
+ hashValue = expr->hash() * Expr::MAGIC_HASH_CONSTANT * Expr::Not;
return hashValue;
}
Loading...