dwarves.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH dwarves 0/4] btf: support btf_type_tag attribute
@ 2021-11-17 20:22 Yonghong Song
  2021-11-17 20:22 ` [PATCH dwarves 1/4] libbpf: sync with latest libbpf repo Yonghong Song
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Yonghong Song @ 2021-11-17 20:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, dwarves
  Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann, kernel-team

btf_type_tag is a new llvm type attribute which is used similar
to kernel __user/__rcu attributes. The format of btf_type_tag looks like
  __attribute__((btf_type_tag("tag1")))
For the case where the attribute applied to a pointee like
  #define __tag1 __attribute__((btf_type_tag("tag1")))
  #define __tag2 __attribute__((btf_type_tag("tag2")))
  int __tag1 * __tag1 __tag2 *g;
the information will be encoded in dwarf.

In BTF, the attribute is encoded as a new kind
BTF_KIND_TYPE_TAG and latest bpf-next supports it.

The patch added support in pahole, specifically
converts llvm dwarf btf_type_tag attributes to
BTF types. Please see individual patches for details.

Yonghong Song (4):
  libbpf: sync with latest libbpf repo
  dutil: move DW_TAG_LLVM_annotation definition to dutil.h
  dwarf_loader: support btf_type_tag attribute
  btf_encoder: support btf_type_tag attribute

 btf_encoder.c  |   7 +++
 dutil.h        |   4 ++
 dwarf_loader.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++---
 dwarves.h      |  38 +++++++++++++++-
 lib/bpf        |   2 +-
 pahole.c       |   8 ++++
 6 files changed, 170 insertions(+), 9 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH dwarves 1/4] libbpf: sync with latest libbpf repo
  2021-11-17 20:22 [PATCH dwarves 0/4] btf: support btf_type_tag attribute Yonghong Song
@ 2021-11-17 20:22 ` Yonghong Song
  2021-11-17 20:22 ` [PATCH dwarves 2/4] dutil: move DW_TAG_LLVM_annotation definition to dutil.h Yonghong Song
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Yonghong Song @ 2021-11-17 20:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, dwarves
  Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann, kernel-team

Sync up to commit 94a49850c5ee ("Makefile: enforce gnu89 standard").
This is needed to support BTF_KIND_TYPE_TAG.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 lib/bpf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bpf b/lib/bpf
index f05791d..94a4985 160000
--- a/lib/bpf
+++ b/lib/bpf
@@ -1 +1 @@
-Subproject commit f05791d8cfcbbf9092b4099b9d011bb72e241ef8
+Subproject commit 94a49850c5ee61ea02dfcbabf48013391e8cecdf
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH dwarves 2/4] dutil: move DW_TAG_LLVM_annotation definition to dutil.h
  2021-11-17 20:22 [PATCH dwarves 0/4] btf: support btf_type_tag attribute Yonghong Song
  2021-11-17 20:22 ` [PATCH dwarves 1/4] libbpf: sync with latest libbpf repo Yonghong Song
