Merge "Revert^2 "Verify that ndk_headers headers are valid C."" into main

cc/ndk_headers.go

@@ -18,6 +18,7 @@ import (
 	"path/filepath"
 
 	"android/soong/android"
+
 	"github.com/google/blueprint"
 )
 
@@ -44,7 +45,7 @@ func init() {
 }
 
 // Returns the NDK base include path for use with sdk_version current. Usable with -I.
-func getCurrentIncludePath(ctx android.ModuleContext) android.OutputPath {
+func getCurrentIncludePath(ctx android.PathContext) android.OutputPath {
 	return getNdkSysrootBase(ctx).Join(ctx, "usr/include")
 }
 
@@ -73,6 +74,13 @@ type headerProperties struct {
 
 	// Path to the NOTICE file associated with the headers.
 	License *string `android:"path"`
+
+	// Set to true if the headers installed by this module should skip
+	// verification. This step ensures that each header is self-contained (can
+	// be #included alone) and is valid C. This should not be disabled except in
+	// rare cases. Outside bionic and external, if you're using this option
+	// you've probably made a mistake.
+	Skip_verification *bool
 }
 
 type headerModule struct {
@@ -309,6 +317,13 @@ type preprocessedHeadersProperties struct {
 
 	// Path to the NOTICE file associated with the headers.
 	License *string
+
+	// Set to true if the headers installed by this module should skip
+	// verification. This step ensures that each header is self-contained (can
+	// be #included alone) and is valid C. This should not be disabled except in
+	// rare cases. Outside bionic and external, if you're using this option
+	// you've probably made a mistake.
+	Skip_verification *bool
 }
 
 type preprocessedHeadersModule struct {

cc/ndk_sysroot.go

@@ -54,7 +54,22 @@ package cc
 
 import (
 	"android/soong/android"
+	"fmt"
+	"path/filepath"
 	"strings"
+
+	"github.com/google/blueprint"
+)
+
+var (
+	verifyCCompat = pctx.AndroidStaticRule("verifyCCompat",
+		blueprint.RuleParams{
+			Command:     "$ccCmd -x c -fsyntax-only $flags $in && touch $out",
+			CommandDeps: []string{"$ccCmd"},
+		},
+		"ccCmd",
+		"flags",
+	)
 )
 
 func init() {
@@ -103,6 +118,45 @@ func getNdkABIHeadersFile(ctx android.PathContext) android.WritablePath {
 	return android.PathForOutput(ctx, "ndk_abi_headers.txt")
 }
 
+func verifyNdkHeaderIsCCompatible(ctx android.SingletonContext,
+	src android.Path, dest android.Path) android.Path {
+	sysrootInclude := getCurrentIncludePath(ctx)
+	baseOutputDir := android.PathForOutput(ctx, "c-compat-verification")
+	installRelPath, err := filepath.Rel(sysrootInclude.String(), dest.String())
+	if err != nil {
+		ctx.Errorf("filepath.Rel(%q, %q) failed: %s", dest, sysrootInclude, err)
+	}
+	output := baseOutputDir.Join(ctx, installRelPath)
+	ctx.Build(pctx, android.BuildParams{
+		Rule:        verifyCCompat,
+		Description: fmt.Sprintf("Verifying C compatibility of %s", src),
+		Output:      output,
+		Input:       dest,
+		// Ensures that all the headers in the sysroot are already installed
+		// before testing any of the headers for C compatibility, and also that
+		// the check will be re-run whenever the sysroot changes. This is
+		// necessary because many of the NDK headers depend on other NDK
+		// headers, but we don't have explicit dependency tracking for that.
+		Implicits: []android.Path{getNdkHeadersTimestampFile(ctx)},
+		Args: map[string]string{
+			"ccCmd": "${config.ClangBin}/clang",
+			"flags": fmt.Sprintf(
+				// Ideally we'd check each ABI, multiple API levels,
+				// fortify/non-fortify, and a handful of other variations. It's
+				// a lot more difficult to do that though, and would eat up more
+				// build time. All the problems we've seen so far that this
+				// check would catch have been in arch-generic and
+				// minSdkVersion-generic code in frameworks though, so this is a
+				// good place to start.
+				"-target aarch64-linux-android%d --sysroot %s",
+				android.FutureApiLevel.FinalOrFutureInt(),
+				getNdkSysrootBase(ctx).String(),
+			),
+		},
+	})
+	return output
+}
+
 func NdkSingleton() android.Singleton {
 	return &ndkSingleton{}
 }
@@ -143,10 +197,17 @@ func writeNdkAbiSrcFilter(ctx android.BuilderContext,
 
 type ndkSingleton struct{}
 
+type srcDestPair struct {
+	src  android.Path
+	dest android.Path
+}
+
 func (n *ndkSingleton) GenerateBuildActions(ctx android.SingletonContext) {
 	var staticLibInstallPaths android.Paths
 	var headerSrcPaths android.Paths
 	var headerInstallPaths android.Paths
+	var headersToVerify []srcDestPair
+	var headerCCompatVerificationTimestampPaths android.Paths
 	var installPaths android.Paths
 	var licensePaths android.Paths
 	ctx.VisitAllModules(func(module android.Module) {
@@ -157,6 +218,14 @@ func (n *ndkSingleton) GenerateBuildActions(ctx android.SingletonContext) {
 		if m, ok := module.(*headerModule); ok {
 			headerSrcPaths = append(headerSrcPaths, m.srcPaths...)
 			headerInstallPaths = append(headerInstallPaths, m.installPaths...)
+			if !Bool(m.properties.Skip_verification) {
+				for i, installPath := range m.installPaths {
+					headersToVerify = append(headersToVerify, srcDestPair{
+						src:  m.srcPaths[i],
+						dest: installPath,
+					})
+				}
+			}
 			installPaths = append(installPaths, m.installPaths...)
 			licensePaths = append(licensePaths, m.licensePath)
 		}
@@ -164,6 +233,10 @@ func (n *ndkSingleton) GenerateBuildActions(ctx android.SingletonContext) {
 		if m, ok := module.(*versionedHeaderModule); ok {
 			headerSrcPaths = append(headerSrcPaths, m.srcPaths...)
 			headerInstallPaths = append(headerInstallPaths, m.installPaths...)
+			// Verification intentionally not done for headers that go through
+			// versioner. It'd be nice to have, but the only user is bionic, and
+			// that one module would also need to use skip_verification, so it
+			// wouldn't help at all.
 			installPaths = append(installPaths, m.installPaths...)
 			licensePaths = append(licensePaths, m.licensePath)
 		}
@@ -171,6 +244,14 @@ func (n *ndkSingleton) GenerateBuildActions(ctx android.SingletonContext) {
 		if m, ok := module.(*preprocessedHeadersModule); ok {
 			headerSrcPaths = append(headerSrcPaths, m.srcPaths...)
 			headerInstallPaths = append(headerInstallPaths, m.installPaths...)
+			if !Bool(m.properties.Skip_verification) {
+				for i, installPath := range m.installPaths {
+					headersToVerify = append(headersToVerify, srcDestPair{
+						src:  m.srcPaths[i],
+						dest: installPath,
+					})
+				}
+			}
 			installPaths = append(installPaths, m.installPaths...)
 			licensePaths = append(licensePaths, m.licensePath)
 		}
@@ -223,6 +304,12 @@ func (n *ndkSingleton) GenerateBuildActions(ctx android.SingletonContext) {
 		Implicits: headerInstallPaths,
 	})
 
+	for _, srcDestPair := range headersToVerify {
+		headerCCompatVerificationTimestampPaths = append(
+			headerCCompatVerificationTimestampPaths,
+			verifyNdkHeaderIsCCompatible(ctx, srcDestPair.src, srcDestPair.dest))
+	}
+
 	writeNdkAbiSrcFilter(ctx, headerSrcPaths, getNdkABIHeadersFile(ctx))
 
 	fullDepPaths := append(staticLibInstallPaths, getNdkBaseTimestampFile(ctx))
@@ -235,6 +322,6 @@ func (n *ndkSingleton) GenerateBuildActions(ctx android.SingletonContext) {
 	ctx.Build(pctx, android.BuildParams{
 		Rule:      android.Touch,
 		Output:    getNdkFullTimestampFile(ctx),
-		Implicits: fullDepPaths,
+		Implicits: append(fullDepPaths, headerCCompatVerificationTimestampPaths...),
 	})
 }