Re: [PATCH dwarves v2] btf_encoder: sanitize non-regular int base type

From: Mark Wielaard <mark@klomp.org>
To: Yonghong Song <yhs@fb.com>, acme@kernel.org, dwarves@vger.kernel.org
Cc: bpf@vger.kernel.org, andriin@fb.com, ndesaulniers@google.com,
	sedat.dilek@gmail.com, Andrii Nakryiko <andrii@kernel.org>
Subject: Re: [PATCH dwarves v2] btf_encoder: sanitize non-regular int base type
Date: Sun, 07 Feb 2021 15:18:03 +0100	[thread overview]
Message-ID: <a02164334d0e991820eefa45e2df1a8b49f5537e.camel@klomp.org> (raw)
In-Reply-To: <20210207071726.3969978-1-yhs@fb.com>

Hi,

On Sat, 2021-02-06 at 23:17 -0800, Yonghong Song wrote:
> clang with dwarf5 may generate non-regular int base type,
> i.e., not a signed/unsigned char/short/int/longlong/__int128.
> Such base types are often used to describe
> how an actual parameter or variable is generated. For example,
> 
> 0x000015cf:   DW_TAG_base_type
>                 DW_AT_name      ("DW_ATE_unsigned_1")
>                 DW_AT_encoding  (DW_ATE_unsigned)
>                 DW_AT_byte_size (0x00)
> 
> 0x00010ed9:         DW_TAG_formal_parameter
>                       DW_AT_location    (DW_OP_lit0,
>                                          DW_OP_not,
>                                          DW_OP_convert (0x000015cf) "DW_ATE_unsigned_1",
>                                          DW_OP_convert (0x000015d4) "DW_ATE_unsigned_8",
>                                          DW_OP_stack_value)
>                       DW_AT_abstract_origin     (0x00013984 "branch")
> 
> What it does is with a literal "0", did a "not" operation, and the converted to
> one-bit unsigned int and then 8-bit unsigned int.

Thanks for tracking this down. Do you have any idea why the clang
compiler emits this? You might be right that it is intended to do what
you describe it does (but then it would simply encode an unsigned
constant 1 char in a very inefficient way). But as implemented it
doesn't seem to make any sense. What would DW_OP_convert of an zero
sized base type even mean (if it is intended as a 1 bit sized typed,
then why is there no DW_AT_bit_size)?

So I do think your patch makes sense. clang clearly is emitting
something bogus. And so some fixup is needed. But maybe we should at
least give a warning about it, otherwise it might never get fixed.

BTW. If these bogus base types are only emitted as part of a location
expression and not as part of an actual function or variable type
description, then why are we even trying to encode it as a BTF type? It
might be cheaper to just skip/drop it. But maybe the code setup makes
it hard to know whether or not such a (bogus) type is actually
referenced from a function or variable description?

Cheers,

Mark