@ 2021-11-17 20:22 ` Yonghong Song
  2021-11-17 20:22 ` [PATCH dwarves 3/4] dwarf_loader: support btf_type_tag attribute Yonghong Song
  2021-11-17 20:22 ` [PATCH dwarves 4/4] btf_encoder: " Yonghong Song
  3 siblings, 0 replies; 11+ messages in thread
From: Yonghong Song @ 2021-11-17 20:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, dwarves
  Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann, kernel-team

Move DW_TAG_LLVM_annotation definition from dwarf_load.c to
dutil.h as it will be used later for btf_encoder.c.
There is no functionality change for this patch.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 dutil.h        | 4 ++++
 dwarf_loader.c | 4 ----
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/dutil.h b/dutil.h
index 25691ea..e45bba0 100644
--- a/dutil.h
+++ b/dutil.h
@@ -31,6 +31,10 @@
 
 #define roundup(x,y) ((((x) + ((y) - 1)) / (y)) * (y))
 
+#ifndef DW_TAG_LLVM_annotation
+#define DW_TAG_LLVM_annotation 0x6000
+#endif
+
 static inline __attribute__((const)) bool is_power_of_2(unsigned long n)
 {
         return (n != 0 && ((n & (n - 1)) == 0));
diff --git a/dwarf_loader.c b/dwarf_loader.c
index cf005da..1b07a62 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -52,10 +52,6 @@
 #define DW_OP_addrx 0xa1
 #endif
 
-#ifndef DW_TAG_LLVM_annotation
-#define DW_TAG_LLVM_annotation 0x6000
-#endif
-
 static pthread_mutex_t libdw__lock = PTHREAD_MUTEX_INITIALIZER;
 
 static uint32_t hashtags__bits = 12;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH dwarves 3/4] dwarf_loader: support btf_type_tag attribute
  2021-11-17 20:22 [PATCH dwarves 0/4] btf: support btf_type_tag attribute Yonghong Song
  2021-11-17 20:22 ` [PATCH dwarves 1/4] libbpf: sync with latest libbpf repo Yonghong Song
  2021-11-17 20:22 ` [PATCH dwarves 2/4] dutil: move DW_TAG_LLVM_annotation definition to dutil.h Yonghong Song
@ 2021-11-17 20:22 ` Yonghong Song
  2021-11-18 13:00   ` Arnaldo Carvalho de Melo
  2021-11-23  1:52   ` Andrii Nakryiko
  2021-11-17 20:22 ` [PATCH dwarves 4/4] btf_encoder: " Yonghong Song
  3 siblings, 2 replies; 11+ messages in thread
From: Yonghong Song @ 2021-11-17 20:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, dwarves
  Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann, kernel-team

LLVM patches ([1] for clang, [2] and [3] for BPF backend)
added support for btf_type_tag attributes. The following is
an example:
  [$ ~] cat t.c
  #define __tag1 __attribute__((btf_type_tag("tag1")))
  #define __tag2 __attribute__((btf_type_tag("tag2")))
  int __tag1 * __tag1 __tag2 *g __attribute__((section(".data..percpu")));
  [$ ~] clang -O2 -g -c t.c
  [$ ~] llvm-dwarfdump --debug-info t.o
  t.o:    file format elf64-x86-64
  ...
  0x0000001e:   DW_TAG_variable
                  DW_AT_name      ("g")
                  DW_AT_type      (0x00000033 "int **")
                  DW_AT_external  (true)
                  DW_AT_decl_file ("/home/yhs/t.c")
                  DW_AT_decl_line (3)
                  DW_AT_location  (DW_OP_addr 0x0)
  0x00000033:   DW_TAG_pointer_type
                  DW_AT_type      (0x0000004b "int *")
  0x00000038:     DW_TAG_LLVM_annotation
                    DW_AT_name    ("btf_type_tag")
                    DW_AT_const_value     ("tag1")
  0x00000041:     DW_TAG_LLVM_annotation
                    DW_AT_name    ("btf_type_tag")
                    DW_AT_const_value     ("tag2")
  0x0000004a:     NULL
  0x0000004b:   DW_TAG_pointer_type
                  DW_AT_type      (0x0000005a "int")
  0x00000050:     DW_TAG_LLVM_annotation
                    DW_AT_name    ("btf_type_tag")
                    DW_AT_const_value     ("tag1")
  0x00000059:     NULL
  0x0000005a:   DW_TAG_base_type
                  DW_AT_name      ("int")
                  DW_AT_encoding  (DW_ATE_signed)
                  DW_AT_byte_size (0x04)
  0x00000061:   NULL

From the above example, you can see that DW_TAG_pointer_type
may contain one or more DW_TAG_LLVM_annotation btf_type_tag tags.
If DW_TAG_LLVM_annotation tags are present inside
DW_TAG_pointer_type, for BTF encoding, pahole will need
to follow [3] to generate a type chain like
  var -> ptr -> tag2 -> tag1 -> ptr -> tag1 -> int

This patch implemented dwarf_loader support. If a pointer type
contains DW_TAG_LLVM_annotation tags, a new type
btf_type_tag_ptr_type will be created which will store
the pointer tag itself and all DW_TAG_LLVM_annotation tags.
During recoding stage, the type chain will be formed properly
based on the above example.

An option "--skip_encoding_btf_type_tag" is added to disable
this new functionality.

  [1] https://reviews.llvm.org/D111199
  [2] https://reviews.llvm.org/D113222
  [3] https://reviews.llvm.org/D113496
---
 dwarf_loader.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++--
 dwarves.h      |  33 +++++++++++++-
 pahole.c       |   8 ++++
 3 files changed, 153 insertions(+), 4 deletions(-)

diff --git a/dwarf_loader.c b/dwarf_loader.c
index 1b07a62..5b2bebb 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -1206,6 +1206,88 @@ static struct tag *die__create_new_tag(Dwarf_Die *die, struct cu *cu)
 	return tag;
 }
 
+static struct btf_type_tag_ptr_type *die__create_new_btf_type_tag_ptr_type(Dwarf_Die *die, struct cu *cu)
+{
+	struct btf_type_tag_ptr_type *tag;
+
+	tag  = tag__alloc_with_spec(cu, sizeof(struct btf_type_tag_ptr_type));
+	if (tag == NULL)
+		return NULL;
+
+	tag__init(&tag->tag, cu, die);
+	tag->tag.has_btf_type_tag = true;
+	INIT_LIST_HEAD(&tag->tags);
+	return tag;
+}
+
+static struct btf_type_tag_type *die__create_new_btf_type_tag_type(Dwarf_Die *die, struct cu *cu,
+								   struct conf_load *conf)
+{
+	struct btf_type_tag_type *tag;
+
+	tag  = tag__alloc_with_spec(cu, sizeof(struct btf_type_tag_type));
+	if (tag == NULL)
+		return NULL;
+
+	tag__init(&tag->tag, cu, die);
+	tag->value = attr_string(die, DW_AT_const_value, conf);
+	return tag;
+}
+
+static struct tag *die__create_new_pointer_tag(Dwarf_Die *die, struct cu *cu,
+					       struct conf_load *conf)
+{
+	struct btf_type_tag_ptr_type *tag = NULL;
+	struct btf_type_tag_type *annot;
+	Dwarf_Die *cdie, child;
+	const char *name;
+	uint32_t id;
+
+	/* If no child tags or skipping btf_type_tag encoding, just create a new tag
+	 * and return
+	 */
+	if (!dwarf_haschildren(die) || dwarf_child(die, &child) != 0 ||
+	    conf->skip_encoding_btf_type_tag)
+		return tag__new(die, cu);
+
+	/* Otherwise, check DW_TAG_LLVM_annotation child tags */
+	cdie = &child;
+	do {
+		if (dwarf_tag(cdie) == DW_TAG_LLVM_annotation) {
+			/* Only check btf_type_tag annotations */
+			name = attr_string(cdie, DW_AT_name, conf);
+			if (strcmp(name, "btf_type_tag") != 0)
+				continue;
+
+			if (tag == NULL) {
+				/* Create a btf_type_tag_ptr type. */
+				tag = die__create_new_btf_type_tag_ptr_type(die, cu);
+				if (!tag)
+					return NULL;
+			}
+
+			/* Create a btf_type_tag type for this annotation. */
+			annot = die__create_new_btf_type_tag_type(cdie, cu, conf);
+			if (annot == NULL)
+				return NULL;
+
+			if (cu__table_add_tag(cu, &annot->tag, &id) < 0)
+				return NULL;
+
+			struct dwarf_tag *dtag = annot->tag.priv;
+			dtag->small_id = id;
+			cu__hash(cu, &annot->tag);
+
+			/* For a list of DW_TAG_LLVM_annotation like tag1 -> tag2 -> tag3,
+			 * the tag->tags contains tag3 -> tag2 -> tag1.
+			 */
+			list_add(&annot->node, &tag->tags);
+		}
+	} while (dwarf_siblingof(cdie, cdie) == 0);
+
+	return tag ? &tag->tag : tag__new(die, cu);
+}
+
 static struct tag *die__create_new_ptr_to_member_type(Dwarf_Die *die,
 						      struct cu *cu)
 {
@@ -1903,12 +1985,13 @@ static struct tag *__die__process_tag(Dwarf_Die *die, struct cu *cu,
 	case DW_TAG_const_type:
 	case DW_TAG_imported_declaration:
 	case DW_TAG_imported_module:
-	case DW_TAG_pointer_type:
 	case DW_TAG_reference_type:
 	case DW_TAG_restrict_type:
 	case DW_TAG_unspecified_type:
 	case DW_TAG_volatile_type:
 		tag = die__create_new_tag(die, cu);		break;
+	case DW_TAG_pointer_type:
+		tag = die__create_new_pointer_tag(die, cu, conf);	break;
 	case DW_TAG_ptr_to_member_type:
 		tag = die__create_new_ptr_to_member_type(die, cu); break;
 	case DW_TAG_enumeration_type:
@@ -2192,6 +2275,26 @@ static void lexblock__recode_dwarf_types(struct lexblock *tag, struct cu *cu)
 	}
 }
 
+static void dwarf_cu__recode_btf_type_tag_ptr(struct btf_type_tag_ptr_type *tag,
+					      uint32_t pointee_type)
+{
+	struct btf_type_tag_type *annot;
+	struct dwarf_tag *annot_dtag;
+	struct tag *prev_tag;
+
+	/* If tag->tags contains tag3 -> tag2 -> tag1, the final type chain
+	 * looks like:
+	 *   pointer -> tag3 -> tag2 -> tag1 -> pointee
+	 */
+	prev_tag = &tag->tag;
+	list_for_each_entry(annot, &tag->tags, node) {
+		annot_dtag = annot->tag.priv;
+		prev_tag->type = annot_dtag->small_id;
+		prev_tag = &annot->tag;
+	}
+	prev_tag->type = pointee_type;
+}
+
 static int tag__recode_dwarf_type(struct tag *tag, struct cu *cu)
 {
 	struct dwarf_tag *dtag = tag->priv;
@@ -2301,7 +2404,10 @@ static int tag__recode_dwarf_type(struct tag *tag, struct cu *cu)
 	}
 
 	if (dtag->type.off == 0) {
-		tag->type = 0; /* void */
+		if (tag->tag != DW_TAG_pointer_type || !tag->has_btf_type_tag)
+			tag->type = 0; /* void */
+		else
+			dwarf_cu__recode_btf_type_tag_ptr(tag__btf_type_tag_ptr(tag), 0);
 		return 0;
 	}
 
@@ -2313,7 +2419,11 @@ check_type:
 		return 0;
 	}
 out:
-	tag->type = dtype->small_id;
+	if (tag->tag != DW_TAG_pointer_type || !tag->has_btf_type_tag)
+		tag->type = dtype->small_id;
+	else
+		dwarf_cu__recode_btf_type_tag_ptr(tag__btf_type_tag_ptr(tag), dtype->small_id);
+
 	return 0;
 }
 
diff --git a/dwarves.h b/dwarves.h
index 0d3e204..4425d3c 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -63,6 +63,7 @@ struct conf_load {
 	bool			ptr_table_stats;
 	bool			skip_encoding_btf_decl_tag;
 	bool			skip_missing;
+	bool			skip_encoding_btf_type_tag;
 	uint8_t			hashtable_bits;
 	uint8_t			max_hashtable_bits;
 	uint16_t		kabi_prefix_len;
@@ -413,6 +414,7 @@ struct tag {
 	uint16_t	 tag;
 	bool		 visited;
 	bool		 top_level;
+	bool		 has_btf_type_tag;
 	uint16_t	 recursivity_level;
 	void		 *priv;
 };
@@ -533,7 +535,8 @@ static inline int tag__is_tag_type(const struct tag *tag)
 	       tag->tag == DW_TAG_restrict_type ||
 	       tag->tag == DW_TAG_subroutine_type ||
 	       tag->tag == DW_TAG_unspecified_type ||
-	       tag->tag == DW_TAG_volatile_type;
+	       tag->tag == DW_TAG_volatile_type ||
+	       tag->tag == DW_TAG_LLVM_annotation;
 }
 
 static inline const char *tag__decl_file(const struct tag *tag,
@@ -606,6 +609,34 @@ struct llvm_annotation {
 	struct list_head	node;
 };
 
+/** struct btf_type_tag_type - representing a btf_type_tag annotation
+ *
+ * @tag   - DW_TAG_LLVM_annotation tag
+ * @value - btf_type_tag value string
+ * @node  - list_head node
+ */
+struct btf_type_tag_type {
+	struct tag		tag;
+	const char		*value;
+	struct list_head	node;
+};
+
+/** The struct btf_type_tag_ptr_type - type containing both pointer type and
+ *  its btf_type_tag annotations
+ *
+ * @tag  - pointer type tag
+ * @tags - btf_type_tag annotations for the pointer type
+ */
+struct btf_type_tag_ptr_type {
+	struct tag		tag;
+	struct list_head 	tags;
+};
+
+static inline struct btf_type_tag_ptr_type *tag__btf_type_tag_ptr(struct tag *tag)
+{
+	return (struct btf_type_tag_ptr_type *)tag;
+}
+
 /** struct namespace - base class for enums, structs, unions, typedefs, etc
  *
  * @tags - class_member, enumerators, etc
diff --git a/pahole.c b/pahole.c
index 5fc1cca..f3a51cb 100644
--- a/pahole.c
+++ b/pahole.c
@@ -1126,6 +1126,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version;
 #define ARGP_devel_stats	   330
 #define ARGP_skip_encoding_btf_decl_tag 331
 #define ARGP_skip_missing          332
+#define ARGP_skip_encoding_btf_type_tag 333
 
 static const struct argp_option pahole__options[] = {
 	{
@@ -1506,6 +1507,11 @@ static const struct argp_option pahole__options[] = {
 		.key  = ARGP_skip_missing,
 		.doc = "skip missing types passed to -C rather than stop",
 	},
+	{
+		.name = "skip_encoding_btf_type_tag",
+		.key  = ARGP_skip_encoding_btf_type_tag,
+		.doc  = "Do not encode TAGs in BTF."
+	},
 	{
 		.name = NULL,
 	}
@@ -1658,6 +1664,8 @@ static error_t pahole__options_parser(int key, char *arg,
 		conf_load.skip_encoding_btf_decl_tag = true;	break;
 	case ARGP_skip_missing:
 		conf_load.skip_missing = true;          break;
+	case ARGP_skip_encoding_btf_type_tag:
+		conf_load.skip_encoding_btf_type_tag = true;	break;
 	default:
 		return ARGP_ERR_UNKNOWN;
 	}
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH dwarves 4/4] btf_encoder: support btf_type_tag attribute
  2021-11-17 20:22 [PATCH dwarves 0/4] btf: support btf_type_tag attribute Yonghong Song
                   ` (2 preceding siblings ...)
  2021-11-17 20:22 ` [PATCH dwarves 3/4] dwarf_loader: support btf_type_tag attribute Yonghong Song
@ 2021-11-17 20:22 ` Yonghong Song
  3 siblings, 0 replies; 11+ messages in thread
From: Yonghong Song @ 2021-11-17 20:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, dwarves
  Cc: Alexei Starovoitov, Andrii Nakryiko, bpf, Daniel Borkmann, kernel-team

The following is an example.
  [$ ~] cat t.c
  #define __tag1 __attribute__((btf_type_tag("tag1")))
  #define __tag2 __attribute__((btf_type_tag("tag2")))
  int __tag1 * __tag1 __tag2 *g __attribute__((section(".data..percpu")));
  [$ ~] clang -O2 -g -c t.c
  [$ ~] pahole -JV t.o
  Found per-CPU symbol 'g' at address 0x0
  Found 1 per-CPU variables!
  File t.o:
  [1] TYPE_TAG tag1 type_id=5
  [2] TYPE_TAG tag2 type_id=1
  [3] PTR (anon) type_id=2
  [4] TYPE_TAG tag1 type_id=6
  [5] PTR (anon) type_id=4
  [6] INT int size=4 nr_bits=32 encoding=SIGNED
  search cu 't.c' for percpu global variables.
  Variable 'g' from CU 't.c' at address 0x0 encoded
  [7] VAR g type=3 linkage=1
  [8] DATASEC .data..percpu size=8 vlen=1
          type=7 offset=0 size=8
  [$ ~]

