[clang] [RFC][clang][BPF] Make trivial uninit var value to be 0 (PR #125601)

via cfe-commits cfe-commits at lists.llvm.org
Sat Feb 8 14:27:50 PST 2025


yonghong-song wrote:

> what veristat says before/after?

I tried with latest bpf-next and latest llvm trunk. The following is the difference
with/without this patch:
```
[~/work/bpf-next/tools/testing/selftests/bpf (master)]$ ./veristat *.bpf.o -o csv > old.csv
[~/work/bpf-next/tools/testing/selftests/bpf (master)]$ ./veristat *.bpf.o -o csv > new.csv
[~/work/bpf-next/tools/testing/selftests/bpf (master)]$ ./veristat -e file,prog,insns -C old.csv new.csv -f 'insns_diff!=0'
File                             Program                             Insns (A)  Insns (B)  Insns  (DIFF)
-------------------------------  ----------------------------------  ---------  ---------  -------------
cgroup_skb_sk_lookup_kern.bpf.o  ingress_lookup                             96        106  +10 (+10.42%)
cgroup_tcp_skb.bpf.o             client_egress                              94        104  +10 (+10.64%)
cgroup_tcp_skb.bpf.o             client_egress_srv                         115        125   +10 (+8.70%)
cgroup_tcp_skb.bpf.o             client_ingress                            115        125   +10 (+8.70%)
cgroup_tcp_skb.bpf.o             client_ingress_srv                         94        104  +10 (+10.64%)
cgroup_tcp_skb.bpf.o             server_egress                             115        125   +10 (+8.70%)
cgroup_tcp_skb.bpf.o             server_egress_srv                          94        104  +10 (+10.64%)
cgroup_tcp_skb.bpf.o             server_ingress                            129        139   +10 (+7.75%)
cgroup_tcp_skb.bpf.o             server_ingress_srv                        137        147   +10 (+7.30%)
verifier_array_access.bpf.o      an_array_with_a_constant_too_small          7          9   +2 (+28.57%)
[~/work/bpf-next/tools/testing/selftests/bpf (master)]$
```
For cgroup_tcp_skb.c, e.g.,
```
SEC("cgroup_skb/egress")
int server_egress_srv(struct __sk_buff *skb)
{
        struct tcphdr tcph;

        if (!needed_tcp_pkt(skb, &tcph))
                return 1;
...
```
the tcphdr is larger so it costs more with initializing to 0.
The same for cgroup_skb_sk_lookup_kern.c.

For verifier_array_access.c,
```
SEC("socket")
__description("invalid map access into an array using constant smaller than key_size")
__failure __msg("R0 invalid mem access 'map_value_or_null'")
unsigned int an_array_with_a_constant_too_small(void)
{       
        __u32 __attribute__((aligned(8))) key;
        struct test_val *val;

        /* Mark entire key as STACK_MISC */
        bpf_probe_read_user(&key, sizeof(key), NULL);
        ...
}
```
the 'key' is initialized.

> 
> re: selftests
> 
> there should be a flag to disable this, so the tests can remain as-is (with only Makefile change).

Will do.

> 
> There is also -ftrivial-auto-var-init-max-size= flag. What is the default? Looks like: char buf[64]; will be zero inited as well? 

The default is 0 (-ftrivial-auto-var-init-max-size=0) which means there is no limitation.
yes, char buf[64] and all buf[64] will be initialized to 0 if the compiler does not know whehter any buf[i](i = 0...63) is used or not.

>That will probably hurt performance/verification for cases like: char comm[16]; bpf_get_current_comm(comm, ...) ?
I checked. In almost all cases in bpf selftest progs, comm[16] is a field in a map, so it is not impacted.



https://github.com/llvm/llvm-project/pull/125601


More information about the cfe-commits mailing list