diff options
author | Dan Willemsen <dwillemsen@google.com> | 2016-05-25 14:47:21 -0700 |
---|---|---|
committer | Dan Willemsen <dwillemsen@google.com> | 2016-05-25 17:50:05 -0700 |
commit | 20acc5c520c8d6021824b3f839000db45be36a6e (patch) | |
tree | ca1698bd3e5f55b3128a595cf1b4f91bfc0ab24a /cc | |
parent | b36ab1a1a0355d15d692e2b6ff5955cae1275174 (diff) | |
download | build_soong-20acc5c520c8d6021824b3f839000db45be36a6e.tar.gz build_soong-20acc5c520c8d6021824b3f839000db45be36a6e.tar.bz2 build_soong-20acc5c520c8d6021824b3f839000db45be36a6e.zip |
Add flag property checking
Some checks for common errors with user-provided compiler and linker
flags:
* Using -I instead of include_dirs
* Using -l<lib> in ldflags instead of host_ldlibs (or shared_libs)
* Using -L in ldflags
* Splitting a multi-word flag into two flags
* Combining two flags into one list entry
* Using a path that could search outside the source or output trees
* Using a non-whitelisted library in host_ldlibs
Maybe some of the flag checks should happen during a static analysis
pass, but we don't have one right now, and this only adds ~1/2 second to
our 73 second Mega_device runs (recompile the changed code, run
soong_build, then report unknown target).
Change-Id: Icea7436260f1caa62c0cec29817a1cfea27b3e7c
Diffstat (limited to 'cc')
-rw-r--r-- | cc/cc.go | 14 | ||||
-rw-r--r-- | cc/check.go | 96 | ||||
-rw-r--r-- | cc/toolchain.go | 6 | ||||
-rw-r--r-- | cc/util.go | 7 | ||||
-rw-r--r-- | cc/x86_darwin_host.go | 10 | ||||
-rw-r--r-- | cc/x86_linux_host.go | 18 |
6 files changed, 151 insertions, 0 deletions
@@ -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() { diff --git a/cc/check.go b/cc/check.go new file mode 100644 index 00000000..e6a80361 --- /dev/null +++ b/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) + } + } +} diff --git a/cc/toolchain.go b/cc/toolchain.go index d85f1ce7..efb79536 100644 --- a/cc/toolchain.go +++ b/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 } @@ -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 +} diff --git a/cc/x86_darwin_host.go b/cc/x86_darwin_host.go index bd164e35..f3cf1c97 100644 --- a/cc/x86_darwin_host.go +++ b/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{} diff --git a/cc/x86_linux_host.go b/cc/x86_linux_host.go index 6562e6a5..a75de900 100644 --- a/cc/x86_linux_host.go +++ b/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{} |