dwarves.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: bpf: Question about odd BPF verifier behaviour
       [not found]                         ` <CAEf4BzYO+Rgcfbr+QzJ-8BdQg-x-mC6c4bOhA+Z4cvu_1ObX+g@mail.gmail.com>
@ 2023-02-27 20:48                           ` Eduard Zingerman
  2023-02-28  2:55                             ` KP Singh
  0 siblings, 1 reply; 4+ messages in thread
From: Eduard Zingerman @ 2023-02-27 20:48 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: KP Singh, Matt Bobrowski, bpf, andrii, acme, dwarves

On Mon, 2023-02-27 at 11:31 -0800, Andrii Nakryiko wrote:
[...]
> > > I'd start with understanding what BTF and DWARF differences are
> > > causing the issue before trying to come up with the fix. For that we
> > > don't even need config or repro steps, it should be enough to share
> > > vmlinux with BTF and DWARF, and start from there.
> > > 
> > 
> > Yes, I suspect that there is some kind of unanticipated
> > anomaly for some DWARF encoding for some kind of objects,
> > just need to find the root for the diverging type hierarchies.
> > 
> > > But I'm sure Eduard is on top of this already (especially that he can
> > > repro the issue now).
> > 
> > I'm working on it, nothing to report yet, but I'm working on it.
> > 
> 
> Thanks, please keep us posted!

It is interesting how everything is interconnected. The patch for
pahole below happens to help. I prepared it last week while working on
new DWARF encoding scheme for btf_type_tag.

I still need to track down which "unspecified_type" entries caused the
issue in this particular case. Will post an update tomorrow.

Meanwhile, Matt, KP, could you please verify the patch on your side?
It is for the "next" branch of pahole.

---

From 09fac63ca08e25aea499f827283b07cc87a7daab Mon Sep 17 00:00:00 2001
From: Eduard Zingerman <eddyz87@gmail.com>
Date: Tue, 21 Feb 2023 19:23:00 +0200
Subject: [PATCH] dwarf_loader: Fix for BTF id drift caused by adding
 unspecified types

Recent changes to handle unspecified types (see [1]) cause BTF ID drift.

Specifically, the intent of commits [2], [3] and [4] is to render
references to unspecified types as void type.
However, as a consequence:
- in `die__process_unit()` call to `cu__add_tag()` allocates `small_id`
  for unspecified type tags and adds these tags to `cu->types_table`;
- `btf_encoder__encode_tag()` skips generation of BTF entries for
  `DW_TAG_unspecified_type` tags.

Such logic causes ID drift if unspecified type is not the last type
processed for compilation unit. `small_id` of each type following
unspecified type in the `cu->types_table` would have its BTF id off by -1.
Thus renders references established on recode phase invalid.

This commit reverts `unspecified_type` id/tag tracking, instead:
- `small_id` for unspecified type tags is set to 0, thus reference to
  unspecified type tag would render BTF id of a `void` on recode phase;
- unspecified type tags are not added to `cu->types_table`.

[1] https://lore.kernel.org/all/Y0R7uu3s%2FimnvPzM@kernel.org/
[2] bcc648a10cbc ("btf_encoder: Encode DW_TAG_unspecified_type returning routines as void")
[3] cffe5e1f75e1 ("core: Record if a CU has a DW_TAG_unspecified_type")
[4] 75e0fe28bb02 ("core: Add DW_TAG_unspecified_type to tag__is_tag_type() set")

Fixes: bcc648a10cbc ("btf_encoder: Encode DW_TAG_unspecified_type returning routines as void")
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 btf_encoder.c  |  8 --------
 dwarf_loader.c | 25 +++++++++++++++++++------
 dwarves.h      |  8 --------
 3 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index da776f4..07a9dc5 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -69,7 +69,6 @@ struct btf_encoder {
 	const char	  *filename;
 	struct elf_symtab *symtab;
 	uint32_t	  type_id_off;
-	uint32_t	  unspecified_type;
 	int		  saved_func_cnt;
 	bool		  has_index_type,
 			  need_index_type,
@@ -635,11 +634,6 @@ static int32_t btf_encoder__tag_type(struct btf_encoder *encoder, uint32_t tag_t
 	if (tag_type == 0)
 		return 0;
 
-	if (encoder->unspecified_type && tag_type == encoder->unspecified_type) {
-		// No provision for encoding this, turn it into void.
-		return 0;
-	}
-
 	return encoder->type_id_off + tag_type;
 }
 
@@ -1746,8 +1740,6 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
 
 	encoder->cu = cu;
 	encoder->type_id_off = btf__type_cnt(encoder->btf) - 1;
-	if (encoder->cu->unspecified_type.tag)
-		encoder->unspecified_type = encoder->cu->unspecified_type.type;
 
 	if (!encoder->has_index_type) {
 		/* cu__find_base_type_by_name() takes "type_id_t *id" */
diff --git a/dwarf_loader.c b/dwarf_loader.c
index 014e130..c37bd7b 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -2155,8 +2155,7 @@ static struct tag *__die__process_tag(Dwarf_Die *die, struct cu *cu,
 	case DW_TAG_atomic_type:
 		tag = die__create_new_tag(die, cu);		break;
 	case DW_TAG_unspecified_type:
-		cu->unspecified_type.tag =
-			tag = die__create_new_tag(die, cu);     break;
+		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:
@@ -2219,13 +2218,27 @@ static int die__process_unit(Dwarf_Die *die, struct cu *cu, struct conf_load *co
 			continue;
 		}
 
-		uint32_t id;
-		cu__add_tag(cu, tag, &id);
+		uint32_t id = 0;
+		/* There is no BTF representation for unspecified types.
+		 * Currently we want such types to be represented as `void`
+		 * (and thus skip BTF encoding).
+		 *
+		 * As BTF encoding is skipped, such types must not be added to type table,
+		 * otherwise an ID for a type would be allocated and we would be forced
+		 * to put something in BTF at this ID.
+		 * Thus avoid `cu__add_tag()` call for such types.
+		 *
+		 * On the other hand, there might be references to this type from other
+		 * tags, so `dwarf_cu__find_tag_by_ref()` must return something.
+		 * Thus call `cu__hash()` for such types.
+		 *
+		 * Note, that small_id of zero would be assigned to unspecified type entry.
+		 */
+		if (tag->tag != DW_TAG_unspecified_type)
+			cu__add_tag(cu, tag, &id);
 		cu__hash(cu, tag);
 		struct dwarf_tag *dtag = tag->priv;
 		dtag->small_id = id;
-		if (tag->tag == DW_TAG_unspecified_type)
-			cu->unspecified_type.type = id;
 	} while (dwarf_siblingof(die, die) == 0);
 
 	return 0;
diff --git a/dwarves.h b/dwarves.h
index 5074cf8..e92b2fd 100644
--- a/dwarves.h
+++ b/dwarves.h
@@ -236,10 +236,6 @@ struct debug_fmt_ops {
 
 #define ARCH_MAX_REGISTER_PARAMS	8
 
-/*
- * unspecified_type: If this CU has a DW_TAG_unspecified_type, as BTF doesn't have a representation for this
- * 		     and thus we need to check functions returning this to convert it to void.
- */
 struct cu {
 	struct list_head node;
 	struct list_head tags;
@@ -248,10 +244,6 @@ struct cu {
 	struct ptr_table functions_table;
 	struct ptr_table tags_table;
 	struct rb_root	 functions;
-	struct {
-		struct tag	 *tag;
-		uint32_t	 type;
-	} unspecified_type;
 	char		 *name;
 	char		 *filename;
 	void 		 *priv;
-- 
2.39.1


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

* Re: bpf: Question about odd BPF verifier behaviour
  2023-02-27 20:48                           ` bpf: Question about odd BPF verifier behaviour Eduard Zingerman
