This is an archive of the discontinued LLVM Phabricator instance.

[ADT]Fix AddPointer() in FoldingSet
AbandonedPublic

Authored by kevgs on Feb 18 2016, 6:37 AM.

Details

Reviewers
None
Summary

Put a pointer value instead of some garbage from stack to hash buffer.

Diff Detail

Repository
rL LLVM

Event Timeline

kevgs updated this revision to Diff 48310.Feb 18 2016, 6:37 AM
kevgs retitled this revision from to [ADT]Fix AddPointer() in FoldingSet.
kevgs updated this object.
kevgs set the repository for this revision to rL LLVM.
kevgs added a subscriber: llvm-commits.

What makes you think that this was adding garbage?

kevgs added a comment.Feb 18 2016, 7:54 AM

&Ptr is an address of a local variable which depends on a previous call stack and doesn't depends on a pointer value, isn't it? I just do not understand why do we need a this. If it has some purpose why passing a value and ignore it? We could just write
void FoldingSetNodeID::AddPointer() {

const void *Ptr;
Bits.append(reinterpret_cast<unsigned *>(&Ptr),
            reinterpret_cast<unsigned *>(&Ptr+1));

}

We take a pointer (of the argument on the stack) and it gets dereferenced inside of append. What platform are you on that this causes problems for you?

kevgs added a comment.Feb 18 2016, 1:57 PM

Linux AMD64, but I have no problems with that code. I just thought it was incorrect and feel pretty silly now. However, patched method looks nicer and it's probably faster because compiler is not forced to put variable on the stack ;)

Anyway, thanks four your help and sorry for taking your time on such a stupid case.

kevgs abandoned this revision.Apr 15 2016, 3:45 AM