| Message ID | 1645801f-7dbe-65e8-a991-eee6a9c2f4fc@samsung.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PR,sanitizer/78651] Incorrect exception handling when catch clause uses local class and PIC and sanitizer are active | expand |
On Mon, Mar 19, 2018 at 06:44:39PM +0300, Maxim Ostapenko wrote: > as noted in bugzilla, ASan inserts redzones for `.LDFCM*' variables and > breaks internal ABI between GCC and libstdc++ because libstdc++ tries to > obtain a pointer to `typeinfo for (anonymous namespace)::SomeRandomType' > from a constant offset from `.LDFCM*' labels and hits these redzones. This > can be trivially fixed by not sanitizing `.LDFCM*' variables (and other > debug variables) at all. I don't like very much adding an extra argument for such so frequently used function to handle a corner case. Wouldn't just: /* Don't instrument this decl with -fsanitize=*address. */ unsigned int save_flag_sanitize = flag_sanitize; flag_sanitize &= ~(SANITIZE_ADDRESS | SANITIZE_USER_ADDRESS | SANITIZE_KERNEL_ADDRESS); assemble_variable (decl, 1, 1, 1); flag_sanitize = save_flag_sanitize; DTRT? Jakub
On 03/19/2018 06:55 PM, Jakub Jelinek wrote: > On Mon, Mar 19, 2018 at 06:44:39PM +0300, Maxim Ostapenko wrote: >> as noted in bugzilla, ASan inserts redzones for `.LDFCM*' variables and >> breaks internal ABI between GCC and libstdc++ because libstdc++ tries to >> obtain a pointer to `typeinfo for (anonymous namespace)::SomeRandomType' >> from a constant offset from `.LDFCM*' labels and hits these redzones. This >> can be trivially fixed by not sanitizing `.LDFCM*' variables (and other >> debug variables) at all. > I don't like very much adding an extra argument for such so frequently used > function to handle a corner case. > Wouldn't just: > /* Don't instrument this decl with -fsanitize=*address. */ > unsigned int save_flag_sanitize = flag_sanitize; > flag_sanitize &= ~(SANITIZE_ADDRESS | SANITIZE_USER_ADDRESS > | SANITIZE_KERNEL_ADDRESS); > assemble_variable (decl, 1, 1, 1); > flag_sanitize = save_flag_sanitize; > DTRT? > > Yes, it works, attaching the patch. -Maxim diff --git a/gcc/ChangeLog b/gcc/ChangeLog index b5b5559..356e68c 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +2018-03-19 Maxim Ostapenko <m.ostapenko@samsung.com> + + PR sanitizer/78651 + * dwarf2asm.c (dw2_output_indirect_constant_1): Disable ASan before + calling assemble_variable. + 2018-03-19 Richard Biener <rguenther@suse.de> PR tree-optimization/84929 diff --git a/gcc/dwarf2asm.c b/gcc/dwarf2asm.c index e9b18b8..065406b 100644 --- a/gcc/dwarf2asm.c +++ b/gcc/dwarf2asm.c @@ -967,7 +967,11 @@ dw2_output_indirect_constant_1 (const char *sym, tree id) } sym_ref = gen_rtx_SYMBOL_REF (Pmode, sym); + unsigned int save_flag_sanitize = flag_sanitize; + flag_sanitize &= ~(SANITIZE_ADDRESS | SANITIZE_USER_ADDRESS + | SANITIZE_KERNEL_ADDRESS); assemble_variable (decl, 1, 1, 1); + flag_sanitize = save_flag_sanitize; assemble_integer (sym_ref, POINTER_SIZE_UNITS, POINTER_SIZE, 1); return 0; diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 868d8e8..6ff9217 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2018-03-19 Maxim Ostapenko <m.ostapenko@samsung.com> + + PR sanitizer/78651 + * g++.dg/asan/pr78651.C: New test. + 2018-03-19 Richard Biener <rguenther@suse.de> PR tree-optimization/84929 diff --git a/gcc/testsuite/g++.dg/asan/pr78651.C b/gcc/testsuite/g++.dg/asan/pr78651.C new file mode 100644 index 0000000..3f14be7 --- /dev/null +++ b/gcc/testsuite/g++.dg/asan/pr78651.C @@ -0,0 +1,24 @@ +// { dg-do run { target fpic } } + +struct A { }; + +namespace { + +void thisThrows () { + throw A(); +} + +struct SomeRandomType {}; +} + +int main() { + try { + thisThrows(); + } + catch (SomeRandomType) { + throw; + } + catch (A) { + } + return 0; +}
On Mon, Mar 19, 2018 at 07:28:11PM +0300, Maxim Ostapenko wrote: > Yes, it works, attaching the patch. Can you please add a short comment why we do this? Also, the testcase needs work, see below. Ok for trunk with those changes and after a while for affected release branches as well. Thanks. > --- /dev/null > +++ b/gcc/testsuite/g++.dg/asan/pr78651.C > @@ -0,0 +1,24 @@ > +// { dg-do run { target fpic } } Effective target fpic just means that -fpic or -fPIC is supported, nothing else. So, you want instead: // PR sanitizer/78651 // { dg-do run } // { dg-additional-options "-fpic" { target fpic } } and verify make check-c++-all RUNTESTFLAGS=asan.exp=pr78651.C fails without the dwarf2asm.c patch and succeeds with it. Jakub
On 03/19/2018 07:35 PM, Jakub Jelinek wrote: > On Mon, Mar 19, 2018 at 07:28:11PM +0300, Maxim Ostapenko wrote: >> Yes, it works, attaching the patch. > Can you please add a short comment why we do this? Also, the testcase > needs work, see below. Ok for trunk with those changes and after a > while for affected release branches as well. Ok, thanks, attaching the patch I'm going to apply. -Maxim diff --git a/gcc/ChangeLog b/gcc/ChangeLog index b5b5559..356e68c 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +2018-03-19 Maxim Ostapenko <m.ostapenko@samsung.com> + + PR sanitizer/78651 + * dwarf2asm.c (dw2_output_indirect_constant_1): Disable ASan before + calling assemble_variable. + 2018-03-19 Richard Biener <rguenther@suse.de> PR tree-optimization/84929 diff --git a/gcc/dwarf2asm.c b/gcc/dwarf2asm.c index e9b18b8..2e108ac 100644 --- a/gcc/dwarf2asm.c +++ b/gcc/dwarf2asm.c @@ -967,7 +967,13 @@ dw2_output_indirect_constant_1 (const char *sym, tree id) } sym_ref = gen_rtx_SYMBOL_REF (Pmode, sym); + /* Disable ASan for decl because redzones cause ABI breakage between GCC and + libstdc++ for `.LDFCM*' variables. See PR 78651 for details. */ + unsigned int save_flag_sanitize = flag_sanitize; + flag_sanitize &= ~(SANITIZE_ADDRESS | SANITIZE_USER_ADDRESS + | SANITIZE_KERNEL_ADDRESS); assemble_variable (decl, 1, 1, 1); + flag_sanitize = save_flag_sanitize; assemble_integer (sym_ref, POINTER_SIZE_UNITS, POINTER_SIZE, 1); return 0; diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 868d8e8..6ff9217 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2018-03-19 Maxim Ostapenko <m.ostapenko@samsung.com> + + PR sanitizer/78651 + * g++.dg/asan/pr78651.C: New test. + 2018-03-19 Richard Biener <rguenther@suse.de> PR tree-optimization/84929 diff --git a/gcc/testsuite/g++.dg/asan/pr78651.C b/gcc/testsuite/g++.dg/asan/pr78651.C new file mode 100644 index 0000000..09f1be5 --- /dev/null +++ b/gcc/testsuite/g++.dg/asan/pr78651.C @@ -0,0 +1,26 @@ +// PR sanitizer/78651 +// { dg-do run } +// { dg-additional-options "-fpic" { target fpic } } + +struct A { }; + +namespace { + +void thisThrows () { + throw A(); +} + +struct SomeRandomType {}; +} + +int main() { + try { + thisThrows(); + } + catch (SomeRandomType) { + throw; + } + catch (A) { + } + return 0; +}
gcc/ PR sanitizer/78651 * dwarf2asm.c (dw2_output_indirect_constant_1): Call assemble_variable with new parameter. * output.h (assemble_variable): Add new parameter. * varasm.c (assemble_variable): Add new parameter. Do not protect variable with ASan if is_debug_var is true. gcc/testsuite/ChangeLog: PR sanitizer/78651 * g++.dg/asan/pr78651.C: New test. diff --git a/gcc/ChangeLog b/gcc/ChangeLog index b5b5559..a26f05e 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,12 @@ +2018-03-19 Maxim Ostapenko <m.ostapenko@samsung.com> + + PR sanitizer/78651 + * dwarf2asm.c (dw2_output_indirect_constant_1): Call assemble_variable + with new parameter. + * output.h (assemble_variable): Add new parameter. + * varasm.c (assemble_variable): Add new parameter. Do not protect + variable with ASan if is_debug_var is true. + 2018-03-19 Richard Biener <rguenther@suse.de> PR tree-optimization/84929 diff --git a/gcc/dwarf2asm.c b/gcc/dwarf2asm.c index e9b18b8..5c90994 100644 --- a/gcc/dwarf2asm.c +++ b/gcc/dwarf2asm.c @@ -967,7 +967,7 @@ dw2_output_indirect_constant_1 (const char *sym, tree id) } sym_ref = gen_rtx_SYMBOL_REF (Pmode, sym); - assemble_variable (decl, 1, 1, 1); + assemble_variable (decl, 1, 1, 1, 1); assemble_integer (sym_ref, POINTER_SIZE_UNITS, POINTER_SIZE, 1); return 0; diff --git a/gcc/output.h b/gcc/output.h index f708cc7..678c370 100644 --- a/gcc/output.h +++ b/gcc/output.h @@ -200,7 +200,7 @@ extern void assemble_end_function (tree, const char *); to define things that have had only tentative definitions. DONT_OUTPUT_DATA if nonzero means don't actually output the initial value (that will be done by the caller). */ -extern void assemble_variable (tree, int, int, int); +extern void assemble_variable (tree, int, int, int, int is_debug_var = 0); /* Put the vtable verification constructor initialization function into the preinit array. */ diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 868d8e8..6ff9217 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2018-03-19 Maxim Ostapenko <m.ostapenko@samsung.com> + + PR sanitizer/78651 + * g++.dg/asan/pr78651.C: New test. + 2018-03-19 Richard Biener <rguenther@suse.de> PR tree-optimization/84929 diff --git a/gcc/testsuite/g++.dg/asan/pr78651.C b/gcc/testsuite/g++.dg/asan/pr78651.C new file mode 100644 index 0000000..3f14be7 --- /dev/null +++ b/gcc/testsuite/g++.dg/asan/pr78651.C @@ -0,0 +1,24 @@ +// { dg-do run { target fpic } } + +struct A { }; + +namespace { + +void thisThrows () { + throw A(); +} + +struct SomeRandomType {}; +} + +int main() { + try { + thisThrows(); + } + catch (SomeRandomType) { + throw; + } + catch (A) { + } + return 0; +} diff --git a/gcc/varasm.c b/gcc/varasm.c index 2b5c70c..b18d011 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -2158,11 +2158,14 @@ assemble_undefined_decl (tree decl) AT_END is nonzero if this is the special handling, at end of compilation, to define things that have had only tentative definitions. DONT_OUTPUT_DATA if nonzero means don't actually output the - initial value (that will be done by the caller). */ + initial value (that will be done by the caller). + IS_DEBUG_VAR if nonzero that we are assembling debug variable. + This information is useful for ASan, see pr78651. */ void assemble_variable (tree decl, int top_level ATTRIBUTE_UNUSED, - int at_end ATTRIBUTE_UNUSED, int dont_output_data) + int at_end ATTRIBUTE_UNUSED, int dont_output_data, + int is_debug_var) { const char *name; rtx decl_rtl, symbol; @@ -2258,6 +2261,7 @@ assemble_variable (tree decl, int top_level ATTRIBUTE_UNUSED, align_variable (decl, dont_output_data); if ((flag_sanitize & SANITIZE_ADDRESS) + && !is_debug_var && asan_protect_global (decl)) { asan_protected = true;