| Message ID | 20241011200005.1422103-1-ezulian@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | tools/resolve_btfids: Fix 'variable' may be used uninitialized warnings | expand |
The parse-options change was sent before as https://lore.kernel.org/all/20240731085217.94928-1-michael.weiss@aisec.fraunhofer.de/ but seems to have fallen through the cracks.
Hi Sam, thank you for pointing it out. On Sat, Oct 12, 2024 at 05:14:29AM +0100, Sam James wrote: > The parse-options change was sent before as > https://lore.kernel.org/all/20240731085217.94928-1-michael.weiss@aisec.fraunhofer.de/ Sorry, I missed Michael's patch. My suggestion is to initialize 'o' to NULL instead. An illegal dereferencing (if any) would then be evident. > but seems to have fallen through the cracks. > > Would it be better to revert this part and wait a bit for Michael's patch to be merged, please let me know. Thank you, Eder
Eder Zulian <ezulian@redhat.com> writes: > Hi Sam, thank you for pointing it out. > > On Sat, Oct 12, 2024 at 05:14:29AM +0100, Sam James wrote: >> The parse-options change was sent before as >> https://lore.kernel.org/all/20240731085217.94928-1-michael.weiss@aisec.fraunhofer.de/ > > Sorry, I missed Michael's patch. > My suggestion is to initialize 'o' to NULL instead. An illegal dereferencing > (if any) would then be evident. Yeah, I was wondering the same. > >> but seems to have fallen through the cracks. >> >> > Would it be better to revert this part and wait a bit for Michael's patch to > be merged, please let me know. I'll defer to the maintainers. > > Thank you, > Eder
On 10/11/24 22:00, Eder Zulian wrote: > - tools/bpf/resolve_btfids/main.c: Initialize 'set' and 'set8' pointers > to NULL in to fix compiler warnings. > > - tools/lib/bpf/btf_dump.c: Initialize 'new_off' and 'pad_bits' to 0 and > 'pad_type' to NULL to prevent compiler warnings. > > - tools/lib/subcmd/parse-options.c: Initiazlide pointer 'o' to NULL Typo: "Initiazlide" -> "Initialize" > avoiding a compiler warning. I think that the three changes should be split into three patches so that we don't have one patch touching three different tools. Then, maintainers can also decide whether they prefer your patch of subcmd or the one mentioned in the other thread. Viktor > > Tested on x86_64 with clang version 17.0.6 and gcc (GCC) 13.3.1. > > $ cd tools/bpf/resolve_btfids > $ for c in gcc clang; do \ > for o in fast g s z $(seq 0 3); do \ > make clean && \ > make HOST_CC=${c} "HOSTCFLAGS=-O${o} -Wall" 2>&1 | tee ${c}-O${o}.out; \ > done; done && grep 'warning:\|error:' *.out > > [...] > clang-O1.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] > clang-O1.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] > clang-O2.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] > clang-O2.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] > clang-O3.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] > clang-O3.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] > clang-Ofast.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] > clang-Ofast.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] > clang-Og.out:btf_dump.c:903:42: error: ‘new_off’ may be used uninitialized [-Werror=maybe-uninitialized] > clang-Og.out:btf_dump.c:917:25: error: ‘pad_type’ may be used uninitialized [-Werror=maybe-uninitialized] > clang-Og.out:btf_dump.c:930:20: error: ‘pad_bits’ may be used uninitialized [-Werror=maybe-uninitialized] > clang-Os.out:parse-options.c:832:9: error: ‘o’ may be used uninitialized [-Werror=maybe-uninitialized] > clang-Oz.out:parse-options.c:832:9: error: ‘o’ may be used uninitialized [-Werror=maybe-uninitialized] > gcc-O1.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] > gcc-O1.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] > gcc-O2.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] > gcc-O2.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] > gcc-O3.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] > gcc-O3.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] > gcc-Ofast.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] > gcc-Ofast.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] > gcc-Og.out:btf_dump.c:903:42: error: ‘new_off’ may be used uninitialized [-Werror=maybe-uninitialized] > gcc-Og.out:btf_dump.c:917:25: error: ‘pad_type’ may be used uninitialized [-Werror=maybe-uninitialized] > gcc-Og.out:btf_dump.c:930:20: error: ‘pad_bits’ may be used uninitialized [-Werror=maybe-uninitialized] > gcc-Os.out:parse-options.c:832:9: error: ‘o’ may be used uninitialized [-Werror=maybe-uninitialized] > gcc-Oz.out:parse-options.c:832:9: error: ‘o’ may be used uninitialized [-Werror=maybe-uninitialized] > > The above warnings and/or errors are not observed after applying this > patch. > > Signed-off-by: Eder Zulian <ezulian@redhat.com> > --- > tools/bpf/resolve_btfids/main.c | 4 ++-- > tools/lib/bpf/btf_dump.c | 4 ++-- > tools/lib/subcmd/parse-options.c | 2 +- > 3 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c > index d54aaa0619df..bd9f960bce3d 100644 > --- a/tools/bpf/resolve_btfids/main.c > +++ b/tools/bpf/resolve_btfids/main.c > @@ -679,8 +679,8 @@ static int sets_patch(struct object *obj) > > next = rb_first(&obj->sets); > while (next) { > - struct btf_id_set8 *set8; > - struct btf_id_set *set; > + struct btf_id_set8 *set8 = NULL; > + struct btf_id_set *set = NULL; > unsigned long addr, off; > struct btf_id *id; > > diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c > index 8440c2c5ad3e..468392f9882d 100644 > --- a/tools/lib/bpf/btf_dump.c > +++ b/tools/lib/bpf/btf_dump.c > @@ -867,8 +867,8 @@ static void btf_dump_emit_bit_padding(const struct btf_dump *d, > } pads[] = { > {"long", d->ptr_sz * 8}, {"int", 32}, {"short", 16}, {"char", 8} > }; > - int new_off, pad_bits, bits, i; > - const char *pad_type; > + int new_off = 0, pad_bits = 0, bits, i; > + const char *pad_type = NULL; > > if (cur_off >= next_off) > return; /* no gap */ > diff --git a/tools/lib/subcmd/parse-options.c b/tools/lib/subcmd/parse-options.c > index eb896d30545b..555d617c1f50 100644 > --- a/tools/lib/subcmd/parse-options.c > +++ b/tools/lib/subcmd/parse-options.c > @@ -807,7 +807,7 @@ static int option__cmp(const void *va, const void *vb) > static struct option *options__order(const struct option *opts) > { > int nr_opts = 0, nr_group = 0, nr_parent = 0, len; > - const struct option *o, *p = opts; > + const struct option *o = NULL, *p = opts; > struct option *opt, *ordered = NULL, *group; > > /* flatten the options that have parents */
On 10/11/24 1:00 PM, Eder Zulian wrote: > - tools/bpf/resolve_btfids/main.c: Initialize 'set' and 'set8' pointers > to NULL in to fix compiler warnings. > > - tools/lib/bpf/btf_dump.c: Initialize 'new_off' and 'pad_bits' to 0 and > 'pad_type' to NULL to prevent compiler warnings. > > - tools/lib/subcmd/parse-options.c: Initiazlide pointer 'o' to NULL > avoiding a compiler warning. > > Tested on x86_64 with clang version 17.0.6 and gcc (GCC) 13.3.1. > > $ cd tools/bpf/resolve_btfids > $ for c in gcc clang; do \ > for o in fast g s z $(seq 0 3); do \ > make clean && \ > make HOST_CC=${c} "HOSTCFLAGS=-O${o} -Wall" 2>&1 | tee ${c}-O${o}.out; \ > done; done && grep 'warning:\|error:' *.out > > [...] > clang-O1.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] > clang-O1.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] > clang-O2.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] > clang-O2.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] > clang-O3.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] > clang-O3.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] > clang-Ofast.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] > clang-Ofast.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] > clang-Og.out:btf_dump.c:903:42: error: ‘new_off’ may be used uninitialized [-Werror=maybe-uninitialized] > clang-Og.out:btf_dump.c:917:25: error: ‘pad_type’ may be used uninitialized [-Werror=maybe-uninitialized] > clang-Og.out:btf_dump.c:930:20: error: ‘pad_bits’ may be used uninitialized [-Werror=maybe-uninitialized] > clang-Os.out:parse-options.c:832:9: error: ‘o’ may be used uninitialized [-Werror=maybe-uninitialized] > clang-Oz.out:parse-options.c:832:9: error: ‘o’ may be used uninitialized [-Werror=maybe-uninitialized] > gcc-O1.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] > gcc-O1.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] > gcc-O2.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] > gcc-O2.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] > gcc-O3.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] > gcc-O3.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] > gcc-Ofast.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] > gcc-Ofast.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] > gcc-Og.out:btf_dump.c:903:42: error: ‘new_off’ may be used uninitialized [-Werror=maybe-uninitialized] > gcc-Og.out:btf_dump.c:917:25: error: ‘pad_type’ may be used uninitialized [-Werror=maybe-uninitialized] > gcc-Og.out:btf_dump.c:930:20: error: ‘pad_bits’ may be used uninitialized [-Werror=maybe-uninitialized] > gcc-Os.out:parse-options.c:832:9: error: ‘o’ may be used uninitialized [-Werror=maybe-uninitialized] > gcc-Oz.out:parse-options.c:832:9: error: ‘o’ may be used uninitialized [-Werror=maybe-uninitialized] > > The above warnings and/or errors are not observed after applying this > patch. > > Signed-off-by: Eder Zulian <ezulian@redhat.com> Currently compilation is okay although it uses -O0. One choice is to move from -O0 to -O2 and fix corresponding -Wmaybe-uninitialized issues. Otherwise, since there is no bug, not sure whether we should make changes or not. Maintainers can decide the next step.
diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c index d54aaa0619df..bd9f960bce3d 100644 --- a/tools/bpf/resolve_btfids/main.c +++ b/tools/bpf/resolve_btfids/main.c @@ -679,8 +679,8 @@ static int sets_patch(struct object *obj) next = rb_first(&obj->sets); while (next) { - struct btf_id_set8 *set8; - struct btf_id_set *set; + struct btf_id_set8 *set8 = NULL; + struct btf_id_set *set = NULL; unsigned long addr, off; struct btf_id *id; diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c index 8440c2c5ad3e..468392f9882d 100644 --- a/tools/lib/bpf/btf_dump.c +++ b/tools/lib/bpf/btf_dump.c @@ -867,8 +867,8 @@ static void btf_dump_emit_bit_padding(const struct btf_dump *d, } pads[] = { {"long", d->ptr_sz * 8}, {"int", 32}, {"short", 16}, {"char", 8} }; - int new_off, pad_bits, bits, i; - const char *pad_type; + int new_off = 0, pad_bits = 0, bits, i; + const char *pad_type = NULL; if (cur_off >= next_off) return; /* no gap */ diff --git a/tools/lib/subcmd/parse-options.c b/tools/lib/subcmd/parse-options.c index eb896d30545b..555d617c1f50 100644 --- a/tools/lib/subcmd/parse-options.c +++ b/tools/lib/subcmd/parse-options.c @@ -807,7 +807,7 @@ static int option__cmp(const void *va, const void *vb) static struct option *options__order(const struct option *opts) { int nr_opts = 0, nr_group = 0, nr_parent = 0, len; - const struct option *o, *p = opts; + const struct option *o = NULL, *p = opts; struct option *opt, *ordered = NULL, *group; /* flatten the options that have parents */
- tools/bpf/resolve_btfids/main.c: Initialize 'set' and 'set8' pointers to NULL in to fix compiler warnings. - tools/lib/bpf/btf_dump.c: Initialize 'new_off' and 'pad_bits' to 0 and 'pad_type' to NULL to prevent compiler warnings. - tools/lib/subcmd/parse-options.c: Initiazlide pointer 'o' to NULL avoiding a compiler warning. Tested on x86_64 with clang version 17.0.6 and gcc (GCC) 13.3.1. $ cd tools/bpf/resolve_btfids $ for c in gcc clang; do \ for o in fast g s z $(seq 0 3); do \ make clean && \ make HOST_CC=${c} "HOSTCFLAGS=-O${o} -Wall" 2>&1 | tee ${c}-O${o}.out; \ done; done && grep 'warning:\|error:' *.out [...] clang-O1.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] clang-O1.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] clang-O2.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] clang-O2.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] clang-O3.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] clang-O3.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] clang-Ofast.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] clang-Ofast.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] clang-Og.out:btf_dump.c:903:42: error: ‘new_off’ may be used uninitialized [-Werror=maybe-uninitialized] clang-Og.out:btf_dump.c:917:25: error: ‘pad_type’ may be used uninitialized [-Werror=maybe-uninitialized] clang-Og.out:btf_dump.c:930:20: error: ‘pad_bits’ may be used uninitialized [-Werror=maybe-uninitialized] clang-Os.out:parse-options.c:832:9: error: ‘o’ may be used uninitialized [-Werror=maybe-uninitialized] clang-Oz.out:parse-options.c:832:9: error: ‘o’ may be used uninitialized [-Werror=maybe-uninitialized] gcc-O1.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] gcc-O1.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] gcc-O2.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] gcc-O2.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] gcc-O3.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] gcc-O3.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] gcc-Ofast.out:main.c:163:9: warning: ‘set8’ may be used uninitialized [-Wmaybe-uninitialized] gcc-Ofast.out:main.c:163:9: warning: ‘set’ may be used uninitialized [-Wmaybe-uninitialized] gcc-Og.out:btf_dump.c:903:42: error: ‘new_off’ may be used uninitialized [-Werror=maybe-uninitialized] gcc-Og.out:btf_dump.c:917:25: error: ‘pad_type’ may be used uninitialized [-Werror=maybe-uninitialized] gcc-Og.out:btf_dump.c:930:20: error: ‘pad_bits’ may be used uninitialized [-Werror=maybe-uninitialized] gcc-Os.out:parse-options.c:832:9: error: ‘o’ may be used uninitialized [-Werror=maybe-uninitialized] gcc-Oz.out:parse-options.c:832:9: error: ‘o’ may be used uninitialized [-Werror=maybe-uninitialized] The above warnings and/or errors are not observed after applying this patch. Signed-off-by: Eder Zulian <ezulian@redhat.com> --- tools/bpf/resolve_btfids/main.c | 4 ++-- tools/lib/bpf/btf_dump.c | 4 ++-- tools/lib/subcmd/parse-options.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)