Hi, any one got a chance to take look at this patch? It seems that some other guys are also interested in this patch, the clang developer is also proposing implement this "-fstack-protect-strong" option.
Patch has just been merged with newest trunk and fixed a bug reported by Kees. Tested fox x86_64 and arm. Below patches also uploaded as patchset#4 at https://codereview.appspot.com/6303078/ ========================================================================== diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index 299150e..6eb18d6 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -1244,6 +1244,11 @@ clear_tree_used (tree block) #define SPCT_HAS_ARRAY 4 #define SPCT_HAS_AGGREGATE 8 +/* Constants for flag_stack_protect. */ +#define SPCT_ALL 3 +#define SPCT_STRONG 2 +#define SPCT_DEFAULT 1 + static unsigned int stack_protect_classify_type (tree type) { @@ -1306,7 +1311,8 @@ stack_protect_decl_phase (tree decl) if (bits & SPCT_HAS_SMALL_CHAR_ARRAY) has_short_buffer = true; - if (flag_stack_protect == 2) + if (flag_stack_protect == SPCT_ALL || + flag_stack_protect == SPCT_STRONG) { if ((bits & (SPCT_HAS_SMALL_CHAR_ARRAY | SPCT_HAS_LARGE_CHAR_ARRAY)) && !(bits & SPCT_HAS_AGGREGATE)) @@ -1444,6 +1450,29 @@ estimated_stack_frame_size (struct cgraph_node *node) return size; } +/* Helper routine to check if a record or union contains an array field. */ + +static int +record_or_union_type_has_array_p (const_tree tree_type) +{ + tree fields = TYPE_FIELDS (tree_type); + tree f; + + for (f = fields; f; f = DECL_CHAIN (f)) + { + if (TREE_CODE (f) == FIELD_DECL) + { + tree field_type = TREE_TYPE (f); + if (RECORD_OR_UNION_TYPE_P (field_type) && + record_or_union_type_has_array_p (field_type)) + return 1; + if (TREE_CODE (field_type) == ARRAY_TYPE) + return 1; + } + } + return 0; +} + /* Expand all variables used in the function. */ static void @@ -1454,6 +1483,7 @@ expand_used_vars (void) struct pointer_map_t *ssa_name_decls; unsigned i; unsigned len; + int gen_stack_protect_signal = 0; /* Compute the phase of the stack frame for this function. */ { @@ -1505,6 +1535,23 @@ expand_used_vars (void) } pointer_map_destroy (ssa_name_decls); + FOR_EACH_LOCAL_DECL (cfun, i, var) + { + tree var_type = TREE_TYPE (var); + /* Examine local referenced variables that have their addresses taken, + contain an array, or are arrays. */ + if (TREE_CODE (var) == VAR_DECL + && (TREE_CODE (var_type) == ARRAY_TYPE + || TREE_ADDRESSABLE (var) + || (RECORD_OR_UNION_TYPE_P (var_type) + && record_or_union_type_has_array_p (var_type)))) + { + ++gen_stack_protect_signal; + break; + } + } + + /* At this point all variables on the local_decls with TREE_USED set are not associated with any block scope. Lay them out. */ @@ -1591,11 +1638,18 @@ expand_used_vars (void) dump_stack_var_partition (); } - /* There are several conditions under which we should create a - stack guard: protect-all, alloca used, protected decls present. */ - if (flag_stack_protect == 2 - || (flag_stack_protect - && (cfun->calls_alloca || has_protected_decls))) + /* Create stack guard, if + a) "-fstack-protector-all" - always; + b) "-fstack-protector-strong" - if there are arrays, memory + references to local variables, alloca used, or protected decls present; + c) "-fstack-protector" - if alloca used, or protected decls present */ + if (flag_stack_protect == SPCT_ALL /* -fstack-protector-all */ + || (flag_stack_protect == SPCT_STRONG /* -fstack-protector-strong */ + && (gen_stack_protect_signal || cfun->calls_alloca + || has_protected_decls)) + || (flag_stack_protect == SPCT_DEFAULT /* -fstack-protector */ + && (cfun->calls_alloca + || has_protected_decls))) create_stack_guard (); /* Assign rtl to each variable based on these partitions. */ @@ -1612,7 +1666,8 @@ expand_used_vars (void) expand_stack_vars (stack_protect_decl_phase_1); /* Phase 2 contains other kinds of arrays. */ - if (flag_stack_protect == 2) + if (flag_stack_protect == SPCT_ALL || + flag_stack_protect == SPCT_STRONG) expand_stack_vars (stack_protect_decl_phase_2); } diff --git a/gcc/common.opt b/gcc/common.opt index f0e757c..942fbc0 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -1892,8 +1892,12 @@ fstack-protector Common Report Var(flag_stack_protect, 1) Use propolice as a stack protection method -fstack-protector-all +fstack-protector-strong Common Report RejectNegative Var(flag_stack_protect, 2) +Use a smart stack protection method for certain functions + +fstack-protector-all +Common Report RejectNegative Var(flag_stack_protect, 3) Use a stack protection method for every function fstack-usage diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 7578dda..e1f2f2d 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -406,7 +406,7 @@ Objective-C and Objective-C++ Dialects}. -fsel-sched-pipelining -fsel-sched-pipelining-outer-loops @gol -fshrink-wrap -fsignaling-nans -fsingle-precision-constant @gol -fsplit-ivs-in-unroller -fsplit-wide-types -fstack-protector @gol --fstack-protector-all -fstrict-aliasing -fstrict-overflow @gol +-fstack-protector-all -fstack-protector-strong -fstrict-aliasing -fstrict-overflow @gol -fthread-jumps -ftracer -ftree-bit-ccp @gol -ftree-builtin-call-dce -ftree-ccp -ftree-ch @gol -ftree-coalesce-inline-vars -ftree-coalesce-vars -ftree-copy-prop @gol @@ -8697,15 +8697,23 @@ branch target registers within any basic block. @opindex fstack-protector Emit extra code to check for buffer overflows, such as stack smashing attacks. This is done by adding a guard variable to functions with -vulnerable objects. This includes functions that call @code{alloca}, and -functions with buffers larger than 8 bytes. The guards are initialized -when a function is entered and then checked when the function exits. -If a guard check fails, an error message is printed and the program exits. +vulnerable objects. This includes functions that call @code{alloca}, +and functions with buffers larger than 8 bytes. (Note, covering +@code{alloca} requires optimizations to be enable.) The guards are +initialized when a function is entered and then checked when the +function exits. If a guard check fails, an error message is printed +and the program exits. @item -fstack-protector-all @opindex fstack-protector-all Like @option{-fstack-protector} except that all functions are protected. +@item -fstack-protector-strong +@opindex fstack-protector-strong +Like @option{-fstack-protector} but includes additional functions to be +protected - those that have local array definitions, or have references to +local frame addresses. + @item -fsection-anchors @opindex fsection-anchors Try to reduce the number of symbolic address calculations by using diff --git a/gcc/testsuite/g++.dg/fstack-protector-strong.C b/gcc/testsuite/g++.dg/fstack-protector-strong.C new file mode 100644 index 0000000..a4f0f81 --- /dev/null +++ b/gcc/testsuite/g++.dg/fstack-protector-strong.C @@ -0,0 +1,35 @@ +/* Test that stack protection is done on chosen functions. */ + +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */ +/* { dg-options "-O2 -fstack-protector-strong" } */ + +class A +{ +public: + A() {} + ~A() {} + void method(); + int state; +}; + +/* Frame address exposed to A::method via "this". */ +int +foo1 () +{ + A a; + a.method (); + return a.state; +} + +/* Possible destroying foo2's stack via &a. */ +int +global_func (A& a); + +/* Frame address exposed to global_func. */ +int foo2 () +{ + A a; + return global_func (a); +} + +/* { dg-final { scan-assembler-times "stack_chk_fail" 2 } } */ diff --git a/gcc/testsuite/gcc.dg/fstack-protector-strong.c b/gcc/testsuite/gcc.dg/fstack-protector-strong.c new file mode 100644 index 0000000..5a5cf98 --- /dev/null +++ b/gcc/testsuite/gcc.dg/fstack-protector-strong.c @@ -0,0 +1,135 @@ +/* Test that stack protection is done on chosen functions. */ + +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */ +/* { dg-options "-O2 -fstack-protector-strong" } */ + +#include<string.h> +#include<stdlib.h> + +extern int g0; +extern int* pg0; +int +goo (int *); +int +hoo (int); + +/* Function frame address escaped function call. */ +int +foo1 () +{ + int i; + return goo (&i); +} + +struct ArrayStruct +{ + int a; + int array[10]; +}; + +struct AA +{ + int b; + struct ArrayStruct as; +}; + +/* Function frame contains array. */ +int +foo2 () +{ + struct AA aa; + int i; + for (i = 0; i < 10; ++i) + { + aa.as.array[i] = i * (i-1) + i / 2; + } + return aa.as.array[5]; +} + +/* Address computation based on a function frame address. */ +int +foo3 () +{ + int a; + int *p; + p = &a + 5; + return goo (p); +} + +/* Address cast based on a function frame address. */ +int +foo4 () +{ + int a; + return goo (g0 << 2 ? (int *)(3 * (long)(void *)(&a)) : 0); +} + +/* Address cast based on a local array. */ +int +foo5 () +{ + short array[10]; + return goo ((int *)(array + 5)); +} + +struct BB +{ + int one; + int two; + int three; +}; + +/* Address computaton based on a function frame address.*/ +int +foo6 () +{ + struct BB bb; + return goo (&bb.one + sizeof(int)); +} + +/* Function frame address escaped via global variable. */ +int +foo7 () +{ + int a; + pg0 = &a; + goo (pg0); + return *pg0; +} + +/* Check that this covers -fstack-protector. */ +int +foo8 () +{ + char base[100]; + memcpy ((void *)base, (const void *)pg0, 105); + return (int)(base[32]); +} + +/* Check that this covers -fstack-protector. */ +int +foo9 () +{ + char* p = alloca (100); + return goo ((int *)(p + 50)); +} + +int +global2 (struct BB* pbb); + +/* Address taken on struct. */ +int +foo10 () +{ + struct BB bb; + int i; + bb.one = global2 (&bb); + for (i = 0; i < 10; ++i) + { + bb.two = bb.one + bb.two; + bb.three = bb.one + bb.two + bb.three; + } + return bb.three; +} + +/* { dg-final { scan-assembler-times "stack_chk_fail" 10 } } */ On Tue, Sep 11, 2012 at 9:50 AM, Carrot Wei <car...@google.com> wrote: > > > ---------- Forwarded message ---------- > From: Han Shen(沈涵) <shen...@google.com> > Date: Thu, Jun 14, 2012 at 4:09 PM > Subject: [PATCH] Add a new option "-fstack-protector-strong" (patch / doc > inside) > To: gcc-patches@gcc.gnu.org > Cc: Diego Novillo <dnovi...@google.com>, Jing Yu <jin...@google.com>, Kees > Cook <keesc...@google.com>, Ahmad Sharif <asha...@google.com>, David Li > <davi...@google.com>, Rong Xu <x...@google.com> > > > Hi, > > This is to port the patch from google/main to trunk, which provides a new > stack > protection option - "fstack-protector-strong". > > Previous review for google trunk is here - > http://codereview.appspot.com/5461043 > > Status - it has been used in google/main for 2 quarters, building the whole > chromiumos with no securiy degradation. > > Benefit - gain big performance while sacrificing little security (for > scenarios > using -fstack-protector-all) > > Background - some times stack-protector is too-simple while > stack-protector-all > over-kills, for example, to build one of our core systems, we forcibly add > "-fstack-protector-all" to all compile commands, which brings big > performance > penalty (due to extra stack guard/check insns on function prologue and > epilogue) > on both atom and arm. To use "-fstack-protector" is just regarded as not > secure > enough (only "protects" <2% functions) by the system secure team. So I'd > like to > add the option "-fstack-protector-strong", that hits the balance between > "-fstack-protector" and "-fstack-protector-all". > > Tested - building chromiumos from scratch. > > Changelog - > > gcc/ChangeLog: > * cfgexpand.c (expand_used_vars): Add logic handling > stack-protector-strong. > (record_or_union_type_has_array_p): New, tests if a record or > union type contains an array. > * common.opt (fstack-protector-all): New option. > * doc/invoke.texi: Added reference to "-fstack-protector-strong". > > gcc/testsuite/ChangeLog > * gcc.dg/fstack-protector-strong.c: New. > * g++.dg/fstack-protector-strong.C: New. > > > > Patch - > > diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c > index 8a31a9f..0911b6c 100644 > --- a/gcc/cfgexpand.c > +++ b/gcc/cfgexpand.c > @@ -1511,15 +1511,39 @@ estimated_stack_frame_size (struct cgraph_node > *node) > return size; > } > > +/* Helper routine to check if a record or union contains an array field. */ > + > +static int > +record_or_union_type_has_array_p (const_tree tree_type) > +{ > + tree fields = TYPE_FIELDS (tree_type); > + tree f; > + > + for (f = fields; f; f = DECL_CHAIN (f)) > + { > + if (TREE_CODE (f) == FIELD_DECL) > + { > + tree field_type = TREE_TYPE (f); > + if (RECORD_OR_UNION_TYPE_P (field_type)) > + return record_or_union_type_has_array_p (field_type); > + if (TREE_CODE (field_type) == ARRAY_TYPE) > + return 1; > + } > + } > + return 0; > +} > + > /* Expand all variables used in the function. */ > > static void > expand_used_vars (void) > { > tree var, outer_block = DECL_INITIAL (current_function_decl); > + referenced_var_iterator rvi; > VEC(tree,heap) *maybe_local_decls = NULL; > unsigned i; > unsigned len; > + int gen_stack_protect_signal = 0; > > /* Compute the phase of the stack frame for this function. */ > { > @@ -1552,6 +1576,23 @@ expand_used_vars (void) > } > } > > + FOR_EACH_REFERENCED_VAR (cfun, var, rvi) > + if (!is_global_var (var)) > + { > + tree var_type = TREE_TYPE (var); > + /* Examine local referenced variables that have their addresses > taken, > + contain an array, or are arrays. */ > + if (TREE_CODE (var) == VAR_DECL > + && (TREE_CODE (var_type) == ARRAY_TYPE > + || TREE_ADDRESSABLE (var) > + || (RECORD_OR_UNION_TYPE_P (var_type) > + && record_or_union_type_has_array_p (var_type)))) > + { > + ++gen_stack_protect_signal; > + break; > + } > + } > + > /* At this point all variables on the local_decls with TREE_USED > set are not associated with any block scope. Lay them out. */ > > @@ -1642,11 +1683,18 @@ expand_used_vars (void) > dump_stack_var_partition (); > } > > - /* There are several conditions under which we should create a > - stack guard: protect-all, alloca used, protected decls present. */ > - if (flag_stack_protect == 2 > - || (flag_stack_protect > - && (cfun->calls_alloca || has_protected_decls))) > + /* Create stack guard, if > + a) "-fstack-protector-all" - always; > + b) "-fstack-protector-strong" - if there are arrays, memory > + references to local variables, alloca used, or protected decls > present; > + c) "-fstack-protector" - if alloca used, or protected decls present > */ > + if (flag_stack_protect == 3 /* -fstack-protector-all */ > + || (flag_stack_protect == 2 /* -fstack-protector-strong */ > + && (gen_stack_protect_signal || cfun->calls_alloca > + || has_protected_decls)) > + || (flag_stack_protect == 1 /* -fstack-protector */ > + && (cfun->calls_alloca > + || has_protected_decls))) > create_stack_guard (); > > /* Assign rtl to each variable based on these partitions. */ > diff --git a/gcc/common.opt b/gcc/common.opt > index 5b1b4d8..44447f6 100644 > --- a/gcc/common.opt > +++ b/gcc/common.opt > @@ -1846,8 +1846,12 @@ fstack-protector > Common Report Var(flag_stack_protect, 1) > Use propolice as a stack protection method > > -fstack-protector-all > +fstack-protector-strong > Common Report RejectNegative Var(flag_stack_protect, 2) > +Use a smart stack protection method for certain functions > + > +fstack-protector-all > +Common Report RejectNegative Var(flag_stack_protect, 3) > Use a stack protection method for every function > > fstack-usage > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index 9fa0085..44f2034 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -403,7 +403,7 @@ Objective-C and Objective-C++ Dialects}. > -fsel-sched-pipelining -fsel-sched-pipelining-outer-loops @gol > -fshrink-wrap -fsignaling-nans -fsingle-precision-constant @gol > -fsplit-ivs-in-unroller -fsplit-wide-types -fstack-protector @gol > --fstack-protector-all -fstrict-aliasing -fstrict-overflow @gol > +-fstack-protector-all -fstack-protector-strong -fstrict-aliasing > -fstrict-overflow @gol > -fthread-jumps -ftracer -ftree-bit-ccp @gol > -ftree-builtin-call-dce -ftree-ccp -ftree-ch @gol > -ftree-coalesce-inline-vars -ftree-coalesce-vars -ftree-copy-prop @gol > @@ -8564,6 +8564,12 @@ If a guard check fails, an error message is > printed and the program exits. > @opindex fstack-protector-all > Like @option{-fstack-protector} except that all functions are protected. > > +@item -fstack-protector-strong > +@opindex fstack-protector-strong > +Like @option{-fstack-protector} but includes additional functions to be > +protected - those that have local array definitions, or have references to > +local frame addresses. > + > @item -fsection-anchors > @opindex fsection-anchors > Try to reduce the number of symbolic address calculations by using > diff --git a/gcc/testsuite/g++.dg/fstack-protector-strong.C > b/gcc/testsuite/g++.dg/fstack-protector-strong.C > new file mode 100644 > index 0000000..a4f0f81 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/fstack-protector-strong.C > @@ -0,0 +1,35 @@ > +/* Test that stack protection is done on chosen functions. */ > + > +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */ > +/* { dg-options "-O2 -fstack-protector-strong" } */ > + > +class A > +{ > +public: > + A() {} > + ~A() {} > + void method(); > + int state; > +}; > + > +/* Frame address exposed to A::method via "this". */ > +int > +foo1 () > +{ > + A a; > + a.method (); > + return a.state; > +} > + > +/* Possible destroying foo2's stack via &a. */ > +int > +global_func (A& a); > + > +/* Frame address exposed to global_func. */ > +int foo2 () > +{ > + A a; > + return global_func (a); > +} > + > +/* { dg-final { scan-assembler-times "stack_chk_fail" 2 } } */ > diff --git a/gcc/testsuite/gcc.dg/fstack-protector-strong.c > b/gcc/testsuite/gcc.dg/fstack-protector-strong.c > new file mode 100644 > index 0000000..5a5cf98 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/fstack-protector-strong.c > @@ -0,0 +1,135 @@ > +/* Test that stack protection is done on chosen functions. */ > + > +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */ > +/* { dg-options "-O2 -fstack-protector-strong" } */ > + > +#include<string.h> > +#include<stdlib.h> > + > +extern int g0; > +extern int* pg0; > +int > +goo (int *); > +int > +hoo (int); > + > +/* Function frame address escaped function call. */ > +int > +foo1 () > +{ > + int i; > + return goo (&i); > +} > + > +struct ArrayStruct > +{ > + int a; > + int array[10]; > +}; > + > +struct AA > +{ > + int b; > + struct ArrayStruct as; > +}; > + > +/* Function frame contains array. */ > +int > +foo2 () > +{ > + struct AA aa; > + int i; > + for (i = 0; i < 10; ++i) > + { > + aa.as.array[i] = i * (i-1) + i / 2; > + } > + return aa.as.array[5]; > +} > + > +/* Address computation based on a function frame address. */ > +int > +foo3 () > +{ > + int a; > + int *p; > + p = &a + 5; > + return goo (p); > +} > + > +/* Address cast based on a function frame address. */ > +int > +foo4 () > +{ > + int a; > + return goo (g0 << 2 ? (int *)(3 * (long)(void *)(&a)) : 0); > +} > + > +/* Address cast based on a local array. */ > +int > +foo5 () > +{ > + short array[10]; > + return goo ((int *)(array + 5)); > +} > + > +struct BB > +{ > + int one; > + int two; > + int three; > +}; > + > +/* Address computaton based on a function frame address.*/ > +int > +foo6 () > +{ > + struct BB bb; > + return goo (&bb.one + sizeof(int)); > +} > + > +/* Function frame address escaped via global variable. */ > +int > +foo7 () > +{ > + int a; > + pg0 = &a; > + goo (pg0); > + return *pg0; > +} > + > +/* Check that this covers -fstack-protector. */ > +int > +foo8 () > +{ > + char base[100]; > + memcpy ((void *)base, (const void *)pg0, 105); > + return (int)(base[32]); > +} > + > +/* Check that this covers -fstack-protector. */ > +int > +foo9 () > +{ > + char* p = alloca (100); > + return goo ((int *)(p + 50)); > +} > + > +int > +global2 (struct BB* pbb); > + > +/* Address taken on struct. */ > +int > +foo10 () > +{ > + struct BB bb; > + int i; > + bb.one = global2 (&bb); > + for (i = 0; i < 10; ++i) > + { > + bb.two = bb.one + bb.two; > + bb.three = bb.one + bb.two + bb.three; > + } > + return bb.three; > +} > + > +/* { dg-final { scan-assembler-times "stack_chk_fail" 10 } } */ > > > Design doc - > > Proposal to add compiler option “-fstack-protector-strong” > > 1. Current stack protection options > Currently, gcc only has 2 options regarding stack protector > against SSA (stack smashing attack) > - stack-protector > Add stack protection to functions that have “alloca” or have > a (signed or unsigned) char array with size > 8 (SSP_BUFFER_SIZE) > - stack-protector-all > Add stack protection to ALL functions. > > 2. Problems with current stack protection options > The stack-protector option is over-simplified, which ignores > pointer cast, address computation, while the stack-protector-all is > over-killing, using this option brings too much performance overhead > to both arm- and atom- chrome browser. > > 3. Propose a new stack protector option - stack-protector-strong > This option tries to hit the balance between an over-simplified > version and an over-killing protection schema. > > It chooses more functions to be protected than > “stack-protector”, it is a superset of “stack-protector”, for > functions not chosen by “stack-protector”, “stack-protector-strong” > will apply protection if any of the following conditions meets. > - if any of its local variable’s address is taken,as part of the > RHS of an assignment > - or if any of its local variable’s address is taken as part of > a function argument. > - or if it has an array, regardless of array type or length > - or if it has a struct/union which contains an array, > regardless of array type or length. > - or if function has register local variables > > 4. Possible performance gain for atom and tegra board > Replacing “stack-protector-all” with “stack-protector-strong” > sees a good performance gain. > > 5. Ideas behind this implementation: > The basic idea is that any stack smashing attack needs a frame > address to perform the attack. The frame address of function F can be > from one of the following: > - (A) an address taking operator (&) on any local variables of F > - (B) any address computation based on (A) > - (C) any address casting operation from either (A) or (B) > - (D) the name of a local array of F > - (E) the address from calling “alloca” > Function F is said to be vulnerable if its frame address is > exposed via (A) ~ (E). > “stack-protector-strong” just protects these vulnerable functions. > > > 6. Stack smashing attack illustrated > > If in function F, we have pointer P, which points to function > G's stack, you can > only attack frames of function G and functions calling G, for > example X and XX. > > To add guard0 to frame G protects everything above guard0 being > attacked from > P". (Note, v0 and v1 are still attack-able, this won't be fixed > even if you add > guard to all frames.) > > Suppose now you have Q, which points to XX's frame, guard0 will > not prevent > attacking from Q, so we have to add guard1. > > To summarize, we just need to add guard to functions whose frame > address is > exposed(escaped) - either by address taken operator (&), or by > passing address > (or array) around via function call. > > (See picture below) (I cannot upload pictures here, to see the > origin picture - refer here - > https://docs.google.com/a/google.com/document/d/1xXBH6rRZue4f296vGt9YQcuLVQHeE516stHwt8M9xyU/edit?hl=en_US) > > > > Patch also uploaded to http://codereview.appspot.com/6303078/ > > > Regards, > -Han > Regards, -Han