[PATCH dwarves] fprintf: Fix `*` not being printed for pointers with btf_type_tag

* [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