You can see for the source
  int __tag1 * __tag1 __tag2 *g __attribute__((section(".data..percpu")));
the following type chain is generated:
  var -> ptr -> tag2 -> tag1 -> ptr -> tag1 -> int

The following shows pahole option "--skip_encoding_btf_type_tag"
can prevent BTF_KIND_TYPE_TAG generation.
  [$ ~] pahole -JV t.o --skip_encoding_btf_type_tag
  Found per-CPU symbol 'g' at address 0x0
  Found 1 per-CPU variables!
  File t.o:
  [1] PTR (anon) type_id=2
  [2] PTR (anon) type_id=3
  [3] INT int size=4 nr_bits=32 encoding=SIGNED
  search cu 't.c' for percpu global variables.
  Variable 'g' from CU 't.c' at address 0x0 encoded
  [4] VAR g type=1 linkage=1
  [5] DATASEC .data..percpu size=8 vlen=1
          type=4 offset=0 size=8
  [$ ~]

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 btf_encoder.c | 7 +++++++
 dwarves.h     | 5 +++++
 2 files changed, 12 insertions(+)

diff --git a/btf_encoder.c b/btf_encoder.c
index 117656e..9d015f3 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -143,6 +143,7 @@ static const char * const btf_kind_str[NR_BTF_KINDS] = {
 	[BTF_KIND_DATASEC]      = "DATASEC",
 	[BTF_KIND_FLOAT]        = "FLOAT",
 	[BTF_KIND_DECL_TAG]     = "DECL_TAG",
+	[BTF_KIND_TYPE_TAG]     = "TYPE_TAG",
 };
 
 static const char *btf__printable_name(const struct btf *btf, uint32_t offset)
