| Message ID | 20240815173903.4172139-29-samitolvanen@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Implement DWARF modversions | expand |
On 8/15/24 19:39, Sami Tolvanen wrote: > Add support for expanding DW_TAG_subroutine_type and the parameters > in DW_TAG_formal_parameter. Use this to also expand subprograms. > > Example output with --debug: > > subprogram( > formal_parameter base_type usize byte_size(8), > formal_parameter base_type usize byte_size(8), > ) > -> base_type void; > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > --- > scripts/gendwarfksyms/dwarf.c | 57 ++++++++++++++++++++++++++- > scripts/gendwarfksyms/gendwarfksyms.h | 1 + > 2 files changed, 57 insertions(+), 1 deletion(-) > > diff --git a/scripts/gendwarfksyms/dwarf.c b/scripts/gendwarfksyms/dwarf.c > index 82185737fa2a..c81652426be8 100644 > --- a/scripts/gendwarfksyms/dwarf.c > +++ b/scripts/gendwarfksyms/dwarf.c > [...] > > +static int __process_subroutine_type(struct state *state, struct die *cache, > + Dwarf_Die *die, const char *type) > +{ > + check(process(state, cache, type)); > + check(process(state, cache, "(")); > + check(process_linebreak(cache, 1)); > + /* Parameters */ > + check(process_die_container(state, cache, die, process_type, > + match_formal_parameter_type)); > + check(process_linebreak(cache, -1)); > + check(process(state, cache, ")")); > + process_linebreak(cache, 0); > + /* Return type */ > + check(process(state, cache, "-> ")); > + return check(process_type_attr(state, cache, die)); > +} If I understand correctly, this formatting logic also affects the symtypes output. Looking at its format, I would like to propose a few minor changes. Example of the current symtypes output: kprobe_event_cmd_init subprogram( formal_parameter pointer_type <unnamed> { s#dynevent_cmd } byte_size(8), formal_parameter pointer_type <unnamed> { base_type char byte_size(1) encoding(8) } byte_size(8), formal_parameter base_type int byte_size(4) encoding(5), ) -> base_type void Proposed changes: kprobe_event_cmd_init subprogram ( formal_parameter pointer_type <unnamed> { s#dynevent_cmd } byte_size(8) , formal_parameter pointer_type <unnamed> { base_type char byte_size(1) encoding(8) } byte_size(8) , formal_parameter base_type int byte_size(4) encoding(5) ) -> base_type void ^- (1) ^- (2) ^- (3) (1) "subprogram(" is split to "subprogram (". (2) A space is added prior to ",". (3) String ", " is removed after the last parameter. Separating each token with a whitespace matches the current genksyms format, makes the data trivially parsable and easy to pretty-print by additional tools. If some tokens are merged, as in "subprogram(", then such a tool needs to have extra logic to parse each word and split it into tokens. For attributes with one value, such as "byte_size(4)", I think the current format is probably ok.
On Tue, Sep 3, 2024 at 8:11 AM Petr Pavlu <petr.pavlu@suse.com> wrote: > > On 8/15/24 19:39, Sami Tolvanen wrote: > > > > +static int __process_subroutine_type(struct state *state, struct die *cache, > > + Dwarf_Die *die, const char *type) > > +{ > > + check(process(state, cache, type)); > > + check(process(state, cache, "(")); > > + check(process_linebreak(cache, 1)); > > + /* Parameters */ > > + check(process_die_container(state, cache, die, process_type, > > + match_formal_parameter_type)); > > + check(process_linebreak(cache, -1)); > > + check(process(state, cache, ")")); > > + process_linebreak(cache, 0); > > + /* Return type */ > > + check(process(state, cache, "-> ")); > > + return check(process_type_attr(state, cache, die)); > > +} > > If I understand correctly, this formatting logic also affects the > symtypes output. Looking at its format, I would like to propose a few > minor changes. Correct, it's passed directly to the symtypes output. > Example of the current symtypes output: > kprobe_event_cmd_init subprogram( formal_parameter pointer_type <unnamed> { s#dynevent_cmd } byte_size(8), formal_parameter pointer_type <unnamed> { base_type char byte_size(1) encoding(8) } byte_size(8), formal_parameter base_type int byte_size(4) encoding(5), ) -> base_type void > > Proposed changes: > kprobe_event_cmd_init subprogram ( formal_parameter pointer_type <unnamed> { s#dynevent_cmd } byte_size(8) , formal_parameter pointer_type <unnamed> { base_type char byte_size(1) encoding(8) } byte_size(8) , formal_parameter base_type int byte_size(4) encoding(5) ) -> base_type void > ^- (1) ^- (2) ^- (3) > > (1) "subprogram(" is split to "subprogram (". > (2) A space is added prior to ",". > (3) String ", " is removed after the last parameter. > > Separating each token with a whitespace matches the current genksyms > format, makes the data trivially parsable and easy to pretty-print by > additional tools. If some tokens are merged, as in "subprogram(", then > such a tool needs to have extra logic to parse each word and split it > into tokens. Sure, that makes sense. I'll clean this up. Sami
diff --git a/scripts/gendwarfksyms/dwarf.c b/scripts/gendwarfksyms/dwarf.c index 82185737fa2a..c81652426be8 100644 --- a/scripts/gendwarfksyms/dwarf.c +++ b/scripts/gendwarfksyms/dwarf.c @@ -210,6 +210,15 @@ DEFINE_PROCESS_UDATA_ATTRIBUTE(alignment) DEFINE_PROCESS_UDATA_ATTRIBUTE(byte_size) DEFINE_PROCESS_UDATA_ATTRIBUTE(encoding) +/* Match functions -- die_match_callback_t */ +#define DEFINE_MATCH(type) \ + static bool match_##type##_type(Dwarf_Die *die) \ + { \ + return dwarf_tag(die) == DW_TAG_##type##_type; \ + } + +DEFINE_MATCH(formal_parameter) + bool match_all(Dwarf_Die *die) { return true; @@ -247,6 +256,25 @@ static int process_type_attr(struct state *state, struct die *cache, return check(process(state, cache, "base_type void")); } +/* Comma-separated with DW_AT_type */ +static int __process_list_type(struct state *state, struct die *cache, + Dwarf_Die *die, const char *type) +{ + check(process(state, cache, type)); + check(process_type_attr(state, cache, die)); + check(process(state, cache, ",")); + return check(process_linebreak(cache, 0)); +} + +#define DEFINE_PROCESS_LIST_TYPE(type) \ + static int process_##type##_type(struct state *state, \ + struct die *cache, Dwarf_Die *die) \ + { \ + return __process_list_type(state, cache, die, #type " "); \ + } + +DEFINE_PROCESS_LIST_TYPE(formal_parameter) + /* Container types with DW_AT_type */ static int __process_type(struct state *state, struct die *cache, Dwarf_Die *die, const char *type) @@ -281,6 +309,29 @@ DEFINE_PROCESS_TYPE(shared) DEFINE_PROCESS_TYPE(volatile) DEFINE_PROCESS_TYPE(typedef) +static int __process_subroutine_type(struct state *state, struct die *cache, + Dwarf_Die *die, const char *type) +{ + check(process(state, cache, type)); + check(process(state, cache, "(")); + check(process_linebreak(cache, 1)); + /* Parameters */ + check(process_die_container(state, cache, die, process_type, + match_formal_parameter_type)); + check(process_linebreak(cache, -1)); + check(process(state, cache, ")")); + process_linebreak(cache, 0); + /* Return type */ + check(process(state, cache, "-> ")); + return check(process_type_attr(state, cache, die)); +} + +static int process_subroutine_type(struct state *state, struct die *cache, + Dwarf_Die *die) +{ + return check(__process_subroutine_type(state, cache, die, + "subroutine_type")); +} static int process_base_type(struct state *state, struct die *cache, Dwarf_Die *die) { @@ -358,8 +409,11 @@ static int process_type(struct state *state, struct die *parent, Dwarf_Die *die) PROCESS_TYPE(rvalue_reference) PROCESS_TYPE(shared) PROCESS_TYPE(volatile) + /* Subtypes */ + PROCESS_TYPE(formal_parameter) /* Other types */ PROCESS_TYPE(base) + PROCESS_TYPE(subroutine) PROCESS_TYPE(typedef) default: debug("unimplemented type: %x", tag); @@ -379,7 +433,8 @@ static int process_type(struct state *state, struct die *parent, Dwarf_Die *die) */ static int process_subprogram(struct state *state, Dwarf_Die *die) { - return check(process(state, NULL, "subprogram;\n")); + check(__process_subroutine_type(state, NULL, die, "subprogram")); + return check(process(state, NULL, ";\n")); } static int process_variable(struct state *state, Dwarf_Die *die) diff --git a/scripts/gendwarfksyms/gendwarfksyms.h b/scripts/gendwarfksyms/gendwarfksyms.h index e8b4da0fc9a8..25f87485eb69 100644 --- a/scripts/gendwarfksyms/gendwarfksyms.h +++ b/scripts/gendwarfksyms/gendwarfksyms.h @@ -59,6 +59,7 @@ extern bool debug; #define checkp(expr) __check(expr, __res < 0, __res) /* Consistent aliases (DW_TAG_<type>_type) for DWARF tags */ +#define DW_TAG_formal_parameter_type DW_TAG_formal_parameter #define DW_TAG_typedef_type DW_TAG_typedef /*
Add support for expanding DW_TAG_subroutine_type and the parameters in DW_TAG_formal_parameter. Use this to also expand subprograms. Example output with --debug: subprogram( formal_parameter base_type usize byte_size(8), formal_parameter base_type usize byte_size(8), ) -> base_type void; Signed-off-by: Sami Tolvanen <samitolvanen@google.com> --- scripts/gendwarfksyms/dwarf.c | 57 ++++++++++++++++++++++++++- scripts/gendwarfksyms/gendwarfksyms.h | 1 + 2 files changed, 57 insertions(+), 1 deletion(-)