Skip to content

Commit ddeea49

Browse files
svens-s390rostedt
svens-s390
authored and
rostedt
committed
tracing/synthetic: Use union instead of casts
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. Link: https://lkml.kernel.org/r/[email protected] Cc: [email protected] Cc: Masami Hiramatsu <[email protected]> Fixes: 00cf3d6 ("tracing: Allow synthetic events to pass around stacktraces") Signed-off-by: Sven Schnelle <[email protected]> Signed-off-by: Steven Rostedt (Google) <[email protected]>
1 parent 5450be6 commit ddeea49

3 files changed

Lines changed: 56 additions & 50 deletions

File tree

‎include/linux/trace_events.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,17 @@ int trace_raw_output_prep(struct trace_iterator *iter,
5959
extern __printf(2, 3)
6060
void trace_event_printf(struct trace_iterator *iter, const char *fmt, ...);
6161

62+
/* Used to find the offset and length of dynamic fields in trace events */
63+
struct trace_dynamic_info {
64+
#ifdef CONFIG_CPU_BIG_ENDIAN
65+
u16 offset;
66+
u16 len;
67+
#else
68+
u16 len;
69+
u16 offset;
70+
#endif
71+
};
72+
6273
/*
6374
* The trace entry - the most basic unit of tracing. This is what
6475
* is printed in the end as a single line in the trace output, such as:

‎kernel/trace/trace.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,6 +1295,14 @@ static inline void trace_branch_disable(void)
12951295
/* set ring buffers to default size if not already done so */
12961296
int tracing_update_buffers(void);
12971297

1298+
union trace_synth_field {
1299+
u8 as_u8;
1300+
u16 as_u16;
1301+
u32 as_u32;
1302+
u64 as_u64;
1303+
struct trace_dynamic_info as_dynamic;
1304+
};
1305+
12981306
struct ftrace_event_field {
12991307
struct list_head link;
13001308
const char *name;

‎kernel/trace/trace_events_synth.c‎

Lines changed: 37 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ static bool synth_event_match(const char *system, const char *event,
127127

128128
struct synth_trace_event {
129129
struct trace_entry ent;
130-
u64 fields[];
130+
union trace_synth_field fields[];
131131
};
132132

133133
static int synth_event_define_fields(struct trace_event_call *call)
@@ -321,19 +321,19 @@ static const char *synth_field_fmt(char *type)
321321

322322
static void print_synth_event_num_val(struct trace_seq *s,
323323
char *print_fmt, char *name,
324-
int size, u64 val, char *space)
324+
int size, union trace_synth_field *val, char *space)
325325
{
326326
switch (size) {
327327
case 1:
328-
trace_seq_printf(s, print_fmt, name, (u8)val, space);
328+
trace_seq_printf(s, print_fmt, name, val->as_u8, space);
329329
break;
330330

331331
case 2:
332-
trace_seq_printf(s, print_fmt, name, (u16)val, space);
332+
trace_seq_printf(s, print_fmt, name, val->as_u16, space);
333333
break;
334334

335335
case 4:
336-
trace_seq_printf(s, print_fmt, name, (u32)val, space);
336+
trace_seq_printf(s, print_fmt, name, val->as_u32, space);
337337
break;
338338

339339
default:
@@ -374,36 +374,26 @@ static enum print_line_t print_synth_event(struct trace_iterator *iter,
374374
/* parameter values */
375375
if (se->fields[i]->is_string) {
376376
if (se->fields[i]->is_dynamic) {
377-
u32 offset, data_offset;
378-
char *str_field;
379-
380-
offset = (u32)entry->fields[n_u64];
381-
data_offset = offset & 0xffff;
382-
383-
str_field = (char *)entry + data_offset;
377+
union trace_synth_field *data = &entry->fields[n_u64];
384378

385379
trace_seq_printf(s, print_fmt, se->fields[i]->name,
386380
STR_VAR_LEN_MAX,
387-
str_field,
381+
(char *)entry + data->as_dynamic.offset,
388382
i == se->n_fields - 1 ? "" : " ");
389383
n_u64++;
390384
} else {
391385
trace_seq_printf(s, print_fmt, se->fields[i]->name,
392386
STR_VAR_LEN_MAX,
393-
(char *)&entry->fields[n_u64],
387+
(char *)&entry->fields[n_u64].as_u64,
394388
i == se->n_fields - 1 ? "" : " ");
395389
n_u64 += STR_VAR_LEN_MAX / sizeof(u64);
396390
}
397391
} else if (se->fields[i]->is_stack) {
398-
u32 offset, data_offset, len;
399392
unsigned long *p, *end;
393+
union trace_synth_field *data = &entry->fields[n_u64];
400394

401-
offset = (u32)entry->fields[n_u64];
402-
data_offset = offset & 0xffff;
403-
len = offset >> 16;
404-
405-
p = (void *)entry + data_offset;
406-
end = (void *)p + len - (sizeof(long) - 1);
395+
p = (void *)entry + data->as_dynamic.offset;
396+
end = (void *)p + data->as_dynamic.len - (sizeof(long) - 1);
407397

408398
trace_seq_printf(s, "%s=STACK:\n", se->fields[i]->name);
409399

@@ -419,13 +409,13 @@ static enum print_line_t print_synth_event(struct trace_iterator *iter,
419409
print_synth_event_num_val(s, print_fmt,
420410
se->fields[i]->name,
421411
se->fields[i]->size,
422-
entry->fields[n_u64],
412+
&entry->fields[n_u64],
423413
space);
424414

425415
if (strcmp(se->fields[i]->type, "gfp_t") == 0) {
426416
trace_seq_puts(s, " (");
427417
trace_print_flags_seq(s, "|",
428-
entry->fields[n_u64],
418+
entry->fields[n_u64].as_u64,
429419
__flags);
430420
trace_seq_putc(s, ')');
431421
}
@@ -454,21 +444,16 @@ static unsigned int trace_string(struct synth_trace_event *entry,
454444
int ret;
455445

456446
if (is_dynamic) {
457-
u32 data_offset;
447+
union trace_synth_field *data = &entry->fields[*n_u64];
458448

459-
data_offset = struct_size(entry, fields, event->n_u64);
460-
data_offset += data_size;
461-
462-
len = fetch_store_strlen((unsigned long)str_val);
463-
464-
data_offset |= len << 16;
465-
*(u32 *)&entry->fields[*n_u64] = data_offset;
449+
data->as_dynamic.offset = struct_size(entry, fields, event->n_u64) + data_size;
450+
data->as_dynamic.len = fetch_store_strlen((unsigned long)str_val);
466451

467452
ret = fetch_store_string((unsigned long)str_val, &entry->fields[*n_u64], entry);
468453

469454
(*n_u64)++;
470455
} else {
471-
str_field = (char *)&entry->fields[*n_u64];
456+
str_field = (char *)&entry->fields[*n_u64].as_u64;
472457

473458
#ifdef CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
474459
if ((unsigned long)str_val < TASK_SIZE)
@@ -492,6 +477,7 @@ static unsigned int trace_stack(struct synth_trace_event *entry,
492477
unsigned int data_size,
493478
unsigned int *n_u64)
494479
{
480+
union trace_synth_field *data = &entry->fields[*n_u64];
495481
unsigned int len;
496482
u32 data_offset;
497483
void *data_loc;
@@ -515,8 +501,9 @@ static unsigned int trace_stack(struct synth_trace_event *entry,
515501
memcpy(data_loc, stack, len);
516502

517503
/* Fill in the field that holds the offset/len combo */
518-
data_offset |= len << 16;
519-
*(u32 *)&entry->fields[*n_u64] = data_offset;
504+
505+
data->as_dynamic.offset = data_offset;
506+
data->as_dynamic.len = len;
520507

521508
(*n_u64)++;
522509

@@ -592,19 +579,19 @@ static notrace void trace_event_raw_event_synth(void *__data,
592579

593580
switch (field->size) {
594581
case 1:
595-
*(u8 *)&entry->fields[n_u64] = (u8)val;
582+
entry->fields[n_u64].as_u8 = (u8)val;
596583
break;
597584

598585
case 2:
599-
*(u16 *)&entry->fields[n_u64] = (u16)val;
586+
entry->fields[n_u64].as_u16 = (u16)val;
600587
break;
601588

602589
case 4:
603-
*(u32 *)&entry->fields[n_u64] = (u32)val;
590+
entry->fields[n_u64].as_u32 = (u32)val;
604591
break;
605592

606593
default:
607-
entry->fields[n_u64] = val;
594+
entry->fields[n_u64].as_u64 = val;
608595
break;
609596
}
610597
n_u64++;
@@ -1791,19 +1778,19 @@ int synth_event_trace(struct trace_event_file *file, unsigned int n_vals, ...)
17911778

17921779
switch (field->size) {
17931780
case 1:
1794-
*(u8 *)&state.entry->fields[n_u64] = (u8)val;
1781+
state.entry->fields[n_u64].as_u8 = (u8)val;
17951782
break;
17961783

17971784
case 2:
1798-
*(u16 *)&state.entry->fields[n_u64] = (u16)val;
1785+
state.entry->fields[n_u64].as_u16 = (u16)val;
17991786
break;
18001787

18011788
case 4:
1802-
*(u32 *)&state.entry->fields[n_u64] = (u32)val;
1789+
state.entry->fields[n_u64].as_u32 = (u32)val;
18031790
break;
18041791

18051792
default:
1806-
state.entry->fields[n_u64] = val;
1793+
state.entry->fields[n_u64].as_u64 = val;
18071794
break;
18081795
}
18091796
n_u64++;
@@ -1884,19 +1871,19 @@ int synth_event_trace_array(struct trace_event_file *file, u64 *vals,
18841871

18851872
switch (field->size) {
18861873
case 1:
1887-
*(u8 *)&state.entry->fields[n_u64] = (u8)val;
1874+
state.entry->fields[n_u64].as_u8 = (u8)val;
18881875
break;
18891876

18901877
case 2:
1891-
*(u16 *)&state.entry->fields[n_u64] = (u16)val;
1878+
state.entry->fields[n_u64].as_u16 = (u16)val;
18921879
break;
18931880

18941881
case 4:
1895-
*(u32 *)&state.entry->fields[n_u64] = (u32)val;
1882+
state.entry->fields[n_u64].as_u32 = (u32)val;
18961883
break;
18971884

18981885
default:
1899-
state.entry->fields[n_u64] = val;
1886+
state.entry->fields[n_u64].as_u64 = val;
19001887
break;
19011888
}
19021889
n_u64++;
@@ -2031,19 +2018,19 @@ static int __synth_event_add_val(const char *field_name, u64 val,
20312018
} else {
20322019
switch (field->size) {
20332020
case 1:
2034-
*(u8 *)&trace_state->entry->fields[field->offset] = (u8)val;
2021+
trace_state->entry->fields[field->offset].as_u8 = (u8)val;
20352022
break;
20362023

20372024
case 2:
2038-
*(u16 *)&trace_state->entry->fields[field->offset] = (u16)val;
2025+
trace_state->entry->fields[field->offset].as_u16 = (u16)val;
20392026
break;
20402027

20412028
case 4:
2042-
*(u32 *)&trace_state->entry->fields[field->offset] = (u32)val;
2029+
trace_state->entry->fields[field->offset].as_u32 = (u32)val;
20432030
break;
20442031

20452032
default:
2046-
trace_state->entry->fields[field->offset] = val;
2033+
trace_state->entry->fields[field->offset].as_u64 = val;
20472034
break;
20482035
}
20492036
}

0 commit comments

Comments
 (0)