@@ -393,6 +394,9 @@ static int32_t btf_encoder__add_ref_type(struct btf_encoder *encoder, uint16_t k
 	case BTF_KIND_TYPEDEF:
 		id = btf__add_typedef(btf, name, type);
 		break;
+	case BTF_KIND_TYPE_TAG:
+		id = btf__add_type_tag(btf, name, type);
+		break;
 	case BTF_KIND_FWD:
 		id = btf__add_fwd(btf, name, kind_flag);
 		break;
@@ -862,6 +866,9 @@ static int btf_encoder__encode_tag(struct btf_encoder *encoder, struct tag *tag,
 	case DW_TAG_typedef:
 		name = namespace__name(tag__namespace(tag));
 		return btf_encoder__add_ref_type(encoder, BTF_KIND_TYPEDEF, ref_type_id, name, false);
+	case DW_TAG_LLVM_annotation:
+		name = tag__btf_type_tag(tag)->value;
+		return btf_encoder__add_ref_type(encoder, BTF_KIND_TYPE_TAG, ref_type_id, name, false);
 	case DW_TAG_structure_type:
 	case DW_TAG_union_type:
 	case DW_TAG_class_type:
diff --git a/dwarves.h b/dwarves.h
index 4425d3c..52d162d 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -637,6 +637,11 @@ static inline struct btf_type_tag_ptr_type *tag__btf_type_tag_ptr(struct tag *ta
 	return (struct btf_type_tag_ptr_type *)tag;
 }
 
+static inline struct btf_type_tag_type *tag__btf_type_tag(struct tag *tag)
+{
+	return (struct btf_type_tag_type *)tag;
+}
+
 /** struct namespace - base class for enums, structs, unions, typedefs, etc
  *
  * @tags - class_member, enumerators, etc
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH dwarves 3/4] dwarf_loader: support btf_type_tag attribute
  2021-11-17 20:22 ` [PATCH dwarves 3/4] dwarf_loader: support btf_type_tag attribute Yonghong Song
@ 2021-11-18 13:00   ` Arnaldo Carvalho de Melo
  2021-11-18 16:11     ` Yonghong Song
  2021-11-23  1:52   ` Andrii Nakryiko
  1 sibling, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-11-18 13:00 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Andrii Nakryiko, bpf, Daniel Borkmann, kernel-team

Em Wed, Nov 17, 2021 at 12:22:29PM -0800, Yonghong Song escreveu:
> This patch implemented dwarf_loader support. If a pointer type
> contains DW_TAG_LLVM_annotation tags, a new type
> btf_type_tag_ptr_type will be created which will store
> the pointer tag itself and all DW_TAG_LLVM_annotation tags.
> During recoding stage, the type chain will be formed properly
> based on the above example.
> 
> An option "--skip_encoding_btf_type_tag" is added to disable
> this new functionality.
> 
>   [1] https://reviews.llvm.org/D111199
>   [2] https://reviews.llvm.org/D113222
>   [3] https://reviews.llvm.org/D113496

You forgot to add your S-o-B and to add this entry to
man-pages/pahole.1, I'm fixing both cases, bellow is a followup
patch, I'll add one as well for the recently added
--skip_encoding_btf_decl_tag.

- Arnaldo

commit 9c3101db76acf364607d90adb3052e34d81fa1bd
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date:   Thu Nov 18 09:56:35 2021 -0300

    man pages: Add missing --skip_encoding_btf_type_tag entry
    
    In the past we saw the value of being able to disable specific features
    due to problems in in its implementation, allowing users to use a subset
    of functionality, without the problematic one.
    
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/man-pages/pahole.1 b/man-pages/pahole.1
index edcf58b8ca5814a3..f9f64b67945b45cb 100644
--- a/man-pages/pahole.1
+++ b/man-pages/pahole.1
@@ -197,6 +197,10 @@ the debugging information.
 .B \-\-skip_encoding_btf_vars
 Do not encode VARs in BTF.
 
+.TP
+.B \-\-skip_encoding_btf_type_tag
+Do not encode type tags in BTF.
+
 .TP
 .B \-j, \-\-jobs=N
 Run N jobs in parallel. Defaults to number of online processors + 10% (like



> ---
>  dwarf_loader.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++--
>  dwarves.h      |  33 +++++++++++++-
>  pahole.c       |   8 ++++
>  3 files changed, 153 insertions(+), 4 deletions(-)
> 
> diff --git a/dwarf_loader.c b/dwarf_loader.c
> index 1b07a62..5b2bebb 100644
> --- a/dwarf_loader.c
> +++ b/dwarf_loader.c
> @@ -1206,6 +1206,88 @@ static struct tag *die__create_new_tag(Dwarf_Die *die, struct cu *cu)
>  	return tag;
>  }
>  
> +static struct btf_type_tag_ptr_type *die__create_new_btf_type_tag_ptr_type(Dwarf_Die *die, struct cu *cu)
> +{
> +	struct btf_type_tag_ptr_type *tag;
> +
> +	tag  = tag__alloc_with_spec(cu, sizeof(struct btf_type_tag_ptr_type));
> +	if (tag == NULL)
> +		return NULL;
> +
> +	tag__init(&tag->tag, cu, die);
> +	tag->tag.has_btf_type_tag = true;
> +	INIT_LIST_HEAD(&tag->tags);
> +	return tag;
> +}
> +
> +static struct btf_type_tag_type *die__create_new_btf_type_tag_type(Dwarf_Die *die, struct cu *cu,
> +								   struct conf_load *conf)
> +{
> +	struct btf_type_tag_type *tag;
> +
> +	tag  = tag__alloc_with_spec(cu, sizeof(struct btf_type_tag_type));
> +	if (tag == NULL)
> +		return NULL;
> +
> +	tag__init(&tag->tag, cu, die);
> +	tag->value = attr_string(die, DW_AT_const_value, conf);
> +	return tag;
> +}
> +
> +static struct tag *die__create_new_pointer_tag(Dwarf_Die *die, struct cu *cu,
> +					       struct conf_load *conf)
> +{
> +	struct btf_type_tag_ptr_type *tag = NULL;
> +	struct btf_type_tag_type *annot;
> +	Dwarf_Die *cdie, child;
> +	const char *name;
> +	uint32_t id;
> +
> +	/* If no child tags or skipping btf_type_tag encoding, just create a new tag
> +	 * and return
> +	 */
> +	if (!dwarf_haschildren(die) || dwarf_child(die, &child) != 0 ||
> +	    conf->skip_encoding_btf_type_tag)
> +		return tag__new(die, cu);
> +
> +	/* Otherwise, check DW_TAG_LLVM_annotation child tags */
> +	cdie = &child;
> +	do {
> +		if (dwarf_tag(cdie) == DW_TAG_LLVM_annotation) {
> +			/* Only check btf_type_tag annotations */
> +			name = attr_string(cdie, DW_AT_name, conf);
> +			if (strcmp(name, "btf_type_tag") != 0)
> +				continue;
> +
> +			if (tag == NULL) {
> +				/* Create a btf_type_tag_ptr type. */
> +				tag = die__create_new_btf_type_tag_ptr_type(die, cu);
> +				if (!tag)
> +					return NULL;
> +			}
> +
> +			/* Create a btf_type_tag type for this annotation. */
> +			annot = die__create_new_btf_type_tag_type(cdie, cu, conf);
> +			if (annot == NULL)
> +				return NULL;
> +
> +			if (cu__table_add_tag(cu, &annot->tag, &id) < 0)
> +				return NULL;
> +
> +			struct dwarf_tag *dtag = annot->tag.priv;
> +			dtag->small_id = id;
> +			cu__hash(cu, &annot->tag);
> +
> +			/* For a list of DW_TAG_LLVM_annotation like tag1 -> tag2 -> tag3,
> +			 * the tag->tags contains tag3 -> tag2 -> tag1.
> +			 */
> +			list_add(&annot->node, &tag->tags);
> +		}
> +	} while (dwarf_siblingof(cdie, cdie) == 0);
> +
> +	return tag ? &tag->tag : tag__new(die, cu);
> +}
> +
>  static struct tag *die__create_new_ptr_to_member_type(Dwarf_Die *die,
>  						      struct cu *cu)
>  {
> @@ -1903,12 +1985,13 @@ static struct tag *__die__process_tag(Dwarf_Die *die, struct cu *cu,
>  	case DW_TAG_const_type:
>  	case DW_TAG_imported_declaration:
>  	case DW_TAG_imported_module:
> -	case DW_TAG_pointer_type:
>  	case DW_TAG_reference_type:
>  	case DW_TAG_restrict_type:
>  	case DW_TAG_unspecified_type:
>  	case DW_TAG_volatile_type:
>  		tag = die__create_new_tag(die, cu);		break;
> +	case DW_TAG_pointer_type:
> +		tag = die__create_new_pointer_tag(die, cu, conf);	break;
>  	case DW_TAG_ptr_to_member_type:
>  		tag = die__create_new_ptr_to_member_type(die, cu); break;
>  	case DW_TAG_enumeration_type:
> @@ -2192,6 +2275,26 @@ static void lexblock__recode_dwarf_types(struct lexblock *tag, struct cu *cu)
>  	}
>  }
>  
> +static void dwarf_cu__recode_btf_type_tag_ptr(struct btf_type_tag_ptr_type *tag,
> +					      uint32_t pointee_type)
> +{
> +	struct btf_type_tag_type *annot;
> +	struct dwarf_tag *annot_dtag;
> +	struct tag *prev_tag;
> +
> +	/* If tag->tags contains tag3 -> tag2 -> tag1, the final type chain
> +	 * looks like:
> +	 *   pointer -> tag3 -> tag2 -> tag1 -> pointee
> +	 */
> +	prev_tag = &tag->tag;
> +	list_for_each_entry(annot, &tag->tags, node) {
> +		annot_dtag = annot->tag.priv;
> +		prev_tag->type = annot_dtag->small_id;
> +		prev_tag = &annot->tag;
> +	}
> +	prev_tag->type = pointee_type;
> +}
> +
>  static int tag__recode_dwarf_type(struct tag *tag, struct cu *cu)
>  {
>  	struct dwarf_tag *dtag = tag->priv;
> @@ -2301,7 +2404,10 @@ static int tag__recode_dwarf_type(struct tag *tag, struct cu *cu)
>  	}
>  
>  	if (dtag->type.off == 0) {
> -		tag->type = 0; /* void */
> +		if (tag->tag != DW_TAG_pointer_type || !tag->has_btf_type_tag)
> +			tag->type = 0; /* void */
> +		else
> +			dwarf_cu__recode_btf_type_tag_ptr(tag__btf_type_tag_ptr(tag), 0);
>  		return 0;
>  	}
>  
> @@ -2313,7 +2419,11 @@ check_type:
>  		return 0;
>  	}
>  out:
> -	tag->type = dtype->small_id;
> +	if (tag->tag != DW_TAG_pointer_type || !tag->has_btf_type_tag)
> +		tag->type = dtype->small_id;
> +	else
> +		dwarf_cu__recode_btf_type_tag_ptr(tag__btf_type_tag_ptr(tag), dtype->small_id);
> +
>  	return 0;
>  }
>  
> diff --git a/dwarves.h b/dwarves.h
> index 0d3e204..4425d3c 100644
> --- a/dwarves.h
> +++ b/dwarves.h
> @@ -63,6 +63,7 @@ struct conf_load {
>  	bool			ptr_table_stats;
>  	bool			skip_encoding_btf_decl_tag;
>  	bool			skip_missing;
> +	bool			skip_encoding_btf_type_tag;
>  	uint8_t			hashtable_bits;
>  	uint8_t			max_hashtable_bits;
>  	uint16_t		kabi_prefix_len;
> @@ -413,6 +414,7 @@ struct tag {
>  	uint16_t	 tag;
>  	bool		 visited;
>  	bool		 top_level;
> +	bool		 has_btf_type_tag;
>  	uint16_t	 recursivity_level;
>  	void		 *priv;
>  };
> @@ -533,7 +535,8 @@ static inline int tag__is_tag_type(const struct tag *tag)
>  	       tag->tag == DW_TAG_restrict_type ||
>  	       tag->tag == DW_TAG_subroutine_type ||
>  	       tag->tag == DW_TAG_unspecified_type ||
> -	       tag->tag == DW_TAG_volatile_type;
> +	       tag->tag == DW_TAG_volatile_type ||
> +	       tag->tag == DW_TAG_LLVM_annotation;
>  }
>  
>  static inline const char *tag__decl_file(const struct tag *tag,
> @@ -606,6 +609,34 @@ struct llvm_annotation {
>  	struct list_head	node;
>  };
>  
> +/** struct btf_type_tag_type - representing a btf_type_tag annotation
> + *
> + * @tag   - DW_TAG_LLVM_annotation tag
> + * @value - btf_type_tag value string
> + * @node  - list_head node
> + */
> +struct btf_type_tag_type {
> +	struct tag		tag;
> +	const char		*value;
> +	struct list_head	node;
> +};
> +
> +/** The struct btf_type_tag_ptr_type - type containing both pointer type and
> + *  its btf_type_tag annotations
> + *
> + * @tag  - pointer type tag
> + * @tags - btf_type_tag annotations for the pointer type
> + */
> +struct btf_type_tag_ptr_type {
> +	struct tag		tag;
> +	struct list_head 	tags;
> +};
> +
> +static inline struct btf_type_tag_ptr_type *tag__btf_type_tag_ptr(struct tag *tag)
> +{
> +	return (struct btf_type_tag_ptr_type *)tag;
> +}
> +
>  /** struct namespace - base class for enums, structs, unions, typedefs, etc
>   *
>   * @tags - class_member, enumerators, etc
> diff --git a/pahole.c b/pahole.c
> index 5fc1cca..f3a51cb 100644
> --- a/pahole.c
> +++ b/pahole.c
> @@ -1126,6 +1126,7 @@ ARGP_PROGRAM_VERSION_HOOK_DEF = dwarves_print_version;
>  #define ARGP_devel_stats	   330
>  #define ARGP_skip_encoding_btf_decl_tag 331
>  #define ARGP_skip_missing          332
> +#define ARGP_skip_encoding_btf_type_tag 333
>  
>  static const struct argp_option pahole__options[] = {
>  	{
> @@ -1506,6 +1507,11 @@ static const struct argp_option pahole__options[] = {
>  		.key  = ARGP_skip_missing,
>  		.doc = "skip missing types passed to -C rather than stop",
>  	},
> +	{
> +		.name = "skip_encoding_btf_type_tag",
> +		.key  = ARGP_skip_encoding_btf_type_tag,
> +		.doc  = "Do not encode TAGs in BTF."
> +	},
>  	{
>  		.name = NULL,
>  	}
> @@ -1658,6 +1664,8 @@ static error_t pahole__options_parser(int key, char *arg,
>  		conf_load.skip_encoding_btf_decl_tag = true;	break;
>  	case ARGP_skip_missing:
>  		conf_load.skip_missing = true;          break;
> +	case ARGP_skip_encoding_btf_type_tag:
> +		conf_load.skip_encoding_btf_type_tag = true;	break;
>  	default:
>  		return ARGP_ERR_UNKNOWN;
>  	}
> -- 
> 2.30.2

-- 

- Arnaldo

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH dwarves 3/4] dwarf_loader: support btf_type_tag attribute
  2021-11-18 13:00   ` Arnaldo Carvalho de Melo
@ 2021-11-18 16:11     ` Yonghong Song
  0 siblings, 0 replies; 11+ messages in thread
From: Yonghong Song @ 2021-11-18 16:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Andrii Nakryiko, bpf, Daniel Borkmann, kernel-team



On 11/18/21 5:00 AM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Nov 17, 2021 at 12:22:29PM -0800, Yonghong Song escreveu:
>> This patch implemented dwarf_loader support. If a pointer type
>> contains DW_TAG_LLVM_annotation tags, a new type
>> btf_type_tag_ptr_type will be created which will store
>> the pointer tag itself and all DW_TAG_LLVM_annotation tags.
>> During recoding stage, the type chain will be formed properly
>> based on the above example.
>>
>> An option "--skip_encoding_btf_type_tag" is added to disable
>> this new functionality.
>>
>>    [1] https://reviews.llvm.org/D111199
>>    [2] https://reviews.llvm.org/D113222
>>    [3] https://reviews.llvm.org/D113496
> 
> You forgot to add your S-o-B and to add this entry to
> man-pages/pahole.1, I'm fixing both cases, bellow is a followup
> patch, I'll add one as well for the recently added
> --skip_encoding_btf_decl_tag.

Thanks for the fixup! It is my fault as I never changed man page
before and not aware of that.

> 
> - Arnaldo
> 
> commit 9c3101db76acf364607d90adb3052e34d81fa1bd
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date:   Thu Nov 18 09:56:35 2021 -0300
> 
>      man pages: Add missing --skip_encoding_btf_type_tag entry
>      
>      In the past we saw the value of being able to disable specific features
>      due to problems in in its implementation, allowing users to use a subset
>      of functionality, without the problematic one.
>      
>      Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> diff --git a/man-pages/pahole.1 b/man-pages/pahole.1
> index edcf58b8ca5814a3..f9f64b67945b45cb 100644
> --- a/man-pages/pahole.1
> +++ b/man-pages/pahole.1
> @@ -197,6 +197,10 @@ the debugging information.
>   .B \-\-skip_encoding_btf_vars
>   Do not encode VARs in BTF.
>   
> +.TP
> +.B \-\-skip_encoding_btf_type_tag
> +Do not encode type tags in BTF.
> +
>   .TP
>   .B \-j, \-\-jobs=N
>   Run N jobs in parallel. Defaults to number of online processors + 10% (like
> 
> 

LGTM.

[...]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH dwarves 3/4] dwarf_loader: support btf_type_tag attribute
  2021-11-17 20:22 ` [PATCH dwarves 3/4] dwarf_loader: support btf_type_tag attribute Yonghong Song
  2021-11-18 13:00   ` Arnaldo Carvalho de Melo
@ 2021-11-23  1:52   ` Andrii Nakryiko
  2021-11-23  3:32     ` Yonghong Song
  1 sibling, 1 reply; 11+ messages in thread
From: Andrii Nakryiko @ 2021-11-23  1:52 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Andrii Nakryiko, bpf, Daniel Borkmann, Kernel Team

On Wed, Nov 17, 2021 at 12:25 PM Yonghong Song <yhs@fb.com> wrote:
>
> LLVM patches ([1] for clang, [2] and [3] for BPF backend)
> added support for btf_type_tag attributes. The following is
> an example:
>   [$ ~] cat t.c
>   #define __tag1 __attribute__((btf_type_tag("tag1")))
>   #define __tag2 __attribute__((btf_type_tag("tag2")))
>   int __tag1 * __tag1 __tag2 *g __attribute__((section(".data..percpu")));
>   [$ ~] clang -O2 -g -c t.c
>   [$ ~] llvm-dwarfdump --debug-info t.o
>   t.o:    file format elf64-x86-64
>   ...
>   0x0000001e:   DW_TAG_variable
>                   DW_AT_name      ("g")
>                   DW_AT_type      (0x00000033 "int **")
>                   DW_AT_external  (true)
>                   DW_AT_decl_file ("/home/yhs/t.c")
>                   DW_AT_decl_line (3)
>                   DW_AT_location  (DW_OP_addr 0x0)
>   0x00000033:   DW_TAG_pointer_type
>                   DW_AT_type      (0x0000004b "int *")
>   0x00000038:     DW_TAG_LLVM_annotation
>                     DW_AT_name    ("btf_type_tag")
>                     DW_AT_const_value     ("tag1")
>   0x00000041:     DW_TAG_LLVM_annotation
>                     DW_AT_name    ("btf_type_tag")
>                     DW_AT_const_value     ("tag2")
>   0x0000004a:     NULL
>   0x0000004b:   DW_TAG_pointer_type
>                   DW_AT_type      (0x0000005a "int")
>   0x00000050:     DW_TAG_LLVM_annotation
>                     DW_AT_name    ("btf_type_tag")
>                     DW_AT_const_value     ("tag1")
>   0x00000059:     NULL
>   0x0000005a:   DW_TAG_base_type
>                   DW_AT_name      ("int")
>                   DW_AT_encoding  (DW_ATE_signed)
>                   DW_AT_byte_size (0x04)
>   0x00000061:   NULL
>
> From the above example, you can see that DW_TAG_pointer_type
> may contain one or more DW_TAG_LLVM_annotation btf_type_tag tags.
> If DW_TAG_LLVM_annotation tags are present inside
> DW_TAG_pointer_type, for BTF encoding, pahole will need
> to follow [3] to generate a type chain like
>   var -> ptr -> tag2 -> tag1 -> ptr -> tag1 -> int
>
> This patch implemented dwarf_loader support. If a pointer type
> contains DW_TAG_LLVM_annotation tags, a new type
> btf_type_tag_ptr_type will be created which will store
> the pointer tag itself and all DW_TAG_LLVM_annotation tags.
> During recoding stage, the type chain will be formed properly
> based on the above example.
>
> An option "--skip_encoding_btf_type_tag" is added to disable
> this new functionality.
>
>   [1] https://reviews.llvm.org/D111199
>   [2] https://reviews.llvm.org/D113222
>   [3] https://reviews.llvm.org/D113496
> ---
>  dwarf_loader.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++--
>  dwarves.h      |  33 +++++++++++++-
>  pahole.c       |   8 ++++
>  3 files changed, 153 insertions(+), 4 deletions(-)
>

[...]

> +
> +static struct tag *die__create_new_pointer_tag(Dwarf_Die *die, struct cu *cu,
> +                                              struct conf_load *conf)
> +{
> +       struct btf_type_tag_ptr_type *tag = NULL;
> +       struct btf_type_tag_type *annot;
> +       Dwarf_Die *cdie, child;
> +       const char *name;
> +       uint32_t id;
> +
> +       /* If no child tags or skipping btf_type_tag encoding, just create a new tag
> +        * and return
> +        */
> +       if (!dwarf_haschildren(die) || dwarf_child(die, &child) != 0 ||
> +           conf->skip_encoding_btf_type_tag)
> +               return tag__new(die, cu);
> +
> +       /* Otherwise, check DW_TAG_LLVM_annotation child tags */
> +       cdie = &child;
> +       do {
> +               if (dwarf_tag(cdie) == DW_TAG_LLVM_annotation) {

nit: inverting the condition and doing continue would reduce nestedness level

> +                       /* Only check btf_type_tag annotations */
> +                       name = attr_string(cdie, DW_AT_name, conf);
> +                       if (strcmp(name, "btf_type_tag") != 0)
> +                               continue;
> +
> +                       if (tag == NULL) {
> +                               /* Create a btf_type_tag_ptr type. */
> +                               tag = die__create_new_btf_type_tag_ptr_type(die, cu);
> +                               if (!tag)
> +                                       return NULL;
> +                       }
> +
> +                       /* Create a btf_type_tag type for this annotation. */
> +                       annot = die__create_new_btf_type_tag_type(cdie, cu, conf);
> +                       if (annot == NULL)
> +                               return NULL;
> +
> +                       if (cu__table_add_tag(cu, &annot->tag, &id) < 0)
> +                               return NULL;
> +
> +                       struct dwarf_tag *dtag = annot->tag.priv;
> +                       dtag->small_id = id;
> +                       cu__hash(cu, &annot->tag);
> +
> +                       /* For a list of DW_TAG_LLVM_annotation like tag1 -> tag2 -> tag3,
> +                        * the tag->tags contains tag3 -> tag2 -> tag1.
> +                        */
> +                       list_add(&annot->node, &tag->tags);
> +               }
> +       } while (dwarf_siblingof(cdie, cdie) == 0);
> +
> +       return tag ? &tag->tag : tag__new(die, cu);
> +}
> +
>  static struct tag *die__create_new_ptr_to_member_type(Dwarf_Die *die,
>                                                       struct cu *cu)
>  {
> @@ -1903,12 +1985,13 @@ static struct tag *__die__process_tag(Dwarf_Die *die, struct cu *cu,
>         case DW_TAG_const_type:
>         case DW_TAG_imported_declaration:
>         case DW_TAG_imported_module:
> -       case DW_TAG_pointer_type:
>         case DW_TAG_reference_type:
>         case DW_TAG_restrict_type:
>         case DW_TAG_unspecified_type:
>         case DW_TAG_volatile_type:
>                 tag = die__create_new_tag(die, cu);             break;
> +       case DW_TAG_pointer_type:
> +               tag = die__create_new_pointer_tag(die, cu, conf);       break;
>         case DW_TAG_ptr_to_member_type:
>                 tag = die__create_new_ptr_to_member_type(die, cu); break;
>         case DW_TAG_enumeration_type:
> @@ -2192,6 +2275,26 @@ static void lexblock__recode_dwarf_types(struct lexblock *tag, struct cu *cu)
>         }
>  }
>
> +static void dwarf_cu__recode_btf_type_tag_ptr(struct btf_type_tag_ptr_type *tag,
> +                                             uint32_t pointee_type)
> +{
> +       struct btf_type_tag_type *annot;
> +       struct dwarf_tag *annot_dtag;
> +       struct tag *prev_tag;
> +
> +       /* If tag->tags contains tag3 -> tag2 -> tag1, the final type chain
> +        * looks like:
> +        *   pointer -> tag3 -> tag2 -> tag1 -> pointee
> +        */

is the comment accurate or the final one should have looked like
pointer -> tag1 -> tag2 -> tag3 -> pointee? Basically, trying to
understand if the final BTF represents the source-level order of tags
or not?

> +       prev_tag = &tag->tag;
> +       list_for_each_entry(annot, &tag->tags, node) {
> +               annot_dtag = annot->tag.priv;
> +               prev_tag->type = annot_dtag->small_id;
> +               prev_tag = &annot->tag;
> +       }
> +       prev_tag->type = pointee_type;
> +}
> +

