[PATCH bpf] libbpf: handle GCC noreturn-turned-volatile quirk

[PATCH bpf] libbpf: handle GCC noreturn-turned-volatile quirk

[Andrii Nakryiko]

Handle a GCC quirk of emitting extra volatile modifier in DWARF (and
subsequently preserved in BTF by pahole) for function pointers marked as
__attribute__((noreturn)). This was the way to mark such functions before GCC
2.5 added noreturn attribute. Drop such func_proto modifiers, similarly to how
it's done for array (also to handle GCC quirk/bug).

Such volatile attribute is emitted by GCC only, so existing selftests can't
express such test. Simple repro is like this (compiled with GCC + BTF
generated by pahole):

  struct my_struct {
      void __attribute__((noreturn)) (*fn)(int);
  };
  struct my_struct a;

Without this fix, output will be:

struct my_struct {
    voidvolatile  (*fn)(int);
};

With the fix:

struct my_struct {
    void (*fn)(int);
};

Reported-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Fixes: 351131b51c7a ("libbpf: add btf_dump API for BTF-to-C conversion")
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/btf_dump.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
index de07e559a11d..bbb430317260 100644
--- a/tools/lib/bpf/btf_dump.c
+++ b/tools/lib/bpf/btf_dump.c
@@ -1137,6 +1137,20 @@ static void btf_dump_emit_mods(struct btf_dump *d, struct id_stack *decl_stack)
 	}
 }
 
+static void btf_dump_drop_mods(struct btf_dump *d, struct id_stack *decl_stack)
+{
+	const struct btf_type *t;
+	__u32 id;
+
+	while (decl_stack->cnt) {
+		id = decl_stack->ids[decl_stack->cnt - 1];
+		t = btf__type_by_id(d->btf, id);
+		if (!btf_is_mod(t))
+			return;
+		decl_stack->cnt--;
+	}
+}
+
 static void btf_dump_emit_name(const struct btf_dump *d,
 			       const char *name, bool last_was_ptr)
 {
@@ -1235,14 +1249,7 @@ static void btf_dump_emit_type_chain(struct btf_dump *d,
 			 * a const/volatile modifier for array, so we are
 			 * going to silently skip them here.
 			 */
-			while (decls->cnt) {
-				next_id = decls->ids[decls->cnt - 1];
-				next_t = btf__type_by_id(d->btf, next_id);
-				if (btf_is_mod(next_t))
-					decls->cnt--;
-				else
-					break;
-			}
+			btf_dump_drop_mods(d, decls);
 
 			if (decls->cnt == 0) {
 				btf_dump_emit_name(d, fname, last_was_ptr);
@@ -1270,7 +1277,15 @@ static void btf_dump_emit_type_chain(struct btf_dump *d,
 			__u16 vlen = btf_vlen(t);
 			int i;
 
-			btf_dump_emit_mods(d, decls);
+			/*
+			 * GCC emits extra volatile qualifier for
+			 * __attribute__((noreturn)) function pointers. Clang
+			 * doesn't do it. It's a GCC quirk for backwards
+			 * compatibility with code written for GCC <2.5. So,
+			 * similarly to extra qualifiers for array, just drop
+			 * them, instead of handling them.
+			 */
+			btf_dump_drop_mods(d, decls);
 			if (decls->cnt) {
 				btf_dump_printf(d, " (");
 				btf_dump_emit_type_chain(d, decls, fname, lvl);
-- 
2.24.1

Re: [PATCH bpf] libbpf: handle GCC noreturn-turned-volatile quirk

[Jean-Philippe Brucker]

On Tue, Jun 09, 2020 at 10:23:35PM -0700, Andrii Nakryiko wrote:
> Handle a GCC quirk of emitting extra volatile modifier in DWARF (and
> subsequently preserved in BTF by pahole) for function pointers marked as
> __attribute__((noreturn)). This was the way to mark such functions before GCC
> 2.5 added noreturn attribute. Drop such func_proto modifiers, similarly to how
> it's done for array (also to handle GCC quirk/bug).
> 
> Such volatile attribute is emitted by GCC only, so existing selftests can't
> express such test. Simple repro is like this (compiled with GCC + BTF
> generated by pahole):
> 
>   struct my_struct {
>       void __attribute__((noreturn)) (*fn)(int);
>   };
>   struct my_struct a;
> 
> Without this fix, output will be:
> 
> struct my_struct {
>     voidvolatile  (*fn)(int);
> };
> 
> With the fix:
> 
> struct my_struct {
>     void (*fn)(int);
> };
> 
> Reported-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> Fixes: 351131b51c7a ("libbpf: add btf_dump API for BTF-to-C conversion")
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Thanks, this fixes the issue I was seeing with the arm64 vmlinux

Tested-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

Re: [PATCH bpf] libbpf: handle GCC noreturn-turned-volatile quirk

[Daniel Borkmann]

On 6/10/20 7:23 AM, Andrii Nakryiko wrote:
> Handle a GCC quirk of emitting extra volatile modifier in DWARF (and
> subsequently preserved in BTF by pahole) for function pointers marked as
> __attribute__((noreturn)). This was the way to mark such functions before GCC
> 2.5 added noreturn attribute. Drop such func_proto modifiers, similarly to how
> it's done for array (also to handle GCC quirk/bug).
> 
> Such volatile attribute is emitted by GCC only, so existing selftests can't
> express such test. Simple repro is like this (compiled with GCC + BTF
> generated by pahole):
> 
>    struct my_struct {
>        void __attribute__((noreturn)) (*fn)(int);
>    };
>    struct my_struct a;
> 
> Without this fix, output will be:
> 
> struct my_struct {
>      voidvolatile  (*fn)(int);
> };
> 
> With the fix:
> 
> struct my_struct {
>      void (*fn)(int);
> };
> 
> Reported-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> Fixes: 351131b51c7a ("libbpf: add btf_dump API for BTF-to-C conversion")
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Applied, thanks!