build/advanced: add option to check for use of cdefs.h

We want to catch programs that directly include sys/cdefs.h so that we can fix them not to. So, we want to instrument sys/cdefs.h to emit a warning when it is included.

However, there are two cases: musl and the other C libraries.

For musl, it is pretty trivial to do so, because we install our own sys/cdefs.h header, so we are free to instrument it at will. Not content to emit a warning when it is included, we can also instrument each macro to emit its own warning on being used, so we can easily pinpoint what macro is used and why cdefs.h is used at all.

This is made even easier because musl does not include sys/cdefs.h from its own headers, so any inclusion of sys/cdefs.h is entirely due to an explicit include by a package.

But for glibc and uClibc, the matter is a little bit different: there are two problems we have to solve:

- both glibc and uClibc install their own sys/cdefs.h, so we are not

    free to instrument it at will;

- both include it from their own headers, so that the inclusion of it is not necessarily an explicit include by a package.

We solve these two problems in two ways:

- the first point is solved by renaming the existing header, adding our own header that acts as a trampoline to the original one, after adding a #warning to warn about inclusion of sys/cdefs.h;

- the second point is solved by changing all the headers installed by the toolchain to directly include the original, renamed header instead of our trampoline.

This way, we can catch direct inclusion of sys/cdefs.h and ignore indirect inclusions that are internal to the C library headers.

This also means that we are limited in what we can catch. We can not instrument each macro individually, so all we know is that sys/cdefs.h was included; we don't know what is being used from it.

Because instrumenting sys/cdefs.h to emit even just a warning has the potential to break packages the hard way (e.g. those using -Werror that would normally not have any warning), we make that an advanced build option that defaults to not instrumenting sys/cdefs.h.

When we are confident that the instrumentation of sys/cdefs.h has no harmful side effects, we can change the default to emit the warning.

Beside no instrumentation and adding a warning, we add a third option, to treat use of sys/cdefs.h as an error. This does break a lot of things, so this is only available to actively track down the use of sys/cdefs.h with the aim of removing its use from a pacakge.

---
 Config.in                                          | 52 ++++++++++++
 package/musl-compat-headers/cdefs.h                | 51 ------------
 package/musl-compat-headers/cdefs.h.in             | 94 ++++++++++++++++++++++
 package/musl-compat-headers/musl-compat-headers.mk |  8 +-
 toolchain/cdefs.h.in                               |  8 ++
 toolchain/toolchain.mk                             | 30 +++++++
 6 files changed, 191 insertions(+), 52 deletions(-)
 delete mode 100644 package/musl-compat-headers/cdefs.h
 create mode 100644 package/musl-compat-headers/cdefs.h.in
 create mode 100644 toolchain/cdefs.h.in

index 98096df..c68b8b1 100644
@@ -699,6 +699,58 @@ config BR2_COMPILER_PARANOID_UNSAFE_PATH
 	  toolchain (through the toolchain wrapper and binutils patches)
 	  and external toolchain backends (through the toolchain wrapper).
 
+choice
+	bool "paranoid check for use of cdefs.h"
+	default BR2_LEGACY_CDEFS_H_NO_CHECK
+	help
+	  sys/cdefs.h is a non-standard header, originating from glibc,
+	  that defines non-standard and legacy macros. This header is not
+	  always available (e.g. musl does not provide it, but Buildroot
+	  installs a surrogate, minimalist one in this case).
+
+	  Some packages use this header, which is wrong, as that makes
+	  then non-portable.
+
+	  By default, Buildroot does not detect the use of that header.
+	  In the future, this default may change to warning, or even to
+	  erroring out.
+
+config BR2_LEGACY_CDEFS_H_NO_CHECK
+	bool "don't check"
+	help
+	  Do not detect any use of sys/cdefs.h.
+
+config BR2_LEGACY_CDEFS_H_WARN
+	bool "check and emit warnings"
+	help
+	  Detect and warn about the use of sys/cdefs.h.
+
+	  When using the musl C library, this also warns for each of the
+	  legacy macros being used:
+		__P()
+		__BEGIN_DECLS
+		__END_DECLS
+		__THROW
+		__NTH()
+
+config BR2_LEGACY_CDEFS_H_ERROR
+	bool "check and exit in error"
+	help
+	  Same as BR2_LEGACY_CDEFS_H_WARN, above, but treated as
+	  an error.
+
+endchoice
+
+config BR2_LEGACY_CDEFS_H_PRAGMA_COND
+	int
+	default 0 if BR2_LEGACY_CDEFS_H_NO_CHECK
+	default 1
+
+config BR2_LEGACY_CDEFS_H_PRAGMA
+	string
+	default "warning" if BR2_LEGACY_CDEFS_H_WARN
+	default "error"   if BR2_LEGACY_CDEFS_H_ERROR
+
 config BR2_REPRODUCIBLE
 	bool "Make the build reproducible (experimental)"
 	help

