llvm dev - Jun 2021 - put "str" in __attribute__((annotate("str"))) to dwarf

On Thu, Jun 10, 2021 at 8:47 PM Alexei Starovoitov
<alexei.starovoitov at gmail.com> wrote:
>
> On Thu, Jun 10, 2021 at 12:42 PM David Blaikie <dblaikie at
gmail.com> wrote:
> >
> >> >
> >> >
> >> > Any suggestions/preferences for the spelling, Aaron?
> >>
> >> I don't know this domain particularly well, so takes these
suggestions
> >> with a giant grain of salt:
> >>
> >> If the concept is specific to DWARF and you don't think
it'll need to
> >> extend into other debug formats, you could go with
`dwarf_annotate`.
> >> If it's not really a DWARF thing but is more about B[P|T]F,
then
> >> `btf_annotate`  or `bpf_annotate` could work, but those may be a
bit
> >> mysterious to folks outside of the domain. If it's a generic
debug
> >> info concept, probably `debug_info_annotate` or something.
> >
> >
> > Arguably it can/could be a generic debug info or dwarf thing, but for
now we don't have any use for it other than to squirrel info along to
BTF/BPF so I'm on the fence about which prefix to use exactly
> >
>
> A bit more bike shedding colors...
>
> The __rcu and __user annations might be used by the clang itself
eventually.
> Currently the "sparse" tool is doing this analysis and warns
users
> when __rcu pointer is incorrectly accessed in the kernel C code.
> If clang can do that directly that could be a huge selling point
> for folks to switch from gcc to clang for kernel builds.
> The front-end would treat such annotations as arbitrary string, but
> special "building-linux-kernel-pass" would interpret the
semantical context.
Are __rcu and __user annotations notionally distinct things from bpf
(and perhaps each other as well)? Distinct enough that it would make
sense to use a different attribute name for user source for each need?
I suspect the answer is yes given that the existing annotations have
their own names which are distinct, but I don't know this domain
enough to be sure.
> Considering above the dwarf_annotate, btf_annotate, debug_info_annotate
> names don't fit that well. The accuracy of the annotations is important
> unlike debug info that can be dropped on a whim of some optimization pass.
>
> bpf_annotate wouldn't fit either, since the kernel might use that
> without any bpf bits.
>
> kernel_annotate might sound like it's not applicable to user space.
>
> How about __attribute__((note("str"))) or
__attribute__((tag("str"))) ?
I don't think we'd want to use such generic terms for this
functionality. e.g., a note attribute could complement the existing
diagnose_if attribute for providing diagnostic notes, and a tag
attribute could be specific to tag types (struct, union, enum), etc.

I'm skeptical of using the same attribute for all these purposes
because that usually leads to needing some sort of uniqueness to the
string argument so it can be properly distinguished. e.g.,
__attribute__((note("bpf.instruction"))) vs
__attribute__((note("rcu.instruction")), which is harder for tooling
to handle if it feels the need to inspect the string literal (such as
for diagnostic purposes).

~Aaron

On Fri, Jun 11, 2021 at 07:17:32AM -0400, Aaron Ballman
wrote:
> On Thu, Jun 10, 2021 at 8:47 PM Alexei Starovoitov
> <alexei.starovoitov at gmail.com> wrote:
> >
> > On Thu, Jun 10, 2021 at 12:42 PM David Blaikie <dblaikie at
gmail.com> wrote:
> > >
> > >> >
> > >> >
> > >> > Any suggestions/preferences for the spelling, Aaron?
> > >>
> > >> I don't know this domain particularly well, so takes
these suggestions
> > >> with a giant grain of salt:
> > >>
> > >> If the concept is specific to DWARF and you don't think
it'll need to
> > >> extend into other debug formats, you could go with
`dwarf_annotate`.
> > >> If it's not really a DWARF thing but is more about
B[P|T]F, then
> > >> `btf_annotate`  or `bpf_annotate` could work, but those may
be a bit
> > >> mysterious to folks outside of the domain. If it's a
generic debug
> > >> info concept, probably `debug_info_annotate` or something.
> > >
> > >
> > > Arguably it can/could be a generic debug info or dwarf thing, but
for now we don't have any use for it other than to squirrel info along to
BTF/BPF so I'm on the fence about which prefix to use exactly
> > >
> >
> > A bit more bike shedding colors...
> >
> > The __rcu and __user annations might be used by the clang itself
eventually.
> > Currently the "sparse" tool is doing this analysis and warns
users
> > when __rcu pointer is incorrectly accessed in the kernel C code.
> > If clang can do that directly that could be a huge selling point
> > for folks to switch from gcc to clang for kernel builds.
> > The front-end would treat such annotations as arbitrary string, but
> > special "building-linux-kernel-pass" would interpret the
semantical context.
> 
> Are __rcu and __user annotations notionally distinct things from bpf
> (and perhaps each other as well)? Distinct enough that it would make
> sense to use a different attribute name for user source for each need?
> I suspect the answer is yes given that the existing annotations have
> their own names which are distinct, but I don't know this domain
> enough to be sure.
__rcu and __user don't overlap. __rcu is not a single annotation though.
It's a combination of annotations in pointers, functions, macros.
Some functions have:
__acquires(rcu)
another function might have:
__acquires(rcu_bh)
There are several flavors of the RCU in the kernel.
So single __attribute__((rcu_annotate("foo"))) won't work even
within RCU scope.
But if we do:
struct foo {
  void * __attribute__((tag("ptr.rcu_bh")) ptr;
};
int bar(int) __attribute__((tag("acquires.rcu_bh")) { ... }
int baz(int) __attribute__((tag("releases.rcu_bh")) { ... }
int qux(int) __attribute__((tag("acquires.rcu_sched")) { ... }
...
The clang pass can parse these strings and correlate one tag to another.
RCU flavors come and go, so clang cannot hard code the names.