[...]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH dwarves 3/4] dwarf_loader: support btf_type_tag attribute
  2021-11-23  1:52   ` Andrii Nakryiko
@ 2021-11-23  3:32     ` Yonghong Song
  2021-11-23  3:45       ` Andrii Nakryiko
  0 siblings, 1 reply; 11+ messages in thread
From: Yonghong Song @ 2021-11-23  3:32 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Andrii Nakryiko, bpf, Daniel Borkmann, Kernel Team



On 11/22/21 5:52 PM, Andrii Nakryiko wrote:
> On Wed, Nov 17, 2021 at 12:25 PM Yonghong Song <yhs@fb.com> wrote:
>>
>> LLVM patches ([1] for clang, [2] and [3] for BPF backend)
>> added support for btf_type_tag attributes. The following is
>> an example:
>>    [$ ~] cat t.c
>>    #define __tag1 __attribute__((btf_type_tag("tag1")))
>>    #define __tag2 __attribute__((btf_type_tag("tag2")))
>>    int __tag1 * __tag1 __tag2 *g __attribute__((section(".data..percpu")));
>>    [$ ~] clang -O2 -g -c t.c
>>    [$ ~] llvm-dwarfdump --debug-info t.o
>>    t.o:    file format elf64-x86-64
>>    ...
>>    0x0000001e:   DW_TAG_variable
>>                    DW_AT_name      ("g")
>>                    DW_AT_type      (0x00000033 "int **")
>>                    DW_AT_external  (true)
>>                    DW_AT_decl_file ("/home/yhs/t.c")
>>                    DW_AT_decl_line (3)
>>                    DW_AT_location  (DW_OP_addr 0x0)
>>    0x00000033:   DW_TAG_pointer_type
>>                    DW_AT_type      (0x0000004b "int *")
>>    0x00000038:     DW_TAG_LLVM_annotation
>>                      DW_AT_name    ("btf_type_tag")
>>                      DW_AT_const_value     ("tag1")
>>    0x00000041:     DW_TAG_LLVM_annotation
>>                      DW_AT_name    ("btf_type_tag")
>>                      DW_AT_const_value     ("tag2")
>>    0x0000004a:     NULL
>>    0x0000004b:   DW_TAG_pointer_type
>>                    DW_AT_type      (0x0000005a "int")
>>    0x00000050:     DW_TAG_LLVM_annotation
>>                      DW_AT_name    ("btf_type_tag")
>>                      DW_AT_const_value     ("tag1")
>>    0x00000059:     NULL
>>    0x0000005a:   DW_TAG_base_type
>>                    DW_AT_name      ("int")
>>                    DW_AT_encoding  (DW_ATE_signed)
>>                    DW_AT_byte_size (0x04)
>>    0x00000061:   NULL
>>
>>  From the above example, you can see that DW_TAG_pointer_type
>> may contain one or more DW_TAG_LLVM_annotation btf_type_tag tags.
>> If DW_TAG_LLVM_annotation tags are present inside
>> DW_TAG_pointer_type, for BTF encoding, pahole will need
>> to follow [3] to generate a type chain like
>>    var -> ptr -> tag2 -> tag1 -> ptr -> tag1 -> int
>>
>> This patch implemented dwarf_loader support. If a pointer type
>> contains DW_TAG_LLVM_annotation tags, a new type
>> btf_type_tag_ptr_type will be created which will store
>> the pointer tag itself and all DW_TAG_LLVM_annotation tags.
>> During recoding stage, the type chain will be formed properly
>> based on the above example.
>>
>> An option "--skip_encoding_btf_type_tag" is added to disable
>> this new functionality.
>>
>>    [1] https://reviews.llvm.org/D111199
>>    [2] https://reviews.llvm.org/D113222
>>    [3] https://reviews.llvm.org/D113496
>> ---
>>   dwarf_loader.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++--
>>   dwarves.h      |  33 +++++++++++++-
>>   pahole.c       |   8 ++++
>>   3 files changed, 153 insertions(+), 4 deletions(-)
>>
> 
> [...]
> 
>> +
>> +static struct tag *die__create_new_pointer_tag(Dwarf_Die *die, struct cu *cu,
>> +                                              struct conf_load *conf)
>> +{
>> +       struct btf_type_tag_ptr_type *tag = NULL;
>> +       struct btf_type_tag_type *annot;
>> +       Dwarf_Die *cdie, child;
>> +       const char *name;
>> +       uint32_t id;
>> +
>> +       /* If no child tags or skipping btf_type_tag encoding, just create a new tag
>> +        * and return
>> +        */
>> +       if (!dwarf_haschildren(die) || dwarf_child(die, &child) != 0 ||
>> +           conf->skip_encoding_btf_type_tag)
>> +               return tag__new(die, cu);
>> +
>> +       /* Otherwise, check DW_TAG_LLVM_annotation child tags */
>> +       cdie = &child;
>> +       do {
>> +               if (dwarf_tag(cdie) == DW_TAG_LLVM_annotation) {
> 
> nit: inverting the condition and doing continue would reduce nestedness level

good point. Will send another revision.

> 
>> +                       /* Only check btf_type_tag annotations */
>> +                       name = attr_string(cdie, DW_AT_name, conf);
>> +                       if (strcmp(name, "btf_type_tag") != 0)
>> +                               continue;
>> +
>> +                       if (tag == NULL) {
>> +                               /* Create a btf_type_tag_ptr type. */
>> +                               tag = die__create_new_btf_type_tag_ptr_type(die, cu);
>> +                               if (!tag)
>> +                                       return NULL;
>> +                       }
>> +
>> +                       /* Create a btf_type_tag type for this annotation. */
>> +                       annot = die__create_new_btf_type_tag_type(cdie, cu, conf);
>> +                       if (annot == NULL)
>> +                               return NULL;
>> +
>> +                       if (cu__table_add_tag(cu, &annot->tag, &id) < 0)
>> +                               return NULL;
>> +
>> +                       struct dwarf_tag *dtag = annot->tag.priv;
>> +                       dtag->small_id = id;
>> +                       cu__hash(cu, &annot->tag);
>> +
>> +                       /* For a list of DW_TAG_LLVM_annotation like tag1 -> tag2 -> tag3,
>> +                        * the tag->tags contains tag3 -> tag2 -> tag1.
>> +                        */
>> +                       list_add(&annot->node, &tag->tags);
>> +               }
>> +       } while (dwarf_siblingof(cdie, cdie) == 0);
>> +
>> +       return tag ? &tag->tag : tag__new(die, cu);
>> +}
>> +
>>   static struct tag *die__create_new_ptr_to_member_type(Dwarf_Die *die,
>>                                                        struct cu *cu)
>>   {
>> @@ -1903,12 +1985,13 @@ static struct tag *__die__process_tag(Dwarf_Die *die, struct cu *cu,
>>          case DW_TAG_const_type:
>>          case DW_TAG_imported_declaration:
>>          case DW_TAG_imported_module:
>> -       case DW_TAG_pointer_type:
>>          case DW_TAG_reference_type:
>>          case DW_TAG_restrict_type:
>>          case DW_TAG_unspecified_type:
>>          case DW_TAG_volatile_type:
>>                  tag = die__create_new_tag(die, cu);             break;
>> +       case DW_TAG_pointer_type:
>> +               tag = die__create_new_pointer_tag(die, cu, conf);       break;
>>          case DW_TAG_ptr_to_member_type:
>>                  tag = die__create_new_ptr_to_member_type(die, cu); break;
>>          case DW_TAG_enumeration_type:
>> @@ -2192,6 +2275,26 @@ static void lexblock__recode_dwarf_types(struct lexblock *tag, struct cu *cu)
>>          }
>>   }
>>
>> +static void dwarf_cu__recode_btf_type_tag_ptr(struct btf_type_tag_ptr_type *tag,
>> +                                             uint32_t pointee_type)
>> +{
>> +       struct btf_type_tag_type *annot;
>> +       struct dwarf_tag *annot_dtag;
>> +       struct tag *prev_tag;
>> +
>> +       /* If tag->tags contains tag3 -> tag2 -> tag1, the final type chain
>> +        * looks like:
>> +        *   pointer -> tag3 -> tag2 -> tag1 -> pointee
>> +        */
> 
> is the comment accurate or the final one should have looked like
> pointer -> tag1 -> tag2 -> tag3 -> pointee? Basically, trying to
> understand if the final BTF represents the source-level order of tags
> or not?

The comment is accurate. Given source like
    int tag1 tag2 tag3 *p;
the final type chain is
    p -> tag3 -> tag2 -> tag1 -> int

basically it means
    - '*' applies to "int tag1 tag2 tag3"
    - tag3 applies to "int tag1 tag2"
    - tag2 applies to "int tag1"
    - tag1 applies to "int"

This also makes final source code (format c) easier as
we can do
    emit for "tag3 -> tag2 -> tag1 -> int"
    emit '*'

For 'tag3 -> tag2 -> tag1 -> int":
    emit for "tag2 -> tag1 -> int"
    emit tag3

Eventually we can get the source code like
    int tag1 tag2 tag3 *p
and this matches the user/kernel code.

> 
>> +       prev_tag = &tag->tag;
>> +       list_for_each_entry(annot, &tag->tags, node) {
>> +               annot_dtag = annot->tag.priv;
>> +               prev_tag->type = annot_dtag->small_id;
>> +               prev_tag = &annot->tag;
>> +       }
>> +       prev_tag->type = pointee_type;
>> +}
>> +
> 
> [...]
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH dwarves 3/4] dwarf_loader: support btf_type_tag attribute
  2021-11-23  3:32     ` Yonghong Song