index 25e032c..7879123 100644
@@ -21,7 +21,13 @@ MUSL_COMPAT_HEADERS_INSTALL_STAGING = YES
 # Copying both headers so legal-info finds them (they are _LICENSE_FILES)
 define MUSL_COMPAT_HEADERS_EXTRACT_CMDS
 	$(INSTALL) -m 0644 -D $(DL_DIR)/$(notdir $(MUSL_COMPAT_HEADERS_QUEUE_H)) $(@D)/queue.h
-	$(INSTALL) -m 0644 -D $(MUSL_COMPAT_HEADERS_PKGDIR)/cdefs.h $(@D)/cdefs.h
+	$(INSTALL) -m 0644 -D $(MUSL_COMPAT_HEADERS_PKGDIR)/cdefs.h.in $(@D)/cdefs.h.in
+endef
+
+define MUSL_COMPAT_HEADERS_BUILD_CMDS
+	sed -r -e 's/@pragma_cond@/$(BR2_LEGACY_CDEFS_H_PRAGMA_COND)/' \
+	       -e 's/@pragma@/$(call qstrip,$(BR2_LEGACY_CDEFS_H_PRAGMA))/' \
+	       $(@D)/cdefs.h.in >$(@D)/cdefs.h
 endef
 
 define MUSL_COMPAT_HEADERS_INSTALL_STAGING_CMDS

new file mode 100644
index 0000000..23bd3e8
@@ -0,0 +1,8 @@
+#ifndef BUILDROOT_SYS_CDEFS_H_WRAPPER
+#define BUILDROOT_SYS_CDEFS_H_WRAPPER
+
+#@pragma@ Use of deprecated <sys/cdefs.h> header.
+
+#include <sys/cdefs.h.wrapped>
+
+#endif /* BUILDROOT_SYS_CDEFS_H_WRAPPER */

index 59fc905..13109a9 100644
@@ -54,3 +54,33 @@ define COPY_GCONV_LIBS
 endef
 TOOLCHAIN_TARGET_FINALIZE_HOOKS += COPY_GCONV_LIBS
 endif
+
+# For non-musl toolchain, install our wrapper to sys/cdefs.h and
+# set the proper checking level. Also `fix' toolchain headers to
+# directly include the real sys/cdefs.h instead of our wrapper.
+#
+# This is a target post install hook, because the virtual package
+# "toolchain" is not `installed' to staging.
+ifeq ($(BR2_TOOLCHAIN_USES_MUSL),)
+ifneq ($(BR2_LEGACY_CDEFS_H_NO_CHECK),y)
+define TOOLCHAIN_WRAP_CDEFS_H
+	cdefs_h=$(STAGING_DIR)/usr/include/sys/cdefs.h; \
+	if [ ! -f $${cdefs_h} ]; then \
+		echo "*** Error: toolchain does not have <sys/cdefs.h>" >&2; \
+		exit 1; \
+	fi; \
+	if [ ! -f $${cdefs_h}.wrapped ]; then \
+		mv $${cdefs_h} $${cdefs_h}.wrapped || exit 1; \
+		$(INSTALL) -m 0644 -D toolchain/cdefs.h.in \
+			$${cdefs_h} || exit 1; \
+		$(SED) 's/@pragma_cond@/$(BR2_LEGACY_CDEFS_H_PRAGMA_COND)/' \
+			-e 's/@pragma@/$(call qstrip,$(BR2_LEGACY_CDEFS_H_PRAGMA))/' \
+			$${cdefs_h} || exit 1; \
+		find $(STAGING_DIR)/usr/include/ -type f -name '*.h' -exec \
+			sed -r -i -e 's:^(#[[:space:]]*include[[:space:]]+<sys/cdefs\.h)(>)$$:\1.wrapped\2 /* Redirected by Buildroot */:' \
+				{} + || exit 1; \
+	fi
+endef
+TOOLCHAIN_POST_INSTALL_TARGET_HOOKS += TOOLCHAIN_WRAP_CDEFS_H
+endif # CDEFS_H_NONE
+endif # USES_MUSL

2.7.4

Hello,

Thanks for working on this. However, I am still not convinced that we want to merge such an additional complexity (one new Config.in choice with three sub-options, two additional Config.in options), a cdefs.h wrapper for glibc/uclibc, etc.

Do we really care about upstream packages using sys/cdefs.h? Is it really the most important battle to fight against upstream projects using sys/cdefs.h?

If we start having instrumentation in Buildroot to detect such very specific "standard violation", then where do we stop? There's plenty of upstream projects doing bogus things all over the place.

I believe the paranoid checks for bogus -I/-L flags are fine because they really potentially break cross-compilation. But this cdefs.h mis-use doesn't break anything, now that we have the cdefs.h replacement for musl.

Peter, Arnout, what do you think?

Thomas

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Thomas, All,

On 2016-12-04 13:52 +0100, Thomas Petazzoni spake thusly:

I don't really care. I did some cleanups and changes according to comments to the previous, initial iteration, and just sent it to the list for a final review before I delete it locally.

So, at least, it is in the list archives for posterity, in case we want/need to revisit this later on.

Regards,
Yann E. MORIN.

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

I agree with Thomas. It's not our responsibility to fix packages, just to
cross-compile them.

Regards,
Arnout

Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF