-
Notifications
You must be signed in to change notification settings - Fork 435
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
LLVM 4 Merge #23
LLVM 4 Merge #23
Conversation
… like so far it's line and file information, i.e diagnostic so..
…ysisUtil looks more like I might miss some diagnostic info
…vs X* in the node references, but let me take a look at this before rushing right in (I can do that this afternoon)
…arification of pointers or double pointers in WPASolver
…ngRef, inserting a few NodeRef, etc, and now compiling a fair amount of the project, but still more to go...
…e some new virtual methods in 4.0 aren't implemented...
…back to Yulei (with notes to help him...)
…ing the LLVMGetGlobalContext rather than getGlobalContext
…est should be pretty much ready to go
…re - just for a clean environment, tired of my wonkiness
Hi Jared, Thanks! I will take a look at it today. |
Your PR works very well on small test cases including |
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.
Option -ander and -fspta work perfectly for all the micro-benchmarks.
I have also tested on CPU2000. Your PR produces identical points-to sets.
Very good work!!
Unfortunately, option wpa -ander -locMM *.bc causes crashes for some micro-benchmarks in fi_tests.
Could you please fix it as below?
Thanks
lib/MemoryModel/LocMemModel.cpp
Outdated
@@ -60,7 +60,7 @@ bool LocSymTableInfo::computeGepOffset(const llvm::User *V, LocationSet& ls) { | |||
if (index <= baseIndex) { | |||
/// variant offset | |||
// Handling pointer types | |||
if (const PointerType* pty = dyn_cast<PointerType>(*gi)) { | |||
if (const PointerType* pty = dyn_cast<PointerType>(gi.getIndexedType())) { |
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.
For LocMemModel, we can also get rid of the gep_type_iterator
and gi.getIndexedType()
, and stick with your BridgedGepIterator for now:).
Please use *gi
intead of gi.getIndexedType()
in LocMemModel. Please also replace gep_type_iterator
at lines 50 and 57 with the BridgedGepIterator.
lib/MemoryModel/LocMemModel.cpp
Outdated
const Type* et = at->getElementType(); | ||
Size_t sz = getTypeSizeInBytes(et); | ||
ls.offset += idx * sz; | ||
} | ||
// Handling struct here | ||
else if (const StructType *ST = dyn_cast<StructType>(*gi)) { |
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.
Shall we use gi.getStructTypeOrNull
? I am not very sure whether these two are equivalent or not. Please keep the original one as you did in MemModel.cpp
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.
LocMemModel.cpp updated!
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.
Thanks, Jared!
Finally, we almost get there:) I will merge your PR once the following two are fixed.
I will revise the wiki accordingly.
lib/MemoryModel/LocMemModel.cpp
Outdated
@@ -94,19 +95,19 @@ bool LocSymTableInfo::computeGepOffset(const llvm::User *V, LocationSet& ls) { | |||
// Handling pointer types | |||
// These GEP instructions are simply making address computations from the base pointer address | |||
// e.g. idx1 = (char*) &MyVar + 4, at this case gep only one offset index (idx) | |||
if (const PointerType* pty = dyn_cast<PointerType>(*gi)) { | |||
if (const PointerType* pty = dyn_cast<PointerType>(gi.getIndexedType())) { |
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.
Please change this back.
lib/MemoryModel/LocMemModel.cpp
Outdated
const Type* et = pty->getElementType(); | ||
Size_t sz = getTypeSizeInBytes(et); | ||
ls.offset += idx * sz; | ||
} | ||
// Calculate the size of the array element | ||
else if(const ArrayType* at = dyn_cast<ArrayType>(*gi)) { |
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.
See above.
lib/MemoryModel/LocMemModel.cpp
Outdated
const Type* et = pty->getElementType(); | ||
Size_t sz = getTypeSizeInBytes(et); | ||
ls.offset += idx * sz; | ||
} | ||
// Calculate the size of the array element | ||
else if(const ArrayType* at = dyn_cast<ArrayType>(*gi)) { | ||
else if(const ArrayType* at = dyn_cast<ArrayType>(gi.getIndexedType())) { |
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.
Should be this line.
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, my bad on missing this one, trying to update the commits in between breakfast ;-P
Jared, Please link your account to your commits, so we can acknowledge your contribution: Thanks |
Thanks @rockysui! I think I just took care of that (added the additional email). Thanks again. |
I tested this with fi_tests under PTABen and while there were 4 failures (I think, it was just handful for sure) I don't think (although I'm not entirely sure) these were unexpected (non-default cases referred to Questions on PTABen test cases ). I ran a few other tests and had generally positive results as well so, I didn't run them all (On my Linux VM this went pretty smooth but on my Mac I'm still struggling a bit although I did find some of my issues, perhaps the key ones).
This has the BridgedGEPIterator solution implemented along with the previous PR's more mundane fixes, again the NodeRef inclusion in the GraphTraits was the most notable as I recall, to update SVF with LLVM 4.