Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Commit

Permalink
Land hash collision fix for V8 3.6 by Erik Corry.
Browse files Browse the repository at this point in the history
- If V8 snapshots are enabled then the hash is only randomized at build time.
- Breaks MIPS

---
Backport hash collision workaround to 3.6.
This is made up of 9956, 10351, 10338 and 10330.
This change bakes the string hash key into the snapshot, so
it is determined at build time for shapshot configs.
Review URL: http://codereview.chromium.org/9124004
  • Loading branch information
piscisaureus committed Jan 6, 2012
1 parent dd9593c commit 4a899c9
Show file tree
Hide file tree
Showing 23 changed files with 446 additions and 162 deletions.
5 changes: 1 addition & 4 deletions deps/v8/src/arm/builtins-arm.cc
Expand Up @@ -1006,10 +1006,7 @@ static void Generate_JSEntryTrampolineHelper(MacroAssembler* masm,
// Set up the context from the function argument.
__ ldr(cp, FieldMemOperand(r1, JSFunction::kContextOffset));

// Set up the roots register.
ExternalReference roots_address =
ExternalReference::roots_address(masm->isolate());
__ mov(r10, Operand(roots_address));
__ InitializeRootRegister();

// Push the function and the receiver onto the stack.
__ push(r1);
Expand Down
86 changes: 14 additions & 72 deletions deps/v8/src/arm/code-stubs-arm.cc
Expand Up @@ -5043,70 +5043,6 @@ void StringCharAtGenerator::GenerateSlow(
}


class StringHelper : public AllStatic {
public:
// Generate code for copying characters using a simple loop. This should only
// be used in places where the number of characters is small and the
// additional setup and checking in GenerateCopyCharactersLong adds too much
// overhead. Copying of overlapping regions is not supported.
// Dest register ends at the position after the last character written.
static void GenerateCopyCharacters(MacroAssembler* masm,
Register dest,
Register src,
Register count,
Register scratch,
bool ascii);

// Generate code for copying a large number of characters. This function
// is allowed to spend extra time setting up conditions to make copying
// faster. Copying of overlapping regions is not supported.
// Dest register ends at the position after the last character written.
static void GenerateCopyCharactersLong(MacroAssembler* masm,
Register dest,
Register src,
Register count,
Register scratch1,
Register scratch2,
Register scratch3,
Register scratch4,
Register scratch5,
int flags);


// Probe the symbol table for a two character string. If the string is
// not found by probing a jump to the label not_found is performed. This jump
// does not guarantee that the string is not in the symbol table. If the
// string is found the code falls through with the string in register r0.
// Contents of both c1 and c2 registers are modified. At the exit c1 is
// guaranteed to contain halfword with low and high bytes equal to
// initial contents of c1 and c2 respectively.
static void GenerateTwoCharacterSymbolTableProbe(MacroAssembler* masm,
Register c1,
Register c2,
Register scratch1,
Register scratch2,
Register scratch3,
Register scratch4,
Register scratch5,
Label* not_found);

// Generate string hash.
static void GenerateHashInit(MacroAssembler* masm,
Register hash,
Register character);

static void GenerateHashAddCharacter(MacroAssembler* masm,
Register hash,
Register character);

static void GenerateHashGetHash(MacroAssembler* masm,
Register hash);

private:
DISALLOW_IMPLICIT_CONSTRUCTORS(StringHelper);
};


void StringHelper::GenerateCopyCharacters(MacroAssembler* masm,
Register dest,
Register src,
Expand Down Expand Up @@ -5359,9 +5295,8 @@ void StringHelper::GenerateTwoCharacterSymbolTableProbe(MacroAssembler* masm,
static const int kProbes = 4;
Label found_in_symbol_table;
Label next_probe[kProbes];
Register candidate = scratch5; // Scratch register contains candidate.
for (int i = 0; i < kProbes; i++) {
Register candidate = scratch5; // Scratch register contains candidate.

// Calculate entry in symbol table.
if (i > 0) {
__ add(candidate, hash, Operand(SymbolTable::GetProbeOffset(i)));
Expand Down Expand Up @@ -5418,7 +5353,7 @@ void StringHelper::GenerateTwoCharacterSymbolTableProbe(MacroAssembler* masm,
__ jmp(not_found);

// Scratch register contains result when we fall through to here.
Register result = scratch;
Register result = candidate;
__ bind(&found_in_symbol_table);
__ Move(r0, result);
}
Expand All @@ -5428,9 +5363,13 @@ void StringHelper::GenerateHashInit(MacroAssembler* masm,
Register hash,
Register character) {
// hash = character + (character << 10);
__ add(hash, character, Operand(character, LSL, 10));
__ LoadRoot(hash, Heap::kStringHashSeedRootIndex);
// Untag smi seed and add the character.
__ add(hash, character, Operand(hash, LSR, kSmiTagSize));
// hash += hash << 10;
__ add(hash, hash, Operand(hash, LSL, 10));
// hash ^= hash >> 6;
__ eor(hash, hash, Operand(hash, ASR, 6));
__ eor(hash, hash, Operand(hash, LSR, 6));
}


Expand All @@ -5442,7 +5381,7 @@ void StringHelper::GenerateHashAddCharacter(MacroAssembler* masm,
// hash += hash << 10;
__ add(hash, hash, Operand(hash, LSL, 10));
// hash ^= hash >> 6;
__ eor(hash, hash, Operand(hash, ASR, 6));
__ eor(hash, hash, Operand(hash, LSR, 6));
}


Expand All @@ -5451,12 +5390,15 @@ void StringHelper::GenerateHashGetHash(MacroAssembler* masm,
// hash += hash << 3;
__ add(hash, hash, Operand(hash, LSL, 3));
// hash ^= hash >> 11;
__ eor(hash, hash, Operand(hash, ASR, 11));
__ eor(hash, hash, Operand(hash, LSR, 11));
// hash += hash << 15;
__ add(hash, hash, Operand(hash, LSL, 15), SetCC);

uint32_t kHashShiftCutOffMask = (1 << (32 - String::kHashShift)) - 1;
__ and_(hash, hash, Operand(kHashShiftCutOffMask));

// if (hash == 0) hash = 27;
__ mov(hash, Operand(27), LeaveCC, ne);
__ mov(hash, Operand(27), LeaveCC, eq);
}


Expand Down
64 changes: 64 additions & 0 deletions deps/v8/src/arm/code-stubs-arm.h
Expand Up @@ -225,6 +225,70 @@ class BinaryOpStub: public CodeStub {
};


class StringHelper : public AllStatic {
public:
// Generate code for copying characters using a simple loop. This should only
// be used in places where the number of characters is small and the
// additional setup and checking in GenerateCopyCharactersLong adds too much
// overhead. Copying of overlapping regions is not supported.
// Dest register ends at the position after the last character written.
static void GenerateCopyCharacters(MacroAssembler* masm,
Register dest,
Register src,
Register count,
Register scratch,
bool ascii);

// Generate code for copying a large number of characters. This function
// is allowed to spend extra time setting up conditions to make copying
// faster. Copying of overlapping regions is not supported.
// Dest register ends at the position after the last character written.
static void GenerateCopyCharactersLong(MacroAssembler* masm,
Register dest,
Register src,
Register count,
Register scratch1,
Register scratch2,
Register scratch3,
Register scratch4,
Register scratch5,
int flags);


// Probe the symbol table for a two character string. If the string is
// not found by probing a jump to the label not_found is performed. This jump
// does not guarantee that the string is not in the symbol table. If the
// string is found the code falls through with the string in register r0.
// Contents of both c1 and c2 registers are modified. At the exit c1 is
// guaranteed to contain halfword with low and high bytes equal to
// initial contents of c1 and c2 respectively.
static void GenerateTwoCharacterSymbolTableProbe(MacroAssembler* masm,
Register c1,
Register c2,
Register scratch1,
Register scratch2,
Register scratch3,
Register scratch4,
Register scratch5,
Label* not_found);

// Generate string hash.
static void GenerateHashInit(MacroAssembler* masm,
Register hash,
Register character);

static void GenerateHashAddCharacter(MacroAssembler* masm,
Register hash,
Register character);

static void GenerateHashGetHash(MacroAssembler* masm,
Register hash);

private:
DISALLOW_IMPLICIT_CONSTRUCTORS(StringHelper);
};


// Flag that indicates how to generate code for the stub StringAddStub.
enum StringAddFlags {
NO_STRING_ADD_FLAGS = 0,
Expand Down
4 changes: 1 addition & 3 deletions deps/v8/src/arm/deoptimizer-arm.cc
Expand Up @@ -736,9 +736,7 @@ void Deoptimizer::EntryGenerator::Generate() {
__ pop(ip); // remove sp
__ pop(ip); // remove lr

// Set up the roots register.
ExternalReference roots_address = ExternalReference::roots_address(isolate);
__ mov(r10, Operand(roots_address));
__ InitializeRootRegister();

__ pop(ip); // remove pc
__ pop(r7); // get continuation, leave pc on stack
Expand Down
4 changes: 2 additions & 2 deletions deps/v8/src/arm/macro-assembler-arm.cc
Expand Up @@ -395,14 +395,14 @@ void MacroAssembler::Usat(Register dst, int satpos, const Operand& src,
void MacroAssembler::LoadRoot(Register destination,
Heap::RootListIndex index,
Condition cond) {
ldr(destination, MemOperand(roots, index << kPointerSizeLog2), cond);
ldr(destination, MemOperand(kRootRegister, index << kPointerSizeLog2), cond);
}


void MacroAssembler::StoreRoot(Register source,
Heap::RootListIndex index,
Condition cond) {
str(source, MemOperand(roots, index << kPointerSizeLog2), cond);
str(source, MemOperand(kRootRegister, index << kPointerSizeLog2), cond);
}


Expand Down
8 changes: 7 additions & 1 deletion deps/v8/src/arm/macro-assembler-arm.h
Expand Up @@ -51,7 +51,7 @@ static inline Operand SmiUntagOperand(Register object) {

// Give alias names to registers
const Register cp = { 8 }; // JavaScript context pointer
const Register roots = { 10 }; // Roots array pointer.
const Register kRootRegister = { 10 }; // Roots array pointer.

// Flags used for the AllocateInNewSpace functions.
enum AllocationFlags {
Expand Down Expand Up @@ -350,6 +350,12 @@ class MacroAssembler: public Assembler {
Register map,
Register scratch);

void InitializeRootRegister() {
ExternalReference roots_address =
ExternalReference::roots_address(isolate());
mov(kRootRegister, Operand(roots_address));
}

// ---------------------------------------------------------------------------
// JavaScript invokes

Expand Down
7 changes: 6 additions & 1 deletion deps/v8/src/d8.cc
Expand Up @@ -760,8 +760,13 @@ Persistent<Context> Shell::CreateEvaluationContext() {
#endif // V8_SHARED
// Initialize the global objects
Handle<ObjectTemplate> global_template = CreateGlobalTemplate();

v8::TryCatch try_catch;
Persistent<Context> context = Context::New(NULL, global_template);
ASSERT(!context.IsEmpty());
if (context.IsEmpty()) {
v8::Local<v8::Value> st = try_catch.StackTrace();
ASSERT(!context.IsEmpty());
}
Context::Scope scope(context);

#ifndef V8_SHARED
Expand Down
8 changes: 8 additions & 0 deletions deps/v8/src/flag-definitions.h
Expand Up @@ -319,6 +319,14 @@ DEFINE_bool(trace_exception, false,
"print stack trace when throwing exceptions")
DEFINE_bool(preallocate_message_memory, false,
"preallocate some memory to build stack traces.")
DEFINE_bool(randomize_string_hashes,
true,
"randomize string hashes to avoid predictable hash collisions "
"(with snapshots this option cannot override the baked-in seed)")
DEFINE_int(string_hash_seed,
0,
"Fixed seed to use to string hashing (0 means random)"
"(with snapshots this option cannot override the baked-in seed)")

// v8.cc
DEFINE_bool(preemption, false,
Expand Down
11 changes: 11 additions & 0 deletions deps/v8/src/heap.cc
Expand Up @@ -5362,6 +5362,17 @@ bool Heap::Setup(bool create_heap_objects) {
if (lo_space_ == NULL) return false;
if (!lo_space_->Setup()) return false;

// Set up the seed that is used to randomize the string hash function.
ASSERT(string_hash_seed() == 0);
if (FLAG_randomize_string_hashes) {
if (FLAG_string_hash_seed == 0) {
set_string_hash_seed(
Smi::FromInt(V8::RandomPrivate(isolate()) & 0x3fffffff));
} else {
set_string_hash_seed(Smi::FromInt(FLAG_string_hash_seed));
}
}

if (create_heap_objects) {
// Create initial maps.
if (!CreateInitialMaps()) return false;
Expand Down
10 changes: 8 additions & 2 deletions deps/v8/src/heap.h
Expand Up @@ -79,6 +79,7 @@ inline Heap* _inline_get_heap_();
V(FixedArray, single_character_string_cache, SingleCharacterStringCache) \
V(FixedArray, string_split_cache, StringSplitCache) \
V(Object, termination_exception, TerminationException) \
V(Smi, string_hash_seed, StringHashSeed) \
V(FixedArray, empty_fixed_array, EmptyFixedArray) \
V(ByteArray, empty_byte_array, EmptyByteArray) \
V(FixedDoubleArray, empty_fixed_double_array, EmptyFixedDoubleArray) \
Expand Down Expand Up @@ -841,8 +842,7 @@ class Heap {
// Please note this function does not perform a garbage collection.
MUST_USE_RESULT MaybeObject* LookupSymbol(Vector<const char> str);
MUST_USE_RESULT MaybeObject* LookupAsciiSymbol(Vector<const char> str);
MUST_USE_RESULT MaybeObject* LookupTwoByteSymbol(
Vector<const uc16> str);
MUST_USE_RESULT MaybeObject* LookupTwoByteSymbol(Vector<const uc16> str);
MUST_USE_RESULT MaybeObject* LookupAsciiSymbol(const char* str) {
return LookupSymbol(CStrVector(str));
}
Expand Down Expand Up @@ -1301,6 +1301,12 @@ class Heap {
if (global_gc_epilogue_callback_ != NULL) global_gc_epilogue_callback_();
}

uint32_t StringHashSeed() {
uint32_t seed = static_cast<uint32_t>(string_hash_seed()->value());
ASSERT(FLAG_randomize_string_hashes || seed == 0);
return seed;
}

private:
Heap();

Expand Down

0 comments on commit 4a899c9

Please sign in to comment.