Skip to content

fix shift overflow in irep_hash_container #493

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/util/irep_hash_container.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Function: irep_hash_container_baset::number

\*******************************************************************/

unsigned irep_hash_container_baset::number(const irept &irep)
size_t irep_hash_container_baset::number(const irept &irep)
{
// the ptr-hash provides a speedup of up to 3x

Expand All @@ -32,7 +32,7 @@ unsigned irep_hash_container_baset::number(const irept &irep)

packedt packed;
pack(irep, packed);
unsigned id=numbering.number(packed);
size_t id=numbering.number(packed);

ptr_hash[&irep.read()]=id;

Expand Down
22 changes: 12 additions & 10 deletions src/util/irep_hash_container.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,17 @@ Author: Daniel Kroening, [email protected]
#include <cstdlib> // for size_t
#include <vector>

#include "irep_hash.h"
#include "numbering.h"

class irept;

class irep_hash_container_baset
{
public:
unsigned number(const irept &irep);
size_t number(const irept &irep);

irep_hash_container_baset(bool _full):full(_full)
explicit irep_hash_container_baset(bool _full):full(_full)
{
}

Expand All @@ -36,33 +37,34 @@ class irep_hash_container_baset

// this is the first level: address of the content

struct pointer_hash
struct pointer_hasht
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #376 I took the view that those are effectively operations, and thus went for a linter override. I don't claim any authority on that, and your approach seems perfectly valid. Just noting.

{
inline size_t operator()(const void *p) const
{
return (size_t)p;
}
};

typedef std::unordered_map<const void *, unsigned, pointer_hash> ptr_hasht;
typedef std::unordered_map<const void *, size_t, pointer_hasht>
ptr_hasht;
ptr_hasht ptr_hash;

// this is the second level: content

typedef std::vector<unsigned> packedt;
typedef std::vector<size_t> packedt;

struct vector_hash
struct vector_hasht
{
inline size_t operator()(const packedt &p) const
{
size_t result=p.size();
for(unsigned i=0; i<p.size(); i++)
result^=p[i]<<i;
size_t result=p.size(); // seed
for(auto elem : p)
result=hash_combine(result, elem);
return result;
}
};

typedef hash_numbering<packedt, vector_hash> numberingt;
typedef hash_numbering<packedt, vector_hasht> numberingt;
numberingt numbering;

void pack(const irept &irep, packedt &);
Expand Down