BPF Archive on lore.kernel.org
 help / color / Atom feed
* CO-RE: Weird immediate for bpf_core_field_exists
@ 2021-04-29 10:29 Lorenz Bauer
  2021-04-29 15:31 ` Yonghong Song
  0 siblings, 1 reply; 2+ messages in thread
From: Lorenz Bauer @ 2021-04-29 10:29 UTC (permalink / raw)
  To: Andrii Nakryiko, Yonghong Song; +Cc: bpf

Hi Andrii and Yonghong,

This is probably a case of me holding it wrong, but I figured I would
share this nonetheless. Given the following C:

struct s {
    int _1;
    char _2;
};

typedef struct s s_t;

union u {
    int *_1;
    char *_2;
};

__section("socket_filter/fields") int fields() {
    struct t {
        union {
            s_t s[10];
        };
        struct {
            union u u;
        };
    } bar;
    return bpf_core_field_exists((&bar)[1]);
}

clang-12 generates the following instructions:

0000000000000000 <fields>:
;     return bpf_core_field_exists((&bar)[1]);
       0:    b7 00 00 00 58 00 00 00    r0 = 88
       1:    95 00 00 00 00 00 00 00    exit

The weird bit is that the immediate for instruction 0 isn't 1 but 88.
Coincidentally sizeof(bar) is also 88 bytes.

$ clang-12 --version
Ubuntu clang version
12.0.0-++20210126113614+510b3d4b3e02-1~exp1~20210126104320.178

I've tried clang-13 as well, same result.

Best,
Lorenz

-- 
Lorenz Bauer  |  Systems Engineer
6th Floor, County Hall/The Riverside Building, SE1 7PB, UK

www.cloudflare.com

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

* Re: CO-RE: Weird immediate for bpf_core_field_exists
  2021-04-29 10:29 CO-RE: Weird immediate for bpf_core_field_exists Lorenz Bauer
@ 2021-04-29 15:31 ` Yonghong Song
  0 siblings, 0 replies; 2+ messages in thread
From: Yonghong Song @ 2021-04-29 15:31 UTC (permalink / raw)
  To: Lorenz Bauer, Andrii Nakryiko; +Cc: bpf



On 4/29/21 3:29 AM, Lorenz Bauer wrote:
> Hi Andrii and Yonghong,
> 
> This is probably a case of me holding it wrong, but I figured I would
> share this nonetheless. Given the following C:
> 
> struct s {
>      int _1;
>      char _2;
> };
> 
> typedef struct s s_t;
> 
> union u {
>      int *_1;
>      char *_2;
> };
> 
> __section("socket_filter/fields") int fields() {
>      struct t {
>          union {
>              s_t s[10];
>          };
>          struct {
>              union u u;
>          };
>      } bar;
>      return bpf_core_field_exists((&bar)[1]);
> }
> 
> clang-12 generates the following instructions:
> 
> 0000000000000000 <fields>:
> ;     return bpf_core_field_exists((&bar)[1]);
>         0:    b7 00 00 00 58 00 00 00    r0 = 88
>         1:    95 00 00 00 00 00 00 00    exit
> 
> The weird bit is that the immediate for instruction 0 isn't 1 but 88.
> Coincidentally sizeof(bar) is also 88 bytes.

Thanks for the reporting. This is a compiler issue which didn't handle
invalid case nicely. The following is an explanation.

After macro expansion,
   bpf_core_field_exists((&bar)[1]);
is actually
   __builtin_preserve_field_info((&bar)[1], BPF_FIELD_EXISTS);

For BPF_FIELD_EXISTS, the first argument should be a field access.
But in the above, we got an array access, (&bar)[1], internally,
the compiler keeps track of the offset from the base address, and
this offset is used for most other builtin kinds like
FIELD_BYPE_OFFSET etc.

For relative to &bar, (&bar)[1] has an offset 88. In your
particular case, the code never went inside the routine to
generate correct "patch_imm" (i.e., r0 = <patch_imm> in the above)
since we didn't get a field access. So we got a wrong result.

I will fix this in the compiler by issuing an error so people
can correct their usage. Thanks for reporting!

> 
> $ clang-12 --version
> Ubuntu clang version
> 12.0.0-++20210126113614+510b3d4b3e02-1~exp1~20210126104320.178
> 
> I've tried clang-13 as well, same result.
> 
> Best,
> Lorenz
> 

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

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-29 10:29 CO-RE: Weird immediate for bpf_core_field_exists Lorenz Bauer
2021-04-29 15:31 ` Yonghong Song

BPF Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/bpf/0 bpf/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 bpf bpf/ https://lore.kernel.org/bpf \
		bpf@vger.kernel.org
	public-inbox-index bpf

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.bpf


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git