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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2/7/21 6:18 AM, Mark Wielaard wrote:
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

No, I don't know.

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

We need to report back to llvm community about this instance.
DW_AT_byte_size = 0 only for DW_ATE_unsigned_1 does not sound right.
DW_AT_bit_size might be needed as you suggested.


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.

In pahole, to deal with dwarf weirdness, we have several other places
for workaround without warning. I think it is okay not to have
warning here as users cannot do anything about it. Also we have
this special int type with name __SANITIZED_FAKE_INT__ which will
signal the issue still exists if looking at BTF or generated vmlinux.h.


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

Let us still encode it as a BTF type as an evidence that we do
changed some types. After deduplication, we will only have ONE __SANITIZED_FAKE_INT__ type. I think leave this one type in BTF is okay.

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?

I guess it is possible that we do a preprocessing to check whether
any types BTF intends to emit (var/func definition, etc.) referencing
such types or not. This will involves overhead for every CU most of which does not have this issue. And this should not really happen given a sane dwarf.

With __SANITIZED_FAKE_INT__ as the type name, if something e.g., a variable definition use this type, you will have
   __SANITIZED_FAKE_INT__ a;
in skeleton, or have
   struct {
      __SANITIZED_FAKE_INT__ member;
      ...
   };
in vmlinux.h,
we should be able to catch such an error easily.


Cheers,

Mark




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux