Skip to content

Commit 4a899c9

Browse files
piscisaureuspiscisaureus
committed
Land hash collision fix for V8 3.6 by Erik Corry.
- 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
1 parent dd9593c commit 4a899c9

23 files changed

+446
-162
lines changed

‎deps/v8/src/arm/builtins-arm.cc

+1-4
Original file line numberDiff line numberDiff line change
@@ -1006,10 +1006,7 @@ static void Generate_JSEntryTrampolineHelper(MacroAssembler* masm,
10061006
// Set up the context from the function argument.
10071007
__ ldr(cp, FieldMemOperand(r1, JSFunction::kContextOffset));
10081008

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

10141011
// Push the function and the receiver onto the stack.
10151012
__ push(r1);

‎deps/v8/src/arm/code-stubs-arm.cc

+14-72
Original file line numberDiff line numberDiff line change
@@ -5043,70 +5043,6 @@ void StringCharAtGenerator::GenerateSlow(
50435043
}
50445044

50455045

5046-
class StringHelper : public AllStatic {
5047-
public:
5048-
// Generate code for copying characters using a simple loop. This should only
5049-
// be used in places where the number of characters is small and the
5050-
// additional setup and checking in GenerateCopyCharactersLong adds too much
5051-
// overhead. Copying of overlapping regions is not supported.
5052-
// Dest register ends at the position after the last character written.
5053-
static void GenerateCopyCharacters(MacroAssembler* masm,
5054-
Register dest,
5055-
Register src,
5056-
Register count,
5057-
Register scratch,
5058-
bool ascii);
5059-
5060-
// Generate code for copying a large number of characters. This function
5061-
// is allowed to spend extra time setting up conditions to make copying
5062-
// faster. Copying of overlapping regions is not supported.
5063-
// Dest register ends at the position after the last character written.
5064-
static void GenerateCopyCharactersLong(MacroAssembler* masm,
5065-
Register dest,
5066-
Register src,
5067-
Register count,
5068-
Register scratch1,
5069-
Register scratch2,
5070-
Register scratch3,
5071-
Register scratch4,
5072-
Register scratch5,
5073-
int flags);
5074-
5075-
5076-
// Probe the symbol table for a two character string. If the string is
5077-
// not found by probing a jump to the label not_found is performed. This jump
5078-
// does not guarantee that the string is not in the symbol table. If the
5079-
// string is found the code falls through with the string in register r0.
5080-
// Contents of both c1 and c2 registers are modified. At the exit c1 is
5081-
// guaranteed to contain halfword with low and high bytes equal to
5082-
// initial contents of c1 and c2 respectively.
5083-
static void GenerateTwoCharacterSymbolTableProbe(MacroAssembler* masm,
5084-
Register c1,
5085-
Register c2,
5086-
Register scratch1,
5087-
Register scratch2,
5088-
Register scratch3,
5089-
Register scratch4,
5090-
Register scratch5,
5091-
Label* not_found);
5092-
5093-
// Generate string hash.
5094-
static void GenerateHashInit(MacroAssembler* masm,
5095-
Register hash,
5096-
Register character);
5097-
5098-
static void GenerateHashAddCharacter(MacroAssembler* masm,
5099-
Register hash,
5100-
Register character);
5101-
5102-
static void GenerateHashGetHash(MacroAssembler* masm,
5103-
Register hash);
5104-
5105-
private:
5106-
DISALLOW_IMPLICIT_CONSTRUCTORS(StringHelper);
5107-
};
5108-
5109-
51105046
void StringHelper::GenerateCopyCharacters(MacroAssembler* masm,
51115047
Register dest,
51125048
Register src,
@@ -5359,9 +5295,8 @@ void StringHelper::GenerateTwoCharacterSymbolTableProbe(MacroAssembler* masm,
53595295
static const int kProbes = 4;
53605296
Label found_in_symbol_table;
53615297
Label next_probe[kProbes];
5298+
Register candidate = scratch5; // Scratch register contains candidate.
53625299
for (int i = 0; i < kProbes; i++) {
5363-
Register candidate = scratch5; // Scratch register contains candidate.
5364-
53655300
// Calculate entry in symbol table.
53665301
if (i > 0) {
53675302
__ add(candidate, hash, Operand(SymbolTable::GetProbeOffset(i)));
@@ -5418,7 +5353,7 @@ void StringHelper::GenerateTwoCharacterSymbolTableProbe(MacroAssembler* masm,
54185353
__ jmp(not_found);
54195354

54205355
// Scratch register contains result when we fall through to here.
5421-
Register result = scratch;
5356+
Register result = candidate;
54225357
__ bind(&found_in_symbol_table);
54235358
__ Move(r0, result);
54245359
}
@@ -5428,9 +5363,13 @@ void StringHelper::GenerateHashInit(MacroAssembler* masm,
54285363
Register hash,
54295364
Register character) {
54305365
// hash = character + (character << 10);
5431-
__ add(hash, character, Operand(character, LSL, 10));
5366+
__ LoadRoot(hash, Heap::kStringHashSeedRootIndex);
5367+
// Untag smi seed and add the character.
5368+
__ add(hash, character, Operand(hash, LSR, kSmiTagSize));
5369+
// hash += hash << 10;
5370+
__ add(hash, hash, Operand(hash, LSL, 10));
54325371
// hash ^= hash >> 6;
5433-
__ eor(hash, hash, Operand(hash, ASR, 6));
5372+
__ eor(hash, hash, Operand(hash, LSR, 6));
54345373
}
54355374

54365375

@@ -5442,7 +5381,7 @@ void StringHelper::GenerateHashAddCharacter(MacroAssembler* masm,
54425381
// hash += hash << 10;
54435382
__ add(hash, hash, Operand(hash, LSL, 10));
54445383
// hash ^= hash >> 6;
5445-
__ eor(hash, hash, Operand(hash, ASR, 6));
5384+
__ eor(hash, hash, Operand(hash, LSR, 6));
54465385
}
54475386

54485387

@@ -5451,12 +5390,15 @@ void StringHelper::GenerateHashGetHash(MacroAssembler* masm,
54515390
// hash += hash << 3;
54525391
__ add(hash, hash, Operand(hash, LSL, 3));
54535392
// hash ^= hash >> 11;
5454-
__ eor(hash, hash, Operand(hash, ASR, 11));
5393+
__ eor(hash, hash, Operand(hash, LSR, 11));
54555394
// hash += hash << 15;
54565395
__ add(hash, hash, Operand(hash, LSL, 15), SetCC);
54575396

5397+
uint32_t kHashShiftCutOffMask = (1 << (32 - String::kHashShift)) - 1;
5398+
__ and_(hash, hash, Operand(kHashShiftCutOffMask));
5399+
54585400
// if (hash == 0) hash = 27;
5459-
__ mov(hash, Operand(27), LeaveCC, ne);
5401+
__ mov(hash, Operand(27), LeaveCC, eq);
54605402
}
54615403

54625404

‎deps/v8/src/arm/code-stubs-arm.h

+64
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,70 @@ class BinaryOpStub: public CodeStub {
225225
};
226226

227227

228+
class StringHelper : public AllStatic {
229+
public:
230+
// Generate code for copying characters using a simple loop. This should only
231+
// be used in places where the number of characters is small and the
232+
// additional setup and checking in GenerateCopyCharactersLong adds too much
233+
// overhead. Copying of overlapping regions is not supported.
234+
// Dest register ends at the position after the last character written.
235+
static void GenerateCopyCharacters(MacroAssembler* masm,
236+
Register dest,
237+
Register src,
238+
Register count,
239+
Register scratch,
240+
bool ascii);
241+
242+
// Generate code for copying a large number of characters. This function
243+
// is allowed to spend extra time setting up conditions to make copying
244+
// faster. Copying of overlapping regions is not supported.
245+
// Dest register ends at the position after the last character written.
246+
static void GenerateCopyCharactersLong(MacroAssembler* masm,
247+
Register dest,
248+
Register src,
249+
Register count,
250+
Register scratch1,
251+
Register scratch2,
252+
Register scratch3,
253+
Register scratch4,
254+
Register scratch5,
255+
int flags);
256+
257+
258+
// Probe the symbol table for a two character string. If the string is
259+
// not found by probing a jump to the label not_found is performed. This jump
260+
// does not guarantee that the string is not in the symbol table. If the
261+
// string is found the code falls through with the string in register r0.
262+
// Contents of both c1 and c2 registers are modified. At the exit c1 is
263+
// guaranteed to contain halfword with low and high bytes equal to
264+
// initial contents of c1 and c2 respectively.
265+
static void GenerateTwoCharacterSymbolTableProbe(MacroAssembler* masm,
266+
Register c1,
267+
Register c2,
268+
Register scratch1,
269+
Register scratch2,
270+
Register scratch3,
271+
Register scratch4,
272+
Register scratch5,
273+
Label* not_found);
274+
275+
// Generate string hash.
276+
static void GenerateHashInit(MacroAssembler* masm,
277+
Register hash,
278+
Register character);
279+
280+
static void GenerateHashAddCharacter(MacroAssembler* masm,
281+
Register hash,
282+
Register character);
283+
284+
static void GenerateHashGetHash(MacroAssembler* masm,
285+
Register hash);
286+
287+
private:
288+
DISALLOW_IMPLICIT_CONSTRUCTORS(StringHelper);
289+
};
290+
291+
228292
// Flag that indicates how to generate code for the stub StringAddStub.
229293
enum StringAddFlags {
230294
NO_STRING_ADD_FLAGS = 0,

‎deps/v8/src/arm/deoptimizer-arm.cc

+1-3
Original file line numberDiff line numberDiff line change
@@ -736,9 +736,7 @@ void Deoptimizer::EntryGenerator::Generate() {
736736
__ pop(ip); // remove sp
737737
__ pop(ip); // remove lr
738738

739-
// Set up the roots register.
740-
ExternalReference roots_address = ExternalReference::roots_address(isolate);
741-
__ mov(r10, Operand(roots_address));
739+
__ InitializeRootRegister();
742740

743741
__ pop(ip); // remove pc
744742
__ pop(r7); // get continuation, leave pc on stack

‎deps/v8/src/arm/macro-assembler-arm.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -395,14 +395,14 @@ void MacroAssembler::Usat(Register dst, int satpos, const Operand& src,
395395
void MacroAssembler::LoadRoot(Register destination,
396396
Heap::RootListIndex index,
397397
Condition cond) {
398-
ldr(destination, MemOperand(roots, index << kPointerSizeLog2), cond);
398+
ldr(destination, MemOperand(kRootRegister, index << kPointerSizeLog2), cond);
399399
}
400400

401401

402402
void MacroAssembler::StoreRoot(Register source,
403403
Heap::RootListIndex index,
404404
Condition cond) {
405-
str(source, MemOperand(roots, index << kPointerSizeLog2), cond);
405+
str(source, MemOperand(kRootRegister, index << kPointerSizeLog2), cond);
406406
}
407407

408408

‎deps/v8/src/arm/macro-assembler-arm.h

+7-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ static inline Operand SmiUntagOperand(Register object) {
5151

5252
// Give alias names to registers
5353
const Register cp = { 8 }; // JavaScript context pointer
54-
const Register roots = { 10 }; // Roots array pointer.
54+
const Register kRootRegister = { 10 }; // Roots array pointer.
5555

5656
// Flags used for the AllocateInNewSpace functions.
5757
enum AllocationFlags {
@@ -350,6 +350,12 @@ class MacroAssembler: public Assembler {
350350
Register map,
351351
Register scratch);
352352

353+
void InitializeRootRegister() {
354+
ExternalReference roots_address =
355+
ExternalReference::roots_address(isolate());
356+
mov(kRootRegister, Operand(roots_address));
357+
}
358+
353359
// ---------------------------------------------------------------------------
354360
// JavaScript invokes
355361

‎deps/v8/src/d8.cc

+6-1
Original file line numberDiff line numberDiff line change
@@ -760,8 +760,13 @@ Persistent<Context> Shell::CreateEvaluationContext() {
760760
#endif // V8_SHARED
761761
// Initialize the global objects
762762
Handle<ObjectTemplate> global_template = CreateGlobalTemplate();
763+
764+
v8::TryCatch try_catch;
763765
Persistent<Context> context = Context::New(NULL, global_template);
764-
ASSERT(!context.IsEmpty());
766+
if (context.IsEmpty()) {
767+
v8::Local<v8::Value> st = try_catch.StackTrace();
768+
ASSERT(!context.IsEmpty());
769+
}
765770
Context::Scope scope(context);
766771

767772
#ifndef V8_SHARED

‎deps/v8/src/flag-definitions.h

+8
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,14 @@ DEFINE_bool(trace_exception, false,
319319
"print stack trace when throwing exceptions")
320320
DEFINE_bool(preallocate_message_memory, false,
321321
"preallocate some memory to build stack traces.")
322+
DEFINE_bool(randomize_string_hashes,
323+
true,
324+
"randomize string hashes to avoid predictable hash collisions "
325+
"(with snapshots this option cannot override the baked-in seed)")
326+
DEFINE_int(string_hash_seed,
327+
0,
328+
"Fixed seed to use to string hashing (0 means random)"
329+
"(with snapshots this option cannot override the baked-in seed)")
322330

323331
// v8.cc
324332
DEFINE_bool(preemption, false,

‎deps/v8/src/heap.cc

+11
Original file line numberDiff line numberDiff line change
@@ -5362,6 +5362,17 @@ bool Heap::Setup(bool create_heap_objects) {
53625362
if (lo_space_ == NULL) return false;
53635363
if (!lo_space_->Setup()) return false;
53645364

5365+
// Set up the seed that is used to randomize the string hash function.
5366+
ASSERT(string_hash_seed() == 0);
5367+
if (FLAG_randomize_string_hashes) {
5368+
if (FLAG_string_hash_seed == 0) {
5369+
set_string_hash_seed(
5370+
Smi::FromInt(V8::RandomPrivate(isolate()) & 0x3fffffff));
5371+
} else {
5372+
set_string_hash_seed(Smi::FromInt(FLAG_string_hash_seed));
5373+
}
5374+
}
5375+
53655376
if (create_heap_objects) {
53665377
// Create initial maps.
53675378
if (!CreateInitialMaps()) return false;

‎deps/v8/src/heap.h

+8-2
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ inline Heap* _inline_get_heap_();
7979
V(FixedArray, single_character_string_cache, SingleCharacterStringCache) \
8080
V(FixedArray, string_split_cache, StringSplitCache) \
8181
V(Object, termination_exception, TerminationException) \
82+
V(Smi, string_hash_seed, StringHashSeed) \
8283
V(FixedArray, empty_fixed_array, EmptyFixedArray) \
8384
V(ByteArray, empty_byte_array, EmptyByteArray) \
8485
V(FixedDoubleArray, empty_fixed_double_array, EmptyFixedDoubleArray) \
@@ -841,8 +842,7 @@ class Heap {
841842
// Please note this function does not perform a garbage collection.
842843
MUST_USE_RESULT MaybeObject* LookupSymbol(Vector<const char> str);
843844
MUST_USE_RESULT MaybeObject* LookupAsciiSymbol(Vector<const char> str);
844-
MUST_USE_RESULT MaybeObject* LookupTwoByteSymbol(
845-
Vector<const uc16> str);
845+
MUST_USE_RESULT MaybeObject* LookupTwoByteSymbol(Vector<const uc16> str);
846846
MUST_USE_RESULT MaybeObject* LookupAsciiSymbol(const char* str) {
847847
return LookupSymbol(CStrVector(str));
848848
}
@@ -1301,6 +1301,12 @@ class Heap {
13011301
if (global_gc_epilogue_callback_ != NULL) global_gc_epilogue_callback_();
13021302
}
13031303

1304+
uint32_t StringHashSeed() {
1305+
uint32_t seed = static_cast<uint32_t>(string_hash_seed()->value());
1306+
ASSERT(FLAG_randomize_string_hashes || seed == 0);
1307+
return seed;
1308+
}
1309+
13041310
private:
13051311
Heap();
13061312

0 commit comments

Comments
 (0)