[for-next][PATCH 04/21] ftrace/x86: Add back ftrace_expected assignment

[for-next][PATCH 04/21] ftrace/x86: Add back ftrace_expected assignment

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

When a ftrace_bug happens (where ftrace fails to modify a location) it is
helpful to have what was at that location as well as what was expected to
be there.

But with the conversion to text_poke() the variable that assigns the
expected for debugging was dropped. Unfortunately, I noticed this when I
needed it. Add it back.

Link: https://lkml.kernel.org/r/20220726101851.069d2e70@gandalf.local.home

Cc: "x86@kernel.org" <x86@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: stable@vger.kernel.org
Fixes: 768ae4406a5c ("x86/ftrace: Use text_poke()")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 arch/x86/kernel/ftrace.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 5b4efc927d80..f5cdb5f675b3 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -91,6 +91,7 @@ static int ftrace_verify_code(unsigned long ip, const char *old_code)
 
 	/* Make sure it is what we expect it to be */
 	if (memcmp(cur_code, old_code, MCOUNT_INSN_SIZE) != 0) {
+		ftrace_expected = old_code;
 		WARN_ON(1);
 		return -EINVAL;
 	}
-- 
2.35.1

[for-next][PATCH 21/21] tracing: Use a struct alignof to determine trace event field alignment

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

alignof() gives an alignment of types as they would be as standalone
variables. But alignment in structures might be different, and when
building the fields of events, the alignment must be the actual
alignment otherwise the field offsets may not match what they actually
are.

This caused trace-cmd to crash, as libtraceevent did not check if the
field offset was bigger than the event. The write_msr and read_msr
events on 32 bit had their fields incorrect, because it had a u64 field
between two ints. alignof(u64) would give 8, but the u64 field was at a
4 byte alignment.

Define a macro as:

   ALIGN_STRUCTFIELD(type) ((int)(offsetof(struct {char a; type b;}, b)))

which gives the actual alignment of types in a structure.

Link: https://lkml.kernel.org/r/20220731015928.7ab3a154@rorschach.local.home

Cc: Ingo Molnar <mingo@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: stable@vger.kernel.org
Fixes: 04ae87a52074e ("ftrace: Rework event_create_dir()")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 include/trace/stages/stage4_event_fields.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/trace/stages/stage4_event_fields.h b/include/trace/stages/stage4_event_fields.h
index c3790ec7a453..80d34f396555 100644
--- a/include/trace/stages/stage4_event_fields.h
+++ b/include/trace/stages/stage4_event_fields.h
@@ -2,16 +2,18 @@
 
 /* Stage 4 definitions for creating trace events */
 
+#define ALIGN_STRUCTFIELD(type) ((int)(offsetof(struct {char a; type b;}, b)))
+
 #undef __field_ext
 #define __field_ext(_type, _item, _filter_type) {			\
 	.type = #_type, .name = #_item,					\
-	.size = sizeof(_type), .align = __alignof__(_type),		\
+	.size = sizeof(_type), .align = ALIGN_STRUCTFIELD(_type),	\
 	.is_signed = is_signed_type(_type), .filter_type = _filter_type },
 
 #undef __field_struct_ext
 #define __field_struct_ext(_type, _item, _filter_type) {		\
 	.type = #_type, .name = #_item,					\
-	.size = sizeof(_type), .align = __alignof__(_type),		\
+	.size = sizeof(_type), .align = ALIGN_STRUCTFIELD(_type),	\
 	0, .filter_type = _filter_type },
 
 #undef __field
@@ -23,7 +25,7 @@
 #undef __array
 #define __array(_type, _item, _len) {					\
 	.type = #_type"["__stringify(_len)"]", .name = #_item,		\
-	.size = sizeof(_type[_len]), .align = __alignof__(_type),	\
+	.size = sizeof(_type[_len]), .align = ALIGN_STRUCTFIELD(_type),	\
 	.is_signed = is_signed_type(_type), .filter_type = FILTER_OTHER },
 
 #undef __dynamic_array
-- 
2.35.1

RE: [for-next][PATCH 21/21] tracing: Use a struct alignof to determine trace event field alignment

[David Laight]

From: Steven Rostedt
> Sent: 31 July 2022 20:04
> 
> alignof() gives an alignment of types as they would be as standalone
> variables. But alignment in structures might be different, and when
> building the fields of events, the alignment must be the actual
> alignment otherwise the field offsets may not match what they actually
> are.
> 
> This caused trace-cmd to crash, as libtraceevent did not check if the
> field offset was bigger than the event. The write_msr and read_msr
> events on 32 bit had their fields incorrect, because it had a u64 field
> between two ints. alignof(u64) would give 8, but the u64 field was at a
> 4 byte alignment.
> 
> Define a macro as:
> 
>    ALIGN_STRUCTFIELD(type) ((int)(offsetof(struct {char a; type b;}, b)))
> 
> which gives the actual alignment of types in a structure.

The simpler:
	__alignof__(struct {type b;})
also works.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [for-next][PATCH 21/21] tracing: Use a struct alignof to determine trace event field alignment

[Steven Rostedt]

On Mon, 1 Aug 2022 07:46:22 +0000
David Laight <David.Laight@ACULAB.COM> wrote:

> > Define a macro as:
> > 
> >    ALIGN_STRUCTFIELD(type) ((int)(offsetof(struct {char a; type b;}, b)))
> > 
> > which gives the actual alignment of types in a structure.  
> 
> The simpler:
> 	__alignof__(struct {type b;})
> also works.

I'll have to try that out.

For now, as the previous version made it through all my tests, I may be
pushing it, but change it to this for simplicity if that also works and
passes all my test.

Thanks,

-- Steve