All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH dwarves] fprintf: Fix `*` not being printed for pointers with btf_type_tag
@ 2023-03-30 21:27 Eduard Zingerman
  2023-03-31 12:13 ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 8+ messages in thread
From: Eduard Zingerman @ 2023-03-30 21:27 UTC (permalink / raw)
  To: dwarves, arnaldo.melo
  Cc: bpf, kernel-team, ast, daniel, andrii, yhs, Eduard Zingerman

Recent change to fprintf (see below) causes incorrect `type_fprintf()`
behavior for pointers annotated with btf_type_tag, for example:

    $ cat tag-test.c
    #define __t __attribute__((btf_type_tag("t1")))

    struct foo {
      int __t *a;
    } g;

    $ clang -g -c tag-test.c -o tag-test.o && \
      pahole -J tag-test.o && pahole --sort -F dwarf tag-test.o
    struct foo {
    	int                        a;                    /*     0     8 */
    	...
    };

Note that `*` is missing in the pahole output.
The issue is caused by `goto next_type` that jumps over the
`tag__name()` that is responsible for pointer printing.

As agreed in [1] `type__fprintf()` is modified to skip type tags for
now and would be modified to emit type tags later.

The change in `__tag__name()` is necessary to avoid the following behavior:

    $ cat tag-test.c
    #define __t __attribute__((btf_type_tag("t1")))

    struct foo {
      int __t *a;
      int __t __t *b;
    } g;

    $ clang -g -c tag-test.c -o tag-test.o && \
      pahole -J tag-test.o && pahole --sort -F dwarf tag-test.o
    struct foo {
    	int  *                     a;                    /*     0     8 */
    	int   *                    b;                    /*     8     8 */
            ...
    };

Note the extra space before `*` for field `b`.

The issue was reported and tracked down to the root cause by
Arnaldo Carvalho de Melo.

Links:
[1] https://lore.kernel.org/dwarves/20230314230417.1507266-1-eddyz87@gmail.com/T/#md82b04f66867434524beec746138951f26a3977e

Fixes: e7fb771f2649 ("fprintf: Correct names for types with btf_type_tag attribute")
Reported-by: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
Link: https://lore.kernel.org/dwarves/20230314230417.1507266-1-eddyz87@gmail.com/T/#mc630bcd474ddd64c70d237edd4e0590dc048d63d
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 dwarves_fprintf.c | 37 +++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
index 1e6147a..818db2d 100644
--- a/dwarves_fprintf.c
+++ b/dwarves_fprintf.c
@@ -572,7 +572,6 @@ static const char *__tag__name(const struct tag *tag, const struct cu *cu,
 	case DW_TAG_restrict_type:
 	case DW_TAG_atomic_type:
 	case DW_TAG_unspecified_type:
-	case DW_TAG_LLVM_annotation:
 		type = cu__type(cu, tag->type);
 		if (type == NULL && tag->type != 0)
 			tag__id_not_found_snprintf(bf, len, tag->type);
@@ -618,6 +617,13 @@ static const char *__tag__name(const struct tag *tag, const struct cu *cu,
 	case DW_TAG_variable:
 		snprintf(bf, len, "%s", variable__name(tag__variable(tag)));
 		break;
+	case DW_TAG_LLVM_annotation:
+		type = cu__type(cu, tag->type);
+		if (type == NULL && tag->type != 0)
+			tag__id_not_found_snprintf(bf, len, tag->type);
+		else if (!tag__has_type_loop(tag, type, bf, len, NULL))
+			__tag__name(type, cu, bf, len, conf);
+		break;
 	default:
 		snprintf(bf, len, "%s%s", tag__prefix(cu, tag->tag, pconf),
 			 type__name(tag__type(tag)) ?: "");
@@ -677,6 +683,22 @@ static size_t type__fprintf_stats(struct type *type, const struct cu *cu,
 	return printed;
 }
 
+static type_id_t skip_llvm_annotations(const struct cu *cu, type_id_t id)
+{
+	struct tag *type;
+
+	for (;;) {
+		if (id == 0)
+			break;
+		type = cu__type(cu, id);
+		if (type == NULL || type->tag != DW_TAG_LLVM_annotation || type->type == id)
+			break;
+		id = type->type;
+	}
+
+	return id;
+}
+
 static size_t union__fprintf(struct type *type, const struct cu *cu,
 			     const struct conf_fprintf *conf, FILE *fp);
 
@@ -778,19 +800,17 @@ inner_struct:
 
 next_type:
 	switch (type->tag) {
-	case DW_TAG_pointer_type:
-		if (type->type != 0) {
+	case DW_TAG_pointer_type: {
+		type_id_t ptype_id = skip_llvm_annotations(cu, type->type);
+
+		if (ptype_id != 0) {
 			int n;
-			struct tag *ptype = cu__type(cu, type->type);
+			struct tag *ptype = cu__type(cu, ptype_id);
 			if (ptype == NULL)
 				goto out_type_not_found;
 			n = tag__has_type_loop(type, ptype, NULL, 0, fp);
 			if (n)
 				return printed + n;
-			if (ptype->tag == DW_TAG_LLVM_annotation) {
-				type = ptype;
-				goto next_type;
-			}
 			if (ptype->tag == DW_TAG_subroutine_type) {
 				printed += ftype__fprintf(tag__ftype(ptype),
 							  cu, name, 0, 1,
@@ -811,6 +831,7 @@ next_type:
 			}
 		}
 		/* Fall Thru */
+	}
 	default:
 print_default:
 		printed += fprintf(fp, "%-*s %s", tconf.type_spacing,
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-03-31 18:33 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-30 21:27 [PATCH dwarves] fprintf: Fix `*` not being printed for pointers with btf_type_tag Eduard Zingerman
2023-03-31 12:13 ` Arnaldo Carvalho de Melo
2023-03-31 12:14   ` Arnaldo Carvalho de Melo
2023-03-31 12:20     ` Arnaldo Carvalho de Melo
2023-03-31 14:12       ` Eduard Zingerman
2023-03-31 18:33         ` Arnaldo Carvalho de Melo
2023-03-31 13:35   ` Eduard Zingerman
2023-03-31 14:05     ` Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.