2010-02-24 18:59:26

by Jeff Mahoney

[permalink] [raw]
Subject: [PATCH] ftrace: fix ftrace_event_call alignment for use with gcc 4.5

GCC 4.5 introduces behavior that forces the alignment of structures to
use the largest possible value. The default value is 32 bytes, so if
some structures are defined with a 4-byte alignment and others aren't
declared with an alignment constraint at all - it will align at 32-bytes.

For things like the ftrace events, this results in a non-standard array.
When initializing the ftrace subsystem, we traverse the _ftrace_events
section and call the initialization callback for each event. When the
structures are misaligned, we could be treating another part of the
structure (or the zeroed out space between them) as a function pointer.

This patch forces the alignment for all the ftrace_event_call structures
to 4 bytes.

Without this patch, the kernel fails to boot very early when built with
gcc 4.5.

It's trivial to check the alignment of the members of the array, so it
might be worthwhile to add something to the build system to do that
automatically. Unfortunately, that only covers this case. I've asked one
of the gcc developers about adding a warning when this condition is seen.

Signed-off-by: Jeff Mahoney <[email protected]>
---
include/linux/syscalls.h | 6 ++++--
include/trace/ftrace.h | 3 ++-
kernel/trace/trace.h | 3 ++-
3 files changed, 8 insertions(+), 4 deletions(-)

--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -132,7 +132,8 @@ struct perf_event_attr;

#define SYSCALL_TRACE_ENTER_EVENT(sname) \
static const struct syscall_metadata __syscall_meta_##sname; \
- static struct ftrace_event_call event_enter_##sname; \
+ static struct ftrace_event_call \
+ __attribute__((__aligned__(4))) event_enter_##sname; \
static struct trace_event enter_syscall_print_##sname = { \
.trace = print_syscall_enter, \
}; \
@@ -154,7 +155,8 @@ struct perf_event_attr;

#define SYSCALL_TRACE_EXIT_EVENT(sname) \
static const struct syscall_metadata __syscall_meta_##sname; \
- static struct ftrace_event_call event_exit_##sname; \
+ static struct ftrace_event_call \
+ __attribute__((__aligned__(4))) event_exit_##sname; \
static struct trace_event exit_syscall_print_##sname = { \
.trace = print_syscall_exit, \
}; \
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -65,7 +65,8 @@
};
#undef DEFINE_EVENT
#define DEFINE_EVENT(template, name, proto, args) \
- static struct ftrace_event_call event_##name
+ static struct ftrace_event_call \
+ __attribute__((__aligned__(4))) event_##name

#undef DEFINE_EVENT_PRINT
#define DEFINE_EVENT_PRINT(template, name, proto, args, print) \
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -791,7 +791,8 @@ extern const char *__stop___trace_bprint

#undef FTRACE_ENTRY
#define FTRACE_ENTRY(call, struct_name, id, tstruct, print) \
- extern struct ftrace_event_call event_##call;
+ extern struct ftrace_event_call \
+ __attribute__((__aligned__(4))) event_##call;
#undef FTRACE_ENTRY_DUP
#define FTRACE_ENTRY_DUP(call, struct_name, id, tstruct, print) \
FTRACE_ENTRY(call, struct_name, id, PARAMS(tstruct), PARAMS(print))
--
Jeff Mahoney
SUSE Labs


2010-02-24 19:33:24

by Jeff Mahoney

[permalink] [raw]
Subject: Re: [PATCH] ftrace: fix ftrace_event_call alignment for use with gcc 4.5

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/24/2010 01:59 PM, Jeff Mahoney wrote:
> GCC 4.5 introduces behavior that forces the alignment of structures to
> use the largest possible value. The default value is 32 bytes, so if
> some structures are defined with a 4-byte alignment and others aren't
> declared with an alignment constraint at all - it will align at 32-bytes.
>
> For things like the ftrace events, this results in a non-standard array.
> When initializing the ftrace subsystem, we traverse the _ftrace_events
> section and call the initialization callback for each event. When the
> structures are misaligned, we could be treating another part of the
> structure (or the zeroed out space between them) as a function pointer.
>
> This patch forces the alignment for all the ftrace_event_call structures
> to 4 bytes.
>
> Without this patch, the kernel fails to boot very early when built with
> gcc 4.5.
>
> It's trivial to check the alignment of the members of the array, so it
> might be worthwhile to add something to the build system to do that
> automatically. Unfortunately, that only covers this case. I've asked one
> of the gcc developers about adding a warning when this condition is seen.

BTW, the analysis for this is available here:

https://bugzilla.novell.com/show_bug.cgi?id=582222

- -Jeff

- --
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.13 (GNU/Linux)
Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org/

iEYEARECAAYFAkuFfwAACgkQLPWxlyuTD7KvZgCgluUzuO91K0wARk0SaLJeCV7x
we0AnAjPT/M1c4wD0Ti6mUjsZjKx5CXX
=zJLJ
-----END PGP SIGNATURE-----

2010-02-25 14:09:29

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH] ftrace: fix ftrace_event_call alignment for use with gcc 4.5