@ 2021-11-23  3:45       ` Andrii Nakryiko
  2021-11-23  4:18         ` Yonghong Song
  0 siblings, 1 reply; 11+ messages in thread
From: Andrii Nakryiko @ 2021-11-23  3:45 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Andrii Nakryiko, bpf, Daniel Borkmann, Kernel Team

On Mon, Nov 22, 2021 at 7:32 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 11/22/21 5:52 PM, Andrii Nakryiko wrote:
> > On Wed, Nov 17, 2021 at 12:25 PM Yonghong Song <yhs@fb.com> wrote:
> >>
> >> LLVM patches ([1] for clang, [2] and [3] for BPF backend)
> >> added support for btf_type_tag attributes. The following is
> >> an example:
> >>    [$ ~] cat t.c
> >>    #define __tag1 __attribute__((btf_type_tag("tag1")))
> >>    #define __tag2 __attribute__((btf_type_tag("tag2")))
> >>    int __tag1 * __tag1 __tag2 *g __attribute__((section(".data..percpu")));
> >>    [$ ~] clang -O2 -g -c t.c
> >>    [$ ~] llvm-dwarfdump --debug-info t.o
> >>    t.o:    file format elf64-x86-64
> >>    ...
> >>    0x0000001e:   DW_TAG_variable
> >>                    DW_AT_name      ("g")
> >>                    DW_AT_type      (0x00000033 "int **")
> >>                    DW_AT_external  (true)
> >>                    DW_AT_decl_file ("/home/yhs/t.c")
> >>                    DW_AT_decl_line (3)
> >>                    DW_AT_location  (DW_OP_addr 0x0)
> >>    0x00000033:   DW_TAG_pointer_type
> >>                    DW_AT_type      (0x0000004b "int *")
> >>    0x00000038:     DW_TAG_LLVM_annotation
> >>                      DW_AT_name    ("btf_type_tag")
> >>                      DW_AT_const_value     ("tag1")
> >>    0x00000041:     DW_TAG_LLVM_annotation
> >>                      DW_AT_name    ("btf_type_tag")
> >>                      DW_AT_const_value     ("tag2")
> >>    0x0000004a:     NULL
> >>    0x0000004b:   DW_TAG_pointer_type
> >>                    DW_AT_type      (0x0000005a "int")
> >>    0x00000050:     DW_TAG_LLVM_annotation
> >>                      DW_AT_name    ("btf_type_tag")
> >>                      DW_AT_const_value     ("tag1")
> >>    0x00000059:     NULL
> >>    0x0000005a:   DW_TAG_base_type
> >>                    DW_AT_name      ("int")
> >>                    DW_AT_encoding  (DW_ATE_signed)
> >>                    DW_AT_byte_size (0x04)
> >>    0x00000061:   NULL
> >>
> >>  From the above example, you can see that DW_TAG_pointer_type
> >> may contain one or more DW_TAG_LLVM_annotation btf_type_tag tags.
> >> If DW_TAG_LLVM_annotation tags are present inside
> >> DW_TAG_pointer_type, for BTF encoding, pahole will need
> >> to follow [3] to generate a type chain like
> >>    var -> ptr -> tag2 -> tag1 -> ptr -> tag1 -> int
> >>
> >> This patch implemented dwarf_loader support. If a pointer type
> >> contains DW_TAG_LLVM_annotation tags, a new type
> >> btf_type_tag_ptr_type will be created which will store
> >> the pointer tag itself and all DW_TAG_LLVM_annotation tags.
> >> During recoding stage, the type chain will be formed properly
> >> based on the above example.
> >>
> >> An option "--skip_encoding_btf_type_tag" is added to disable
> >> this new functionality.
> >>
> >>    [1] https://reviews.llvm.org/D111199
> >>    [2] https://reviews.llvm.org/D113222
> >>    [3] https://reviews.llvm.org/D113496
> >> ---
> >>   dwarf_loader.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++--
> >>   dwarves.h      |  33 +++++++++++++-
> >>   pahole.c       |   8 ++++
> >>   3 files changed, 153 insertions(+), 4 deletions(-)
> >>
> >
> > [...]
> >
> >> +
> >> +static struct tag *die__create_new_pointer_tag(Dwarf_Die *die, struct cu *cu,
> >> +                                              struct conf_load *conf)
> >> +{
> >> +       struct btf_type_tag_ptr_type *tag = NULL;
> >> +       struct btf_type_tag_type *annot;
> >> +       Dwarf_Die *cdie, child;
> >> +       const char *name;
> >> +       uint32_t id;
> >> +
> >> +       /* If no child tags or skipping btf_type_tag encoding, just create a new tag
> >> +        * and return
> >> +        */
> >> +       if (!dwarf_haschildren(die) || dwarf_child(die, &child) != 0 ||
> >> +           conf->skip_encoding_btf_type_tag)
> >> +               return tag__new(die, cu);
> >> +
> >> +       /* Otherwise, check DW_TAG_LLVM_annotation child tags */
> >> +       cdie = &child;
> >> +       do {
> >> +               if (dwarf_tag(cdie) == DW_TAG_LLVM_annotation) {
> >
> > nit: inverting the condition and doing continue would reduce nestedness level
>
> good point. Will send another revision.
>
> >
> >> +                       /* Only check btf_type_tag annotations */
> >> +                       name = attr_string(cdie, DW_AT_name, conf);
> >> +                       if (strcmp(name, "btf_type_tag") != 0)
> >> +                               continue;
> >> +
> >> +                       if (tag == NULL) {
> >> +                               /* Create a btf_type_tag_ptr type. */
> >> +                               tag = die__create_new_btf_type_tag_ptr_type(die, cu);
> >> +                               if (!tag)
> >> +                                       return NULL;
> >> +                       }
> >> +
> >> +                       /* Create a btf_type_tag type for this annotation. */
> >> +                       annot = die__create_new_btf_type_tag_type(cdie, cu, conf);
> >> +                       if (annot == NULL)
> >> +                               return NULL;
> >> +
> >> +                       if (cu__table_add_tag(cu, &annot->tag, &id) < 0)
> >> +                               return NULL;
> >> +
> >> +                       struct dwarf_tag *dtag = annot->tag.priv;
> >> +                       dtag->small_id = id;
> >> +                       cu__hash(cu, &annot->tag);
> >> +
> >> +                       /* For a list of DW_TAG_LLVM_annotation like tag1 -> tag2 -> tag3,
> >> +                        * the tag->tags contains tag3 -> tag2 -> tag1.
> >> +                        */
> >> +                       list_add(&annot->node, &tag->tags);
> >> +               }
> >> +       } while (dwarf_siblingof(cdie, cdie) == 0);
> >> +
> >> +       return tag ? &tag->tag : tag__new(die, cu);
> >> +}
> >> +
> >>   static struct tag *die__create_new_ptr_to_member_type(Dwarf_Die *die,
> >>                                                        struct cu *cu)
> >>   {
> >> @@ -1903,12 +1985,13 @@ static struct tag *__die__process_tag(Dwarf_Die *die, struct cu *cu,
> >>          case DW_TAG_const_type:
> >>          case DW_TAG_imported_declaration:
> >>          case DW_TAG_imported_module:
> >> -       case DW_TAG_pointer_type:
> >>          case DW_TAG_reference_type:
> >>          case DW_TAG_restrict_type:
> >>          case DW_TAG_unspecified_type:
> >>          case DW_TAG_volatile_type:
> >>                  tag = die__create_new_tag(die, cu);             break;
> >> +       case DW_TAG_pointer_type:
> >> +               tag = die__create_new_pointer_tag(die, cu, conf);       break;
> >>          case DW_TAG_ptr_to_member_type:
> >>                  tag = die__create_new_ptr_to_member_type(die, cu); break;
> >>          case DW_TAG_enumeration_type:
> >> @@ -2192,6 +2275,26 @@ static void lexblock__recode_dwarf_types(struct lexblock *tag, struct cu *cu)
> >>          }
> >>   }
> >>
> >> +static void dwarf_cu__recode_btf_type_tag_ptr(struct btf_type_tag_ptr_type *tag,
> >> +                                             uint32_t pointee_type)
> >> +{
> >> +       struct btf_type_tag_type *annot;
> >> +       struct dwarf_tag *annot_dtag;
> >> +       struct tag *prev_tag;
> >> +
> >> +       /* If tag->tags contains tag3 -> tag2 -> tag1, the final type chain
> >> +        * looks like:
> >> +        *   pointer -> tag3 -> tag2 -> tag1 -> pointee
> >> +        */
> >
> > is the comment accurate or the final one should have looked like
> > pointer -> tag1 -> tag2 -> tag3 -> pointee? Basically, trying to
> > understand if the final BTF represents the source-level order of tags
> > or not?
>
> The comment is accurate. Given source like
>     int tag1 tag2 tag3 *p;
> the final type chain is
>     p -> tag3 -> tag2 -> tag1 -> int
>
> basically it means
>     - '*' applies to "int tag1 tag2 tag3"
>     - tag3 applies to "int tag1 tag2"
>     - tag2 applies to "int tag1"
>     - tag1 applies to "int"
>
> This also makes final source code (format c) easier as
> we can do
>     emit for "tag3 -> tag2 -> tag1 -> int"
>     emit '*'
>
> For 'tag3 -> tag2 -> tag1 -> int":
>     emit for "tag2 -> tag1 -> int"
>     emit tag3
>
> Eventually we can get the source code like
>     int tag1 tag2 tag3 *p
> and this matches the user/kernel code.

It would be great to add that as a comment somewhere here, it's very
hard to make this inference just from the code.

>
> >
> >> +       prev_tag = &tag->tag;
> >> +       list_for_each_entry(annot, &tag->tags, node) {
> >> +               annot_dtag = annot->tag.priv;
> >> +               prev_tag->type = annot_dtag->small_id;
> >> +               prev_tag = &annot->tag;
> >> +       }
> >> +       prev_tag->type = pointee_type;
> >> +}
> >> +
> >
> > [...]
> >

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH dwarves 3/4] dwarf_loader: support btf_type_tag attribute
  2021-11-23  3:45       ` Andrii Nakryiko
@ 2021-11-23  4:18         ` Yonghong Song
  0 siblings, 0 replies; 11+ messages in thread
