| Message ID | CACT4Y+ZVa_JGi986DgL-H_O9z8b0fEdYgjFQ5G89pjQYBDzgUw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Dec 04, 2014 at 11:10:15PM +0400, Dmitry Vyukov wrote: > That would be an option too. I don't know whether it is much better or not. > Kernel lives without constructors, they are used only by coverage. And > coverage does not need priorities. So it is only kasan that needs > priorities. That would be a plenty of code in lib/module.c only for > kasan... If you change it, we can revert the gcc change. > Meanwhile here is backport to 4.9. Applied w/o conflicts (not counting > ChangeLog). > > OK to commit to gcc.gnu.org/svn/gcc/branches/gcc-4_9-branch ? Ok. Jakub
will wait for Andrey this time On Thu, Dec 4, 2014 at 10:12 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Thu, Dec 04, 2014 at 11:10:15PM +0400, Dmitry Vyukov wrote: >> That would be an option too. I don't know whether it is much better or not. >> Kernel lives without constructors, they are used only by coverage. And >> coverage does not need priorities. So it is only kasan that needs >> priorities. That would be a plenty of code in lib/module.c only for >> kasan... > > If you change it, we can revert the gcc change. > >> Meanwhile here is backport to 4.9. Applied w/o conflicts (not counting >> ChangeLog). >> >> OK to commit to gcc.gnu.org/svn/gcc/branches/gcc-4_9-branch ? > > Ok. > > Jakub
On 12/04/2014 10:10 PM, Dmitry Vyukov wrote: > On Wed, Dec 3, 2014 at 9:19 AM, Andrey Ryabinin <a.ryabinin@samsung.com> wrote: >> On 12/02/2014 08:56 PM, Dmitry Vyukov wrote: >>> Hi, >>> >>> The following patch adds support for instrumentation of globals for >>> Linux kernel (-fsanitize=kernel-address). Kernel only supports >>> constructors with default priority, but the rest works fine. >>> >>> OK for trunk? >>> >> >> I know this is too late already, but why we need this? >> IMO it's much better to add support for constructors with priorities in kernel. >> >> We need to do this in kernel anyway, because GCC 4.9.2 don't have this patch and >> I assume that we want to make instrumentation of globals work in kernel with 4.9.2 > > > That would be an option too. I don't know whether it is much better or not. > Kernel lives without constructors, they are used only by coverage. And > coverage does not need priorities. So it is only kasan that needs > priorities. That would be a plenty of code in lib/module.c only for > kasan... It will be a very little piece of code. I don't think that it will be a problem Perhaps, I'll cook a patch today. > > Meanwhile here is backport to 4.9. Applied w/o conflicts (not counting > ChangeLog). > > OK to commit to gcc.gnu.org/svn/gcc/branches/gcc-4_9-branch ? > > > > Index: gcc/ChangeLog > =================================================================== > --- gcc/ChangeLog (revision 218382) > +++ gcc/ChangeLog (working copy) > @@ -1,3 +1,8 @@ > +2014-12-04 Dmitry Vyukov <dvyukov@google.com> > + > + * asan.c: (asan_finish_file): Use default priority for constructors > + in kernel mode. > + > 2014-12-04 Jakub Jelinek <jakub@redhat.com> > > PR c++/56493 > Index: gcc/asan.c > =================================================================== > --- gcc/asan.c (revision 218382) > +++ gcc/asan.c (working copy) > @@ -1295,7 +1295,9 @@ > the var that is selected by the linker will have > padding or not. */ > || DECL_ONE_ONLY (decl) > - /* Similarly for common vars. People can use -fno-common. */ > + /* Similarly for common vars. People can use -fno-common. > + Note: Linux kernel is built with -fno-common, so we do instrument > + globals there even if it is C. */ > || (DECL_COMMON (decl) && TREE_PUBLIC (decl)) > /* Don't protect if using user section, often vars placed > into user section from multiple TUs are then assumed > @@ -2383,6 +2385,13 @@ > nor after .LASAN* array. */ > flag_sanitize &= ~SANITIZE_ADDRESS; > > + /* For user-space we want asan constructors to run first. > + Linux kernel does not support priorities other than default, and the only > + other user of constructors is coverage. So we run with the default > + priority. */ > + int priority = flag_sanitize & SANITIZE_USER_ADDRESS > + ? MAX_RESERVED_INIT_PRIORITY - 1 : DEFAULT_INIT_PRIORITY; > + > if (flag_sanitize & SANITIZE_USER_ADDRESS) > { > tree fn = builtin_decl_implicit (BUILT_IN_ASAN_INIT); > @@ -2436,12 +2445,10 @@ > build_fold_addr_expr (var), > gcount_tree), > &dtor_statements); > - cgraph_build_static_cdtor ('D', dtor_statements, > - MAX_RESERVED_INIT_PRIORITY - 1); > + cgraph_build_static_cdtor ('D', dtor_statements, priority); > } > if (asan_ctor_statements) > - cgraph_build_static_cdtor ('I', asan_ctor_statements, > - MAX_RESERVED_INIT_PRIORITY - 1); > + cgraph_build_static_cdtor ('I', asan_ctor_statements, priority); > flag_sanitize |= SANITIZE_ADDRESS; > } >
Index: gcc/ChangeLog =================================================================== --- gcc/ChangeLog (revision 218382) +++ gcc/ChangeLog (working copy) @@ -1,3 +1,8 @@ +2014-12-04 Dmitry Vyukov <dvyukov@google.com> + + * asan.c: (asan_finish_file): Use default priority for constructors + in kernel mode. + 2014-12-04 Jakub Jelinek <jakub@redhat.com> PR c++/56493 Index: gcc/asan.c =================================================================== --- gcc/asan.c (revision 218382) +++ gcc/asan.c (working copy) @@ -1295,7 +1295,9 @@ the var that is selected by the linker will have padding or not. */ || DECL_ONE_ONLY (decl) - /* Similarly for common vars. People can use -fno-common. */ + /* Similarly for common vars. People can use -fno-common. + Note: Linux kernel is built with -fno-common, so we do instrument + globals there even if it is C. */ || (DECL_COMMON (decl) && TREE_PUBLIC (decl)) /* Don't protect if using user section, often vars placed into user section from multiple TUs are then assumed @@ -2383,6 +2385,13 @@ nor after .LASAN* array. */ flag_sanitize &= ~SANITIZE_ADDRESS; + /* For user-space we want asan constructors to run first. + Linux kernel does not support priorities other than default, and the only + other user of constructors is coverage. So we run with the default + priority. */ + int priority = flag_sanitize & SANITIZE_USER_ADDRESS + ? MAX_RESERVED_INIT_PRIORITY - 1 : DEFAULT_INIT_PRIORITY; + if (flag_sanitize & SANITIZE_USER_ADDRESS) { tree fn = builtin_decl_implicit (BUILT_IN_ASAN_INIT); @@ -2436,12 +2445,10 @@ build_fold_addr_expr (var), gcount_tree), &dtor_statements); - cgraph_build_static_cdtor ('D', dtor_statements, - MAX_RESERVED_INIT_PRIORITY - 1); + cgraph_build_static_cdtor ('D', dtor_statements, priority); } if (asan_ctor_statements) - cgraph_build_static_cdtor ('I', asan_ctor_statements, - MAX_RESERVED_INIT_PRIORITY - 1); + cgraph_build_static_cdtor ('I', asan_ctor_statements, priority); flag_sanitize |= SANITIZE_ADDRESS; }