On Wed, 2010-02-24 at 14:33 -0500, Jeff Mahoney wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 02/24/2010 01:59 PM, Jeff Mahoney wrote:
> > GCC 4.5 introduces behavior that forces the alignment of structures to
> > use the largest possible value. The default value is 32 bytes, so if
> > some structures are defined with a 4-byte alignment and others aren't
> > declared with an alignment constraint at all - it will align at 32-bytes.
> >
> > For things like the ftrace events, this results in a non-standard array.
> > When initializing the ftrace subsystem, we traverse the _ftrace_events
> > section and call the initialization callback for each event. When the
> > structures are misaligned, we could be treating another part of the
> > structure (or the zeroed out space between them) as a function pointer.
> >
> > This patch forces the alignment for all the ftrace_event_call structures
> > to 4 bytes.
> >
> > Without this patch, the kernel fails to boot very early when built with
> > gcc 4.5.
> >
> > It's trivial to check the alignment of the members of the array, so it
> > might be worthwhile to add something to the build system to do that
> > automatically. Unfortunately, that only covers this case. I've asked one
> > of the gcc developers about adding a warning when this condition is seen.
>
> BTW, the analysis for this is available here:
>
> https://bugzilla.novell.com/show_bug.cgi?id=582222
>

Thanks! I'll apply this and push it forward (as well as to stable)

-- Steve

2010-02-26 09:28:20

by Jeff Mahoney

[permalink] [raw]
Subject: [tip:tracing/core] tracing: Fix ftrace_event_call alignment for use with gcc 4.5

Commit-ID: 86c38a31aa7f2dd6e74a262710bf8ebf7455acc5
Gitweb: http://git.kernel.org/tip/86c38a31aa7f2dd6e74a262710bf8ebf7455acc5
Author: Jeff Mahoney <[email protected]>
AuthorDate: Wed, 24 Feb 2010 13:59:23 -0500
Committer: Steven Rostedt <[email protected]>
CommitDate: Thu, 25 Feb 2010 09:38:11 -0500

tracing: Fix ftrace_event_call alignment for use with gcc 4.5

GCC 4.5 introduces behavior that forces the alignment of structures to
use the largest possible value. The default value is 32 bytes, so if
some structures are defined with a 4-byte alignment and others aren't
declared with an alignment constraint at all - it will align at 32-bytes.

For things like the ftrace events, this results in a non-standard array.
When initializing the ftrace subsystem, we traverse the _ftrace_events
section and call the initialization callback for each event. When the
structures are misaligned, we could be treating another part of the
structure (or the zeroed out space between them) as a function pointer.

This patch forces the alignment for all the ftrace_event_call structures
to 4 bytes.

Without this patch, the kernel fails to boot very early when built with
gcc 4.5.

It's trivial to check the alignment of the members of the array, so it
might be worthwhile to add something to the build system to do that
automatically. Unfortunately, that only covers this case. I've asked one
of the gcc developers about adding a warning when this condition is seen.

Cc: [email protected]
Signed-off-by: Jeff Mahoney <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
---
include/linux/syscalls.h | 6 ++++--
include/trace/ftrace.h | 3 ++-
kernel/trace/trace.h | 3 ++-
3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 7b21969..91bd7d7 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -132,7 +132,8 @@ struct perf_event_attr;

#define SYSCALL_TRACE_ENTER_EVENT(sname) \
static const struct syscall_metadata __syscall_meta_##sname; \
- static struct ftrace_event_call event_enter_##sname; \
+ static struct ftrace_event_call \
+ __attribute__((__aligned__(4))) event_enter_##sname; \
static struct trace_event enter_syscall_print_##sname = { \
.trace = print_syscall_enter, \
}; \
@@ -153,7 +154,8 @@ struct perf_event_attr;

#define SYSCALL_TRACE_EXIT_EVENT(sname) \
static const struct syscall_metadata __syscall_meta_##sname; \
- static struct ftrace_event_call event_exit_##sname; \
+ static struct ftrace_event_call \
+ __attribute__((__aligned__(4))) event_exit_##sname; \
static struct trace_event exit_syscall_print_##sname = { \
.trace = print_syscall_exit, \
}; \
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 09fd9af..f23a0ca 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -65,7 +65,8 @@
};
#undef DEFINE_EVENT
#define DEFINE_EVENT(template, name, proto, args) \
- static struct ftrace_event_call event_##name
+ static struct ftrace_event_call \
+ __attribute__((__aligned__(4))) event_##name

#undef DEFINE_EVENT_PRINT
#define DEFINE_EVENT_PRINT(template, name, proto, args, print) \
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index b477fce..fd05bca 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -792,7 +792,8 @@ extern const char *__stop___trace_bprintk_fmt[];

#undef FTRACE_ENTRY
#define FTRACE_ENTRY(call, struct_name, id, tstruct, print) \
- extern struct ftrace_event_call event_##call;
+ extern struct ftrace_event_call \
+ __attribute__((__aligned__(4))) event_##call;
#undef FTRACE_ENTRY_DUP
#define FTRACE_ENTRY_DUP(call, struct_name, id, tstruct, print) \
FTRACE_ENTRY(call, struct_name, id, PARAMS(tstruct), PARAMS(print))