@ 2023-02-28  2:55                             ` KP Singh
  2023-02-28 18:08                               ` Eduard Zingerman
  0 siblings, 1 reply; 4+ messages in thread
From: KP Singh @ 2023-02-28  2:55 UTC (permalink / raw)
  To: Eduard Zingerman
  Cc: Andrii Nakryiko, Matt Bobrowski, bpf, andrii, acme, dwarves

On Mon, Feb 27, 2023 at 9:48 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Mon, 2023-02-27 at 11:31 -0800, Andrii Nakryiko wrote:
> [...]
> > > > I'd start with understanding what BTF and DWARF differences are
> > > > causing the issue before trying to come up with the fix. For that we
> > > > don't even need config or repro steps, it should be enough to share
> > > > vmlinux with BTF and DWARF, and start from there.
> > > >
> > >
> > > Yes, I suspect that there is some kind of unanticipated
> > > anomaly for some DWARF encoding for some kind of objects,
> > > just need to find the root for the diverging type hierarchies.
> > >
> > > > But I'm sure Eduard is on top of this already (especially that he can
> > > > repro the issue now).
> > >
> > > I'm working on it, nothing to report yet, but I'm working on it.
> > >
> >
> > Thanks, please keep us posted!
>
> It is interesting how everything is interconnected. The patch for
> pahole below happens to help. I prepared it last week while working on
> new DWARF encoding scheme for btf_type_tag.
>
> I still need to track down which "unspecified_type" entries caused the
> issue in this particular case. Will post an update tomorrow.
>
> Meanwhile, Matt, KP, could you please verify the patch on your side?
> It is for the "next" branch of pahole.
>
> ---
>
> From 09fac63ca08e25aea499f827283b07cc87a7daab Mon Sep 17 00:00:00 2001
> From: Eduard Zingerman <eddyz87@gmail.com>
> Date: Tue, 21 Feb 2023 19:23:00 +0200
> Subject: [PATCH] dwarf_loader: Fix for BTF id drift caused by adding
>  unspecified types
>
> Recent changes to handle unspecified types (see [1]) cause BTF ID drift.
>
> Specifically, the intent of commits [2], [3] and [4] is to render
> references to unspecified types as void type.
> However, as a consequence:
> - in `die__process_unit()` call to `cu__add_tag()` allocates `small_id`
>   for unspecified type tags and adds these tags to `cu->types_table`;
> - `btf_encoder__encode_tag()` skips generation of BTF entries for
>   `DW_TAG_unspecified_type` tags.
>
> Such logic causes ID drift if unspecified type is not the last type
> processed for compilation unit. `small_id` of each type following
> unspecified type in the `cu->types_table` would have its BTF id off by -1.
> Thus renders references established on recode phase invalid.
>
> This commit reverts `unspecified_type` id/tag tracking, instead:
> - `small_id` for unspecified type tags is set to 0, thus reference to
>   unspecified type tag would render BTF id of a `void` on recode phase;
> - unspecified type tags are not added to `cu->types_table`.
>
> [1] https://lore.kernel.org/all/Y0R7uu3s%2FimnvPzM@kernel.org/
> [2] bcc648a10cbc ("btf_encoder: Encode DW_TAG_unspecified_type returning routines as void")
> [3] cffe5e1f75e1 ("core: Record if a CU has a DW_TAG_unspecified_type")
> [4] 75e0fe28bb02 ("core: Add DW_TAG_unspecified_type to tag__is_tag_type() set")
>
> Fixes: bcc648a10cbc ("btf_encoder: Encode DW_TAG_unspecified_type returning routines as void")
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>

