<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jun 14, 2021 at 11:23 PM Y Song <<a href="mailto:ys114321@gmail.com">ys114321@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, Jun 14, 2021 at 10:54 PM Andrii Nakryiko<br>
<<a href="mailto:andrii.nakryiko@gmail.com" target="_blank">andrii.nakryiko@gmail.com</a>> wrote:<br>
><br>
> On Mon, Jun 14, 2021 at 8:30 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
> ><br>
> ><br>
> ><br>
> > On Mon, Jun 14, 2021 at 7:52 PM Y Song <<a href="mailto:ys114321@gmail.com" target="_blank">ys114321@gmail.com</a>> wrote:<br>
> >><br>
> >> On Mon, Jun 14, 2021 at 6:44 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
> >> ><br>
> >> ><br>
> >> ><br>
> >> > On Mon, Jun 14, 2021 at 4:54 PM David Rector <<a href="mailto:davrecthreads@gmail.com" target="_blank">davrecthreads@gmail.com</a>> wrote:<br>
> >> >><br>
> >> >><br>
> >> >><br>
> >> >> On Jun 14, 2021, at 5:33 PM, Y Song via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br>
> >> >><br>
> >> >> On Mon, Jun 14, 2021 at 1:25 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
> >> >><br>
> >> >><br>
> >> >><br>
> >> >><br>
> >> >> On Mon, Jun 14, 2021 at 12:25 PM Y Song <<a href="mailto:ys114321@gmail.com" target="_blank">ys114321@gmail.com</a>> wrote:<br>
> >> >><br>
> >> >><br>
> >> >> On Fri, Jun 11, 2021 at 9:59 AM Alexei Starovoitov<br>
> >> >> <<a href="mailto:alexei.starovoitov@gmail.com" target="_blank">alexei.starovoitov@gmail.com</a>> wrote:<br>
> >> >><br>
> >> >><br>
> >> >> On Fri, Jun 11, 2021 at 07:17:32AM -0400, Aaron Ballman wrote:<br>
> >> >><br>
> >> >> On Thu, Jun 10, 2021 at 8:47 PM Alexei Starovoitov<br>
> >> >> <<a href="mailto:alexei.starovoitov@gmail.com" target="_blank">alexei.starovoitov@gmail.com</a>> wrote:<br>
> >> >><br>
> >> >><br>
> >> >> On Thu, Jun 10, 2021 at 12:42 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
> >> >><br>
> >> >><br>
> >> >><br>
> >> >><br>
> >> >> Any suggestions/preferences for the spelling, Aaron?<br>
> >> >><br>
> >> >><br>
> >> >> I don't know this domain particularly well, so takes these suggestions<br>
> >> >> with a giant grain of salt:<br>
> >> >><br>
> >> >> If the concept is specific to DWARF and you don't think it'll need to<br>
> >> >> extend into other debug formats, you could go with `dwarf_annotate`.<br>
> >> >> If it's not really a DWARF thing but is more about B[P|T]F, then<br>
> >> >> `btf_annotate` or `bpf_annotate` could work, but those may be a bit<br>
> >> >> mysterious to folks outside of the domain. If it's a generic debug<br>
> >> >> info concept, probably `debug_info_annotate` or something.<br>
> >> >><br>
> >> >><br>
> >> >><br>
> >> >> 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<br>
> >> >><br>
> >> >><br>
> >> >> A bit more bike shedding colors...<br>
> >> >><br>
> >> >> The __rcu and __user annations might be used by the clang itself eventually.<br>
> >> >> Currently the "sparse" tool is doing this analysis and warns users<br>
> >> >> when __rcu pointer is incorrectly accessed in the kernel C code.<br>
> >> >> If clang can do that directly that could be a huge selling point<br>
> >> >> for folks to switch from gcc to clang for kernel builds.<br>
> >> >> The front-end would treat such annotations as arbitrary string, but<br>
> >> >> special "building-linux-kernel-pass" would interpret the semantical context.<br>
> >> >><br>
> >> >><br>
> >> >> Are __rcu and __user annotations notionally distinct things from bpf<br>
> >> >> (and perhaps each other as well)? Distinct enough that it would make<br>
> >> >> sense to use a different attribute name for user source for each need?<br>
> >> >> I suspect the answer is yes given that the existing annotations have<br>
> >> >> their own names which are distinct, but I don't know this domain<br>
> >> >> enough to be sure.<br>
> >> >><br>
> >> >><br>
> >> >> __rcu and __user don't overlap. __rcu is not a single annotation though.<br>
> >> >> It's a combination of annotations in pointers, functions, macros.<br>
> >> >> Some functions have:<br>
> >> >> __acquires(rcu)<br>
> >> >> another function might have:<br>
> >> >> __acquires(rcu_bh)<br>
> >> >> There are several flavors of the RCU in the kernel.<br>
> >> >> So single __attribute__((rcu_annotate("foo"))) won't work even within RCU scope.<br>
> >> >> But if we do:<br>
> >> >> struct foo {<br>
> >> >> void * __attribute__((tag("ptr.rcu_bh")) ptr;<br>
> >> >> };<br>
> >> >> int bar(int) __attribute__((tag("acquires.rcu_bh")) { ... }<br>
> >> >> int baz(int) __attribute__((tag("releases.rcu_bh")) { ... }<br>
> >> >> int qux(int) __attribute__((tag("acquires.rcu_sched")) { ... }<br>
> >> >> ...<br>
> >> >> The clang pass can parse these strings and correlate one tag to another.<br>
> >> >> RCU flavors come and go, so clang cannot hard code the names.<br>
> >> >><br>
> >> >><br>
> >> >> Maybe we can name it as "bpf_tag" as it is a "tag" for "bpf" use case?<br>
> >> >><br>
> >> >> David, in one of your early emails, you mentioned:<br>
> >> >><br>
> >> >> ===<br>
> >> >> Arguably it can/could be a generic debug info or dwarf thing, but for<br>
> >> >> now we don't have any use for it other than to squirrel info along to<br>
> >> >> BTF/BPF so I'm on the fence about which prefix to use exactly<br>
> >> >> ===<br>
> >> >><br>
> >> >> and suggests since it might be used in the future for non-bpf things,<br>
> >> >> maybe the name could be a little more generic then bpf-specific.<br>
> >> >><br>
> >> >> Do you have any suggestions on what name to pick?<br>
> >> >><br>
> >> >><br>
> >> >><br>
> >> >> Nah, not especially. bpf_tag sounds OK-ish to me if it suits you.<br>
> >> >><br>
> >> >><br>
> >> >><br>
> >> >> The more generic the better IMO. And, the less the need to parse string literals the better.<br>
> >> >><br>
> >> >> Why not simply `__attribute__((debuginfo("arg1", "arg2", ...)))`, e.g.:<br>
> >> >><br>
> >> >> ```<br>
> >> >> #define BPF_TAG(...) __attribute__((debuginfo("bpf", __VA_ARGS__)))<br>
> >> >> struct foo {<br>
> >> >> void * BPF_TAG("ptr","rcu","bh") ptr;<br>
> >> >> };<br>
> >> >> #define BPF_RCU_TAG(PFX, ...) BPF(PFX, "rcu", __VA_ARGS__)<br>
> >> >> int bar(int) BPF_RCU_TAG("acquires","bh") { ... }<br>
> >> >> int baz(int) BPF_RCU_TAG("releases","bh") { ... }<br>
> >> >> int qux(int) BPF_RCU_TAG("acquires","sched") { ... }<br>
> >> >> ```<br>
> >> ><br>
> >> ><br>
> >> > Unless Paul & Adrian, etc chime in in agreement of a more general name, like 'debuginfo', I'm inclined to avoid that/go with something bpf specific until there's a broader use case/proposal, something we might be able to/want to encourage GCC to support too. Otherwise we're taking a pretty broad attribute name & choosing its behavior when we don't necessarily have a lot of leverage if GCC ends up using that name for something else.<br>
> >> ><br>
> >> > & as for separate strings - maybe, but I'm not sure what that'll look like in the resulting DWARF, what sort of form would you propose using to encode that? (same question below \/)<br>
> >> ><br>
> >> >><br>
> >> >><br>
> >> >> Sounds good. I will use "bpf_tag" as the starting point now.<br>
> >> >> Also, it is possible "bpf_tag" may appear multiple times for the same<br>
> >> >> function, declaration etc.<br>
> >> >><br>
> >> >> For example,<br>
> >> >> #define __bpf_tag(s) __attribute__((bpf_tag(s)))<br>
> >> >> int g __bpf_tag("str1") __bpf_tag("str2");<br>
> >> >> Let us say we introduced a LLVM vendor tag DWARF_AT_LLVM_bpf_tag.<br>
> >> >><br>
> >> >> How do you want the above to be represented in dwarf?<br>
> >> >><br>
> >> >> My current scheme is to put all bpf_tag's in a string, separated by ",".<br>
> >> >> This will make things simpler. So the final output will be<br>
> >> >> DWARF_AT_LLVM_bpf_tag "str1,str2"<br>
> >> >> I may need to do a discussion with the kernel folks to use a different<br>
> >> >> delimiter than ",", but we still represent all tags with ONE string.<br>
> >> >><br>
> >> >> But alternatively, it could be represented as a list of strings like<br>
> >> >> DWARF_AT_LLVM_bpf_tag<br>
> >> >> "str1"<br>
> >> >> "str2"<br>
> >> >> is similar to DWARF_AT_location.<br>
> >> ><br>
> >> ><br>
> >> > What DWARF form were you thinking of using for this? There isn't a built in form that provides encoding for multiple delimited/separated strings that I know of.<br>
> >><br>
> >> Actually I have not looked at the details on how to implement multiple<br>
> >> separated strings yet. Since you are mentioning there exists no such a<br>
> >> built-in form and the attribute is bpf specific, I will then just go<br>
> >> to one string only approach (e.g. "str1;str2" where ";" is the<br>
> >> delimiter). I just checked linux:include/linux/compiler_*.h, it is<br>
> >> possible "," may appear in some attributes, so I will use ";" as the<br>
> >> delimiter. Thanks for the clarification!<br>
> ><br>
> ><br>
> > Do you need to support multiple distinct __attribute__((XXX("stuff"))) on one entity? If so, maybe it's worth considering how to encode them separately, rather than having the frontend have to concatenate them together?<br>
<br>
Typically linux kernel is to use macros to represent attributes, e.g.,<br>
in linux/include/linux/compiler_types.h, we have<br>
# define __kernel __attribute__((address_space(0)))<br>
# define __user __attribute__((noderef, address_space(__user)))<br>
# define __iomem __attribute__((noderef, address_space(__iomem)))<br>
# define __percpu __attribute__((noderef, address_space(__percpu)))<br>
# define __rcu __attribute__((noderef, address_space(__rcu)))<br>
<br>
drivers/scsi/arcmsr/arcmsr_hba.c: struct MessageUnit_A __iomem<br>
*reg = acb->pmuA;<br>
drivers/scsi/arm/acornscsi.h: void __iomem *base;<br>
/* memc base address */<br>
...<br>
<br>
As you can see, the convention is to define attributes with some<br>
easy-to-understand macros.<br>
<br>
For bpf_tag attributes, most use cases will be a bunch of "# define"<br>
for a single property, e.g.,<br>
#define __property1 __attribute__((bpf_tag("str1")))<br>
#define __property2 __attribute__((bpf_tag("str2")))<br>
#define __property3(str) __attribute__((bpf_tag("info " # str)))<br>
int var1 __property1;<br>
int var2 __property2;<br>
int var3 __property1 __property2;<br>
int var4 __property1 __property3("lock");<br>
<br>
So yes, we do want to support multiple bpf_tag attributes on one<br>
entity. Looks like the easiest way<br>
for llvm internals and dwarf output is having one consolidated string<br>
(see below too).<br>
<br>
> ><br>
> > One option would be to support multiple of the same attribute on the DIE in question - though that's probably still difficult to encode in the LLVM IR metadata (we don't have any repeating fields in the LLVM IR debug info metadata) - which, maybe comes back to the idea of having the frontend concatenate all the attributes together with some separator like ";".<br>
><br>
><br>
> I'd prefer to not have to parse strings and rather have multiple<br>
> attributes individual "tag" attributes, but seems like DWARFv5<br>
> reference explicitly prohibits multiple tags of the same type under<br>
> single DIE:<br>
><br>
> 2.2 Attribute Types<br>
> Each attribute value is characterized by an attribute name. No more than one<br>
> attribute with a given name may appear in any debugging information entry.<br>
> There are no limitations on the ordering of attributes within a debugging<br>
> information entry.<br></blockquote><div><br></div><div>Ah, my mistake then - yeah, then we'd have to go to a custom form (super expensive for a bunch of reasons) or a whole custom tag (possible - tags can be repeated) which might be overkill, more complicated in the LLVM IR metadata, etc.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Thanks for the pointer. I also checked dwarf 5 spec<br>
7.5.5 Classes and Forms<br>
and indeed, I didn't find a FORM to represent a list of strings.<br>
So it looks like one consolidated string for all bpf_tag attributes might be<br>
the easiest way to go.<br></blockquote><div><br>Yeah, I think that's probably the simplest thing to do as a first pass - single aggregate value (single string, using whatever format/separators seem suitable) which goes on the DIWhatever things it needs to and gets lowered to a custom bpf-specific attribute in the resulting DWARF. </div></div></div>