From: Yonghong Song @ 2021-11-23  4:18 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Arnaldo Carvalho de Melo, dwarves, Alexei Starovoitov,
	Andrii Nakryiko, bpf, Daniel Borkmann, Kernel Team



On 11/22/21 7:45 PM, Andrii Nakryiko wrote:
> On Mon, Nov 22, 2021 at 7:32 PM Yonghong Song <yhs@fb.com> wrote:
>>
>>
>>
>> On 11/22/21 5:52 PM, Andrii Nakryiko wrote:
>>> On Wed, Nov 17, 2021 at 12:25 PM Yonghong Song <yhs@fb.com> wrote:
>>>>
>>>> LLVM patches ([1] for clang, [2] and [3] for BPF backend)
>>>> added support for btf_type_tag attributes. The following is
>>>> an example:
>>>>     [$ ~] cat t.c
>>>>     #define __tag1 __attribute__((btf_type_tag("tag1")))
>>>>     #define __tag2 __attribute__((btf_type_tag("tag2")))
>>>>     int __tag1 * __tag1 __tag2 *g __attribute__((section(".data..percpu")));
>>>>     [$ ~] clang -O2 -g -c t.c
>>>>     [$ ~] llvm-dwarfdump --debug-info t.o
>>>>     t.o:    file format elf64-x86-64
>>>>     ...
>>>>     0x0000001e:   DW_TAG_variable
>>>>                     DW_AT_name      ("g")
>>>>                     DW_AT_type      (0x00000033 "int **")
>>>>                     DW_AT_external  (true)
>>>>                     DW_AT_decl_file ("/home/yhs/t.c")
>>>>                     DW_AT_decl_line (3)
>>>>                     DW_AT_location  (DW_OP_addr 0x0)
>>>>     0x00000033:   DW_TAG_pointer_type
>>>>                     DW_AT_type      (0x0000004b "int *")
>>>>     0x00000038:     DW_TAG_LLVM_annotation
>>>>                       DW_AT_name    ("btf_type_tag")
>>>>                       DW_AT_const_value     ("tag1")
>>>>     0x00000041:     DW_TAG_LLVM_annotation
>>>>                       DW_AT_name    ("btf_type_tag")
>>>>                       DW_AT_const_value     ("tag2")
>>>>     0x0000004a:     NULL
>>>>     0x0000004b:   DW_TAG_pointer_type
>>>>                     DW_AT_type      (0x0000005a "int")
>>>>     0x00000050:     DW_TAG_LLVM_annotation
>>>>                       DW_AT_name    ("btf_type_tag")
>>>>                       DW_AT_const_value     ("tag1")
>>>>     0x00000059:     NULL
>>>>     0x0000005a:   DW_TAG_base_type
>>>>                     DW_AT_name      ("int")
>>>>                     DW_AT_encoding  (DW_ATE_signed)
>>>>                     DW_AT_byte_size (0x04)
>>>>     0x00000061:   NULL
>>>>
>>>>   From the above example, you can see that DW_TAG_pointer_type
>>>> may contain one or more DW_TAG_LLVM_annotation btf_type_tag tags.
>>>> If DW_TAG_LLVM_annotation tags are present inside
>>>> DW_TAG_pointer_type, for BTF encoding, pahole will need
>>>> to follow [3] to generate a type chain like
>>>>     var -> ptr -> tag2 -> tag1 -> ptr -> tag1 -> int
>>>>
>>>> This patch implemented dwarf_loader support. If a pointer type
>>>> contains DW_TAG_LLVM_annotation tags, a new type
>>>> btf_type_tag_ptr_type will be created which will store
>>>> the pointer tag itself and all DW_TAG_LLVM_annotation tags.
>>>> During recoding stage, the type chain will be formed properly
>>>> based on the above example.
>>>>
>>>> An option "--skip_encoding_btf_type_tag" is added to disable
>>>> this new functionality.
>>>>
>>>>     [1] https://reviews.llvm.org/D111199
>>>>     [2] https://reviews.llvm.org/D113222
>>>>     [3] https://reviews.llvm.org/D113496
>>>> ---
>>>>    dwarf_loader.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++--
>>>>    dwarves.h      |  33 +++++++++++++-
>>>>    pahole.c       |   8 ++++
>>>>    3 files changed, 153 insertions(+), 4 deletions(-)
>>>>
>>>
>>> [...]
>>>
>>>> +
>>>> +static struct tag *die__create_new_pointer_tag(Dwarf_Die *die, struct cu *cu,
>>>> +                                              struct conf_load *conf)
>>>> +{
>>>> +       struct btf_type_tag_ptr_type *tag = NULL;
>>>> +       struct btf_type_tag_type *annot;
>>>> +       Dwarf_Die *cdie, child;
>>>> +       const char *name;
>>>> +       uint32_t id;
>>>> +
>>>> +       /* If no child tags or skipping btf_type_tag encoding, just create a new tag
>>>> +        * and return
>>>> +        */
>>>> +       if (!dwarf_haschildren(die) || dwarf_child(die, &child) != 0 ||
>>>> +           conf->skip_encoding_btf_type_tag)
>>>> +               return tag__new(die, cu);
>>>> +
>>>> +       /* Otherwise, check DW_TAG_LLVM_annotation child tags */
>>>> +       cdie = &child;
>>>> +       do {
>>>> +               if (dwarf_tag(cdie) == DW_TAG_LLVM_annotation) {
>>>
>>> nit: inverting the condition and doing continue would reduce nestedness level
>>
>> good point. Will send another revision.
>>
>>>
>>>> +                       /* Only check btf_type_tag annotations */
>>>> +                       name = attr_string(cdie, DW_AT_name, conf);
>>>> +                       if (strcmp(name, "btf_type_tag") != 0)
>>>> +                               continue;
>>>> +
>>>> +                       if (tag == NULL) {
>>>> +                               /* Create a btf_type_tag_ptr type. */
>>>> +                               tag = die__create_new_btf_type_tag_ptr_type(die, cu);
>>>> +                               if (!tag)
>>>> +                                       return NULL;
>>>> +                       }
>>>> +
>>>> +                       /* Create a btf_type_tag type for this annotation. */
>>>> +                       annot = die__create_new_btf_type_tag_type(cdie, cu, conf);
>>>> +                       if (annot == NULL)
>>>> +                               return NULL;
>>>> +
>>>> +                       if (cu__table_add_tag(cu, &annot->tag, &id) < 0)
>>>> +                               return NULL;
>>>> +
>>>> +                       struct dwarf_tag *dtag = annot->tag.priv;
>>>> +                       dtag->small_id = id;
>>>> +                       cu__hash(cu, &annot->tag);
>>>> +
>>>> +                       /* For a list of DW_TAG_LLVM_annotation like tag1 -> tag2 -> tag3,
>>>> +                        * the tag->tags contains tag3 -> tag2 -> tag1.
>>>> +                        */
>>>> +                       list_add(&annot->node, &tag->tags);
>>>> +               }
>>>> +       } while (dwarf_siblingof(cdie, cdie) == 0);
>>>> +
>>>> +       return tag ? &tag->tag : tag__new(die, cu);
>>>> +}
>>>> +
>>>>    static struct tag *die__create_new_ptr_to_member_type(Dwarf_Die *die,
>>>>                                                         struct cu *cu)
>>>>    {
>>>> @@ -1903,12 +1985,13 @@ static struct tag *__die__process_tag(Dwarf_Die *die, struct cu *cu,
>>>>           case DW_TAG_const_type:
>>>>           case DW_TAG_imported_declaration:
>>>>           case DW_TAG_imported_module:
>>>> -       case DW_TAG_pointer_type:
>>>>           case DW_TAG_reference_type:
>>>>           case DW_TAG_restrict_type:
>>>>           case DW_TAG_unspecified_type:
>>>>           case DW_TAG_volatile_type:
>>>>                   tag = die__create_new_tag(die, cu);             break;
>>>> +       case DW_TAG_pointer_type:
>>>> +               tag = die__create_new_pointer_tag(die, cu, conf);       break;
>>>>           case DW_TAG_ptr_to_member_type:
>>>>                   tag = die__create_new_ptr_to_member_type(die, cu); break;
>>>>           case DW_TAG_enumeration_type:
>>>> @@ -2192,6 +2275,26 @@ static void lexblock__recode_dwarf_types(struct lexblock *tag, struct cu *cu)
>>>>           }
>>>>    }
>>>>
>>>> +static void dwarf_cu__recode_btf_type_tag_ptr(struct btf_type_tag_ptr_type *tag,
>>>> +                                             uint32_t pointee_type)
>>>> +{
>>>> +       struct btf_type_tag_type *annot;
>>>> +       struct dwarf_tag *annot_dtag;
>>>> +       struct tag *prev_tag;
>>>> +
>>>> +       /* If tag->tags contains tag3 -> tag2 -> tag1, the final type chain
>>>> +        * looks like:
>>>> +        *   pointer -> tag3 -> tag2 -> tag1 -> pointee
>>>> +        */
>>>
>>> is the comment accurate or the final one should have looked like
>>> pointer -> tag1 -> tag2 -> tag3 -> pointee? Basically, trying to
>>> understand if the final BTF represents the source-level order of tags
>>> or not?
>>
>> The comment is accurate. Given source like
>>      int tag1 tag2 tag3 *p;
>> the final type chain is
>>      p -> tag3 -> tag2 -> tag1 -> int
>>
>> basically it means
>>      - '*' applies to "int tag1 tag2 tag3"
>>      - tag3 applies to "int tag1 tag2"
>>      - tag2 applies to "int tag1"
>>      - tag1 applies to "int"
>>
>> This also makes final source code (format c) easier as
>> we can do
>>      emit for "tag3 -> tag2 -> tag1 -> int"
>>      emit '*'
>>
>> For 'tag3 -> tag2 -> tag1 -> int":
>>      emit for "tag2 -> tag1 -> int"
>>      emit tag3
>>
>> Eventually we can get the source code like
>>      int tag1 tag2 tag3 *p
>> and this matches the user/kernel code.
> 
> It would be great to add that as a comment somewhere here, it's very
> hard to make this inference just from the code.

Will add detailed comments in next pahole revision and will also
add them in the next kernel btf_type_tag patch set.

> 
>>
>>>
>>>> +       prev_tag = &tag->tag;
>>>> +       list_for_each_entry(annot, &tag->tags, node) {
>>>> +               annot_dtag = annot->tag.priv;
>>>> +               prev_tag->type = annot_dtag->small_id;
>>>> +               prev_tag = &annot->tag;
>>>> +       }
>>>> +       prev_tag->type = pointee_type;
>>>> +}
>>>> +
>>>
>>> [...]
>>>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-11-23  4:19 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-17 20:22 [PATCH dwarves 0/4] btf: support btf_type_tag attribute Yonghong Song
2021-11-17 20:22 ` [PATCH dwarves 1/4] libbpf: sync with latest libbpf repo Yonghong Song
2021-11-17 20:22 ` [PATCH dwarves 2/4] dutil: move DW_TAG_LLVM_annotation definition to dutil.h Yonghong Song
2021-11-17 20:22 ` [PATCH dwarves 3/4] dwarf_loader: support btf_type_tag attribute Yonghong Song
2021-11-18 13:00   ` Arnaldo Carvalho de Melo
2021-11-18 16:11     ` Yonghong Song
2021-11-23  1:52   ` Andrii Nakryiko
2021-11-23  3:32     ` Yonghong Song
2021-11-23  3:45       ` Andrii Nakryiko
2021-11-23  4:18         ` Yonghong Song
2021-11-17 20:22 ` [PATCH dwarves 4/4] btf_encoder: " Yonghong Song

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).