Tested-by: KP Singh <kpsingh@kernel.org>
Reported-by: Matt Bobrowski <mattbobrowski@google.com>

Thank you so much Eduard, this worked:

* No duplicate BTF ID warnings
* No 15 minute BTF ID generation
* Matt's reproducer loads successfully.

I had a sneaky suspicion that it was these unspecified types, which is
why my hacky patch which got unspecified types out of the way got
things to *mostly* work.

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

* Re: bpf: Question about odd BPF verifier behaviour
  2023-02-28  2:55                             ` KP Singh
@ 2023-02-28 18:08                               ` Eduard Zingerman
  2023-02-28 18:56                                 ` Andrii Nakryiko
  0 siblings, 1 reply; 4+ messages in thread
From: Eduard Zingerman @ 2023-02-28 18:08 UTC (permalink / raw)
  To: KP Singh; +Cc: Andrii Nakryiko, Matt Bobrowski, bpf, andrii, acme, dwarves

On Tue, 2023-02-28 at 03:55 +0100, KP Singh wrote:
> On Mon, Feb 27, 2023 at 9:48 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > 
> > On Mon, 2023-02-27 at 11:31 -0800, Andrii Nakryiko wrote:
> > [...]
> > > > > I'd start with understanding what BTF and DWARF differences are
> > > > > causing the issue before trying to come up with the fix. For that we
> > > > > don't even need config or repro steps, it should be enough to share
> > > > > vmlinux with BTF and DWARF, and start from there.
> > > > > 
> > > > 
> > > > Yes, I suspect that there is some kind of unanticipated
> > > > anomaly for some DWARF encoding for some kind of objects,
> > > > just need to find the root for the diverging type hierarchies.
> > > > 
> > > > > But I'm sure Eduard is on top of this already (especially that he can
> > > > > repro the issue now).
> > > > 
> > > > I'm working on it, nothing to report yet, but I'm working on it.
> > > > 
> > > 
> > > Thanks, please keep us posted!
> > 
> > It is interesting how everything is interconnected. The patch for
> > pahole below happens to help. I prepared it last week while working on
> > new DWARF encoding scheme for btf_type_tag.
> > 
> > I still need to track down which "unspecified_type" entries caused the
> > issue in this particular case. Will post an update tomorrow.
> > 
> > Meanwhile, Matt, KP, could you please verify the patch on your side?
> > It is for the "next" branch of pahole.
> > 
> > ---
> > 
> > From 09fac63ca08e25aea499f827283b07cc87a7daab Mon Sep 17 00:00:00 2001
> > From: Eduard Zingerman <eddyz87@gmail.com>
> > Date: Tue, 21 Feb 2023 19:23:00 +0200
> > Subject: [PATCH] dwarf_loader: Fix for BTF id drift caused by adding
> >  unspecified types
> > 
> > Recent changes to handle unspecified types (see [1]) cause BTF ID drift.
> > 
> > Specifically, the intent of commits [2], [3] and [4] is to render
> > references to unspecified types as void type.
> > However, as a consequence:
> > - in `die__process_unit()` call to `cu__add_tag()` allocates `small_id`
> >   for unspecified type tags and adds these tags to `cu->types_table`;
> > - `btf_encoder__encode_tag()` skips generation of BTF entries for
> >   `DW_TAG_unspecified_type` tags.
> > 
> > Such logic causes ID drift if unspecified type is not the last type
> > processed for compilation unit. `small_id` of each type following
> > unspecified type in the `cu->types_table` would have its BTF id off by -1.
> > Thus renders references established on recode phase invalid.
> > 
> > This commit reverts `unspecified_type` id/tag tracking, instead:
> > - `small_id` for unspecified type tags is set to 0, thus reference to
> >   unspecified type tag would render BTF id of a `void` on recode phase;
> > - unspecified type tags are not added to `cu->types_table`.
> > 
> > [1] https://lore.kernel.org/all/Y0R7uu3s%2FimnvPzM@kernel.org/
> > [2] bcc648a10cbc ("btf_encoder: Encode DW_TAG_unspecified_type returning routines as void")
> > [3] cffe5e1f75e1 ("core: Record if a CU has a DW_TAG_unspecified_type")
> > [4] 75e0fe28bb02 ("core: Add DW_TAG_unspecified_type to tag__is_tag_type() set")
> > 
> > Fixes: bcc648a10cbc ("btf_encoder: Encode DW_TAG_unspecified_type returning routines as void")
> > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> 
> Tested-by: KP Singh <kpsingh@kernel.org>
> Reported-by: Matt Bobrowski <mattbobrowski@google.com>
> 
> Thank you so much Eduard, this worked:
> 
> * No duplicate BTF ID warnings
> * No 15 minute BTF ID generation
> * Matt's reproducer loads successfully.
> 
> I had a sneaky suspicion that it was these unspecified types, which is
> why my hacky patch which got unspecified types out of the way got
> things to *mostly* work.

Hi KP,

Thanks a lot for testing!

I found the root cause for the bug (took me longer than I would like
to admit...). Using the patch below the reproducer from Matt works as
expected and warnings are gone.

Still, I think that my patch from yesterday is a more general
approach, as it correctly handles unspecified types that occur in
non-tail position, so I'll post that one.

Thanks,
Eduard

---

From daa53248e8a5087edbceaffe1fad51f9eb06e922 Mon Sep 17 00:00:00 2001
From: Eduard Zingerman <eddyz87@gmail.com>
Date: Tue, 28 Feb 2023 19:44:22 +0200
Subject: [PATCH] btf_encoder: reset encoder->unspecified_type for each CU

The field `encoder->unspecified_type` is set but not reset by function
`btf_encoder__encode_cu()` when processed `cu` has unspecified type.
The following sequence of events might occur when BTF encoding is
requested:
- CU with unspecified type is processed:
  - unspecified type id is 42
  - encoder->unspecified_type is set to 42
- CU without unspecified type is processed next using the same
  `encoder` object:
  - some `struct foo` has id 42 in this CU
  - the references to `struct foo` are set 0 by function
    `btf_encoder__tag_type()`.

This commit sets `encoder->unspecified_type` to 0 when CU does not
have unspecified type.

This issue was reported in thread [1].
See also [2].
[1] https://lore.kernel.org/bpf/Y%2FP1yxAuV6Wj3A0K@google.com/
[2] https://lore.kernel.org/all/Y0R7uu3s%2FimnvPzM@kernel.org/

Fixes: 52b25808e44a ("btf_encoder: Store type_id_off, unspecified type in encoder")
Reported-by: Matt Bobrowski <mattbobrowski@google.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 btf_encoder.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/btf_encoder.c b/btf_encoder.c
index da776f4..24f4c65 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -1748,6 +1748,8 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
 	encoder->type_id_off = btf__type_cnt(encoder->btf) - 1;
 	if (encoder->cu->unspecified_type.tag)
 		encoder->unspecified_type = encoder->cu->unspecified_type.type;
+	else
+		encoder->unspecified_type = 0;
 
 	if (!encoder->has_index_type) {
 		/* cu__find_base_type_by_name() takes "type_id_t *id" */
-- 
2.39.1

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

* Re: bpf: Question about odd BPF verifier behaviour
  2023-02-28 18:08                               ` Eduard Zingerman
