The current code uses a lot of casts to access the fields
member in struct synth_trace_events with different sizes.
This makes the code hard to read, and had already introduced
an endianness bug. Use a union and struct instead.
Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
---
include/linux/trace_events.h | 11 ++++
kernel/trace/trace.h | 8 +++
kernel/trace/trace_events_synth.c | 87 +++++++++++++------------------
3 files changed, 56 insertions(+), 50 deletions(-)
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 3930e676436c..1e8bbdb8da90 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -59,6 +59,17 @@ int trace_raw_output_prep(struct trace_iterator *iter,
extern __printf(2, 3)
void trace_event_printf(struct trace_iterator *iter, const char *fmt, ...);
+/* Used to find the offset and length of dynamic fields in trace events */
+struct trace_dynamic_info {
+#ifdef CONFIG_CPU_BIG_ENDIAN
+ u16 offset;
+ u16 len;
+#else
+ u16 len;
+ u16 offset;
+#endif
+};
+
/*
* The trace entry - the most basic unit of tracing. This is what
* is printed in the end as a single line in the trace output, such as:
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index e1edc2197fc8..95956f75bea5 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1295,6 +1295,14 @@ static inline void trace_branch_disable(void)
/* set ring buffers to default size if not already done so */
int tracing_update_buffers(void);
+union trace_synth_field {
+ u8 as_u8;
+ u16 as_u16;
+ u32 as_u32;
+ u64 as_u64;
+ struct trace_dynamic_info as_dynamic;
+};
+
struct ftrace_event_field {
struct list_head link;
const char *name;
diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c
index dd398afc8e25..7fff8235075f 100644
--- a/kernel/trace/trace_events_synth.c
+++ b/kernel/trace/trace_events_synth.c
@@ -127,7 +127,7 @@ static bool synth_event_match(const char *system, const char *event,
struct synth_trace_event {
struct trace_entry ent;
- u64 fields[];
+ union trace_synth_field fields[];
};
static int synth_event_define_fields(struct trace_event_call *call)
@@ -321,19 +321,19 @@ static const char *synth_field_fmt(char *type)
static void print_synth_event_num_val(struct trace_seq *s,
char *print_fmt, char *name,
- int size, u64 val, char *space)
+ int size, union trace_synth_field *val, char *space)
{
switch (size) {
case 1:
- trace_seq_printf(s, print_fmt, name, (u8)val, space);
+ trace_seq_printf(s, print_fmt, name, val->as_u8, space);
break;
case 2:
- trace_seq_printf(s, print_fmt, name, (u16)val, space);
+ trace_seq_printf(s, print_fmt, name, val->as_u16, space);
break;
case 4:
- trace_seq_printf(s, print_fmt, name, (u32)val, space);
+ trace_seq_printf(s, print_fmt, name, val->as_u32, space);
break;
default:
@@ -374,36 +374,26 @@ static enum print_line_t print_synth_event(struct trace_iterator *iter,
/* parameter values */
if (se->fields[i]->is_string) {
if (se->fields[i]->is_dynamic) {
- u32 offset, data_offset;
- char *str_field;
-
- offset = (u32)entry->fields[n_u64];
- data_offset = offset & 0xffff;
-
- str_field = (char *)entry + data_offset;
+ union trace_synth_field *data = &entry->fields[n_u64];
trace_seq_printf(s, print_fmt, se->fields[i]->name,
STR_VAR_LEN_MAX,
- str_field,
+ (char *)entry + data->as_dynamic.offset,
i == se->n_fields - 1 ? "" : " ");
n_u64++;
} else {
trace_seq_printf(s, print_fmt, se->fields[i]->name,
STR_VAR_LEN_MAX,
- (char *)&entry->fields[n_u64],
+ (char *)&entry->fields[n_u64].as_u64,
i == se->n_fields - 1 ? "" : " ");
n_u64 += STR_VAR_LEN_MAX / sizeof(u64);
}
} else if (se->fields[i]->is_stack) {
- u32 offset, data_offset, len;
unsigned long *p, *end;
+ union trace_synth_field *data = &entry->fields[n_u64];
- offset = (u32)entry->fields[n_u64];
- data_offset = offset & 0xffff;
- len = offset >> 16;
-
- p = (void *)entry + data_offset;
- end = (void *)p + len - (sizeof(long) - 1);
+ p = (void *)entry + data->as_dynamic.offset;
+ end = (void *)p + data->as_dynamic.len - (sizeof(long) - 1);
trace_seq_printf(s, "%s=STACK:\n", se->fields[i]->name);
@@ -419,13 +409,13 @@ static enum print_line_t print_synth_event(struct trace_iterator *iter,
print_synth_event_num_val(s, print_fmt,
se->fields[i]->name,
se->fields[i]->size,
- entry->fields[n_u64],
+ &entry->fields[n_u64],
space);
if (strcmp(se->fields[i]->type, "gfp_t") == 0) {
trace_seq_puts(s, " (");
trace_print_flags_seq(s, "|",
- entry->fields[n_u64],
+ entry->fields[n_u64].as_u64,
__flags);
trace_seq_putc(s, ')');
}
@@ -454,21 +444,16 @@ static unsigned int trace_string(struct synth_trace_event *entry,
int ret;
if (is_dynamic) {
- u32 data_offset;
+ union trace_synth_field *data = &entry->fields[*n_u64];
- data_offset = struct_size(entry, fields, event->n_u64);
- data_offset += data_size;
-
- len = fetch_store_strlen((unsigned long)str_val);
-
- data_offset |= len << 16;
- *(u32 *)&entry->fields[*n_u64] = data_offset;
+ data->as_dynamic.offset = struct_size(entry, fields, event->n_u64) + data_size;
+ data->as_dynamic.len = fetch_store_strlen((unsigned long)str_val);
ret = fetch_store_string((unsigned long)str_val, &entry->fields[*n_u64], entry);
(*n_u64)++;
} else {
- str_field = (char *)&entry->fields[*n_u64];
+ str_field = (char *)&entry->fields[*n_u64].as_u64;
#ifdef CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
if ((unsigned long)str_val < TASK_SIZE)
@@ -492,6 +477,7 @@ static unsigned int trace_stack(struct synth_trace_event *entry,
unsigned int data_size,
unsigned int *n_u64)
{
+ union trace_synth_field *data = &entry->fields[*n_u64];
unsigned int len;
u32 data_offset;
void *data_loc;
@@ -515,8 +501,9 @@ static unsigned int trace_stack(struct synth_trace_event *entry,
memcpy(data_loc, stack, len);
/* Fill in the field that holds the offset/len combo */
- data_offset |= len << 16;
- *(u32 *)&entry->fields[*n_u64] = data_offset;
+
+ data->as_dynamic.offset = data_offset;
+ data->as_dynamic.len = len;
(*n_u64)++;
@@ -592,19 +579,19 @@ static notrace void trace_event_raw_event_synth(void *__data,
switch (field->size) {
case 1:
- *(u8 *)&entry->fields[n_u64] = (u8)val;
+ entry->fields[n_u64].as_u8 = (u8)val;
break;
case 2:
- *(u16 *)&entry->fields[n_u64] = (u16)val;
+ entry->fields[n_u64].as_u16 = (u16)val;
break;
case 4:
- *(u32 *)&entry->fields[n_u64] = (u32)val;
+ entry->fields[n_u64].as_u32 = (u32)val;
break;
default:
- entry->fields[n_u64] = val;
+ entry->fields[n_u64].as_u64 = val;
break;
}
n_u64++;
@@ -1791,19 +1778,19 @@ int synth_event_trace(struct trace_event_file *file, unsigned int n_vals, ...)
switch (field->size) {
case 1:
- *(u8 *)&state.entry->fields[n_u64] = (u8)val;
+ state.entry->fields[n_u64].as_u8 = (u8)val;
break;
case 2:
- *(u16 *)&state.entry->fields[n_u64] = (u16)val;
+ state.entry->fields[n_u64].as_u16 = (u16)val;
break;
case 4:
- *(u32 *)&state.entry->fields[n_u64] = (u32)val;
+ state.entry->fields[n_u64].as_u32 = (u32)val;
break;
default:
- state.entry->fields[n_u64] = val;
+ state.entry->fields[n_u64].as_u64 = val;
break;
}
n_u64++;
@@ -1884,19 +1871,19 @@ int synth_event_trace_array(struct trace_event_file *file, u64 *vals,
switch (field->size) {
case 1:
- *(u8 *)&state.entry->fields[n_u64] = (u8)val;
+ state.entry->fields[n_u64].as_u8 = (u8)val;
break;
case 2:
- *(u16 *)&state.entry->fields[n_u64] = (u16)val;
+ state.entry->fields[n_u64].as_u16 = (u16)val;
break;
case 4:
- *(u32 *)&state.entry->fields[n_u64] = (u32)val;
+ state.entry->fields[n_u64].as_u32 = (u32)val;
break;
default:
- state.entry->fields[n_u64] = val;
+ state.entry->fields[n_u64].as_u64 = val;
break;
}
n_u64++;
@@ -2031,19 +2018,19 @@ static int __synth_event_add_val(const char *field_name, u64 val,
} else {
switch (field->size) {
case 1:
- *(u8 *)&trace_state->entry->fields[field->offset] = (u8)val;
+ trace_state->entry->fields[field->offset].as_u8 = (u8)val;
break;
case 2:
- *(u16 *)&trace_state->entry->fields[field->offset] = (u16)val;
+ trace_state->entry->fields[field->offset].as_u16 = (u16)val;
break;
case 4:
- *(u32 *)&trace_state->entry->fields[field->offset] = (u32)val;
+ trace_state->entry->fields[field->offset].as_u32 = (u32)val;
break;
default:
- trace_state->entry->fields[field->offset] = val;
+ trace_state->entry->fields[field->offset].as_u64 = val;
break;
}
}
--
2.39.2
Hello,
kernel test robot noticed "BUG:unable_to_handle_page_fault_for_address" on:
commit: e2c9745169808b98f235c09d1366cf8b53ce0d3c ("[PATCH RESEND v3 1/3] tracing/synthetic: use union instead of casts")
url: https://github.com/intel-lab-lkp/linux/commits/Sven-Schnelle/tracing-synthetic-use-union-instead-of-casts/20230817-002758
base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 4853c74bd7ab7fdb83f319bd9ace8a08c031e9b6
patch link: https://lore.kernel.org/all/20230816154928.4171614-2-svens@linux.ibm.com/
patch subject: [PATCH RESEND v3 1/3] tracing/synthetic: use union instead of casts
in testcase: boot
compiler: clang-16
test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 4G
(please refer to attached dmesg/kmsg for entire log/backtrace)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202308251530.5bd302e0-oliver.sang@intel.com
[ 64.204176][ T48] Dumping ftrace buffer:
[ 64.204632][ T48] ---------------------------------
[ 64.205082][ T48] BUG: unable to handle page fault for address: 001c24ca
[ 64.205641][ T48] #PF: supervisor read access in kernel mode
[ 64.206115][ T48] #PF: error_code(0x0000) - not-present page
[ 64.206588][ T48] *pde = 00000000
[ 64.206897][ T48] Oops: 0000 [#1] SMP
[ 64.207221][ T48] CPU: 0 PID: 48 Comm: rcu_scale_write Tainted: G T 6.5.0-rc6-00037-ge2c974516980 #1
[ 64.208074][ T48] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[ 64.208892][ T48] EIP: string+0x98/0x100
[ 64.209237][ T48] Code: 94 c2 75 b4 a1 8c c2 75 b4 40 89 44 24 18 31 d2 eb 0e 47 89 3d 94 c2 75 b4 42 39 54 24 0c 74 32 8b 44 24 08 8d 0c 10 8b 45 08 <0f> b6 04
10 84 c0 74 2c 8b 74 24 18 01 d6 89 35 8c c2 75 b4 3b 0c
[ 64.210761][ T48] EAX: 001c24ca EBX: 000405c2 ECX: b516e406 EDX: 00000000
[ 64.211325][ T48] ESI: ffff0a00 EDI: 00005af2 EBP: b600bcd4 ESP: b600bca8
[ 64.211876][ T48] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010046
[ 64.212468][ T48] CR0: 80050033 CR2: 001c24ca CR3: 08d2a000 CR4: 000406d0
[ 64.213022][ T48] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[ 64.213577][ T48] DR6: fffe0ff0 DR7: 00000400
[ 64.213945][ T48] Call Trace:
[ 64.214208][ T48] ? __die_body+0x69/0xc0
[ 64.214546][ T48] ? __die+0x7e/0x90
[ 64.214863][ T48] ? page_fault_oops+0x22d/0x2b0
[ 64.215242][ T48] ? is_prefetch+0x4a/0x220
[ 64.215605][ T48] ? kernelmode_fixup_or_oops+0xfa/0x140
[ 64.216051][ T48] ? __bad_area_nosemaphore+0x64/0x2a0
[ 64.216485][ T48] ? bad_area_nosemaphore+0x12/0x20
[ 64.216899][ T48] ? do_user_addr_fault+0x650/0x7e0
[ 64.217313][ T48] ? pvclock_clocksource_read_nowd+0x7c/0x230
[ 64.217800][ T48] ? exc_page_fault+0xc5/0x358
[ 64.218183][ T48] ? pvclock_clocksource_read_nowd+0x230/0x230
[ 64.218666][ T48] ? handle_exception+0x14b/0x14b
[ 64.219059][ T48] ? number+0x9b/0x630
[ 64.219378][ T48] ? pvclock_clocksource_read_nowd+0x230/0x230
[ 64.219866][ T48] ? string+0x98/0x100
[ 64.220187][ T48] ? pvclock_clocksource_read_nowd+0x230/0x230
[ 64.220656][ T48] ? string+0x98/0x100
[ 64.220986][ T48] vsnprintf+0x420/0x580
[ 64.221323][ T48] ? vsnprintf+0x3e7/0x580
[ 64.221669][ T48] seq_buf_vprintf+0x79/0xc0
[ 64.222029][ T48] trace_seq_printf+0x35/0xa0
[ 64.222400][ T48] print_synth_event+0x26f/0x300
[ 64.222805][ T48] ? trace_event_raw_event_synth+0x410/0x410
[ 64.223272][ T48] print_trace_fmt+0xfe/0x170
[ 64.223651][ T48] print_trace_line+0x10d/0x1c0
[ 64.224046][ T48] ftrace_dump+0x2fa/0x410
[ 64.224407][ T48] rcu_scale_writer+0x59c/0x640
[ 64.224808][ T48] kthread+0x158/0x170
[ 64.225146][ T48] ? rcu_scale_reader+0x180/0x180
[ 64.225559][ T48] ? kthread_unuse_mm+0x160/0x160
[ 64.225968][ T48] ? kthread_unuse_mm+0x160/0x160
[ 64.226379][ T48] ret_from_fork+0x43/0x70
[ 64.226742][ T48] ret_from_fork_asm+0x12/0x1c
[ 64.227130][ T48] entry_INT80_32+0x108/0x108
[ 64.227516][ T48] Modules linked in:
[ 64.227832][ T48] CR2: 00000000001c24ca
[ 64.228166][ T48] ---[ end trace 0000000000000000 ]---
[ 64.228586][ T48] EIP: string+0x98/0x100
[ 64.228921][ T48] Code: 94 c2 75 b4 a1 8c c2 75 b4 40 89 44 24 18 31 d2 eb 0e 47 89 3d 94 c2 75 b4 42 39 54 24 0c 74 32 8b 44 24 08 8d 0c 10 8b 45 08 <0f> b6 04
10 84 c0 74 2c 8b 74 24 18 01 d6 89 35 8c c2 75 b4 3b 0c
[ 64.230401][ T48] EAX: 001c24ca EBX: 000405c2 ECX: b516e406 EDX: 00000000
[ 64.230918][ T48] ESI: ffff0a00 EDI: 00005af2 EBP: b600bcd4 ESP: b600bca8
[ 64.231432][ T48] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010046
[ 64.231993][ T48] CR0: 80050033 CR2: 001c24ca CR3: 08d2a000 CR4: 000406d0
[ 64.232527][ T48] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[ 64.233094][ T48] DR6: fffe0ff0 DR7: 00000400
[ 64.233474][ T48] Kernel panic - not syncing: Fatal exception
[ 64.234050][ T48] Kernel Offset: disabled
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20230825/202308251530.5bd302e0-oliver.sang@intel.com
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
© 2016 - 2025 Red Hat, Inc.