bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Menglong Dong' <menglong8.dong@gmail.com>
Cc: Yonghong Song <yhs@meta.com>,
	"alexei.starovoitov@gmail.com" <alexei.starovoitov@gmail.com>,
	"ast@kernel.org" <ast@kernel.org>,
	"daniel@iogearbox.net" <daniel@iogearbox.net>,
	"andrii@kernel.org" <andrii@kernel.org>,
	"martin.lau@linux.dev" <martin.lau@linux.dev>,
	"song@kernel.org" <song@kernel.org>, "yhs@fb.com" <yhs@fb.com>,
	"john.fastabend@gmail.com" <john.fastabend@gmail.com>,
	"kpsingh@kernel.org" <kpsingh@kernel.org>,
	"sdf@google.com" <sdf@google.com>,
	"haoluo@google.com" <haoluo@google.com>,
	"jolsa@kernel.org" <jolsa@kernel.org>,
	"benbjiang@tencent.com" <benbjiang@tencent.com>,
	"bpf@vger.kernel.org" <bpf@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Menglong Dong <imagedong@tencent.com>
Subject: RE: [PATCH bpf-next v5 2/3] bpf, x86: allow function arguments up to 12 for TRACING
Date: Thu, 22 Jun 2023 14:18:59 +0000	[thread overview]
Message-ID: <84050129b8ce4db9b4579be0fc022723@AcuMS.aculab.com> (raw)
In-Reply-To: <CADxym3bY5EcZhuJG=x5s7kH+BS93ySAyvV8yZ7yYoXf7HCsZVw@mail.gmail.com>

...
> > Is that right for 86-64?
> >
> > IIRC arguments always take (at least) 64bits.
> > For any 32bit argument (register or stack) the high bits are undefined.
> > (Maybe in kernel they are always zero?
> > From 32bit userspace they are definitely random.)
> >
> 
> Hello,
> 
> According to my testing, the compiler will always
> pass the arguments on 8-byte size with "push" insn
> if the count of the arguments that need to be passed
> on stack more than 1 and the size of the argument
> doesn't exceed 8-byte. In this case, there won't be
> garbage. For example, the high 4-byte will be made 0
> if the size of the argument is 4-byte, as the "push" insn
> will copy the argument from regs or imm into stack
> in 8-byte.

You have to know whether a value is expected to be 4 or 8
bytes - a negative 32bit value is zero extended so can't
be treated as a 64bit value.

That is even true for values passed in registers.

There is also a common problem with values passed in registers
to system calls by 32bit code (maybe bpf is tracing these).
In this case the high 32 bits of the register are random.
They don't get zerod in 32bit mode.

> If the count of the arguments on-stack is 1 and its size
> doesn't exceed 4-byte, some compiler, like clang, may
> not use the "push" insn. Instead, it allocates 4 bytes in the
> stack, and copies the arguments from regs or imm into
> stack in 4-byte. This is the case we deal with here.

If the compiler sometimes writes a 4 byte (or smaller) value
to pre-allocated stack then it is always allowed to do that.
So the high bytes of the stack slot that contains a 32bit
argument might always be junk.
The count of on-stack arguments isn't relevant.

> I'm not sure if I understand you correctly. Do you mean
> that there will be garbage values for 32bit args?

I'm pretty sure that the function call ABI doesn't require the
caller set the high bits of sub-64bit arguments.
The fact that they are often written with a push instruction
that zeros the high bytes isn't really relevant.

> > I think the called code is also responsible form masking 8 and 16bit
> > values (in reality char/short args and return values just add code
> > bloat).
> >
> > A 128bit value is either passed in two registers or two stack
> > slots. If the last register is skipped it will be used for the
> > next argument.
> >
> 
> Yeah, this point is considered in save_args(). Once
> this happen, the count of stack slots should more
> then 1, and the arguments on-stack will be stored with
> "push" insn in 8-byte. Therefore, there shouldn't be garbage
> values in this case?
> 
> Do I miss something?

The register/stack for these two calls is the same:
	foo(1, 2, 3, 4, 5, 6, (int128_t)7);
	bar(1, 2, 3, 4, 5, (int128_t)7, 6);

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

  reply	other threads:[~2023-06-22 14:19 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-13  2:52 [PATCH bpf-next v5 0/3] bpf, x86: allow function arguments up to 12 for TRACING menglong8.dong
2023-06-13  2:52 ` [PATCH bpf-next v5 1/3] bpf, x86: clean garbage values when store args from regs into stack menglong8.dong
2023-06-18 22:52   ` Yonghong Song
2023-06-19  2:17     ` Menglong Dong
2023-06-13  2:52 ` [PATCH bpf-next v5 2/3] bpf, x86: allow function arguments up to 12 for TRACING menglong8.dong
2023-06-15  4:00   ` Menglong Dong
2023-06-18 23:10   ` Yonghong Song
2023-06-19  2:31     ` Menglong Dong
2023-06-22  9:06     ` David Laight
2023-06-22 13:05       ` Menglong Dong
2023-06-22 14:18         ` David Laight [this message]
2023-06-23 13:08           ` Menglong Dong
2023-06-22 16:26       ` Yonghong Song
2023-06-13  2:52 ` [PATCH bpf-next v5 3/3] selftests/bpf: add testcase for TRACING with 6+ arguments menglong8.dong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=84050129b8ce4db9b4579be0fc022723@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=benbjiang@tencent.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=imagedong@tencent.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=menglong8.dong@gmail.com \
    --cc=sdf@google.com \
    --cc=song@kernel.org \
    --cc=yhs@fb.com \
    --cc=yhs@meta.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).