@ 2023-02-28 18:56                                 ` Andrii Nakryiko
  0 siblings, 0 replies; 4+ messages in thread
From: Andrii Nakryiko @ 2023-02-28 18:56 UTC (permalink / raw)
  To: Eduard Zingerman; +Cc: KP Singh, Matt Bobrowski, bpf, andrii, acme, dwarves

On Tue, Feb 28, 2023 at 10:08 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Tue, 2023-02-28 at 03:55 +0100, KP Singh wrote:
> > On Mon, Feb 27, 2023 at 9:48 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > >
> > > On Mon, 2023-02-27 at 11:31 -0800, Andrii Nakryiko wrote:
> > > [...]
> > > > > > I'd start with understanding what BTF and DWARF differences are
> > > > > > causing the issue before trying to come up with the fix. For that we
> > > > > > don't even need config or repro steps, it should be enough to share
> > > > > > vmlinux with BTF and DWARF, and start from there.
> > > > > >
> > > > >
> > > > > Yes, I suspect that there is some kind of unanticipated
> > > > > anomaly for some DWARF encoding for some kind of objects,
> > > > > just need to find the root for the diverging type hierarchies.
> > > > >
> > > > > > But I'm sure Eduard is on top of this already (especially that he can
> > > > > > repro the issue now).
> > > > >
> > > > > I'm working on it, nothing to report yet, but I'm working on it.
> > > > >
> > > >
> > > > Thanks, please keep us posted!
> > >
> > > It is interesting how everything is interconnected. The patch for
> > > pahole below happens to help. I prepared it last week while working on
> > > new DWARF encoding scheme for btf_type_tag.
> > >
> > > I still need to track down which "unspecified_type" entries caused the
> > > issue in this particular case. Will post an update tomorrow.
> > >
> > > Meanwhile, Matt, KP, could you please verify the patch on your side?
> > > It is for the "next" branch of pahole.
> > >
> > > ---
> > >
> > > From 09fac63ca08e25aea499f827283b07cc87a7daab Mon Sep 17 00:00:00 2001
> > > From: Eduard Zingerman <eddyz87@gmail.com>
> > > Date: Tue, 21 Feb 2023 19:23:00 +0200
> > > Subject: [PATCH] dwarf_loader: Fix for BTF id drift caused by adding
> > >  unspecified types
> > >
> > > Recent changes to handle unspecified types (see [1]) cause BTF ID drift.
> > >
> > > Specifically, the intent of commits [2], [3] and [4] is to render
> > > references to unspecified types as void type.
> > > However, as a consequence:
> > > - in `die__process_unit()` call to `cu__add_tag()` allocates `small_id`
> > >   for unspecified type tags and adds these tags to `cu->types_table`;
> > > - `btf_encoder__encode_tag()` skips generation of BTF entries for
> > >   `DW_TAG_unspecified_type` tags.
> > >
> > > Such logic causes ID drift if unspecified type is not the last type
> > > processed for compilation unit. `small_id` of each type following
> > > unspecified type in the `cu->types_table` would have its BTF id off by -1.
> > > Thus renders references established on recode phase invalid.
> > >
> > > This commit reverts `unspecified_type` id/tag tracking, instead:
> > > - `small_id` for unspecified type tags is set to 0, thus reference to
> > >   unspecified type tag would render BTF id of a `void` on recode phase;
> > > - unspecified type tags are not added to `cu->types_table`.
> > >
> > > [1] https://lore.kernel.org/all/Y0R7uu3s%2FimnvPzM@kernel.org/
> > > [2] bcc648a10cbc ("btf_encoder: Encode DW_TAG_unspecified_type returning routines as void")
> > > [3] cffe5e1f75e1 ("core: Record if a CU has a DW_TAG_unspecified_type")
> > > [4] 75e0fe28bb02 ("core: Add DW_TAG_unspecified_type to tag__is_tag_type() set")
> > >
> > > Fixes: bcc648a10cbc ("btf_encoder: Encode DW_TAG_unspecified_type returning routines as void")
> > > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> >
> > Tested-by: KP Singh <kpsingh@kernel.org>
> > Reported-by: Matt Bobrowski <mattbobrowski@google.com>
> >
> > Thank you so much Eduard, this worked:
> >
> > * No duplicate BTF ID warnings
> > * No 15 minute BTF ID generation
> > * Matt's reproducer loads successfully.
> >
> > I had a sneaky suspicion that it was these unspecified types, which is
> > why my hacky patch which got unspecified types out of the way got
> > things to *mostly* work.
>
> Hi KP,
>
> Thanks a lot for testing!
>
> I found the root cause for the bug (took me longer than I would like
> to admit...). Using the patch below the reproducer from Matt works as
> expected and warnings are gone.
>
> Still, I think that my patch from yesterday is a more general
> approach, as it correctly handles unspecified types that occur in
> non-tail position, so I'll post that one.

I agree, please send it to dwarves@ as a proper patch. Thank you for
digging into this and fixing it quickly!

>
> Thanks,
> Eduard
>
> ---
>
> From daa53248e8a5087edbceaffe1fad51f9eb06e922 Mon Sep 17 00:00:00 2001
> From: Eduard Zingerman <eddyz87@gmail.com>
> Date: Tue, 28 Feb 2023 19:44:22 +0200
> Subject: [PATCH] btf_encoder: reset encoder->unspecified_type for each CU
>
> The field `encoder->unspecified_type` is set but not reset by function
> `btf_encoder__encode_cu()` when processed `cu` has unspecified type.
> The following sequence of events might occur when BTF encoding is
> requested:
> - CU with unspecified type is processed:
>   - unspecified type id is 42
>   - encoder->unspecified_type is set to 42
> - CU without unspecified type is processed next using the same
>   `encoder` object:
>   - some `struct foo` has id 42 in this CU
>   - the references to `struct foo` are set 0 by function
>     `btf_encoder__tag_type()`.
>
> This commit sets `encoder->unspecified_type` to 0 when CU does not
> have unspecified type.
>
> This issue was reported in thread [1].
> See also [2].
> [1] https://lore.kernel.org/bpf/Y%2FP1yxAuV6Wj3A0K@google.com/
> [2] https://lore.kernel.org/all/Y0R7uu3s%2FimnvPzM@kernel.org/
>
> Fixes: 52b25808e44a ("btf_encoder: Store type_id_off, unspecified type in encoder")
> Reported-by: Matt Bobrowski <mattbobrowski@google.com>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
>  btf_encoder.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/btf_encoder.c b/btf_encoder.c
> index da776f4..24f4c65 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -1748,6 +1748,8 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
>         encoder->type_id_off = btf__type_cnt(encoder->btf) - 1;
>         if (encoder->cu->unspecified_type.tag)
>                 encoder->unspecified_type = encoder->cu->unspecified_type.type;
> +       else
> +               encoder->unspecified_type = 0;
>
>         if (!encoder->has_index_type) {
>                 /* cu__find_base_type_by_name() takes "type_id_t *id" */
> --
> 2.39.1

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

end of thread, other threads:[~2023-02-28 18:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <Y/P1yxAuV6Wj3A0K@google.com>
     [not found] ` <e9f7c86ff50b556e08362ebc0b6ce6729a2db7e7.camel@gmail.com>
     [not found]   ` <Y/czygarUnMnDF9m@google.com>
     [not found]     ` <17833347f8cec0e44d856aeafbb1bbe203526237.camel@gmail.com>
     [not found]       ` <Y/hLsgSO3B+2g9iF@google.com>
     [not found]         ` <8f8f9d43d2f3f6d19c477c28d05527250cc6213d.camel@gmail.com>
     [not found]           ` <Y/p0ryf5PcKIs7uj@google.com>
     [not found]             ` <c4cda35711804ec26ebaeedc07d10d5d51901495.camel@gmail.com>
     [not found]               ` <693dffd9571073a47820653fd2de863010491454.camel@gmail.com>
     [not found]                 ` <CAEf4BzYaiD27y=Y85xhrj+VOvJY_5q1oVtg-4vYmFZFEpmW+nQ@mail.gmail.com>
     [not found]                   ` <CACYkzJ7tgbJqwUVxfGd4UKDUcOQjK8zsbEKUUjV79=xHQn1fFg@mail.gmail.com>
     [not found]                     ` <CAEf4BzZauF7V3pY1hgWgnJRN1F6eSkbTOTG3kM0c85uAX-vOfQ@mail.gmail.com>
     [not found]                       ` <9ea9b52fca1300265ce5639a2da809813edb774f.camel@gmail.com>
     [not found]                         ` <CAEf4BzYO+Rgcfbr+QzJ-8BdQg-x-mC6c4bOhA+Z4cvu_1ObX+g@mail.gmail.com>
2023-02-27 20:48                           ` bpf: Question about odd BPF verifier behaviour Eduard Zingerman
2023-02-28  2:55                             ` KP Singh
2023-02-28 18:08                               ` Eduard Zingerman
2023-02-28 18:56                                 ` Andrii Nakryiko

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).