Add flag property checking

am: 20acc5c520

* commit '20acc5c520c8d6021824b3f839000db45be36a6e':
  Add flag property checking

Change-Id: Ia9c635286974db0a363bfbc99c6366781ce12914

Android.bp

@@ -125,6 +125,7 @@ bootstrap_go_package {
         "cc/androidmk.go",
         "cc/builder.go",
         "cc/cc.go",
+        "cc/check.go",
         "cc/clang.go",
         "cc/gen.go",
         "cc/makevars.go",

cc/cc.go

@@ -1019,6 +1019,11 @@ func (compiler *baseCompiler) deps(ctx BaseModuleContext, deps Deps) Deps {
 func (compiler *baseCompiler) flags(ctx ModuleContext, flags Flags) Flags {
 	toolchain := ctx.toolchain()
 
+	CheckBadCompilerFlags(ctx, "cflags", compiler.Properties.Cflags)
+	CheckBadCompilerFlags(ctx, "cppflags", compiler.Properties.Cppflags)
+	CheckBadCompilerFlags(ctx, "conlyflags", compiler.Properties.Conlyflags)
+	CheckBadCompilerFlags(ctx, "asflags", compiler.Properties.Asflags)
+
 	flags.CFlags = append(flags.CFlags, compiler.Properties.Cflags...)
 	flags.CppFlags = append(flags.CppFlags, compiler.Properties.Cppflags...)
 	flags.ConlyFlags = append(flags.ConlyFlags, compiler.Properties.Conlyflags...)
@@ -1066,10 +1071,15 @@ func (compiler *baseCompiler) flags(ctx ModuleContext, flags Flags) Flags {
 		ctx.ModuleErrorf("%s", err)
 	}
 
+	CheckBadCompilerFlags(ctx, "release.cflags", compiler.Properties.Release.Cflags)
+
 	// TODO: debug
 	flags.CFlags = append(flags.CFlags, compiler.Properties.Release.Cflags...)
 
 	if flags.Clang {
+		CheckBadCompilerFlags(ctx, "clang_cflags", compiler.Properties.Clang_cflags)
+		CheckBadCompilerFlags(ctx, "clang_asflags", compiler.Properties.Clang_asflags)
+
 		flags.CFlags = clangFilterUnknownCflags(flags.CFlags)
 		flags.CFlags = append(flags.CFlags, compiler.Properties.Clang_cflags...)
 		flags.AsFlags = append(flags.AsFlags, compiler.Properties.Clang_asflags...)
@@ -1268,10 +1278,14 @@ func (linker *baseLinker) flags(ctx ModuleContext, flags Flags) Flags {
 		}
 
 		if ctx.Host() {
+			CheckBadHostLdlibs(ctx, "host_ldlibs", linker.Properties.Host_ldlibs)
+
 			flags.LdFlags = append(flags.LdFlags, linker.Properties.Host_ldlibs...)
 		}
 	}
 
+	CheckBadLinkerFlags(ctx, "ldflags", linker.Properties.Ldflags)
+
 	flags.LdFlags = append(flags.LdFlags, linker.Properties.Ldflags...)
 
 	if ctx.Host() && !linker.static() {

cc/check.go

@@ -0,0 +1,96 @@
+// Copyright 2016 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package cc
+
+// This file contains utility functions to check for bad or illegal cflags
+// specified by a module
+
+import (
+	"path/filepath"
+	"strings"
+)
+
+// Check for invalid c/conly/cpp/asflags and suggest alternatives. Only use this
+// for flags explicitly passed by the user, since these flags may be used internally.
+func CheckBadCompilerFlags(ctx ModuleContext, prop string, flags []string) {
+	for _, flag := range flags {
+		flag = strings.TrimSpace(flag)
+
+		if !strings.HasPrefix(flag, "-") {
+			ctx.PropertyErrorf(prop, "Flag `%s` must start with `-`", flag)
+		} else if strings.HasPrefix(flag, "-I") || strings.HasPrefix(flag, "-isystem") {
+			ctx.PropertyErrorf(prop, "Bad flag `%s`, use local_include_dirs or include_dirs instead", flag)
+		} else if inList(flag, illegalFlags) {
+			ctx.PropertyErrorf(prop, "Illegal flag `%s`", flag)
+		} else if strings.Contains(flag, " ") {
+			args := strings.Split(flag, " ")
+			if args[0] == "-include" {
+				if len(args) > 2 {
+					ctx.PropertyErrorf(prop, "`-include` only takes one argument: `%s`", flag)
+				}
+				path := filepath.Clean(args[1])
+				if strings.HasPrefix("/", path) {
+					ctx.PropertyErrorf(prop, "Path must not be an absolute path: %s", flag)
+				} else if strings.HasPrefix("../", path) {
+					ctx.PropertyErrorf(prop, "Path must not start with `../`: `%s`. Use include_dirs to -include from a different directory", flag)
+				}
+			} else {
+				ctx.PropertyErrorf(prop, "Bad flag: `%s` is not an allowed multi-word flag. Should it be split into multiple flags?", flag)
+			}
+		}
+	}
+}
+
+// Check for bad ldflags and suggest alternatives. Only use this for flags
+// explicitly passed by the user, since these flags may be used internally.
+func CheckBadLinkerFlags(ctx ModuleContext, prop string, flags []string) {
+	for _, flag := range flags {
+		flag = strings.TrimSpace(flag)
+
+		if !strings.HasPrefix(flag, "-") {
+			ctx.PropertyErrorf(prop, "Flag `%s` must start with `-`", flag)
+		} else if strings.HasPrefix(flag, "-l") {
+			if ctx.Host() {
+				ctx.PropertyErrorf(prop, "Bad flag: `%s`, use shared_libs or host_ldlibs instead", flag)
+			} else {
+				ctx.PropertyErrorf(prop, "Bad flag: `%s`, use shared_libs instead", flag)
+			}
+		} else if strings.HasPrefix(flag, "-L") {
+			ctx.PropertyErrorf(prop, "Bad flag: `%s` is not allowed", flag)
+		} else if strings.Contains(flag, " ") {
+			ctx.PropertyErrorf(prop, "Bad flag: `%s` is not an allowed multi-word flag. Should it be split into multiple flags?", flag)
+		}
+	}
+}
+
+// Check for bad host_ldlibs
+func CheckBadHostLdlibs(ctx ModuleContext, prop string, flags []string) {
+	allowed_ldlibs := ctx.toolchain().AvailableLibraries()
+
+	if !ctx.Host() {
+		panic("Invalid call to CheckBadHostLdlibs")
+	}
+
+	for _, flag := range flags {
+		flag = strings.TrimSpace(flag)
+
+		// TODO: Probably should just redo this property to prefix -l in Soong
+		if !strings.HasPrefix(flag, "-l") {
+			ctx.PropertyErrorf(prop, "Invalid flag: `%s`, must start with `-l`", flag)
+		} else if !inList(flag, allowed_ldlibs) {
+			ctx.PropertyErrorf(prop, "Host library `%s` not available", flag)
+		}
+	}
+}

cc/toolchain.go

@@ -76,6 +76,8 @@ type Toolchain interface {
 	SystemCppLdflags() string
 
 	AddressSanitizerRuntimeLibrary() string
+
+	AvailableLibraries() []string
 }
 
 type toolchainBase struct {
@@ -139,6 +141,10 @@ func (toolchainBase) AddressSanitizerRuntimeLibrary() string {
 	return ""
 }
 
+func (toolchainBase) AvailableLibraries() []string {
+	return []string{}
+}
+
 type toolchain64Bit struct {
 	toolchainBase
 }

cc/util.go

@@ -119,3 +119,10 @@ func variantOrDefault(variants map[string]string, choice string) string {
 	}
 	return variants[""]
 }
+
+func addPrefix(list []string, prefix string) []string {
+	for i := range list {
+		list[i] = prefix + list[i]
+	}
+	return list
+}

cc/x86_darwin_host.go

@@ -84,6 +84,12 @@ var (
 		"10.10",
 		"10.11",
 	}
+
+	darwinAvailableLibraries = addPrefix([]string{
+		"c",
+		"m",
+		"pthread",
+	}, "-l")
 )
 
 const (
@@ -258,6 +264,10 @@ func (t *toolchainDarwin) SystemCppLdflags() string {
 	return "${darwinSystemCppLdflags}"
 }
 
+func (t *toolchainDarwin) AvailableLibraries() []string {
+	return darwinAvailableLibraries
+}
+
 var toolchainDarwinX86Singleton Toolchain = &toolchainDarwinX86{}
 var toolchainDarwinX8664Singleton Toolchain = &toolchainDarwinX8664{}
 

cc/x86_linux_host.go

@@ -97,6 +97,20 @@ var (
 	linuxX8664ClangCppflags = []string{
 		"-isystem ${linuxGccRoot}/${linuxGccTriple}/include/c++/${linuxGccVersion}/${linuxGccTriple}",
 	}
+
+	linuxAvailableLibraries = addPrefix([]string{
+		"c",
+		"dl",
+		"gcc",
+		"gcc_s",
+		"m",
+		"ncurses",
+		"pthread",
+		"resolv",
+		"rt",
+		"util",
+		"z",
+	}, "-l")
 )
 
 const (
@@ -224,6 +238,10 @@ func (t *toolchainLinuxX8664) ClangLdflags() string {
 	return "${linuxClangLdflags} ${linuxX8664ClangLdflags}"
 }
 
+func (t *toolchainLinux) AvailableLibraries() []string {
+	return linuxAvailableLibraries
+}
+
 var toolchainLinuxX86Singleton Toolchain = &toolchainLinuxX86{}
 var toolchainLinuxX8664Singleton Toolchain = &toolchainLinuxX8664{}