On 7/28/22 9:56 AM, Jörn-Thorben Hinz wrote:
Hi,
after compiling a skeleton-using program with -pedantic once and
stumbling across a tiniest incorrectness in skeletons with it[1], I was
debating whether it makes sense to suppress warnings from skeleton
headers.
Happy about comments about this. This change might be too suppressive
towards warnings and maybe ignoring only -Woverlength-strings directly
in OBJ_NAME__elf_bytes() be a better idea. Or keep all warnings from
skeletons available as-is to have them more visible in and around
bpftool’s development.
This is my 2cents. As you mentioned, skeleton file are per program
and not in system header file directory. I would like not to mark
these header files as system files. Since different program will
generate different skeleton headers, suppressing warnings
will prevent from catching potential issues in certain cases.
Also, since the warning is triggered by extra user flags like -pedantic
when building bpftool, user can also add -Wno-overlength-strings
in the extra user flags.
[1] https://lore.kernel.org/r/20220726133203.514087-1-jthinz@xxxxxxxxxxxxxxxxxxxx/
Commit message:
A userspace program including a skeleton generated by bpftool might use
an arbitrary set of compiler flags, including enabling various warnings.
For example, with -Woverlength-strings the string constant in
OBJ_NAME__elf_bytes() causes a warning due to its usually huge length.
This string length is not an actual problem with GCC and clang, though,
it’s “just” not required by the C standard to be supported.
Skeleton headers are likely not placed in a system include path. To
avoid the above warning and similar noise for the *user* of a skeleton,
explicitly mark the header as a system header which disables almost all
warnings for it when included.
Skeleton headers generated during the build of bpftool are not marked to
keep potential warnings available to bpftool’s developers.
Signed-off-by: Jörn-Thorben Hinz <jthinz@xxxxxxxxxxxxxxxxxxxx>
---
tools/bpf/bpftool/Makefile | 2 ++
tools/bpf/bpftool/gen.c | 30 +++++++++++++++++++++++++++---
2 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 6b5b3a99f79d..5f484d7929db 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -196,6 +196,8 @@ endif
CFLAGS += $(if $(BUILD_BPF_SKELS),,-DBPFTOOL_WITHOUT_SKELETONS)
+$(BOOTSTRAP_OUTPUT)%.o: CFLAGS += -DBPFTOOL_BOOTSTRAP
+
$(BOOTSTRAP_OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
$(QUIET_CC)$(HOSTCC) $(HOST_CFLAGS) -c -MMD $< -o $@
diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 1cf53bb01936..82053aceec78 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -1006,7 +1006,15 @@ static int do_skeleton(int argc, char **argv)
/* THIS FILE IS AUTOGENERATED BY BPFTOOL! */ \n\
#ifndef %2$s \n\
#define %2$s \n\
- \n\
+ "
+#ifndef BPFTOOL_BOOTSTRAP
+ "\
+ \n\
+ _Pragma(\"GCC system_header\") \n\
+ "
+#endif
+ "\
+ \n\
#include <bpf/skel_internal.h> \n\
\n\
struct %1$s { \n\
@@ -1022,7 +1030,15 @@ static int do_skeleton(int argc, char **argv)
/* THIS FILE IS AUTOGENERATED BY BPFTOOL! */ \n\
#ifndef %2$s \n\
#define %2$s \n\
- \n\
+ "
+#ifndef BPFTOOL_BOOTSTRAP
+ "\
+ \n\
+ _Pragma(\"GCC system_header\") \n\
+ "
+#endif
+ "\
+ \n\
#include <errno.h> \n\
#include <stdlib.h> \n\
#include <bpf/libbpf.h> \n\
@@ -1415,7 +1431,15 @@ static int do_subskeleton(int argc, char **argv)
/* THIS FILE IS AUTOGENERATED! */ \n\
#ifndef %2$s \n\
#define %2$s \n\
- \n\
+ "
+#ifndef BPFTOOL_BOOTSTRAP
+ "\
+ \n\
+ _Pragma(\"GCC system_header\") \n\
+ "
+#endif
+ "\
+ \n\
#include <errno.h> \n\
#include <stdlib.h> \n\
#include <bpf/libbpf.h> \n\
