From 75f76baea27ef11fa226c5686ab3946a12932d2e Mon Sep 17 00:00:00 2001 From: Mark Seaborn Date: Wed, 10 Oct 2012 17:44:43 -0700 Subject: [PATCH 001/295] PNaCl: Add a pass to convert llvm.global_ctors to __init_array_start/end This pass converts LLVM's special symbols llvm.global_ctors and llvm.global_dtors to concrete arrays, __init_array_start/end and __fini_array_start/end, that are usable by a C library. The pass sorts the contents of global_ctors/dtors according to the priority values they contain and removes the priority values. This pass will allow us to simplify the PNaCl ABI so that less is done at translation time. BUG= http://code.google.com/p/nativeclient/issues/detail?id=3018 TEST=test/Transforms/NaCl/expand-ctors*.ll Review URL: https://codereview.chromium.org/10977030 (cherry picked from commit 7ef7d6db32b8a009c749a4d4f070cc7a72b1f3ff) Conflicts: include/llvm/InitializePasses.h lib/Transforms/CMakeLists.txt lib/Transforms/LLVMBuild.txt lib/Transforms/Makefile --- include/llvm/InitializePasses.h | 1 + include/llvm/Transforms/NaCl.h | 21 +++ lib/Transforms/CMakeLists.txt | 1 + lib/Transforms/LLVMBuild.txt | 2 +- lib/Transforms/Makefile | 2 +- lib/Transforms/NaCl/CMakeLists.txt | 5 + lib/Transforms/NaCl/ExpandCtors.cpp | 143 +++++++++++++++++++++ lib/Transforms/NaCl/LLVMBuild.txt | 23 ++++ lib/Transforms/NaCl/Makefile | 15 +++ test/Transforms/NaCl/expand-ctors-empty.ll | 11 ++ test/Transforms/NaCl/expand-ctors.ll | 36 ++++++ tools/opt/opt.cpp | 1 + 12 files changed, 259 insertions(+), 2 deletions(-) create mode 100644 include/llvm/Transforms/NaCl.h create mode 100644 lib/Transforms/NaCl/CMakeLists.txt create mode 100644 lib/Transforms/NaCl/ExpandCtors.cpp create mode 100644 lib/Transforms/NaCl/LLVMBuild.txt create mode 100644 lib/Transforms/NaCl/Makefile create mode 100644 test/Transforms/NaCl/expand-ctors-empty.ll create mode 100644 test/Transforms/NaCl/expand-ctors.ll diff --git a/include/llvm/InitializePasses.h b/include/llvm/InitializePasses.h index 923571ea4b90..2936992c12be 100644 --- a/include/llvm/InitializePasses.h +++ b/include/llvm/InitializePasses.h @@ -268,6 +268,7 @@ void initializeSLPVectorizerPass(PassRegistry&); void initializeBBVectorizePass(PassRegistry&); void initializeMachineFunctionPrinterPassPass(PassRegistry&); void initializeStackMapLivenessPass(PassRegistry&); +void initializeExpandCtorsPass(PassRegistry&); // @LOCALMOD } #endif diff --git a/include/llvm/Transforms/NaCl.h b/include/llvm/Transforms/NaCl.h new file mode 100644 index 000000000000..fe29463a8bc2 --- /dev/null +++ b/include/llvm/Transforms/NaCl.h @@ -0,0 +1,21 @@ +//===-- NaCl.h - NaCl Transformations ---------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_TRANSFORMS_NACL_H +#define LLVM_TRANSFORMS_NACL_H + +namespace llvm { + +class ModulePass; + +ModulePass *createExpandCtorsPass(); + +} + +#endif diff --git a/lib/Transforms/CMakeLists.txt b/lib/Transforms/CMakeLists.txt index 2bb6e9059094..328bc13cdd08 100644 --- a/lib/Transforms/CMakeLists.txt +++ b/lib/Transforms/CMakeLists.txt @@ -6,3 +6,4 @@ add_subdirectory(IPO) add_subdirectory(Vectorize) add_subdirectory(Hello) add_subdirectory(ObjCARC) +add_subdirectory(NaCl) diff --git a/lib/Transforms/LLVMBuild.txt b/lib/Transforms/LLVMBuild.txt index 15e9fba0a765..3594de54a257 100644 --- a/lib/Transforms/LLVMBuild.txt +++ b/lib/Transforms/LLVMBuild.txt @@ -16,7 +16,7 @@ ;===------------------------------------------------------------------------===; [common] -subdirectories = IPO InstCombine Instrumentation Scalar Utils Vectorize ObjCARC +subdirectories = IPO InstCombine Instrumentation Scalar Utils Vectorize ObjCARC NaCl [component_0] type = Group diff --git a/lib/Transforms/Makefile b/lib/Transforms/Makefile index c390517d07cd..b4bb7cba723b 100644 --- a/lib/Transforms/Makefile +++ b/lib/Transforms/Makefile @@ -8,7 +8,7 @@ ##===----------------------------------------------------------------------===## LEVEL = ../.. -PARALLEL_DIRS = Utils Instrumentation Scalar InstCombine IPO Vectorize Hello ObjCARC +PARALLEL_DIRS = Utils Instrumentation Scalar InstCombine IPO Vectorize Hello ObjCARC NaCl include $(LEVEL)/Makefile.config diff --git a/lib/Transforms/NaCl/CMakeLists.txt b/lib/Transforms/NaCl/CMakeLists.txt new file mode 100644 index 000000000000..d634ad96554b --- /dev/null +++ b/lib/Transforms/NaCl/CMakeLists.txt @@ -0,0 +1,5 @@ +add_llvm_library(LLVMTransformsNaCl + ExpandCtors.cpp + ) + +add_dependencies(LLVMTransformsNaCl intrinsics_gen) diff --git a/lib/Transforms/NaCl/ExpandCtors.cpp b/lib/Transforms/NaCl/ExpandCtors.cpp new file mode 100644 index 000000000000..9c5f2cd2d79c --- /dev/null +++ b/lib/Transforms/NaCl/ExpandCtors.cpp @@ -0,0 +1,143 @@ +//===- ExpandCtors.cpp - Convert ctors/dtors to concrete arrays -----------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This pass converts LLVM's special symbols llvm.global_ctors and +// llvm.global_dtors to concrete arrays, __init_array_start/end and +// __fini_array_start/end, that are usable by a C library. +// +// This pass sorts the contents of global_ctors/dtors according to the +// priority values they contain and removes the priority values. +// +//===----------------------------------------------------------------------===// + +#include + +#include "llvm/Constants.h" +#include "llvm/DerivedTypes.h" +#include "llvm/Instructions.h" +#include "llvm/Module.h" +#include "llvm/Pass.h" +#include "llvm/Support/raw_ostream.h" +#include "llvm/Transforms/NaCl.h" +#include "llvm/TypeBuilder.h" + +using namespace llvm; + +namespace { + struct ExpandCtors : public ModulePass { + static char ID; // Pass identification, replacement for typeid + ExpandCtors() : ModulePass(ID) { + initializeExpandCtorsPass(*PassRegistry::getPassRegistry()); + } + + virtual bool runOnModule(Module &M); + }; +} + +char ExpandCtors::ID = 0; +INITIALIZE_PASS(ExpandCtors, "nacl-expand-ctors", + "Hook up constructor and destructor arrays to libc", + false, false) + +static void setGlobalVariableValue(Module &M, const char *Name, + Constant *Value) { + GlobalVariable *Var = M.getNamedGlobal(Name); + if (!Var) { + // This warning can happen in a program that does not use a libc + // and so does not call the functions in __init_array_start or + // __fini_array_end. Such a program might be linked with + // "-nostdlib". + errs() << "Warning: Variable " << Name << " not referenced\n"; + } else { + if (Var->hasInitializer()) { + report_fatal_error(std::string("Variable ") + Name + + " already has an initializer"); + } + Var->replaceAllUsesWith(ConstantExpr::getBitCast(Value, Var->getType())); + Var->eraseFromParent(); + } +} + +struct FuncArrayEntry { + uint64_t priority; + Constant *func; +}; + +static bool compareEntries(FuncArrayEntry Entry1, FuncArrayEntry Entry2) { + return Entry1.priority < Entry2.priority; +} + +static void defineFuncArray(Module &M, const char *LlvmArrayName, + const char *StartSymbol, + const char *EndSymbol) { + std::vector Funcs; + + GlobalVariable *Array = M.getNamedGlobal(LlvmArrayName); + if (Array && Array->hasInitializer()) { + ConstantArray *InitList = cast(Array->getInitializer()); + std::vector FuncsToSort; + for (unsigned Index = 0; Index < InitList->getNumOperands(); ++Index) { + ConstantStruct *CS = cast(InitList->getOperand(Index)); + FuncArrayEntry Entry; + Entry.priority = cast(CS->getOperand(0))->getZExtValue(); + Entry.func = CS->getOperand(1); + FuncsToSort.push_back(Entry); + } + + std::sort(FuncsToSort.begin(), FuncsToSort.end(), compareEntries); + for (std::vector::iterator Iter = FuncsToSort.begin(); + Iter != FuncsToSort.end(); + ++Iter) { + Funcs.push_back(Iter->func); + } + // No code should be referencing global_ctors/global_dtors, + // because this symbol is internal to LLVM. + Array->eraseFromParent(); + } + + Type *FuncTy = FunctionType::get(Type::getVoidTy(M.getContext()), false); + Type *FuncPtrTy = FuncTy->getPointerTo(); + ArrayType *ArrayTy = ArrayType::get(FuncPtrTy, Funcs.size()); + GlobalVariable *NewArray = + new GlobalVariable(M, ArrayTy, /* isConstant= */ true, + GlobalValue::InternalLinkage, + ConstantArray::get(ArrayTy, Funcs)); + setGlobalVariableValue(M, StartSymbol, NewArray); + // We do this last so that LLVM gives NewArray the name + // "__{init,fini}_array_start" without adding any suffixes to + // disambiguate from the original GlobalVariable's name. This is + // not essential -- it just makes the output easier to understand + // when looking at symbols for debugging. + NewArray->setName(StartSymbol); + + // We replace "__{init,fini}_array_end" with the address of the end + // of NewArray. This removes the name "__{init,fini}_array_end" + // from the output, which is not ideal for debugging. Ideally we + // would convert "__{init,fini}_array_end" to being a GlobalAlias + // that points to the end of the array. However, unfortunately LLVM + // does not generate correct code when a GlobalAlias contains a + // GetElementPtr ConstantExpr. + Constant *NewArrayEnd = + ConstantExpr::getGetElementPtr(NewArray, + ConstantInt::get(M.getContext(), + APInt(32, 1))); + setGlobalVariableValue(M, EndSymbol, NewArrayEnd); +} + +bool ExpandCtors::runOnModule(Module &M) { + defineFuncArray(M, "llvm.global_ctors", + "__init_array_start", "__init_array_end"); + defineFuncArray(M, "llvm.global_dtors", + "__fini_array_start", "__fini_array_end"); + return true; +} + +ModulePass *llvm::createExpandCtorsPass() { + return new ExpandCtors(); +} diff --git a/lib/Transforms/NaCl/LLVMBuild.txt b/lib/Transforms/NaCl/LLVMBuild.txt new file mode 100644 index 000000000000..2f1522b3e5c9 --- /dev/null +++ b/lib/Transforms/NaCl/LLVMBuild.txt @@ -0,0 +1,23 @@ +;===- ./lib/Transforms/NaCl/LLVMBuild.txt ----------------------*- Conf -*--===; +; +; The LLVM Compiler Infrastructure +; +; This file is distributed under the University of Illinois Open Source +; License. See LICENSE.TXT for details. +; +;===------------------------------------------------------------------------===; +; +; This is an LLVMBuild description file for the components in this subdirectory. +; +; For more information on the LLVMBuild system, please see: +; +; http://llvm.org/docs/LLVMBuild.html +; +;===------------------------------------------------------------------------===; + +[component_0] +type = Library +name = NaCl +parent = Transforms +library_name = NaCl +required_libraries = Core diff --git a/lib/Transforms/NaCl/Makefile b/lib/Transforms/NaCl/Makefile new file mode 100644 index 000000000000..ecf8db6eae1e --- /dev/null +++ b/lib/Transforms/NaCl/Makefile @@ -0,0 +1,15 @@ +##===- lib/Transforms/NaCl/Makefile-------------------------*- Makefile -*-===## +# +# The LLVM Compiler Infrastructure +# +# This file is distributed under the University of Illinois Open Source +# License. See LICENSE.TXT for details. +# +##===----------------------------------------------------------------------===## + +LEVEL = ../../.. +LIBRARYNAME = LLVMTransformsNaCl +BUILD_ARCHIVE = 1 + +include $(LEVEL)/Makefile.common + diff --git a/test/Transforms/NaCl/expand-ctors-empty.ll b/test/Transforms/NaCl/expand-ctors-empty.ll new file mode 100644 index 000000000000..4368270765b7 --- /dev/null +++ b/test/Transforms/NaCl/expand-ctors-empty.ll @@ -0,0 +1,11 @@ +; Currently we do not define __{init,fini}_array_end as named aliases. +; RUN: opt < %s -nacl-expand-ctors -S | not grep __init_array_end +; RUN: opt < %s -nacl-expand-ctors -S | not grep __fini_array_end + +; RUN: opt < %s -nacl-expand-ctors -S | FileCheck %s + +; If llvm.global_ctors is not present, it is treated as if it is an +; empty array, and __{init,fini}_array_start are defined anyway. + +; CHECK: @__init_array_start = internal constant [0 x void ()*] zeroinitializer +; CHECK: @__fini_array_start = internal constant [0 x void ()*] zeroinitializer diff --git a/test/Transforms/NaCl/expand-ctors.ll b/test/Transforms/NaCl/expand-ctors.ll new file mode 100644 index 000000000000..7f202618e7dc --- /dev/null +++ b/test/Transforms/NaCl/expand-ctors.ll @@ -0,0 +1,36 @@ +; We expect these symbol names to be removed: +; RUN: opt < %s -nacl-expand-ctors -S | not grep llvm.global_ctors +; RUN: opt < %s -nacl-expand-ctors -S | not grep __init_array_end +; RUN: opt < %s -nacl-expand-ctors -S | not grep __fini_array_end + +; RUN: opt < %s -nacl-expand-ctors -S | FileCheck %s + +@llvm.global_ctors = appending global [3 x { i32, void ()* }] + [{ i32, void ()* } { i32 300, void ()* @init_func_A }, + { i32, void ()* } { i32 100, void ()* @init_func_B }, + { i32, void ()* } { i32 200, void ()* @init_func_C }] + +@__init_array_start = extern_weak global [0 x void ()*] +@__init_array_end = extern_weak global [0 x void ()*] + +; CHECK: @__init_array_start = internal constant [3 x void ()*] [void ()* @init_func_B, void ()* @init_func_C, void ()* @init_func_A] +; CHECK: @__fini_array_start = internal constant [0 x void ()*] zeroinitializer + +define void @init_func_A() { ret void } +define void @init_func_B() { ret void } +define void @init_func_C() { ret void } + +define [0 x void ()*]* @get_array_start() { + ret [0 x void ()*]* @__init_array_start; +} +; CHECK: @get_array_start() +; CHECK: ret {{.*}} @__init_array_start + +define [0 x void ()*]* @get_array_end() { + ret [0 x void ()*]* @__init_array_end; +} + +; @get_array_end() is converted to use a GetElementPtr that returns +; the end of the generated array: +; CHECK: @get_array_end() +; CHECK: ret {{.*}} bitcast ([3 x void ()*]* getelementptr inbounds ([3 x void ()*]* @__init_array_start, i32 1) diff --git a/tools/opt/opt.cpp b/tools/opt/opt.cpp index 49bb3c3b19fc..b111860860b8 100644 --- a/tools/opt/opt.cpp +++ b/tools/opt/opt.cpp @@ -421,6 +421,7 @@ int main(int argc, char **argv) { initializeInstCombine(Registry); initializeInstrumentation(Registry); initializeTarget(Registry); + initializeExpandCtorsPass(Registry); cl::ParseCommandLineOptions(argc, argv, "llvm .bc -> .bc modular optimizer and analysis printer\n"); From 3e2c77d101c83d940e540beba4b1a8c4db36b8ea Mon Sep 17 00:00:00 2001 From: Mark Seaborn Date: Wed, 17 Oct 2012 19:29:37 -0700 Subject: [PATCH 002/295] PNaCl: Fix nacl-expand-ctors pass to handle llvm.global_ctors being zeroinitializer This case can happen if optimisation removes initializers. BUG=http://code.google.com/p/nativeclient/issues/detail?id=3018 TEST=test/Transforms/NaCl/expand-ctors-zeroinit.ll Review URL: https://codereview.chromium.org/11190028 (cherry picked from commit 19bfdca1e43752e134bafb3014508f66003b8873) --- lib/Transforms/NaCl/ExpandCtors.cpp | 34 ++++++++++--------- test/Transforms/NaCl/expand-ctors-zeroinit.ll | 16 +++++++++ 2 files changed, 34 insertions(+), 16 deletions(-) create mode 100644 test/Transforms/NaCl/expand-ctors-zeroinit.ll diff --git a/lib/Transforms/NaCl/ExpandCtors.cpp b/lib/Transforms/NaCl/ExpandCtors.cpp index 9c5f2cd2d79c..6b8130e4fb33 100644 --- a/lib/Transforms/NaCl/ExpandCtors.cpp +++ b/lib/Transforms/NaCl/ExpandCtors.cpp @@ -79,22 +79,24 @@ static void defineFuncArray(Module &M, const char *LlvmArrayName, std::vector Funcs; GlobalVariable *Array = M.getNamedGlobal(LlvmArrayName); - if (Array && Array->hasInitializer()) { - ConstantArray *InitList = cast(Array->getInitializer()); - std::vector FuncsToSort; - for (unsigned Index = 0; Index < InitList->getNumOperands(); ++Index) { - ConstantStruct *CS = cast(InitList->getOperand(Index)); - FuncArrayEntry Entry; - Entry.priority = cast(CS->getOperand(0))->getZExtValue(); - Entry.func = CS->getOperand(1); - FuncsToSort.push_back(Entry); - } - - std::sort(FuncsToSort.begin(), FuncsToSort.end(), compareEntries); - for (std::vector::iterator Iter = FuncsToSort.begin(); - Iter != FuncsToSort.end(); - ++Iter) { - Funcs.push_back(Iter->func); + if (Array) { + if (Array->hasInitializer() && !Array->getInitializer()->isNullValue()) { + ConstantArray *InitList = cast(Array->getInitializer()); + std::vector FuncsToSort; + for (unsigned Index = 0; Index < InitList->getNumOperands(); ++Index) { + ConstantStruct *CS = cast(InitList->getOperand(Index)); + FuncArrayEntry Entry; + Entry.priority = cast(CS->getOperand(0))->getZExtValue(); + Entry.func = CS->getOperand(1); + FuncsToSort.push_back(Entry); + } + + std::sort(FuncsToSort.begin(), FuncsToSort.end(), compareEntries); + for (std::vector::iterator Iter = FuncsToSort.begin(); + Iter != FuncsToSort.end(); + ++Iter) { + Funcs.push_back(Iter->func); + } } // No code should be referencing global_ctors/global_dtors, // because this symbol is internal to LLVM. diff --git a/test/Transforms/NaCl/expand-ctors-zeroinit.ll b/test/Transforms/NaCl/expand-ctors-zeroinit.ll new file mode 100644 index 000000000000..d02741f0b517 --- /dev/null +++ b/test/Transforms/NaCl/expand-ctors-zeroinit.ll @@ -0,0 +1,16 @@ +; Currently we do not define __{init,fini}_array_end as named aliases. +; RUN: opt < %s -nacl-expand-ctors -S | not grep __init_array_end +; RUN: opt < %s -nacl-expand-ctors -S | not grep __fini_array_end + +; We expect this symbol to be removed: +; RUN: opt < %s -nacl-expand-ctors -S | not grep llvm.global_ctors + +; RUN: opt < %s -nacl-expand-ctors -S | FileCheck %s + +; If llvm.global_ctors is zeroinitializer, it should be treated the +; same as an empty array. + +@llvm.global_ctors = appending global [0 x { i32, void ()* }] zeroinitializer + +; CHECK: @__init_array_start = internal constant [0 x void ()*] zeroinitializer +; CHECK: @__fini_array_start = internal constant [0 x void ()*] zeroinitializer From cc78929f118230d1fb141a0c10ea1a78fd9dd8a3 Mon Sep 17 00:00:00 2001 From: Mark Seaborn Date: Thu, 29 Nov 2012 18:42:19 -0800 Subject: [PATCH 003/295] PNaCl: Add ExpandTls pass for expanding out static TLS variables This replaces each reference to a TLS variable "foo" with the LLVM IR equivalent of the expression: ((struct tls_template *) __nacl_read_tp())->foo This pass fills out the global variables __tls_template_start, __tls_template_end etc. which are used by src/untrusted/nacl/tls.c. These are the symbols that are otherwise defined by a binutils linker script. In order to handle the case of TLS variables that occur inside ConstantExprs, we have a helper pass, ExpandTlsConstantExpr. BUG=http://code.google.com/p/nativeclient/issues/detail?id=2837 TEST=test/Transforms/NaCl/expand-tls*.ll Review URL: https://chromiumcodereview.appspot.com/10896042 (cherry picked from commit ffb0eedef4f034996ce59aac3176482617a8044c) Conflicts: include/llvm/InitializePasses.h --- include/llvm/InitializePasses.h | 2 + include/llvm/Transforms/NaCl.h | 2 + lib/Transforms/NaCl/CMakeLists.txt | 2 + lib/Transforms/NaCl/ExpandTls.cpp | 351 ++++++++++++++++++ lib/Transforms/NaCl/ExpandTlsConstantExpr.cpp | 110 ++++++ test/Transforms/NaCl/expand-tls-aligned.ll | 42 +++ test/Transforms/NaCl/expand-tls-bss.ll | 17 + test/Transforms/NaCl/expand-tls-constexpr.ll | 116 ++++++ test/Transforms/NaCl/expand-tls-constexpr2.ll | 12 + test/Transforms/NaCl/expand-tls-phi.ll | 23 ++ test/Transforms/NaCl/expand-tls.ll | 85 +++++ test/Transforms/NaCl/lit.local.cfg | 3 + tools/opt/opt.cpp | 2 + 13 files changed, 767 insertions(+) create mode 100644 lib/Transforms/NaCl/ExpandTls.cpp create mode 100644 lib/Transforms/NaCl/ExpandTlsConstantExpr.cpp create mode 100644 test/Transforms/NaCl/expand-tls-aligned.ll create mode 100644 test/Transforms/NaCl/expand-tls-bss.ll create mode 100644 test/Transforms/NaCl/expand-tls-constexpr.ll create mode 100644 test/Transforms/NaCl/expand-tls-constexpr2.ll create mode 100644 test/Transforms/NaCl/expand-tls-phi.ll create mode 100644 test/Transforms/NaCl/expand-tls.ll create mode 100644 test/Transforms/NaCl/lit.local.cfg diff --git a/include/llvm/InitializePasses.h b/include/llvm/InitializePasses.h index 2936992c12be..195e5646086d 100644 --- a/include/llvm/InitializePasses.h +++ b/include/llvm/InitializePasses.h @@ -269,6 +269,8 @@ void initializeBBVectorizePass(PassRegistry&); void initializeMachineFunctionPrinterPassPass(PassRegistry&); void initializeStackMapLivenessPass(PassRegistry&); void initializeExpandCtorsPass(PassRegistry&); // @LOCALMOD +void initializeExpandTlsPass(PassRegistry&); // @LOCALMOD +void initializeExpandTlsConstantExprPass(PassRegistry&); // @LOCALMOD } #endif diff --git a/include/llvm/Transforms/NaCl.h b/include/llvm/Transforms/NaCl.h index fe29463a8bc2..79c9b9fe7949 100644 --- a/include/llvm/Transforms/NaCl.h +++ b/include/llvm/Transforms/NaCl.h @@ -15,6 +15,8 @@ namespace llvm { class ModulePass; ModulePass *createExpandCtorsPass(); +ModulePass *createExpandTlsPass(); +ModulePass *createExpandTlsConstantExprPass(); } diff --git a/lib/Transforms/NaCl/CMakeLists.txt b/lib/Transforms/NaCl/CMakeLists.txt index d634ad96554b..5e24cc7e28f2 100644 --- a/lib/Transforms/NaCl/CMakeLists.txt +++ b/lib/Transforms/NaCl/CMakeLists.txt @@ -1,5 +1,7 @@ add_llvm_library(LLVMTransformsNaCl ExpandCtors.cpp + ExpandTls.cpp + ExpandTlsConstantExpr.cpp ) add_dependencies(LLVMTransformsNaCl intrinsics_gen) diff --git a/lib/Transforms/NaCl/ExpandTls.cpp b/lib/Transforms/NaCl/ExpandTls.cpp new file mode 100644 index 000000000000..8ce439c018e1 --- /dev/null +++ b/lib/Transforms/NaCl/ExpandTls.cpp @@ -0,0 +1,351 @@ +//===- ExpandTls.cpp - Convert TLS variables to a concrete layout----------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This pass expands out uses of thread-local (TLS) variables into +// more primitive operations. +// +// A reference to the address of a TLS variable is expanded into code +// which gets the current thread's thread pointer using +// @llvm.nacl.read.tp() and adds a fixed offset. +// +// This pass allocates the offsets (relative to the thread pointer) +// that will be used for TLS variables. It sets up the global +// variables __tls_template_start, __tls_template_end etc. to contain +// a template for initializing TLS variables' values for each thread. +// This is a task normally performed by the linker in ELF systems. +// +//===----------------------------------------------------------------------===// + +#include + +#include "llvm/Constants.h" +#include "llvm/DataLayout.h" +#include "llvm/DerivedTypes.h" +#include "llvm/Instructions.h" +#include "llvm/Module.h" +#include "llvm/Pass.h" +#include "llvm/Support/raw_ostream.h" +#include "llvm/Transforms/NaCl.h" + +using namespace llvm; + +namespace { + struct VarInfo { + GlobalVariable *TlsVar; + bool IsBss; // Whether variable is in zero-intialized part of template + int TemplateIndex; + }; + + class PassState { + public: + PassState(Module *M): M(M), DL(M), Offset(0), Alignment(1) {} + + Module *M; + DataLayout DL; + uint64_t Offset; + // 'Alignment' is the maximum variable alignment seen so far, in + // bytes. After visiting all TLS variables, this is the overall + // alignment required for the TLS template. + uint32_t Alignment; + }; + + class ExpandTls : public ModulePass { + public: + static char ID; // Pass identification, replacement for typeid + ExpandTls() : ModulePass(ID) { + initializeExpandTlsPass(*PassRegistry::getPassRegistry()); + } + + virtual bool runOnModule(Module &M); + }; +} + +char ExpandTls::ID = 0; +INITIALIZE_PASS(ExpandTls, "nacl-expand-tls", + "Expand out TLS variables and fix TLS variable layout", + false, false) + +static void setGlobalVariableValue(Module &M, const char *Name, + Constant *Value) { + GlobalVariable *Var = M.getNamedGlobal(Name); + if (!Var) { + // This warning can happen in a program that does not use a libc + // and does not initialize TLS variables. Such a program might be + // linked with "-nostdlib". + errs() << "Warning: Variable " << Name << " not referenced\n"; + } else { + if (Var->hasInitializer()) { + report_fatal_error(std::string("Variable ") + Name + + " already has an initializer"); + } + Var->replaceAllUsesWith(ConstantExpr::getBitCast(Value, Var->getType())); + Var->eraseFromParent(); + } +} + +// Insert alignment padding into the TLS template. +static void padToAlignment(PassState *State, + std::vector *FieldTypes, + std::vector *FieldValues, + unsigned Alignment) { + if ((State->Offset & (Alignment - 1)) != 0) { + unsigned PadSize = Alignment - (State->Offset & (Alignment - 1)); + Type *i8 = Type::getInt8Ty(State->M->getContext()); + Type *PadType = ArrayType::get(i8, PadSize); + FieldTypes->push_back(PadType); + if (FieldValues) + FieldValues->push_back(Constant::getNullValue(PadType)); + State->Offset += PadSize; + } + if (State->Alignment < Alignment) { + State->Alignment = Alignment; + } +} + +static void addVarToTlsTemplate(PassState *State, + std::vector *FieldTypes, + std::vector *FieldValues, + GlobalVariable *TlsVar) { + unsigned Alignment = State->DL.getPreferredAlignment(TlsVar); + padToAlignment(State, FieldTypes, FieldValues, Alignment); + + FieldTypes->push_back(TlsVar->getType()->getElementType()); + if (FieldValues) + FieldValues->push_back(TlsVar->getInitializer()); + State->Offset += + State->DL.getTypeAllocSize(TlsVar->getType()->getElementType()); +} + +static PointerType *buildTlsTemplate(Module &M, std::vector *TlsVars) { + std::vector FieldBssTypes; + std::vector FieldInitTypes; + std::vector FieldInitValues; + PassState State(&M); + + for (Module::global_iterator GV = M.global_begin(); + GV != M.global_end(); + ++GV) { + if (GV->isThreadLocal()) { + if (!GV->hasInitializer()) { + // Since this is a whole-program transformation, "extern" TLS + // variables are not allowed at this point. + report_fatal_error(std::string("TLS variable without an initializer: ") + + GV->getName()); + } + if (!GV->getInitializer()->isNullValue()) { + addVarToTlsTemplate(&State, &FieldInitTypes, + &FieldInitValues, GV); + VarInfo Info; + Info.TlsVar = GV; + Info.IsBss = false; + Info.TemplateIndex = FieldInitTypes.size() - 1; + TlsVars->push_back(Info); + } + } + } + // Handle zero-initialized TLS variables in a second pass, because + // these should follow non-zero-initialized TLS variables. + for (Module::global_iterator GV = M.global_begin(); + GV != M.global_end(); + ++GV) { + if (GV->isThreadLocal() && GV->getInitializer()->isNullValue()) { + addVarToTlsTemplate(&State, &FieldBssTypes, NULL, GV); + VarInfo Info; + Info.TlsVar = GV; + Info.IsBss = true; + Info.TemplateIndex = FieldBssTypes.size() - 1; + TlsVars->push_back(Info); + } + } + // Add final alignment padding so that + // (struct tls_struct *) __nacl_read_tp() - 1 + // gives the correct, aligned start of the TLS variables given the + // x86-style layout we are using. This requires some more bytes to + // be memset() to zero at runtime. This wastage doesn't seem + // important gives that we're not trying to optimize packing by + // reordering to put similarly-aligned variables together. + padToAlignment(&State, &FieldBssTypes, NULL, State.Alignment); + + // We create the TLS template structs as "packed" because we insert + // alignment padding ourselves, and LLVM's implicit insertion of + // padding would interfere with ours. tls_bss_template can start at + // a non-aligned address immediately following the last field in + // tls_init_template. + StructType *InitTemplateType = + StructType::create(M.getContext(), "tls_init_template"); + InitTemplateType->setBody(FieldInitTypes, /*isPacked=*/true); + StructType *BssTemplateType = + StructType::create(M.getContext(), "tls_bss_template"); + BssTemplateType->setBody(FieldBssTypes, /*isPacked=*/true); + + StructType *TemplateType = StructType::create(M.getContext(), "tls_struct"); + SmallVector TemplateTopFields; + TemplateTopFields.push_back(InitTemplateType); + TemplateTopFields.push_back(BssTemplateType); + TemplateType->setBody(TemplateTopFields, /*isPacked=*/true); + PointerType *TemplatePtrType = PointerType::get(TemplateType, 0); + + // We define the following symbols, which are the same as those + // defined by NaCl's original customized binutils linker scripts: + // __tls_template_start + // __tls_template_tdata_end + // __tls_template_end + // We also define __tls_template_alignment, which was not defined by + // the original linker scripts. + + const char *StartSymbol = "__tls_template_start"; + Constant *TemplateData = ConstantStruct::get(InitTemplateType, + FieldInitValues); + GlobalVariable *TemplateDataVar = + new GlobalVariable(M, InitTemplateType, /*isConstant=*/true, + GlobalValue::InternalLinkage, TemplateData); + setGlobalVariableValue(M, StartSymbol, TemplateDataVar); + TemplateDataVar->setName(StartSymbol); + + Constant *TdataEnd = ConstantExpr::getGetElementPtr( + TemplateDataVar, + ConstantInt::get(M.getContext(), APInt(32, 1))); + setGlobalVariableValue(M, "__tls_template_tdata_end", TdataEnd); + + Constant *TotalEnd = ConstantExpr::getGetElementPtr( + ConstantExpr::getBitCast(TemplateDataVar, TemplatePtrType), + ConstantInt::get(M.getContext(), APInt(32, 1))); + setGlobalVariableValue(M, "__tls_template_end", TotalEnd); + + const char *AlignmentSymbol = "__tls_template_alignment"; + Type *i32 = Type::getInt32Ty(M.getContext()); + GlobalVariable *AlignmentVar = new GlobalVariable( + M, i32, /*isConstant=*/true, + GlobalValue::InternalLinkage, + ConstantInt::get(M.getContext(), APInt(32, State.Alignment))); + setGlobalVariableValue(M, AlignmentSymbol, AlignmentVar); + AlignmentVar->setName(AlignmentSymbol); + + return TemplatePtrType; +} + +static void rewriteTlsVars(Module &M, std::vector *TlsVars, + PointerType *TemplatePtrType) { + // Set up the intrinsic that reads the thread pointer. + Type *i8 = Type::getInt8Ty(M.getContext()); + FunctionType *ReadTpType = FunctionType::get(PointerType::get(i8, 0), + /*isVarArg=*/false); + AttrBuilder B; + B.addAttribute(Attributes::ReadOnly); + B.addAttribute(Attributes::NoUnwind); + AttrListPtr ReadTpAttrs = AttrListPtr().addAttr( + M.getContext(), AttrListPtr::FunctionIndex, + Attributes::get(M.getContext(), B)); + Constant *ReadTpFunc = M.getOrInsertTargetIntrinsic("llvm.nacl.read.tp", + ReadTpType, + ReadTpAttrs); + + for (std::vector::iterator VarInfo = TlsVars->begin(); + VarInfo != TlsVars->end(); + ++VarInfo) { + GlobalVariable *Var = VarInfo->TlsVar; + while (!Var->use_empty()) { + Instruction *U = cast(*Var->use_begin()); + Instruction *InsertPt = U; + if (PHINode *PN = dyn_cast(InsertPt)) { + // We cannot insert instructions before a PHI node, so insert + // before the incoming block's terminator. Note that if the + // terminator is conditional, this could be suboptimal, + // because we might be calling ReadTpFunc unnecessarily. + InsertPt = PN->getIncomingBlock(Var->use_begin())->getTerminator(); + } + Value *RawThreadPtr = CallInst::Create(ReadTpFunc, "tls_raw", InsertPt); + Value *TypedThreadPtr = new BitCastInst(RawThreadPtr, TemplatePtrType, + "tls_struct", InsertPt); + SmallVector Indexes; + // We use -1 because we use the x86-style TLS layout in which + // the TLS data is stored at addresses below the thread pointer. + // This is largely because a check in nacl_irt_thread_create() + // in irt/irt_thread.c requires the thread pointer to be a + // self-pointer on x86-32. + // TODO(mseaborn): I intend to remove that check because it is + // non-portable. In the mean time, we want PNaCl pexes to work + // in older Chromium releases when translated to nexes. + Indexes.push_back(ConstantInt::get( + M.getContext(), APInt(32, -1))); + Indexes.push_back(ConstantInt::get( + M.getContext(), APInt(32, VarInfo->IsBss ? 1 : 0))); + Indexes.push_back(ConstantInt::get( + M.getContext(), APInt(32, VarInfo->TemplateIndex))); + Value *TlsField = GetElementPtrInst::Create(TypedThreadPtr, Indexes, + "field", InsertPt); + U->replaceUsesOfWith(Var, TlsField); + } + VarInfo->TlsVar->eraseFromParent(); + } +} + +// Provide fixed definitions for PNaCl's TLS layout intrinsics. We +// adopt the x86-style layout: ExpandTls will output a program that +// uses the x86-style layout wherever it runs. This overrides any +// architecture-specific definitions of the intrinsics that the LLVM +// backend might provide. +static void defineTlsLayoutIntrinsics(Module &M) { + Type *i32 = Type::getInt32Ty(M.getContext()); + SmallVector ArgTypes; + ArgTypes.push_back(i32); + FunctionType *FuncType = FunctionType::get(i32, ArgTypes, /*isVarArg=*/false); + Function *NewFunc; + BasicBlock *BB; + + // Define the intrinsic as follows: + // uint32_t __nacl_tp_tdb_offset(uint32_t tdb_size) { + // return 0; + // } + // This means the thread pointer points to the TDB. + NewFunc = Function::Create(FuncType, GlobalValue::InternalLinkage, + "nacl_tp_tdb_offset", &M); + BB = BasicBlock::Create(M.getContext(), "entry", NewFunc); + ReturnInst::Create(M.getContext(), + ConstantInt::get(M.getContext(), APInt(32, 0)), BB); + if (Function *Intrinsic = M.getFunction("llvm.nacl.tp.tdb.offset")) { + Intrinsic->replaceAllUsesWith(NewFunc); + Intrinsic->eraseFromParent(); + } + + // Define the intrinsic as follows: + // uint32_t __nacl_tp_tls_offset(uint32_t tls_size) { + // return -tls_size; + // } + // This means the TLS variables are stored below the thread pointer. + NewFunc = Function::Create(FuncType, GlobalValue::InternalLinkage, + "nacl_tp_tls_offset", &M); + BB = BasicBlock::Create(M.getContext(), "entry", NewFunc); + Value *Arg = NewFunc->arg_begin(); + Arg->setName("size"); + Value *Result = BinaryOperator::CreateNeg(Arg, "result", BB); + ReturnInst::Create(M.getContext(), Result, BB); + if (Function *Intrinsic = M.getFunction("llvm.nacl.tp.tls.offset")) { + Intrinsic->replaceAllUsesWith(NewFunc); + Intrinsic->eraseFromParent(); + } +} + +bool ExpandTls::runOnModule(Module &M) { + ModulePass *Pass = createExpandTlsConstantExprPass(); + Pass->runOnModule(M); + delete Pass; + + std::vector TlsVars; + PointerType *TemplatePtrType = buildTlsTemplate(M, &TlsVars); + rewriteTlsVars(M, &TlsVars, TemplatePtrType); + + defineTlsLayoutIntrinsics(M); + + return true; +} + +ModulePass *llvm::createExpandTlsPass() { + return new ExpandTls(); +} diff --git a/lib/Transforms/NaCl/ExpandTlsConstantExpr.cpp b/lib/Transforms/NaCl/ExpandTlsConstantExpr.cpp new file mode 100644 index 000000000000..90e007604f6f --- /dev/null +++ b/lib/Transforms/NaCl/ExpandTlsConstantExpr.cpp @@ -0,0 +1,110 @@ +//===- ExpandTlsConstantExpr.cpp - Convert ConstantExprs to Instructions---===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This pass is a helper used by the ExpandTls pass. +// +// LLVM treats the address of a TLS variable as a ConstantExpr. This +// is arguably a bug because the address of a TLS variable is *not* a +// constant: it varies between threads. +// +// See http://llvm.org/bugs/show_bug.cgi?id=14353 +// +// This is also a problem for the ExpandTls pass, which wants to use +// replaceUsesOfWith() to replace each TLS variable with an +// Instruction sequence that calls @llvm.nacl.read.tp(). This doesn't +// work if the TLS variable is used inside other ConstantExprs, +// because ConstantExprs are interned and are not associated with any +// function, whereas each Instruction must be part of a function. +// +// To fix that problem, this pass converts ConstantExprs that +// reference TLS variables into Instructions. +// +// For example, this use of a 'ptrtoint' ConstantExpr: +// +// ret i32 ptrtoint (i32* @tls_var to i32) +// +// is converted into this 'ptrtoint' Instruction: +// +// %expanded = ptrtoint i32* @tls_var to i32 +// ret i32 %expanded +// +//===----------------------------------------------------------------------===// + +#include + +#include "llvm/Constants.h" +#include "llvm/Instructions.h" +#include "llvm/Module.h" +#include "llvm/Pass.h" +#include "llvm/Transforms/NaCl.h" + +using namespace llvm; + +namespace { + class ExpandTlsConstantExpr : public ModulePass { + public: + static char ID; // Pass identification, replacement for typeid + ExpandTlsConstantExpr() : ModulePass(ID) { + initializeExpandTlsConstantExprPass(*PassRegistry::getPassRegistry()); + } + + virtual bool runOnModule(Module &M); + }; +} + +char ExpandTlsConstantExpr::ID = 0; +INITIALIZE_PASS(ExpandTlsConstantExpr, "nacl-expand-tls-constant-expr", + "Eliminate ConstantExpr references to TLS variables", + false, false) + +// This removes ConstantExpr references to the given Constant. +static void expandConstExpr(Constant *Expr) { + // First, ensure that ConstantExpr references to Expr are converted + // to Instructions so that we can modify them. + for (Value::use_iterator UI = Expr->use_begin(); + UI != Expr->use_end(); + ++UI) { + if (ConstantExpr *CE = dyn_cast(*UI)) { + expandConstExpr(CE); + } + } + Expr->removeDeadConstantUsers(); + + if (ConstantExpr *CE = dyn_cast(Expr)) { + while (!Expr->use_empty()) { + Instruction *U = cast(*Expr->use_begin()); + Instruction *InsertPt = U; + if (PHINode *PN = dyn_cast(InsertPt)) { + // We cannot insert instructions before a PHI node, so insert + // before the incoming block's terminator. This could be + // suboptimal if the terminator is a conditional. + InsertPt = PN->getIncomingBlock(Expr->use_begin())->getTerminator(); + } + Instruction *NewInst = CE->getAsInstruction(); + NewInst->insertBefore(InsertPt); + NewInst->setName("expanded"); + U->replaceUsesOfWith(CE, NewInst); + } + } +} + +bool ExpandTlsConstantExpr::runOnModule(Module &M) { + for (Module::global_iterator Global = M.global_begin(); + Global != M.global_end(); + ++Global) { + if (Global->isThreadLocal()) { + expandConstExpr(Global); + } + } + return true; +} + +ModulePass *llvm::createExpandTlsConstantExprPass() { + return new ExpandTlsConstantExpr(); +} diff --git a/test/Transforms/NaCl/expand-tls-aligned.ll b/test/Transforms/NaCl/expand-tls-aligned.ll new file mode 100644 index 000000000000..75f03ba306ff --- /dev/null +++ b/test/Transforms/NaCl/expand-tls-aligned.ll @@ -0,0 +1,42 @@ +; RUN: opt < %s -nacl-expand-tls -S | FileCheck %s + +target datalayout = "p:32:32:32" + + +@var = global i32 123 + +; Put this first to check that the pass handles BSS variables last. +@bss_tvar_aligned = thread_local global i32 0, align 64 + +@tvar1 = thread_local global i16 234 +; Test a pointer to check we are getting the right pointer size. +@tvar2 = thread_local global i32* @var +@tvar_aligned = thread_local global i8 99, align 32 + + +; CHECK: %tls_init_template = type <{ i16, [2 x i8], i32*, [24 x i8], i8 }> +; CHECK: %tls_struct = type <{ %tls_init_template, %tls_bss_template }> + +; This struct type must be "packed" because the 31 byte padding here +; is followed by an i32. +; CHECK: %tls_bss_template = type <{ [31 x i8], i32, [60 x i8] }> + +; CHECK: @__tls_template_start = internal constant %tls_init_template <{ i16 234, [2 x i8] zeroinitializer, i32* @var, [24 x i8] zeroinitializer, i8 99 }> + +; CHECK: @__tls_template_alignment = internal constant i32 64 + + +; Create references to __tls_template_* to keep these live, otherwise +; the definition of %tls_struct (which we check for above) is removed +; from the output. + +@__tls_template_tdata_end = external global i8 +@__tls_template_end = external global i8 + +define i8* @get_tls_template_tdata_end() { + ret i8* @__tls_template_tdata_end +} + +define i8* @get_tls_template_end() { + ret i8* @__tls_template_end +} diff --git a/test/Transforms/NaCl/expand-tls-bss.ll b/test/Transforms/NaCl/expand-tls-bss.ll new file mode 100644 index 000000000000..02504611f091 --- /dev/null +++ b/test/Transforms/NaCl/expand-tls-bss.ll @@ -0,0 +1,17 @@ +; RUN: opt < %s -nacl-expand-tls -S | FileCheck %s + + +@tvar_bss1 = thread_local global i64 0 +@tvar_bss2 = thread_local global i32 0 + + +; CHECK: %tls_struct = type <{ %tls_init_template, %tls_bss_template }> +; CHECK: %tls_bss_template = type <{ i64, i32, [4 x i8] }> + + +define i64* @get_tvar_bss1() { + ret i64* @tvar_bss1 +} +; CHECK: define i64* @get_tvar_bss1() +; CHECK: %field = getelementptr %tls_struct* %tls_struct, i32 -1, i32 1, i32 0 +; CHECK: ret i64* %field diff --git a/test/Transforms/NaCl/expand-tls-constexpr.ll b/test/Transforms/NaCl/expand-tls-constexpr.ll new file mode 100644 index 000000000000..06bb8ed83018 --- /dev/null +++ b/test/Transforms/NaCl/expand-tls-constexpr.ll @@ -0,0 +1,116 @@ +; RUN: opt < %s -nacl-expand-tls-constant-expr -S | FileCheck %s + +@tvar = thread_local global i32 0 + + +define i32 @test_converting_ptrtoint() { + ret i32 ptrtoint (i32* @tvar to i32) +} +; CHECK: define i32 @test_converting_ptrtoint() +; CHECK: %expanded = ptrtoint i32* @tvar to i32 +; CHECK: ret i32 %expanded + + +define i32 @test_converting_add() { + ret i32 add (i32 ptrtoint (i32* @tvar to i32), i32 4) +} +; CHECK: define i32 @test_converting_add() +; CHECK: %expanded1 = ptrtoint i32* @tvar to i32 +; CHECK: %expanded = add i32 %expanded1, 4 +; CHECK: ret i32 %expanded + + +define i32 @test_converting_multiple_operands() { + ret i32 add (i32 ptrtoint (i32* @tvar to i32), + i32 ptrtoint (i32* @tvar to i32)) +} +; CHECK: define i32 @test_converting_multiple_operands() +; CHECK: %expanded1 = ptrtoint i32* @tvar to i32 +; CHECK: %expanded = add i32 %expanded1, %expanded1 +; CHECK: ret i32 %expanded + + +define i32 @test_allocating_new_var_name(i32 %expanded) { + %result = add i32 %expanded, ptrtoint (i32* @tvar to i32) + ret i32 %result +} +; CHECK: define i32 @test_allocating_new_var_name(i32 %expanded) +; CHECK: %expanded1 = ptrtoint i32* @tvar to i32 +; CHECK: %result = add i32 %expanded, %expanded1 +; CHECK: ret i32 %result + + +define i8* @test_converting_bitcast() { + ret i8* bitcast (i32* @tvar to i8*) +} +; CHECK: define i8* @test_converting_bitcast() +; CHECK: %expanded = bitcast i32* @tvar to i8* +; CHECK: ret i8* %expanded + + +define i32* @test_converting_getelementptr() { + ; Use an index >1 to ensure that "inbounds" is not added automatically. + ret i32* getelementptr (i32* @tvar, i32 2) +} +; CHECK: define i32* @test_converting_getelementptr() +; CHECK: %expanded = getelementptr i32* @tvar, i32 2 +; CHECK: ret i32* %expanded + + +; This is identical to @test_converting_getelementptr(). +; We need to check that both copies of getelementptr are fixed. +define i32* @test_converting_getelementptr_copy() { + ret i32* getelementptr (i32* @tvar, i32 2) +} +; CHECK: define i32* @test_converting_getelementptr_copy() +; CHECK: %expanded = getelementptr i32* @tvar, i32 2 +; CHECK: ret i32* %expanded + + +define i32* @test_converting_getelementptr_inbounds() { + ret i32* getelementptr inbounds (i32* @tvar, i32 2) +} +; CHECK: define i32* @test_converting_getelementptr_inbounds() +; CHECK: %expanded = getelementptr inbounds i32* @tvar, i32 2 +; CHECK: ret i32* %expanded + + +define i32* @test_converting_phi(i1 %cmp) { +entry: + br i1 %cmp, label %return, label %else + +else: + br label %return + +return: + %result = phi i32* [ getelementptr (i32* @tvar, i32 1), %entry ], [ null, %else ] + ret i32* %result +} +; The converted ConstantExprs get pushed back into the PHI node's +; incoming block, which might be suboptimal but works in all cases. +; CHECK: define i32* @test_converting_phi(i1 %cmp) +; CHECK: entry: +; CHECK: %expanded = getelementptr inbounds i32* @tvar, i32 1 +; CHECK: else: +; CHECK: return: +; CHECK: %result = phi i32* [ %expanded, %entry ], [ null, %else ] + + +@addr1 = global i8* blockaddress(@test_converting_phi_with_indirectbr, %return) +@addr2 = global i8* blockaddress(@test_converting_phi_with_indirectbr, %else) +define i32* @test_converting_phi_with_indirectbr(i8* %addr) { +entry: + indirectbr i8* %addr, [ label %return, label %else ] + +else: + br label %return + +return: + %result = phi i32* [ getelementptr (i32* @tvar, i32 1), %entry ], [ null, %else ] + ret i32* %result +} +; CHECK: define i32* @test_converting_phi_with_indirectbr(i8* %addr) +; CHECK: entry: +; CHECK: %expanded = getelementptr inbounds i32* @tvar, i32 1 +; CHECK: return: +; CHECK: %result = phi i32* [ %expanded, %entry ], [ null, %else ] diff --git a/test/Transforms/NaCl/expand-tls-constexpr2.ll b/test/Transforms/NaCl/expand-tls-constexpr2.ll new file mode 100644 index 000000000000..ca7054961b7f --- /dev/null +++ b/test/Transforms/NaCl/expand-tls-constexpr2.ll @@ -0,0 +1,12 @@ +; RUN: opt < %s -nacl-expand-tls -S | FileCheck %s + +@tvar = thread_local global i32 0 + +define i32 @get_tvar() { + ret i32 ptrtoint (i32* @tvar to i32) +} +; CHECK: %tls_raw = call i8* @llvm.nacl.read.tp() +; CHECK: %tls_struct = bitcast i8* %tls_raw to %tls_struct* +; CHECK: %field = getelementptr %tls_struct* %tls_struct, i32 -1, i32 1, i32 0 +; CHECK: %expanded = ptrtoint i32* %field to i32 +; CHECK: ret i32 %expanded diff --git a/test/Transforms/NaCl/expand-tls-phi.ll b/test/Transforms/NaCl/expand-tls-phi.ll new file mode 100644 index 000000000000..0292a1d63314 --- /dev/null +++ b/test/Transforms/NaCl/expand-tls-phi.ll @@ -0,0 +1,23 @@ +; RUN: opt < %s -nacl-expand-tls -S | FileCheck %s + + +@tvar = thread_local global i32 123 + +define i32* @get_tvar(i1 %cmp) { +entry: + br i1 %cmp, label %return, label %else + +else: + br label %return + +return: + %result = phi i32* [ @tvar, %entry ], [ null, %else ] + ret i32* %result +} +; The TLS access gets pushed back into the PHI node's incoming block, +; which might be suboptimal but works in all cases. +; CHECK: entry: +; CHECK: %field = getelementptr %tls_struct* %tls_struct, i32 -1, i32 0, i32 0 +; CHECK: else: +; CHECK: return: +; CHECK: %result = phi i32* [ %field, %entry ], [ null, %else ] diff --git a/test/Transforms/NaCl/expand-tls.ll b/test/Transforms/NaCl/expand-tls.ll new file mode 100644 index 000000000000..ec572ffa2c88 --- /dev/null +++ b/test/Transforms/NaCl/expand-tls.ll @@ -0,0 +1,85 @@ +; RUN: opt < %s -nacl-expand-tls -S | FileCheck %s + +; All thread-local variables should be removed +; RUN: opt < %s -nacl-expand-tls -S | not grep thread_local + + +@tvar1 = thread_local global i64 123 +@tvar2 = thread_local global i32 456 + + +; CHECK: %tls_init_template = type <{ i64, i32 }> +; CHECK: %tls_struct = type <{ %tls_init_template, %tls_bss_template }> +; CHECK: %tls_bss_template = type <{ [4 x i8] }> + + +; CHECK: @__tls_template_start = internal constant %tls_init_template <{ i64 123, i32 456 }> + +; CHECK: @__tls_template_alignment = internal constant i32 8 + + +define i64* @get_tvar1() { + ret i64* @tvar1 +} +; CHECK: define i64* @get_tvar1() +; CHECK: %tls_raw = call i8* @llvm.nacl.read.tp() +; CHECK: %tls_struct = bitcast i8* %tls_raw to %tls_struct* +; CHECK: %field = getelementptr %tls_struct* %tls_struct, i32 -1, i32 0, i32 0 +; CHECK: ret i64* %field + + +define i32* @get_tvar2() { + ret i32* @tvar2 +} +; Much the same as for get_tvar1. +; CHECK: define i32* @get_tvar2() +; CHECK: %field = getelementptr %tls_struct* %tls_struct, i32 -1, i32 0, i32 1 + + +; Check that we define global variables for TLS templates + +@__tls_template_start = external global i8 +@__tls_template_tdata_end = external global i8 +@__tls_template_end = external global i8 + +define i8* @get_tls_template_start() { + ret i8* @__tls_template_start +} +; CHECK: define i8* @get_tls_template_start() +; CHECK: ret i8* bitcast (%tls_init_template* @__tls_template_start to i8*) + +define i8* @get_tls_template_tdata_end() { + ret i8* @__tls_template_tdata_end +} +; CHECK: define i8* @get_tls_template_tdata_end() +; CHECK: ret i8* bitcast (%tls_init_template* getelementptr inbounds (%tls_init_template* @__tls_template_start, i32 1) to i8*) + +define i8* @get_tls_template_end() { + ret i8* @__tls_template_end +} +; CHECK: define i8* @get_tls_template_end() +; CHECK: ret i8* bitcast (%tls_struct* getelementptr (%tls_struct* bitcast (%tls_init_template* @__tls_template_start to %tls_struct*), i32 1) to i8*) + + +; Check that we expand out the TLS layout intrinsics + +declare i32 @llvm.nacl.tp.tls.offset(i32) +declare i32 @llvm.nacl.tp.tdb.offset(i32) + +define i32 @test_get_tp_tls_offset(i32 %tls_size) { + %offset = call i32 @llvm.nacl.tp.tls.offset(i32 %tls_size) + ret i32 %offset +} +; Uses of the intrinsic are replaced with uses of a regular function. +; CHECK: define i32 @test_get_tp_tls_offset +; CHECK: call i32 @nacl_tp_tls_offset +; RUN: opt < %s -nacl-expand-tls -S | not grep llvm.nacl.tp.tls.offset + +define i32 @test_get_tp_tdb_offset(i32 %tdb_size) { + %offset = call i32 @llvm.nacl.tp.tdb.offset(i32 %tdb_size) + ret i32 %offset +} +; Uses of the intrinsic are replaced with uses of a regular function. +; CHECK: define i32 @test_get_tp_tdb_offset +; CHECK: call i32 @nacl_tp_tdb_offset +; RUN: opt < %s -nacl-expand-tls -S | not grep llvm.nacl.tp.tdb.offset diff --git a/test/Transforms/NaCl/lit.local.cfg b/test/Transforms/NaCl/lit.local.cfg new file mode 100644 index 000000000000..a43fd3ebdd5a --- /dev/null +++ b/test/Transforms/NaCl/lit.local.cfg @@ -0,0 +1,3 @@ +# -*- Python -*- + +config.suffixes = ['.ll'] diff --git a/tools/opt/opt.cpp b/tools/opt/opt.cpp index b111860860b8..8d2a279ff818 100644 --- a/tools/opt/opt.cpp +++ b/tools/opt/opt.cpp @@ -422,6 +422,8 @@ int main(int argc, char **argv) { initializeInstrumentation(Registry); initializeTarget(Registry); initializeExpandCtorsPass(Registry); + initializeExpandTlsPass(Registry); + initializeExpandTlsConstantExprPass(Registry); cl::ParseCommandLineOptions(argc, argv, "llvm .bc -> .bc modular optimizer and analysis printer\n"); From fc3a584ea4523048fb5ce665bdaedd92c616b3c6 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Thu, 17 Jan 2013 13:15:42 -0800 Subject: [PATCH 004/295] Skeleton for pass-based bitcode ABI verifier This is the skeleton for a verifier for the portion of the PNaCl bitcode ABI that can be verified after the bitcode has been read into a Module object. There is a ModulePass for module-level rules (e.g. GV linkage types) and a FunctionPass for rules that apply to function bodies (e.g. legal instructions). They are separated this way to keep the verifier streaming-friendly. For now, the passes are registered but not hooked up, so they can only be run manually via opt. There are 2 bits of actual functionality, just so each pass has something to do: The ModulePass checks the linkage types of GVs, and the FunctionPass checks instruction opcodes. For now only the terminator instructions are checked, but the idea is to add the rest of the allowed instructions to the whitelist, and possibly call operand checks from the switch statement as well. For now we just print messagees to stderr, but we will probably want a better way to plumb the errors in the browser in the future. R=jvoung@chromium.org,sehr@chromium.org BUG= https://code.google.com/p/nativeclient/issues/detail?id=2196 Review URL: https://codereview.chromium.org/11986002 (cherry picked from commit 6dead1cbf252b2a84f774c9b37f775062281e701) --- include/llvm/Transforms/NaCl.h | 3 + lib/Analysis/LLVMBuild.txt | 2 +- lib/Analysis/Makefile | 2 +- lib/Analysis/NaCl/CMakeLists.txt | 6 ++ lib/Analysis/NaCl/LLVMBuild.txt | 23 +++++ lib/Analysis/NaCl/Makefile | 14 +++ lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp | 66 ++++++++++++++ lib/Analysis/NaCl/PNaClABIVerifyModule.cpp | 86 +++++++++++++++++++ lib/Transforms/NaCl/LLVMBuild.txt | 4 +- 9 files changed, 202 insertions(+), 4 deletions(-) create mode 100644 lib/Analysis/NaCl/CMakeLists.txt create mode 100644 lib/Analysis/NaCl/LLVMBuild.txt create mode 100644 lib/Analysis/NaCl/Makefile create mode 100644 lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp create mode 100644 lib/Analysis/NaCl/PNaClABIVerifyModule.cpp diff --git a/include/llvm/Transforms/NaCl.h b/include/llvm/Transforms/NaCl.h index 79c9b9fe7949..ecdfa9043f47 100644 --- a/include/llvm/Transforms/NaCl.h +++ b/include/llvm/Transforms/NaCl.h @@ -12,11 +12,14 @@ namespace llvm { +class FunctionPass; class ModulePass; ModulePass *createExpandCtorsPass(); ModulePass *createExpandTlsPass(); ModulePass *createExpandTlsConstantExprPass(); +FunctionPass *createPNaClABIVerifyFunctionsPass(); +ModulePass *createPNaClABIVerifyModulePass(); } diff --git a/lib/Analysis/LLVMBuild.txt b/lib/Analysis/LLVMBuild.txt index a8a8079d1e5a..de734ec3f724 100644 --- a/lib/Analysis/LLVMBuild.txt +++ b/lib/Analysis/LLVMBuild.txt @@ -16,7 +16,7 @@ ;===------------------------------------------------------------------------===; [common] -subdirectories = IPA +subdirectories = IPA NaCl [component_0] type = Library diff --git a/lib/Analysis/Makefile b/lib/Analysis/Makefile index 4af6d350a645..426ed1699dd6 100644 --- a/lib/Analysis/Makefile +++ b/lib/Analysis/Makefile @@ -9,7 +9,7 @@ LEVEL = ../.. LIBRARYNAME = LLVMAnalysis -DIRS = IPA +DIRS = IPA NaCl BUILD_ARCHIVE = 1 include $(LEVEL)/Makefile.common diff --git a/lib/Analysis/NaCl/CMakeLists.txt b/lib/Analysis/NaCl/CMakeLists.txt new file mode 100644 index 000000000000..8a5eceb3e4d4 --- /dev/null +++ b/lib/Analysis/NaCl/CMakeLists.txt @@ -0,0 +1,6 @@ +add_llvm_library(LLVMAnalysisNaCl + PNaClABIVerifyFunctions.cpp + PNaClABIVerifyModule.cpp + ) + +add_dependencies(LLVMAnalysisNaCl intrinsics_gen) diff --git a/lib/Analysis/NaCl/LLVMBuild.txt b/lib/Analysis/NaCl/LLVMBuild.txt new file mode 100644 index 000000000000..997803bb5db7 --- /dev/null +++ b/lib/Analysis/NaCl/LLVMBuild.txt @@ -0,0 +1,23 @@ +;===- ./lib/Analysis/NaCl/LLVMBuild.txt ----------------------*- Conf -*--===; +; +; The LLVM Compiler Infrastructure +; +; This file is distributed under the University of Illinois Open Source +; License. See LICENSE.TXT for details. +; +;===------------------------------------------------------------------------===; +; +; This is an LLVMBuild description file for the components in this subdirectory. +; +; For more information on the LLVMBuild system, please see: +; +; http://llvm.org/docs/LLVMBuild.html +; +;===------------------------------------------------------------------------===; + +[component_0] +type = Library +name = NaClAnalysis +parent = Analysis +library_name = NaClAnalysis +required_libraries = Core diff --git a/lib/Analysis/NaCl/Makefile b/lib/Analysis/NaCl/Makefile new file mode 100644 index 000000000000..6a2f0ed8d9dc --- /dev/null +++ b/lib/Analysis/NaCl/Makefile @@ -0,0 +1,14 @@ +##===- lib/Analysis/NaCl/Makefile-------------------------*- Makefile -*-===## +# +# The LLVM Compiler Infrastructure +# +# This file is distributed under the University of Illinois Open Source +# License. See LICENSE.TXT for details. +# +##===----------------------------------------------------------------------===## + +LEVEL = ../../.. +LIBRARYNAME = LLVMAnalysisNaCl +BUILD_ARCHIVE = 1 + +include $(LEVEL)/Makefile.common \ No newline at end of file diff --git a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp new file mode 100644 index 000000000000..a2e5f1b2a57e --- /dev/null +++ b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp @@ -0,0 +1,66 @@ +//===- PNaClABIVerifyFunctions.cpp - Verify PNaCl ABI rules --------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// Verify function-level PNaCl ABI requirements. +// +// +//===----------------------------------------------------------------------===// + +#include "llvm/ADT/Twine.h" +#include "llvm/Function.h" +#include "llvm/Instruction.h" +#include "llvm/Pass.h" +#include "llvm/Support/raw_ostream.h" +#include "llvm/Transforms/NaCl.h" +using namespace llvm; + +namespace { + +// Checks that examine anything in the function body should be in +// FunctionPasses to make them streaming-friendly +struct PNaClABIVerifyFunctions : public FunctionPass { + static char ID; + PNaClABIVerifyFunctions() : FunctionPass(ID) {} + bool runOnFunction(Function &F); +}; +} // and anonymous namespace + +bool PNaClABIVerifyFunctions::runOnFunction(Function &F) { + for (Function::const_iterator FI = F.begin(), FE = F.end(); + FI != FE; ++FI) { + for (BasicBlock::const_iterator BBI = FI->begin(), BBE = FI->end(); + BBI != BBE; ++BBI) { + switch (BBI->getOpcode()) { + // Terminator instructions + case Instruction::Ret: + case Instruction::Br: + case Instruction::Switch: + case Instruction::Resume: + case Instruction::Unreachable: + // indirectbr is not allowed for now. + // invoke and call are handled separately. + break; + default: + errs() << Twine("Function ") + FI->getName() + + " has Disallowed instruction: " + + BBI->getOpcodeName() + "\n"; + } + } + } + return false; +} + +char PNaClABIVerifyFunctions::ID = 0; + +static RegisterPass X("pnaclabi-functions", + "Verify functions for PNaCl", false, false); + +FunctionPass *llvm::createPNaClABIVerifyFunctionsPass() { + return new PNaClABIVerifyFunctions(); +} diff --git a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp new file mode 100644 index 000000000000..f39b83881edc --- /dev/null +++ b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp @@ -0,0 +1,86 @@ +//===- PNaClABIVerifyModule.cpp - Verify PNaCl ABI rules --------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// Verify module-level PNaCl ABI requirements (specifically those that do not +// require looking at the function bodies) +// +// +//===----------------------------------------------------------------------===// + +#include "llvm/ADT/Twine.h" +#include "llvm/Module.h" +#include "llvm/Pass.h" +#include "llvm/Support/raw_ostream.h" +#include "llvm/Transforms/NaCl.h" +using namespace llvm; + +namespace { + +// This pass should not touch function bodies, to stay streaming-friendly +struct PNaClABIVerifyModule : public ModulePass { + static char ID; + PNaClABIVerifyModule() : ModulePass(ID) {} + bool runOnModule(Module &M); +}; + +static const char* LinkageName(GlobalValue::LinkageTypes LT) { + // This logic is taken from PrintLinkage in lib/VMCore/AsmWriter.cpp + switch (LT) { + case GlobalValue::ExternalLinkage: return "external"; + case GlobalValue::PrivateLinkage: return "private "; + case GlobalValue::LinkerPrivateLinkage: return "linker_private "; + case GlobalValue::LinkerPrivateWeakLinkage: return "linker_private_weak "; + case GlobalValue::InternalLinkage: return "internal "; + case GlobalValue::LinkOnceAnyLinkage: return "linkonce "; + case GlobalValue::LinkOnceODRLinkage: return "linkonce_odr "; + case GlobalValue::LinkOnceODRAutoHideLinkage: + return "linkonce_odr_auto_hide "; + case GlobalValue::WeakAnyLinkage: return "weak "; + case GlobalValue::WeakODRLinkage: return "weak_odr "; + case GlobalValue::CommonLinkage: return "common "; + case GlobalValue::AppendingLinkage: return "appending "; + case GlobalValue::DLLImportLinkage: return "dllimport "; + case GlobalValue::DLLExportLinkage: return "dllexport "; + case GlobalValue::ExternalWeakLinkage: return "extern_weak "; + case GlobalValue::AvailableExternallyLinkage: + return "available_externally "; + default: + return "unknown"; + } +} + +} // end anonymous namespace + +bool PNaClABIVerifyModule::runOnModule(Module &M) { + // Check GV linkage types + for (Module::const_global_iterator MI = M.global_begin(), ME = M.global_end(); + MI != ME; ++MI) { + switch(MI->getLinkage()) { + case GlobalValue::ExternalLinkage: + case GlobalValue::AvailableExternallyLinkage: + case GlobalValue::InternalLinkage: + case GlobalValue::PrivateLinkage: + break; + default: + errs() << (Twine("Variable ") + MI->getName() + + " has Disallowed linkage type: " + + LinkageName(MI->getLinkage()) + "\n"); + } + } + return false; +} + +char PNaClABIVerifyModule::ID = 0; + +static RegisterPass X("pnaclabi-module", + "Verify module for PNaCl", false, false); + +ModulePass *llvm::createPNaClABIVerifyModulePass() { + return new PNaClABIVerifyModule(); +} diff --git a/lib/Transforms/NaCl/LLVMBuild.txt b/lib/Transforms/NaCl/LLVMBuild.txt index 2f1522b3e5c9..14c85c9c0464 100644 --- a/lib/Transforms/NaCl/LLVMBuild.txt +++ b/lib/Transforms/NaCl/LLVMBuild.txt @@ -17,7 +17,7 @@ [component_0] type = Library -name = NaCl +name = NaClTransforms parent = Transforms -library_name = NaCl +library_name = NaClTransforms required_libraries = Core From 30cfb464ed21922696a50ed4930fc164f9b68b5e Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Fri, 18 Jan 2013 14:41:11 -0800 Subject: [PATCH 005/295] Move ABI verifier pass declaration to lib/Analysis and add tests This fixes the move of the verifier passes from lib/Transforms to lib/Analysis, and adds tests of the current verifier checks. No new functionality. R=jvoung@chromium.org,eliben@chromium.org,mseaborn@chromium.org BUG= https://code.google.com/p/nativeclient/issues/detail?id=2196 Review URL: https://codereview.chromium.org/12017019 (cherry picked from commit 958cd97d295ba41736615e33c4a1aa641989133e) --- include/llvm/Analysis/NaCl.h | 23 ++++++++++++ include/llvm/Transforms/NaCl.h | 3 -- lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp | 13 +++++-- lib/Analysis/NaCl/PNaClABIVerifyModule.cpp | 11 ++++-- test/NaCl/PNaClABI/instructions.ll | 20 ++++++++++ test/NaCl/PNaClABI/linkagetypes.ll | 37 +++++++++++++++++++ test/NaCl/PNaClABI/lit.local.cfg | 1 + 7 files changed, 98 insertions(+), 10 deletions(-) create mode 100644 include/llvm/Analysis/NaCl.h create mode 100644 test/NaCl/PNaClABI/instructions.ll create mode 100644 test/NaCl/PNaClABI/linkagetypes.ll create mode 100644 test/NaCl/PNaClABI/lit.local.cfg diff --git a/include/llvm/Analysis/NaCl.h b/include/llvm/Analysis/NaCl.h new file mode 100644 index 000000000000..5f8c534c6f9d --- /dev/null +++ b/include/llvm/Analysis/NaCl.h @@ -0,0 +1,23 @@ +//===-- NaCl.h - NaCl Analysis ---------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_ANALYSIS_NACL_H +#define LLVM_ANALYSIS_NACL_H + +namespace llvm { + +class FunctionPass; +class ModulePass; + +FunctionPass *createPNaClABIVerifyFunctionsPass(); +ModulePass *createPNaClABIVerifyModulePass(); + +} + +#endif diff --git a/include/llvm/Transforms/NaCl.h b/include/llvm/Transforms/NaCl.h index ecdfa9043f47..79c9b9fe7949 100644 --- a/include/llvm/Transforms/NaCl.h +++ b/include/llvm/Transforms/NaCl.h @@ -12,14 +12,11 @@ namespace llvm { -class FunctionPass; class ModulePass; ModulePass *createExpandCtorsPass(); ModulePass *createExpandTlsPass(); ModulePass *createExpandTlsConstantExprPass(); -FunctionPass *createPNaClABIVerifyFunctionsPass(); -ModulePass *createPNaClABIVerifyModulePass(); } diff --git a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp index a2e5f1b2a57e..abc66b0addca 100644 --- a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp +++ b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp @@ -17,7 +17,7 @@ #include "llvm/Instruction.h" #include "llvm/Pass.h" #include "llvm/Support/raw_ostream.h" -#include "llvm/Transforms/NaCl.h" +#include "llvm/Analysis/NaCl.h" using namespace llvm; namespace { @@ -28,6 +28,11 @@ struct PNaClABIVerifyFunctions : public FunctionPass { static char ID; PNaClABIVerifyFunctions() : FunctionPass(ID) {} bool runOnFunction(Function &F); + // For now, this print method exists to allow us to run the pass with + // opt -analyze to avoid dumping the result to stdout, to make testing + // simpler. In the future we will probably want to make it do something + // useful. + virtual void print(llvm::raw_ostream &O, const Module *M) const {}; }; } // and anonymous namespace @@ -47,8 +52,8 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) { // invoke and call are handled separately. break; default: - errs() << Twine("Function ") + FI->getName() + - " has Disallowed instruction: " + + errs() << Twine("Function ") + F.getName() + + " has disallowed instruction: " + BBI->getOpcodeName() + "\n"; } } @@ -58,7 +63,7 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) { char PNaClABIVerifyFunctions::ID = 0; -static RegisterPass X("pnaclabi-functions", +static RegisterPass X("verify-pnaclabi-functions", "Verify functions for PNaCl", false, false); FunctionPass *llvm::createPNaClABIVerifyFunctionsPass() { diff --git a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp index f39b83881edc..5062f710f62c 100644 --- a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp +++ b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp @@ -17,7 +17,7 @@ #include "llvm/Module.h" #include "llvm/Pass.h" #include "llvm/Support/raw_ostream.h" -#include "llvm/Transforms/NaCl.h" +#include "llvm/Analysis/NaCl.h" using namespace llvm; namespace { @@ -27,6 +27,11 @@ struct PNaClABIVerifyModule : public ModulePass { static char ID; PNaClABIVerifyModule() : ModulePass(ID) {} bool runOnModule(Module &M); + // For now, this print method exists to allow us to run the pass with + // opt -analyze to avoid dumping the result to stdout, to make testing + // simpler. In the future we will probably want to make it do something + // useful. + virtual void print(llvm::raw_ostream &O, const Module *M) const {}; }; static const char* LinkageName(GlobalValue::LinkageTypes LT) { @@ -69,7 +74,7 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) { break; default: errs() << (Twine("Variable ") + MI->getName() + - " has Disallowed linkage type: " + + " has disallowed linkage type: " + LinkageName(MI->getLinkage()) + "\n"); } } @@ -78,7 +83,7 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) { char PNaClABIVerifyModule::ID = 0; -static RegisterPass X("pnaclabi-module", +static RegisterPass X("verify-pnaclabi-module", "Verify module for PNaCl", false, false); ModulePass *llvm::createPNaClABIVerifyModulePass() { diff --git a/test/NaCl/PNaClABI/instructions.ll b/test/NaCl/PNaClABI/instructions.ll new file mode 100644 index 000000000000..2a8ccbdaca36 --- /dev/null +++ b/test/NaCl/PNaClABI/instructions.ll @@ -0,0 +1,20 @@ +; RUN: opt -verify-pnaclabi-functions -analyze < %s |& FileCheck %s +; Test instruction opcodes allowed by PNaCl ABI + +target datalayout = "e-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-p:32:32:32-v128:32:32" +target triple = "le32-unknown-nacl" + +define i32 @terminators() nounwind { +; Terminator instructions +terminators: + ret i32 0 + br i1 0, label %next2, label %next +next: + switch i32 1, label %next2 [i32 0, label %next] +next2: + unreachable + resume i8 0 + indirectbr i8* undef, [label %next, label %next2] +; CHECK-NOT: disallowed +; CHECK: Function terminators has disallowed instruction: indirectbr +} \ No newline at end of file diff --git a/test/NaCl/PNaClABI/linkagetypes.ll b/test/NaCl/PNaClABI/linkagetypes.ll new file mode 100644 index 000000000000..477b7d6d9178 --- /dev/null +++ b/test/NaCl/PNaClABI/linkagetypes.ll @@ -0,0 +1,37 @@ +; RUN: opt -verify-pnaclabi-module -analyze < %s |& FileCheck %s +; Test linkage types allowed by PNaCl ABI + +target datalayout = "e-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-p:32:32:32-v128:32:32" +target triple = "le32-unknown-nacl" + + +@gv_external = external global i8 +@gv_private = private global i8 0 +@gv_linker_private = linker_private global i32 0 +; CHECK-NOT: disallowed +; CHECK: gv_linker_private has disallowed linkage type: linker_private +@gv_linker_private_weak = linker_private_weak global i32 0 +; CHECK: gv_linker_private_weak has disallowed linkage type: linker_private_weak +@gv_internal = internal global i8 0 +@gv_linkonce = linkonce global i8 0 +; CHECK-NOT: disallowed +; CHECK: gv_linkonce has disallowed linkage type: linkonce +@gv_linkonce_odr = linkonce_odr global i8 0 +; CHECK: gv_linkonce_odr has disallowed linkage type: linkonce_odr +@gv_linkonce_odr_auto_hide = linkonce_odr_auto_hide global i8 0 +; CHECK: gv_linkonce_odr_auto_hide has disallowed linkage type: linkonce_odr_auto_hide +@gv_weak = weak global i8 0 +; CHECK: gv_weak has disallowed linkage type: weak +@gv_weak_odr = weak_odr global i8 0 +; CHECK: gv_weak_odr has disallowed linkage type: weak_odr +@gv_common = common global i8 0 +; CHECK: gv_common has disallowed linkage type: common +@gv_appending = appending global [1 x i8] zeroinitializer +; CHECK: gv_appending has disallowed linkage type: appending +@gv_dllimport = dllimport global i8 +; CHECK: gv_dllimport has disallowed linkage type: dllimport +@gv_dllexport = dllexport global i8 0 +; CHECK: gv_dllexport has disallowed linkage type: dllexport +@gv_extern_weak = extern_weak global i8 +; CHECK: gv_extern_weak has disallowed linkage type: extern_weak +@gv_avilable_externally = available_externally global i8 0 diff --git a/test/NaCl/PNaClABI/lit.local.cfg b/test/NaCl/PNaClABI/lit.local.cfg new file mode 100644 index 000000000000..c6106e4746f2 --- /dev/null +++ b/test/NaCl/PNaClABI/lit.local.cfg @@ -0,0 +1 @@ +config.suffixes = ['.ll'] From 8df922ed3de0eaf1a14b684f8b05e70f105ce0c9 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Tue, 22 Jan 2013 17:11:41 -0800 Subject: [PATCH 006/295] Add remaining instructions to ABI verifier whitelist (the current tests only cover allowed opcodes; no testing yet of types, attributes, etc). R=jvoung@chromium.org,eliben@chromium.org,mseaborn@chromium.org BUG= https://code.google.com/p/nativeclient/issues/detail?id=2196 TEST= LLVM regression Review URL: https://codereview.chromium.org/11896044 (cherry picked from commit 8ccd5689414ade8d6dd50706e64179d6d22cba44) --- lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp | 71 ++++++++++++-- test/NaCl/PNaClABI/instructions.ll | 95 ++++++++++++++++++- 2 files changed, 157 insertions(+), 9 deletions(-) diff --git a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp index abc66b0addca..cf878dc66007 100644 --- a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp +++ b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp @@ -42,19 +42,78 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) { for (BasicBlock::const_iterator BBI = FI->begin(), BBE = FI->end(); BBI != BBE; ++BBI) { switch (BBI->getOpcode()) { + // Disallowed instructions. Default is to disallow. + default: + // indirectbr may interfere with streaming + case Instruction::IndirectBr: + // No vector instructions yet + case Instruction::ExtractElement: + case Instruction::InsertElement: + case Instruction::ShuffleVector: + errs() << Twine("Function ") + F.getName() + + " has disallowed instruction: " + + BBI->getOpcodeName() + "\n"; + break; + // Terminator instructions case Instruction::Ret: case Instruction::Br: case Instruction::Switch: case Instruction::Resume: case Instruction::Unreachable: - // indirectbr is not allowed for now. - // invoke and call are handled separately. + case Instruction::Invoke: + // Binary operations + case Instruction::Add: + case Instruction::FAdd: + case Instruction::Sub: + case Instruction::FSub: + case Instruction::Mul: + case Instruction::FMul: + case Instruction::UDiv: + case Instruction::SDiv: + case Instruction::FDiv: + case Instruction::URem: + case Instruction::SRem: + case Instruction::FRem: + // Bitwise binary operations + case Instruction::Shl: + case Instruction::LShr: + case Instruction::AShr: + case Instruction::And: + case Instruction::Or: + case Instruction::Xor: + case Instruction::ExtractValue: + case Instruction::InsertValue: + // Memory instructions + case Instruction::Alloca: + case Instruction::Load: + case Instruction::Store: + case Instruction::Fence: + case Instruction::AtomicCmpXchg: + case Instruction::AtomicRMW: + case Instruction::GetElementPtr: + // Conversion operations + case Instruction::Trunc: + case Instruction::ZExt: + case Instruction::SExt: + case Instruction::FPTrunc: + case Instruction::FPExt: + case Instruction::FPToUI: + case Instruction::FPToSI: + case Instruction::UIToFP: + case Instruction::SIToFP: + case Instruction::PtrToInt: + case Instruction::IntToPtr: + case Instruction::BitCast: + // Other operations + case Instruction::ICmp: + case Instruction::FCmp: + case Instruction::PHI: + case Instruction::Select: + case Instruction::Call: + case Instruction::VAArg: + case Instruction::LandingPad: break; - default: - errs() << Twine("Function ") + F.getName() + - " has disallowed instruction: " + - BBI->getOpcodeName() + "\n"; } } } diff --git a/test/NaCl/PNaClABI/instructions.ll b/test/NaCl/PNaClABI/instructions.ll index 2a8ccbdaca36..33ee9174e1d2 100644 --- a/test/NaCl/PNaClABI/instructions.ll +++ b/test/NaCl/PNaClABI/instructions.ll @@ -1,13 +1,14 @@ ; RUN: opt -verify-pnaclabi-functions -analyze < %s |& FileCheck %s ; Test instruction opcodes allowed by PNaCl ABI +; No testing yet of operands, types, attributes, etc target datalayout = "e-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-p:32:32:32-v128:32:32" target triple = "le32-unknown-nacl" -define i32 @terminators() nounwind { +define void @terminators() { ; Terminator instructions terminators: - ret i32 0 + ret void br i1 0, label %next2, label %next next: switch i32 1, label %next2 [i32 0, label %next] @@ -17,4 +18,92 @@ next2: indirectbr i8* undef, [label %next, label %next2] ; CHECK-NOT: disallowed ; CHECK: Function terminators has disallowed instruction: indirectbr -} \ No newline at end of file +} + +define void @binops() { +; Binary operations + %a1 = add i32 0, 0 + %a2 = sub i32 0, 0 + %a3 = fsub float 0.0, 0.0 + %a4 = mul i32 0, 0 + %a5 = fmul float 0.0, 0.0 + %a6 = udiv i32 0, 1 + %a7 = sdiv i32 0, 1 + %a8 = fdiv float 0.0, 1.0 + %a9 = urem i32 0, 1 + %a10 = srem i32 0, 1 + %a11 = frem float 0.0, 1.0 +; Bitwise binary operations + %a12 = shl i32 1, 1 + %a13 = lshr i32 1, 1 + %a14 = ashr i32 1, 1 + %a15 = and i32 1, 1 + %a16 = or i32 1, 1 + %a17 = xor i32 1, 1 + ret void +} + +define void @vectors() { +; CHECK-NOT: disallowed + %a1 = extractelement <2 x i32> , i32 0 +; CHECK: Function vectors has disallowed instruction: extractelement + %a2 = shufflevector <2 x i32> undef , <2 x i32> undef, <2 x i32> undef +; CHECK: Function vectors has disallowed instruction: shufflevector + %a3 = insertelement <2 x i32> undef, i32 1, i32 0 +; CHECK: Function vectors has disallowed instruction: insertelement + ret void +} + +define void @aggregates() { +; Aggregate operations + %a1 = extractvalue { i32, i32 } { i32 0, i32 0 }, 0 + %a2 = insertvalue {i32, float} undef, i32 1, 0 + ret void +} + +define void @memory() { +; Memory operations + %a1 = alloca i32 + %a2 = load i32* undef + store i32 undef, i32* undef + fence acq_rel + %a3 = cmpxchg i32* undef, i32 undef, i32 undef acq_rel + %a4 = atomicrmw add i32* undef, i32 1 acquire + %a5 = getelementptr { i32, i32}* undef + ret void +} + +define void @conversion() { +; Conversion operations + %a1 = trunc i32 undef to i8 + %a2 = zext i8 undef to i32 + %a3 = sext i8 undef to i32 + %a4 = fptrunc double undef to float + %a5 = fpext float undef to double + %a6 = fptoui double undef to i64 + %a7 = fptosi double undef to i64 + %a8 = uitofp i64 undef to double + %a9 = sitofp i64 undef to double + %a10 = ptrtoint i8* undef to i32 + %a11 = inttoptr i32 undef to i8* + %a12 = bitcast i8* undef to i32* + ret void +} + +define void @other() { +entry: + %a1 = icmp eq i32 undef, undef + %a2 = fcmp eq float undef, undef + br i1 undef, label %foo, label %bar +foo: +; phi predecessor labels have to match to appease module verifier + %a3 = phi i32 [0, %entry], [0, %foo] + %a4 = select i1 true, i8 undef, i8 undef + %a5 = va_arg i8** undef, i32 + call void @conversion() + br i1 undef, label %foo, label %bar +bar: + ret void +} +; CHECK-NOT: disallowed +; If another check is added, there should be a check-not in between each check From c728ba88410ea49670f869fbbf0c7769093a8102 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Fri, 25 Jan 2013 13:25:07 -0800 Subject: [PATCH 007/295] ABI verifier: Verify types of global variables and initializers. Loosely based on lib/VMCore/TypeFinder.cpp but I can't use that directly because it's not streaming-friendly. constexprs in initializers are also supported. R=jvoung@chromium.org,eliben@chromium.org,mseaborn@chromium.org BUG= https://code.google.com/p/nativeclient/issues/detail?id=2196 TEST=llvm regression Review URL: https://codereview.chromium.org/12038079 (cherry picked from commit ceb093296a82aa7125357dab6ddb388a37cd595c) --- lib/Analysis/NaCl/CheckTypes.h | 109 +++++++++++++++++++++ lib/Analysis/NaCl/PNaClABIVerifyModule.cpp | 34 ++++++- test/NaCl/PNaClABI/types.ll | 89 +++++++++++++++++ 3 files changed, 230 insertions(+), 2 deletions(-) create mode 100644 lib/Analysis/NaCl/CheckTypes.h create mode 100644 test/NaCl/PNaClABI/types.ll diff --git a/lib/Analysis/NaCl/CheckTypes.h b/lib/Analysis/NaCl/CheckTypes.h new file mode 100644 index 000000000000..336dc0302f5c --- /dev/null +++ b/lib/Analysis/NaCl/CheckTypes.h @@ -0,0 +1,109 @@ +//===- CheckTypes.h - Verify PNaCl ABI rules --------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// Common type-checking code for module and function-level passes +// +// +//===----------------------------------------------------------------------===// + +#ifndef LIB_ANALYSIS_NACL_CHECKTYPES_H +#define LIB_ANALYSIS_NACL_CHECKTYPES_H + +#include "llvm/ADT/DenseSet.h" +#include "llvm/DerivedTypes.h" +#include "llvm/Support/raw_ostream.h" + +class TypeChecker { + public: + bool IsValidType(const llvm::Type* Ty) { + if (VisitedTypes.count(Ty)) + return VisitedTypes[Ty]; + + unsigned Width; + bool Valid = false; + switch (Ty->getTypeID()) { + // Allowed primitive types + case llvm::Type::VoidTyID: + case llvm::Type::FloatTyID: + case llvm::Type::DoubleTyID: + case llvm::Type::LabelTyID: + case llvm::Type::MetadataTyID: + Valid = true; + break; + // Disallowed primitive types + case llvm::Type::HalfTyID: + case llvm::Type::X86_FP80TyID: + case llvm::Type::FP128TyID: + case llvm::Type::PPC_FP128TyID: + case llvm::Type::X86_MMXTyID: + Valid = false; + break; + // Derived types + case llvm::Type::VectorTyID: + Valid = false; + break; + case llvm::Type::IntegerTyID: + Width = llvm::cast(Ty)->getBitWidth(); + Valid = (Width == 1 || Width == 8 || Width == 16 || + Width == 32 || Width == 64); + break; + case llvm::Type::FunctionTyID: + case llvm::Type::StructTyID: + case llvm::Type::ArrayTyID: + case llvm::Type::PointerTyID: + // These types are valid if their contained or pointed-to types are + // valid. Since struct/pointer subtype relationships may be circular, + // mark the current type as valid to avoid infinite recursion + Valid = true; + VisitedTypes[Ty] = true; + for (llvm::Type::subtype_iterator I = Ty->subtype_begin(), + E = Ty->subtype_end(); I != E; ++I) + Valid &= IsValidType(*I); + break; + // Handle NumTypeIDs, and no default case, + // so we get a warning if new types are added + case llvm::Type::NumTypeIDs: + Valid = false; + break; + } + + VisitedTypes[Ty] = Valid; + return Valid; + } + + bool CheckTypesInValue(const llvm::Value* V) { + // TODO: Checking types in values probably belongs in its + // own value checker which also handles the various types of + // constexpr (in particular, blockaddr constexprs cause this code + // to assert rather than go off and try to verify the BBs of a function) + // But this code is in a good consistent checkpoint-able state. + assert(llvm::isa(V)); + if (VisitedConstants.count(V)) + return VisitedConstants[V]; + + bool Valid = IsValidType(V->getType()); + VisitedConstants[V] = Valid; + + const llvm::User *U = llvm::cast(V); + for (llvm::Constant::const_op_iterator I = U->op_begin(), + E = U->op_end(); I != E; ++I) + Valid &= CheckTypesInValue(*I); + VisitedConstants[V] = Valid; + return Valid; + } + + private: + // To avoid walking constexprs and types multiple times, keep a cache of + // what we have seen. This is also used to prevent infinite recursion e.g. + // in case of structures like linked lists with pointers to themselves. + llvm::DenseMap VisitedConstants; + llvm::DenseMap VisitedTypes; +}; + +#endif // LIB_ANALYSIS_NACL_CHECKTYPES_H diff --git a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp index 5062f710f62c..3b35668656f2 100644 --- a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp +++ b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp @@ -14,10 +14,14 @@ //===----------------------------------------------------------------------===// #include "llvm/ADT/Twine.h" +#include "llvm/DerivedTypes.h" #include "llvm/Module.h" #include "llvm/Pass.h" + #include "llvm/Support/raw_ostream.h" #include "llvm/Analysis/NaCl.h" + +#include "CheckTypes.h" using namespace llvm; namespace { @@ -32,6 +36,10 @@ struct PNaClABIVerifyModule : public ModulePass { // simpler. In the future we will probably want to make it do something // useful. virtual void print(llvm::raw_ostream &O, const Module *M) const {}; + private: + // Ideally this would share an instance with the Function pass. + // TODO: see if that's feasible when we add checking in bodies + TypeChecker TC; }; static const char* LinkageName(GlobalValue::LinkageTypes LT) { @@ -63,10 +71,26 @@ static const char* LinkageName(GlobalValue::LinkageTypes LT) { } // end anonymous namespace bool PNaClABIVerifyModule::runOnModule(Module &M) { - // Check GV linkage types + for (Module::const_global_iterator MI = M.global_begin(), ME = M.global_end(); MI != ME; ++MI) { - switch(MI->getLinkage()) { + // Check types of global variables and their initializers + if (!TC.IsValidType(MI->getType())) { + errs() << (Twine("Variable ") + MI->getName() + + " has disallowed type: "); + // GVs are pointers, so print the pointed-to type for clarity + MI->getType()->getContainedType(0)->print(errs()); + errs() << "\n"; + } else if (MI->hasInitializer() && + !TC.CheckTypesInValue(MI->getInitializer())) { + errs() << (Twine("Initializer for ") + MI->getName() + + " has disallowed type: "); + MI->getInitializer()->print(errs()); + errs() << "\n"; + } + + // Check GV linkage types + switch (MI->getLinkage()) { case GlobalValue::ExternalLinkage: case GlobalValue::AvailableExternallyLinkage: case GlobalValue::InternalLinkage: @@ -78,6 +102,12 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) { LinkageName(MI->getLinkage()) + "\n"); } } + // No aliases allowed for now. + for (Module::alias_iterator MI = M.alias_begin(), + E = M.alias_end(); MI != E; ++MI) + errs() << (Twine("Variable ") + MI->getName() + + " is an alias (disallowed)\n"); + return false; } diff --git a/test/NaCl/PNaClABI/types.ll b/test/NaCl/PNaClABI/types.ll new file mode 100644 index 000000000000..3656b0a51da3 --- /dev/null +++ b/test/NaCl/PNaClABI/types.ll @@ -0,0 +1,89 @@ +; RUN: opt -verify-pnaclabi-module -analyze < %s |& FileCheck %s +; Test types allowed by PNaCl ABI + +; Basic global types +; CHECK: Variable i4 has disallowed type: i4 +@i4 = private global i4 0 +; CHECK: Variable i33 has disallowed type: i33 +@i33 = private global i33 0 +; CHECK: Variable i128 has disallowed type: i128 +@i128 = private global i128 0 +; CHECK: Variable hlf has disallowed type: half +@hlf = private global half 0.0 +; CHECK: Variable fp80 has disallowed type: x86_fp80 +@fp80 = private global x86_fp80 undef +; CHECK: Variable f128 has disallowed type: fp128 +@f128 = private global fp128 undef +; CHECK: Variable ppc128 has disallowed type: ppc_fp128 +@ppc128 = private global ppc_fp128 undef +; CHECK: Variable mmx has disallowed type: x86_mmx +@mmx = private global x86_mmx undef + +@i1 = private global i1 0 +@i8 = private global i8 0 +@i16 = private global i16 0 +@i32 = private global i32 0 +@i64 = private global i64 0 +@flt = private global float 0.0 +@dbl = private global double 0.0 + + +; global derived types +@p1 = private global i32* undef +@a1 = private global [1 x i32] undef +@s1 = private global { i32, float } undef +@f1 = private global void (i32) * undef +; CHECK-NOT: disallowed +; CHECK: Variable v1 has disallowed type: <2 x i32> +@v1 = private global <2 x i32> undef +; CHECK-NOT: disallowed +@ps1 = private global <{ i8, i32 }> undef + + +; named types. with the current implementation, bogus named types are legal +; until they are actually attempted to be used. Might want to fix that. +%struct.s1 = type { half, float} +; CHECK: Variable s11 has disallowed type: %struct.s1 = type { half, float } +@s11 = private global %struct.s1 undef +; CHECK-NOT: disallowed +%struct.s2 = type { i32, i32} +@s12 = private global %struct.s2 undef + + +; types in arrays, structs, etc +; CHECK: Variable p2 has disallowed type: half* +@p2 = private global half* undef +; CHECK: Variable a2 has disallowed type: [2 x i33] +@a2 = private global [ 2 x i33 ] undef +; CHECK: Variable s2 has disallowed type: { half, i32 } +@s2 = private global { half, i32 } undef +; CHECK: Variable s3 has disallowed type: { float, i33 } +@s3 = private global { float, i33 } undef +; CHECK: Variable s4 has disallowed type: { i32, { i32, half }, float } +@s4 = private global { i32, { i32, half }, float } undef +; CHECK-NOT: disallowed +@s5 = private global { i32, { i32, double }, float } undef + +; Initializers with constexprs +; CHECK: Variable cc1 has disallowed type: half +@cc1 = private global half 0.0 +; CHECK: Initializer for ce1 has disallowed type: i8* bitcast (half* @cc1 to i8*) +@ce1 = private global i8 * bitcast (half* @cc1 to i8*) +@cc2 = private global { i32, half } undef +; CHECK: Initializer for ce2 has disallowed type: i32* getelementptr inbounds ({ i32, half }* @cc2, i32 0, i32 0) +@ce2 = private global i32 * getelementptr ({ i32, half } * @cc2, i32 0, i32 0) + +; Circularities: here to make sure the verifier doesn't crash or assert. + +; This oddity is perfectly legal according to the IR and ABI verifiers. +; Might want to fix that. (good luck initializing one of these, though.) +%struct.snake = type { i32, %struct.tail } +%struct.tail = type { %struct.snake, i32 } +@foo = private global %struct.snake undef + +%struct.linked = type { i32, %struct.linked * } +@list1 = private global %struct.linked { i32 0, %struct.linked* null } +; CHECK-NOT: disallowed + +; CHECK: Variable alias1 is an alias (disallowed) +@alias1 = alias i32* @i32 From 9b3e4604057bd32f9b5810827e0e37200fa1236d Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Fri, 25 Jan 2013 15:49:24 -0800 Subject: [PATCH 008/295] Use 2>&1 to include stderr in pipe instead of |& Apparently OSX's bash doesn't support it. R=eliben@chromium.org,jvoung@chromium.org BUG=none TEST=llvm regression Review URL: https://codereview.chromium.org/12089002 (cherry picked from commit 32cf7c0ab28655e22b5915b8a849451d9b0ff97f) --- test/NaCl/PNaClABI/instructions.ll | 2 +- test/NaCl/PNaClABI/linkagetypes.ll | 2 +- test/NaCl/PNaClABI/types.ll | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/NaCl/PNaClABI/instructions.ll b/test/NaCl/PNaClABI/instructions.ll index 33ee9174e1d2..0ec6cc77669b 100644 --- a/test/NaCl/PNaClABI/instructions.ll +++ b/test/NaCl/PNaClABI/instructions.ll @@ -1,4 +1,4 @@ -; RUN: opt -verify-pnaclabi-functions -analyze < %s |& FileCheck %s +; RUN: opt -verify-pnaclabi-functions -analyze < %s 2>&1 | FileCheck %s ; Test instruction opcodes allowed by PNaCl ABI ; No testing yet of operands, types, attributes, etc diff --git a/test/NaCl/PNaClABI/linkagetypes.ll b/test/NaCl/PNaClABI/linkagetypes.ll index 477b7d6d9178..76a65a8d021c 100644 --- a/test/NaCl/PNaClABI/linkagetypes.ll +++ b/test/NaCl/PNaClABI/linkagetypes.ll @@ -1,4 +1,4 @@ -; RUN: opt -verify-pnaclabi-module -analyze < %s |& FileCheck %s +; RUN: opt -verify-pnaclabi-module -analyze < %s 2>&1 | FileCheck %s ; Test linkage types allowed by PNaCl ABI target datalayout = "e-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-p:32:32:32-v128:32:32" diff --git a/test/NaCl/PNaClABI/types.ll b/test/NaCl/PNaClABI/types.ll index 3656b0a51da3..f5f246f824f7 100644 --- a/test/NaCl/PNaClABI/types.ll +++ b/test/NaCl/PNaClABI/types.ll @@ -1,4 +1,4 @@ -; RUN: opt -verify-pnaclabi-module -analyze < %s |& FileCheck %s +; RUN: opt -verify-pnaclabi-module -analyze < %s 2>&1 | FileCheck %s ; Test types allowed by PNaCl ABI ; Basic global types From e29eabce86ed33e96d30858a928c4c5b0dcf427b Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Wed, 6 Feb 2013 13:21:03 -0800 Subject: [PATCH 009/295] Store bitcode verifier errors rather than dumping to stderr This allows properly implementing the analysis pass print() method, and in the future will allow the error message to be sent somewhere useful R=jvoung@chromium.org,eliben@chromium.org,mseaborn@chromium.org BUG= https://code.google.com/p/nativeclient/issues/detail?id=2309 Review URL: https://codereview.chromium.org/12220028 (cherry picked from commit 10614352cb0e3c4ea03de74d5e3337ab2223a018) --- include/llvm/Analysis/NaCl.h | 21 +++++++++++ lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp | 20 ++++++++-- lib/Analysis/NaCl/PNaClABIVerifyModule.cpp | 37 ++++++++++++------- test/NaCl/PNaClABI/instructions.ll | 2 +- test/NaCl/PNaClABI/linkagetypes.ll | 2 +- test/NaCl/PNaClABI/types.ll | 2 +- 6 files changed, 63 insertions(+), 21 deletions(-) diff --git a/include/llvm/Analysis/NaCl.h b/include/llvm/Analysis/NaCl.h index 5f8c534c6f9d..f1591075d062 100644 --- a/include/llvm/Analysis/NaCl.h +++ b/include/llvm/Analysis/NaCl.h @@ -10,6 +10,10 @@ #ifndef LLVM_ANALYSIS_NACL_H #define LLVM_ANALYSIS_NACL_H +#include "llvm/ADT/Twine.h" +#include +#include + namespace llvm { class FunctionPass; @@ -20,4 +24,21 @@ ModulePass *createPNaClABIVerifyModulePass(); } +// A simple class to store verification errors. This allows them to be saved +// and printed by the analysis passes' print() methods, while still allowing +// the messages to be easily constructed with Twine. +class ABIVerifyErrors { + public: + void addError(const llvm::Twine &Error) { + Messages.push_back(Error.str()); + } + typedef std::vector::const_iterator const_iterator; + const_iterator begin() const { return Messages.begin(); } + const_iterator end() const { return Messages.end(); } + bool empty() const { return Messages.empty(); } + void clear() { Messages.clear(); } + private: + std::vector Messages; +}; + #endif diff --git a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp index cf878dc66007..6b0691fe2450 100644 --- a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp +++ b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp @@ -32,11 +32,16 @@ struct PNaClABIVerifyFunctions : public FunctionPass { // opt -analyze to avoid dumping the result to stdout, to make testing // simpler. In the future we will probably want to make it do something // useful. - virtual void print(llvm::raw_ostream &O, const Module *M) const {}; + virtual void print(llvm::raw_ostream &O, const Module *M) const; + private: + ABIVerifyErrors Errors; }; } // and anonymous namespace bool PNaClABIVerifyFunctions::runOnFunction(Function &F) { + // For now just start with new errors on each function; this may change + // once we want to do something with them other than just calling print() + Errors.clear(); for (Function::const_iterator FI = F.begin(), FE = F.end(); FI != FE; ++FI) { for (BasicBlock::const_iterator BBI = FI->begin(), BBE = FI->end(); @@ -50,9 +55,9 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) { case Instruction::ExtractElement: case Instruction::InsertElement: case Instruction::ShuffleVector: - errs() << Twine("Function ") + F.getName() + - " has disallowed instruction: " + - BBI->getOpcodeName() + "\n"; + Errors.addError(Twine("Function ") + F.getName() + + " has disallowed instruction: " + + BBI->getOpcodeName() + "\n"); break; // Terminator instructions @@ -120,6 +125,13 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) { return false; } +void PNaClABIVerifyFunctions::print(llvm::raw_ostream &O, const Module *M) + const { + for (ABIVerifyErrors::const_iterator I = Errors.begin(), E = Errors.end(); + I != E; ++I) + O << *I; +} + char PNaClABIVerifyFunctions::ID = 0; static RegisterPass X("verify-pnaclabi-functions", diff --git a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp index 3b35668656f2..7579ede22c2e 100644 --- a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp +++ b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp @@ -35,11 +35,12 @@ struct PNaClABIVerifyModule : public ModulePass { // opt -analyze to avoid dumping the result to stdout, to make testing // simpler. In the future we will probably want to make it do something // useful. - virtual void print(llvm::raw_ostream &O, const Module *M) const {}; + virtual void print(llvm::raw_ostream &O, const Module *M) const; private: // Ideally this would share an instance with the Function pass. // TODO: see if that's feasible when we add checking in bodies TypeChecker TC; + ABIVerifyErrors Errors; }; static const char* LinkageName(GlobalValue::LinkageTypes LT) { @@ -76,17 +77,19 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) { MI != ME; ++MI) { // Check types of global variables and their initializers if (!TC.IsValidType(MI->getType())) { - errs() << (Twine("Variable ") + MI->getName() + - " has disallowed type: "); + std::string TypeName; + raw_string_ostream N(TypeName); // GVs are pointers, so print the pointed-to type for clarity - MI->getType()->getContainedType(0)->print(errs()); - errs() << "\n"; + MI->getType()->getContainedType(0)->print(N); + Errors.addError(Twine("Variable ") + MI->getName() + + " has disallowed type: " + N.str() + "\n"); } else if (MI->hasInitializer() && !TC.CheckTypesInValue(MI->getInitializer())) { - errs() << (Twine("Initializer for ") + MI->getName() + - " has disallowed type: "); - MI->getInitializer()->print(errs()); - errs() << "\n"; + std::string TypeName; + raw_string_ostream N(TypeName); + MI->getInitializer()->print(N); + Errors.addError(Twine("Initializer for ") + MI->getName() + + " has disallowed type: " + N.str() + "\n"); } // Check GV linkage types @@ -97,20 +100,26 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) { case GlobalValue::PrivateLinkage: break; default: - errs() << (Twine("Variable ") + MI->getName() + - " has disallowed linkage type: " + - LinkageName(MI->getLinkage()) + "\n"); + Errors.addError(Twine("Variable ") + MI->getName() + + " has disallowed linkage type: " + + LinkageName(MI->getLinkage()) + "\n"); } } // No aliases allowed for now. for (Module::alias_iterator MI = M.alias_begin(), E = M.alias_end(); MI != E; ++MI) - errs() << (Twine("Variable ") + MI->getName() + - " is an alias (disallowed)\n"); + Errors.addError(Twine("Variable ") + MI->getName() + + " is an alias (disallowed)\n"); return false; } +void PNaClABIVerifyModule::print(llvm::raw_ostream &O, const Module *M) const { + for (ABIVerifyErrors::const_iterator I = Errors.begin(), E = Errors.end(); + I != E; ++I) + O << *I; +} + char PNaClABIVerifyModule::ID = 0; static RegisterPass X("verify-pnaclabi-module", diff --git a/test/NaCl/PNaClABI/instructions.ll b/test/NaCl/PNaClABI/instructions.ll index 0ec6cc77669b..77bbbbb6f76a 100644 --- a/test/NaCl/PNaClABI/instructions.ll +++ b/test/NaCl/PNaClABI/instructions.ll @@ -1,4 +1,4 @@ -; RUN: opt -verify-pnaclabi-functions -analyze < %s 2>&1 | FileCheck %s +; RUN: opt -verify-pnaclabi-functions -analyze < %s | FileCheck %s ; Test instruction opcodes allowed by PNaCl ABI ; No testing yet of operands, types, attributes, etc diff --git a/test/NaCl/PNaClABI/linkagetypes.ll b/test/NaCl/PNaClABI/linkagetypes.ll index 76a65a8d021c..ab6d81f351b7 100644 --- a/test/NaCl/PNaClABI/linkagetypes.ll +++ b/test/NaCl/PNaClABI/linkagetypes.ll @@ -1,4 +1,4 @@ -; RUN: opt -verify-pnaclabi-module -analyze < %s 2>&1 | FileCheck %s +; RUN: opt -verify-pnaclabi-module -analyze < %s | FileCheck %s ; Test linkage types allowed by PNaCl ABI target datalayout = "e-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-p:32:32:32-v128:32:32" diff --git a/test/NaCl/PNaClABI/types.ll b/test/NaCl/PNaClABI/types.ll index f5f246f824f7..48630ad9b1be 100644 --- a/test/NaCl/PNaClABI/types.ll +++ b/test/NaCl/PNaClABI/types.ll @@ -1,4 +1,4 @@ -; RUN: opt -verify-pnaclabi-module -analyze < %s 2>&1 | FileCheck %s +; RUN: opt -verify-pnaclabi-module -analyze < %s | FileCheck %s ; Test types allowed by PNaCl ABI ; Basic global types From 65dd78eca0a6ed4f9df10b25d81c2af2d9b32401 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Tue, 12 Feb 2013 10:10:18 -0800 Subject: [PATCH 010/295] This CL supersedes the previous 2 outstanding CLs. 1) Remove the ABIVerifyErrors class since it was just a wrapper with no real functionality. Replace with a simple string ostream for now. If in the future we want to have more intelligent error handling, we can come up with something that does what we actually decide we want. 2) When printing type errors in initializers, just print the Type that was invalid, rather than the whole initializer. This is implemented by having checkTypesInValue return the Type that was invalid, rather than just a boolean. 3) Check the type of each instruction (this is just to make sure it works from the function pass). Do not, however, make the passes dependent as in the previous CL (function passes cannot depend on module passes, but there are no checks for this, and opt does not enforce the dependence, and when llc tries to enforce it, very bad things happen) 2) Style cleanup to match LLVM style. Sorry for the multiple changes in one CL, hopefully it's still small enough to be reviewable. BUG= Review URL: https://codereview.chromium.org/12224109 (cherry picked from commit 421e517aba7ed185ca3aaefcb495a004ae10b72d) --- include/llvm/Analysis/NaCl.h | 20 ------- lib/Analysis/NaCl/CheckTypes.h | 41 +++++++++---- lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp | 36 ++++++----- lib/Analysis/NaCl/PNaClABIVerifyModule.cpp | 59 ++++++++----------- test/NaCl/PNaClABI/instructions.ll | 3 +- test/NaCl/PNaClABI/types-function.ll | 10 ++++ test/NaCl/PNaClABI/types.ll | 6 +- 7 files changed, 95 insertions(+), 80 deletions(-) create mode 100644 test/NaCl/PNaClABI/types-function.ll diff --git a/include/llvm/Analysis/NaCl.h b/include/llvm/Analysis/NaCl.h index f1591075d062..8e599594b271 100644 --- a/include/llvm/Analysis/NaCl.h +++ b/include/llvm/Analysis/NaCl.h @@ -10,10 +10,6 @@ #ifndef LLVM_ANALYSIS_NACL_H #define LLVM_ANALYSIS_NACL_H -#include "llvm/ADT/Twine.h" -#include -#include - namespace llvm { class FunctionPass; @@ -24,21 +20,5 @@ ModulePass *createPNaClABIVerifyModulePass(); } -// A simple class to store verification errors. This allows them to be saved -// and printed by the analysis passes' print() methods, while still allowing -// the messages to be easily constructed with Twine. -class ABIVerifyErrors { - public: - void addError(const llvm::Twine &Error) { - Messages.push_back(Error.str()); - } - typedef std::vector::const_iterator const_iterator; - const_iterator begin() const { return Messages.begin(); } - const_iterator end() const { return Messages.end(); } - bool empty() const { return Messages.empty(); } - void clear() { Messages.clear(); } - private: - std::vector Messages; -}; #endif diff --git a/lib/Analysis/NaCl/CheckTypes.h b/lib/Analysis/NaCl/CheckTypes.h index 336dc0302f5c..dad0ff094d2a 100644 --- a/lib/Analysis/NaCl/CheckTypes.h +++ b/lib/Analysis/NaCl/CheckTypes.h @@ -21,7 +21,7 @@ class TypeChecker { public: - bool IsValidType(const llvm::Type* Ty) { + bool isValidType(const llvm::Type *Ty) { if (VisitedTypes.count(Ty)) return VisitedTypes[Ty]; @@ -64,7 +64,7 @@ class TypeChecker { VisitedTypes[Ty] = true; for (llvm::Type::subtype_iterator I = Ty->subtype_begin(), E = Ty->subtype_end(); I != E; ++I) - Valid &= IsValidType(*I); + Valid &= isValidType(*I); break; // Handle NumTypeIDs, and no default case, // so we get a warning if new types are added @@ -77,7 +77,9 @@ class TypeChecker { return Valid; } - bool CheckTypesInValue(const llvm::Value* V) { + // If the value contains an invalid type, return a pointer to the type. + // Return null if there are no invalid types. + llvm::Type *checkTypesInValue(const llvm::Value *V) { // TODO: Checking types in values probably belongs in its // own value checker which also handles the various types of // constexpr (in particular, blockaddr constexprs cause this code @@ -87,22 +89,41 @@ class TypeChecker { if (VisitedConstants.count(V)) return VisitedConstants[V]; - bool Valid = IsValidType(V->getType()); - VisitedConstants[V] = Valid; + if (!isValidType(V->getType())) { + VisitedConstants[V] = V->getType(); + return V->getType(); + } + // Operand values must also be valid. Values may be circular, so + // mark the current value as valid to avoid infinite recursion. + VisitedConstants[V] = NULL; const llvm::User *U = llvm::cast(V); for (llvm::Constant::const_op_iterator I = U->op_begin(), - E = U->op_end(); I != E; ++I) - Valid &= CheckTypesInValue(*I); - VisitedConstants[V] = Valid; - return Valid; + E = U->op_end(); I != E; ++I) { + llvm::Type *Invalid = checkTypesInValue(*I); + if (Invalid) { + VisitedConstants[V] = Invalid; + return Invalid; + } + } + VisitedConstants[V] = NULL; + return NULL; + } + + // There's no built-in way to get the name of a type, so use a + // string ostream to print it. + static std::string getTypeName(const llvm::Type *T) { + std::string TypeName; + llvm::raw_string_ostream N(TypeName); + T->print(N); + return N.str(); } private: // To avoid walking constexprs and types multiple times, keep a cache of // what we have seen. This is also used to prevent infinite recursion e.g. // in case of structures like linked lists with pointers to themselves. - llvm::DenseMap VisitedConstants; + llvm::DenseMap VisitedConstants; llvm::DenseMap VisitedTypes; }; diff --git a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp index 6b0691fe2450..c561a842949d 100644 --- a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp +++ b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp @@ -18,30 +18,32 @@ #include "llvm/Pass.h" #include "llvm/Support/raw_ostream.h" #include "llvm/Analysis/NaCl.h" + +#include "CheckTypes.h" using namespace llvm; namespace { // Checks that examine anything in the function body should be in // FunctionPasses to make them streaming-friendly -struct PNaClABIVerifyFunctions : public FunctionPass { +class PNaClABIVerifyFunctions : public FunctionPass { + public: static char ID; - PNaClABIVerifyFunctions() : FunctionPass(ID) {} + PNaClABIVerifyFunctions() : FunctionPass(ID), Errors(ErrorsString) {} bool runOnFunction(Function &F); - // For now, this print method exists to allow us to run the pass with - // opt -analyze to avoid dumping the result to stdout, to make testing - // simpler. In the future we will probably want to make it do something - // useful. virtual void print(llvm::raw_ostream &O, const Module *M) const; private: - ABIVerifyErrors Errors; + TypeChecker TC; + std::string ErrorsString; + raw_string_ostream Errors; }; + } // and anonymous namespace bool PNaClABIVerifyFunctions::runOnFunction(Function &F) { // For now just start with new errors on each function; this may change // once we want to do something with them other than just calling print() - Errors.clear(); + ErrorsString.clear(); for (Function::const_iterator FI = F.begin(), FE = F.end(); FI != FE; ++FI) { for (BasicBlock::const_iterator BBI = FI->begin(), BBE = FI->end(); @@ -55,9 +57,9 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) { case Instruction::ExtractElement: case Instruction::InsertElement: case Instruction::ShuffleVector: - Errors.addError(Twine("Function ") + F.getName() + - " has disallowed instruction: " + - BBI->getOpcodeName() + "\n"); + Errors << "Function " + F.getName() + + " has disallowed instruction: " + + BBI->getOpcodeName() + "\n"; break; // Terminator instructions @@ -120,16 +122,22 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) { case Instruction::LandingPad: break; } + // Check the types. First check the type of the instruction. + if (!TC.isValidType(BBI->getType())) { + Errors << "Function " + F.getName() + + " has instruction with disallowed type: " + + TypeChecker::getTypeName(BBI->getType()) + "\n"; + } } } + + Errors.flush(); return false; } void PNaClABIVerifyFunctions::print(llvm::raw_ostream &O, const Module *M) const { - for (ABIVerifyErrors::const_iterator I = Errors.begin(), E = Errors.end(); - I != E; ++I) - O << *I; + O << ErrorsString; } char PNaClABIVerifyFunctions::ID = 0; diff --git a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp index 7579ede22c2e..12f1485b5427 100644 --- a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp +++ b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp @@ -25,25 +25,20 @@ using namespace llvm; namespace { - // This pass should not touch function bodies, to stay streaming-friendly -struct PNaClABIVerifyModule : public ModulePass { +class PNaClABIVerifyModule : public ModulePass { + public: static char ID; - PNaClABIVerifyModule() : ModulePass(ID) {} + PNaClABIVerifyModule(); bool runOnModule(Module &M); - // For now, this print method exists to allow us to run the pass with - // opt -analyze to avoid dumping the result to stdout, to make testing - // simpler. In the future we will probably want to make it do something - // useful. virtual void print(llvm::raw_ostream &O, const Module *M) const; private: - // Ideally this would share an instance with the Function pass. - // TODO: see if that's feasible when we add checking in bodies TypeChecker TC; - ABIVerifyErrors Errors; + std::string ErrorsString; + llvm::raw_string_ostream Errors; }; -static const char* LinkageName(GlobalValue::LinkageTypes LT) { +static const char *linkageName(GlobalValue::LinkageTypes LT) { // This logic is taken from PrintLinkage in lib/VMCore/AsmWriter.cpp switch (LT) { case GlobalValue::ExternalLinkage: return "external"; @@ -71,25 +66,25 @@ static const char* LinkageName(GlobalValue::LinkageTypes LT) { } // end anonymous namespace -bool PNaClABIVerifyModule::runOnModule(Module &M) { +PNaClABIVerifyModule::PNaClABIVerifyModule() : ModulePass(ID), + Errors(ErrorsString) {} +bool PNaClABIVerifyModule::runOnModule(Module &M) { for (Module::const_global_iterator MI = M.global_begin(), ME = M.global_end(); MI != ME; ++MI) { // Check types of global variables and their initializers - if (!TC.IsValidType(MI->getType())) { - std::string TypeName; - raw_string_ostream N(TypeName); + if (!TC.isValidType(MI->getType())) { // GVs are pointers, so print the pointed-to type for clarity - MI->getType()->getContainedType(0)->print(N); - Errors.addError(Twine("Variable ") + MI->getName() + - " has disallowed type: " + N.str() + "\n"); - } else if (MI->hasInitializer() && - !TC.CheckTypesInValue(MI->getInitializer())) { - std::string TypeName; - raw_string_ostream N(TypeName); - MI->getInitializer()->print(N); - Errors.addError(Twine("Initializer for ") + MI->getName() + - " has disallowed type: " + N.str() + "\n"); + Errors << "Variable " + MI->getName() + + " has disallowed type: " + + TypeChecker::getTypeName(MI->getType()->getContainedType(0)) + "\n"; + } else if (MI->hasInitializer()) { + // If the type of the global is bad, no point in checking its initializer + Type *T = TC.checkTypesInValue(MI->getInitializer()); + if (T) + Errors << "Initializer for " + MI->getName() + + " has disallowed type: " + + TypeChecker::getTypeName(T) + "\n"; } // Check GV linkage types @@ -100,24 +95,22 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) { case GlobalValue::PrivateLinkage: break; default: - Errors.addError(Twine("Variable ") + MI->getName() + - " has disallowed linkage type: " + - LinkageName(MI->getLinkage()) + "\n"); + Errors << "Variable " + MI->getName() + + " has disallowed linkage type: " + + linkageName(MI->getLinkage()) + "\n"; } } // No aliases allowed for now. for (Module::alias_iterator MI = M.alias_begin(), E = M.alias_end(); MI != E; ++MI) - Errors.addError(Twine("Variable ") + MI->getName() + - " is an alias (disallowed)\n"); + Errors << "Variable " + MI->getName() + " is an alias (disallowed)\n"; + Errors.flush(); return false; } void PNaClABIVerifyModule::print(llvm::raw_ostream &O, const Module *M) const { - for (ABIVerifyErrors::const_iterator I = Errors.begin(), E = Errors.end(); - I != E; ++I) - O << *I; + O << ErrorsString; } char PNaClABIVerifyModule::ID = 0; diff --git a/test/NaCl/PNaClABI/instructions.ll b/test/NaCl/PNaClABI/instructions.ll index 77bbbbb6f76a..eb6f0a51d60f 100644 --- a/test/NaCl/PNaClABI/instructions.ll +++ b/test/NaCl/PNaClABI/instructions.ll @@ -47,10 +47,11 @@ define void @vectors() { ; CHECK-NOT: disallowed %a1 = extractelement <2 x i32> , i32 0 ; CHECK: Function vectors has disallowed instruction: extractelement - %a2 = shufflevector <2 x i32> undef , <2 x i32> undef, <2 x i32> undef + %a2 = shufflevector <2 x i32> undef, <2 x i32> undef, <2 x i32> undef ; CHECK: Function vectors has disallowed instruction: shufflevector %a3 = insertelement <2 x i32> undef, i32 1, i32 0 ; CHECK: Function vectors has disallowed instruction: insertelement +; CHECK: Function vectors has instruction with disallowed type ret void } diff --git a/test/NaCl/PNaClABI/types-function.ll b/test/NaCl/PNaClABI/types-function.ll new file mode 100644 index 000000000000..0302d3715d95 --- /dev/null +++ b/test/NaCl/PNaClABI/types-function.ll @@ -0,0 +1,10 @@ +; RUN: opt -verify-pnaclabi-functions -analyze < %s | FileCheck %s +; Test type-checking in function bodies. This test is not intended to verify +; all the rules about the various types, but instead to make sure that types +; stashed in various places in function bodies are caught. + +define void @types() { +; CHECK: Function types has instruction with disallowed type: half + %h1 = fptrunc double undef to half + ret void +} \ No newline at end of file diff --git a/test/NaCl/PNaClABI/types.ll b/test/NaCl/PNaClABI/types.ll index 48630ad9b1be..0c68036a73c5 100644 --- a/test/NaCl/PNaClABI/types.ll +++ b/test/NaCl/PNaClABI/types.ll @@ -67,10 +67,10 @@ ; Initializers with constexprs ; CHECK: Variable cc1 has disallowed type: half @cc1 = private global half 0.0 -; CHECK: Initializer for ce1 has disallowed type: i8* bitcast (half* @cc1 to i8*) +; CHECK: Initializer for ce1 has disallowed type: half* @ce1 = private global i8 * bitcast (half* @cc1 to i8*) @cc2 = private global { i32, half } undef -; CHECK: Initializer for ce2 has disallowed type: i32* getelementptr inbounds ({ i32, half }* @cc2, i32 0, i32 0) +; CHECK: Initializer for ce2 has disallowed type: { i32, half }* @ce2 = private global i32 * getelementptr ({ i32, half } * @cc2, i32 0, i32 0) ; Circularities: here to make sure the verifier doesn't crash or assert. @@ -83,6 +83,8 @@ %struct.linked = type { i32, %struct.linked * } @list1 = private global %struct.linked { i32 0, %struct.linked* null } + +@list2 = private global i32* bitcast (i32** @list2 to i32*) ; CHECK-NOT: disallowed ; CHECK: Variable alias1 is an alias (disallowed) From f1846ee6222f981a00eebc347dd76550fd05fcd2 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Tue, 19 Feb 2013 15:13:36 -0800 Subject: [PATCH 011/295] Move ABI type checker to its own file R=jvoung@chromium.org,eliben@chromium.org BUG= Review URL: https://codereview.chromium.org/12302030 (cherry picked from commit 06dd2371667be256a846a68e51d376349f041752) Conflicts: lib/Analysis/NaCl/CheckTypes.h --- lib/Analysis/NaCl/CMakeLists.txt | 1 + lib/Analysis/NaCl/CheckTypes.h | 130 ------------------ lib/Analysis/NaCl/PNaClABITypeChecker.cpp | 106 ++++++++++++++ lib/Analysis/NaCl/PNaClABITypeChecker.h | 52 +++++++ lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp | 8 +- lib/Analysis/NaCl/PNaClABIVerifyModule.cpp | 13 +- 6 files changed, 170 insertions(+), 140 deletions(-) delete mode 100644 lib/Analysis/NaCl/CheckTypes.h create mode 100644 lib/Analysis/NaCl/PNaClABITypeChecker.cpp create mode 100644 lib/Analysis/NaCl/PNaClABITypeChecker.h diff --git a/lib/Analysis/NaCl/CMakeLists.txt b/lib/Analysis/NaCl/CMakeLists.txt index 8a5eceb3e4d4..7fa3a134ac94 100644 --- a/lib/Analysis/NaCl/CMakeLists.txt +++ b/lib/Analysis/NaCl/CMakeLists.txt @@ -1,4 +1,5 @@ add_llvm_library(LLVMAnalysisNaCl + PNaCLABITypeChecker.cpp PNaClABIVerifyFunctions.cpp PNaClABIVerifyModule.cpp ) diff --git a/lib/Analysis/NaCl/CheckTypes.h b/lib/Analysis/NaCl/CheckTypes.h deleted file mode 100644 index dad0ff094d2a..000000000000 --- a/lib/Analysis/NaCl/CheckTypes.h +++ /dev/null @@ -1,130 +0,0 @@ -//===- CheckTypes.h - Verify PNaCl ABI rules --------===// -// -// The LLVM Compiler Infrastructure -// -// This file is distributed under the University of Illinois Open Source -// License. See LICENSE.TXT for details. -// -//===----------------------------------------------------------------------===// -// -// Common type-checking code for module and function-level passes -// -// -//===----------------------------------------------------------------------===// - -#ifndef LIB_ANALYSIS_NACL_CHECKTYPES_H -#define LIB_ANALYSIS_NACL_CHECKTYPES_H - -#include "llvm/ADT/DenseSet.h" -#include "llvm/DerivedTypes.h" -#include "llvm/Support/raw_ostream.h" - -class TypeChecker { - public: - bool isValidType(const llvm::Type *Ty) { - if (VisitedTypes.count(Ty)) - return VisitedTypes[Ty]; - - unsigned Width; - bool Valid = false; - switch (Ty->getTypeID()) { - // Allowed primitive types - case llvm::Type::VoidTyID: - case llvm::Type::FloatTyID: - case llvm::Type::DoubleTyID: - case llvm::Type::LabelTyID: - case llvm::Type::MetadataTyID: - Valid = true; - break; - // Disallowed primitive types - case llvm::Type::HalfTyID: - case llvm::Type::X86_FP80TyID: - case llvm::Type::FP128TyID: - case llvm::Type::PPC_FP128TyID: - case llvm::Type::X86_MMXTyID: - Valid = false; - break; - // Derived types - case llvm::Type::VectorTyID: - Valid = false; - break; - case llvm::Type::IntegerTyID: - Width = llvm::cast(Ty)->getBitWidth(); - Valid = (Width == 1 || Width == 8 || Width == 16 || - Width == 32 || Width == 64); - break; - case llvm::Type::FunctionTyID: - case llvm::Type::StructTyID: - case llvm::Type::ArrayTyID: - case llvm::Type::PointerTyID: - // These types are valid if their contained or pointed-to types are - // valid. Since struct/pointer subtype relationships may be circular, - // mark the current type as valid to avoid infinite recursion - Valid = true; - VisitedTypes[Ty] = true; - for (llvm::Type::subtype_iterator I = Ty->subtype_begin(), - E = Ty->subtype_end(); I != E; ++I) - Valid &= isValidType(*I); - break; - // Handle NumTypeIDs, and no default case, - // so we get a warning if new types are added - case llvm::Type::NumTypeIDs: - Valid = false; - break; - } - - VisitedTypes[Ty] = Valid; - return Valid; - } - - // If the value contains an invalid type, return a pointer to the type. - // Return null if there are no invalid types. - llvm::Type *checkTypesInValue(const llvm::Value *V) { - // TODO: Checking types in values probably belongs in its - // own value checker which also handles the various types of - // constexpr (in particular, blockaddr constexprs cause this code - // to assert rather than go off and try to verify the BBs of a function) - // But this code is in a good consistent checkpoint-able state. - assert(llvm::isa(V)); - if (VisitedConstants.count(V)) - return VisitedConstants[V]; - - if (!isValidType(V->getType())) { - VisitedConstants[V] = V->getType(); - return V->getType(); - } - - // Operand values must also be valid. Values may be circular, so - // mark the current value as valid to avoid infinite recursion. - VisitedConstants[V] = NULL; - const llvm::User *U = llvm::cast(V); - for (llvm::Constant::const_op_iterator I = U->op_begin(), - E = U->op_end(); I != E; ++I) { - llvm::Type *Invalid = checkTypesInValue(*I); - if (Invalid) { - VisitedConstants[V] = Invalid; - return Invalid; - } - } - VisitedConstants[V] = NULL; - return NULL; - } - - // There's no built-in way to get the name of a type, so use a - // string ostream to print it. - static std::string getTypeName(const llvm::Type *T) { - std::string TypeName; - llvm::raw_string_ostream N(TypeName); - T->print(N); - return N.str(); - } - - private: - // To avoid walking constexprs and types multiple times, keep a cache of - // what we have seen. This is also used to prevent infinite recursion e.g. - // in case of structures like linked lists with pointers to themselves. - llvm::DenseMap VisitedConstants; - llvm::DenseMap VisitedTypes; -}; - -#endif // LIB_ANALYSIS_NACL_CHECKTYPES_H diff --git a/lib/Analysis/NaCl/PNaClABITypeChecker.cpp b/lib/Analysis/NaCl/PNaClABITypeChecker.cpp new file mode 100644 index 000000000000..3a1812958895 --- /dev/null +++ b/lib/Analysis/NaCl/PNaClABITypeChecker.cpp @@ -0,0 +1,106 @@ +//===- PNaClABICheckTypes.h - Verify PNaCl ABI rules --------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// Common type-checking code for module and function-level passes +// +// +//===----------------------------------------------------------------------===// + +#include "PNaClABITypeChecker.h" +#include "llvm/IR/Constant.h" +#include "llvm/IR/DerivedTypes.h" + +using namespace llvm; + +bool PNaClABITypeChecker::isValidType(const Type *Ty) { + if (VisitedTypes.count(Ty)) + return VisitedTypes[Ty]; + + unsigned Width; + bool Valid = false; + switch (Ty->getTypeID()) { + // Allowed primitive types + case Type::VoidTyID: + case Type::FloatTyID: + case Type::DoubleTyID: + case Type::LabelTyID: + case Type::MetadataTyID: + Valid = true; + break; + // Disallowed primitive types + case Type::HalfTyID: + case Type::X86_FP80TyID: + case Type::FP128TyID: + case Type::PPC_FP128TyID: + case Type::X86_MMXTyID: + Valid = false; + break; + // Derived types + case Type::VectorTyID: + Valid = false; + break; + case Type::IntegerTyID: + Width = cast(Ty)->getBitWidth(); + Valid = (Width == 1 || Width == 8 || Width == 16 || + Width == 32 || Width == 64); + break; + case Type::FunctionTyID: + case Type::StructTyID: + case Type::ArrayTyID: + case Type::PointerTyID: + // These types are valid if their contained or pointed-to types are + // valid. Since struct/pointer subtype relationships may be circular, + // mark the current type as valid to avoid infinite recursion + Valid = true; + VisitedTypes[Ty] = true; + for (Type::subtype_iterator I = Ty->subtype_begin(), + E = Ty->subtype_end(); I != E; ++I) + Valid &= isValidType(*I); + break; + // Handle NumTypeIDs, and no default case, + // so we get a warning if new types are added + case Type::NumTypeIDs: + Valid = false; + break; + } + + VisitedTypes[Ty] = Valid; + return Valid; +} + +Type *PNaClABITypeChecker::checkTypesInValue(const Value *V) { + // TODO: Checking types in values probably belongs in its + // own value checker which also handles the various types of + // constexpr (in particular, blockaddr constexprs cause this code + // to assert rather than go off and try to verify the BBs of a function) + // But this code is in a good consistent checkpoint-able state. + assert(isa(V)); + if (VisitedConstants.count(V)) + return VisitedConstants[V]; + + if (!isValidType(V->getType())) { + VisitedConstants[V] = V->getType(); + return V->getType(); + } + + // Operand values must also be valid. Values may be circular, so + // mark the current value as valid to avoid infinite recursion. + VisitedConstants[V] = NULL; + const User *U = cast(V); + for (Constant::const_op_iterator I = U->op_begin(), + E = U->op_end(); I != E; ++I) { + Type *Invalid = checkTypesInValue(*I); + if (Invalid) { + VisitedConstants[V] = Invalid; + return Invalid; + } + } + VisitedConstants[V] = NULL; + return NULL; +} diff --git a/lib/Analysis/NaCl/PNaClABITypeChecker.h b/lib/Analysis/NaCl/PNaClABITypeChecker.h new file mode 100644 index 000000000000..1249bf04cd8c --- /dev/null +++ b/lib/Analysis/NaCl/PNaClABITypeChecker.h @@ -0,0 +1,52 @@ +//===- CheckTypes.h - Verify PNaCl ABI rules --------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// Common type-checking code for module and function-level passes +// +// +//===----------------------------------------------------------------------===// + +#ifndef LIB_ANALYSIS_NACL_CHECKTYPES_H +#define LIB_ANALYSIS_NACL_CHECKTYPES_H + +#include "llvm/ADT/DenseSet.h" +#include "llvm/IR/Type.h" +#include "llvm/Support/raw_ostream.h" + +namespace llvm { +class Value; + +class PNaClABITypeChecker { + public: + // Returns true if Ty is a valid type for PNaCl. + bool isValidType(const Type *Ty); + + // If the value contains an invalid type, return a pointer to the type. + // Return null if there are no invalid types. + Type *checkTypesInValue(const Value *V); + + // There's no built-in way to get the name of a type, so use a + // string ostream to print it. + static std::string getTypeName(const Type *T) { + std::string TypeName; + raw_string_ostream N(TypeName); + T->print(N); + return N.str(); + } + + private: + // To avoid walking constexprs and types multiple times, keep a cache of + // what we have seen. This is also used to prevent infinite recursion e.g. + // in case of structures like linked lists with pointers to themselves. + DenseMap VisitedConstants; + DenseMap VisitedTypes; +}; +} // namespace llvm + +#endif // LIB_ANALYSIS_NACL_CHECKTYPES_H diff --git a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp index c561a842949d..430528e3696d 100644 --- a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp +++ b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp @@ -19,7 +19,7 @@ #include "llvm/Support/raw_ostream.h" #include "llvm/Analysis/NaCl.h" -#include "CheckTypes.h" +#include "PNaClABITypeChecker.h" using namespace llvm; namespace { @@ -31,9 +31,9 @@ class PNaClABIVerifyFunctions : public FunctionPass { static char ID; PNaClABIVerifyFunctions() : FunctionPass(ID), Errors(ErrorsString) {} bool runOnFunction(Function &F); - virtual void print(llvm::raw_ostream &O, const Module *M) const; + virtual void print(raw_ostream &O, const Module *M) const; private: - TypeChecker TC; + PNaClABITypeChecker TC; std::string ErrorsString; raw_string_ostream Errors; }; @@ -126,7 +126,7 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) { if (!TC.isValidType(BBI->getType())) { Errors << "Function " + F.getName() + " has instruction with disallowed type: " + - TypeChecker::getTypeName(BBI->getType()) + "\n"; + PNaClABITypeChecker::getTypeName(BBI->getType()) + "\n"; } } } diff --git a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp index 12f1485b5427..aee729612020 100644 --- a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp +++ b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp @@ -21,7 +21,7 @@ #include "llvm/Support/raw_ostream.h" #include "llvm/Analysis/NaCl.h" -#include "CheckTypes.h" +#include "PNaClABITypeChecker.h" using namespace llvm; namespace { @@ -31,11 +31,11 @@ class PNaClABIVerifyModule : public ModulePass { static char ID; PNaClABIVerifyModule(); bool runOnModule(Module &M); - virtual void print(llvm::raw_ostream &O, const Module *M) const; + virtual void print(raw_ostream &O, const Module *M) const; private: - TypeChecker TC; + PNaClABITypeChecker TC; std::string ErrorsString; - llvm::raw_string_ostream Errors; + raw_string_ostream Errors; }; static const char *linkageName(GlobalValue::LinkageTypes LT) { @@ -77,14 +77,15 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) { // GVs are pointers, so print the pointed-to type for clarity Errors << "Variable " + MI->getName() + " has disallowed type: " + - TypeChecker::getTypeName(MI->getType()->getContainedType(0)) + "\n"; + PNaClABITypeChecker::getTypeName(MI->getType()->getContainedType(0)) + + "\n"; } else if (MI->hasInitializer()) { // If the type of the global is bad, no point in checking its initializer Type *T = TC.checkTypesInValue(MI->getInitializer()); if (T) Errors << "Initializer for " + MI->getName() + " has disallowed type: " + - TypeChecker::getTypeName(T) + "\n"; + PNaClABITypeChecker::getTypeName(T) + "\n"; } // Check GV linkage types From 27f9757fce2ab3c6bf9ce3fea9171dd9dea4d809 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Wed, 20 Feb 2013 14:31:59 -0800 Subject: [PATCH 012/295] ABI Verifier: Check function return and argument types, and instruction operands. This should cover everywhere types can hide, except metadata. R=jvoung@chromium.org,eliben@chromium.org BUG= https://code.google.com/p/nativeclient/issues/detail?id=2309 Review URL: https://codereview.chromium.org/12319028 (cherry picked from commit 36749ed09580dfff415e80eb98843e44c123e427) Conflicts: lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp --- lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp | 25 ++++++++++++++++--- lib/Analysis/NaCl/PNaClABIVerifyModule.cpp | 15 +++++++++++ test/NaCl/PNaClABI/instructions.ll | 9 ++++--- test/NaCl/PNaClABI/types-function.ll | 21 +++++++++++++++- test/NaCl/PNaClABI/types.ll | 8 ++++++ 5 files changed, 70 insertions(+), 8 deletions(-) diff --git a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp index 430528e3696d..ef1c26e17501 100644 --- a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp +++ b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp @@ -12,10 +12,10 @@ // //===----------------------------------------------------------------------===// -#include "llvm/ADT/Twine.h" -#include "llvm/Function.h" -#include "llvm/Instruction.h" #include "llvm/Pass.h" +#include "llvm/ADT/Twine.h" +#include "llvm/IR/Function.h" +#include "llvm/IR/Instructions.h" #include "llvm/Support/raw_ostream.h" #include "llvm/Analysis/NaCl.h" @@ -44,6 +44,7 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) { // For now just start with new errors on each function; this may change // once we want to do something with them other than just calling print() ErrorsString.clear(); + // TODO: only report one error per instruction for (Function::const_iterator FI = F.begin(), FE = F.end(); FI != FE; ++FI) { for (BasicBlock::const_iterator BBI = FI->begin(), BBE = FI->end(); @@ -128,6 +129,24 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) { " has instruction with disallowed type: " + PNaClABITypeChecker::getTypeName(BBI->getType()) + "\n"; } + + // Check the instruction operands. Operands which are Instructions will + // be checked on their own here, and GlobalValues will be checked by the + // Module verifier. That leaves Constants. + // Switches are implemented in the in-memory IR with vectors, so don't + // check them. + if (!isa(*BBI)) + for (User::const_op_iterator OI = BBI->op_begin(), OE = BBI->op_end(); + OI != OE; OI++) { + if (isa(OI) && !isa(OI) && + !isa(OI)) { + Type *T = TC.checkTypesInValue(*OI); + if (T) + Errors << "Function " + F.getName() + + " has instruction operand with disallowed type: " + + PNaClABITypeChecker::getTypeName(T) + "\n"; + } + } } } diff --git a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp index aee729612020..f17a6fcf37a3 100644 --- a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp +++ b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp @@ -106,6 +106,21 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) { E = M.alias_end(); MI != E; ++MI) Errors << "Variable " + MI->getName() + " is an alias (disallowed)\n"; + for (Module::iterator MI = M.begin(), ME = M.end(); MI != ME; ++MI) { + // Check types of functions and their arguments + FunctionType *FT = MI->getFunctionType(); + if (!TC.isValidType(FT->getReturnType())) + Errors << "Function " + MI->getName() + " has disallowed return type: " + + PNaClABITypeChecker::getTypeName(FT->getReturnType()) + "\n"; + for (unsigned I = 0, E = FT->getNumParams(); I < E; ++I) { + Type *PT = FT->getParamType(I); + if (!TC.isValidType(PT)) + Errors << "Function " << MI->getName() << " argument " << I + 1 << + " has disallowed type: " << + PNaClABITypeChecker::getTypeName(PT) + "\n"; + } + } + Errors.flush(); return false; } diff --git a/test/NaCl/PNaClABI/instructions.ll b/test/NaCl/PNaClABI/instructions.ll index eb6f0a51d60f..018d66c87634 100644 --- a/test/NaCl/PNaClABI/instructions.ll +++ b/test/NaCl/PNaClABI/instructions.ll @@ -15,9 +15,9 @@ next: next2: unreachable resume i8 0 - indirectbr i8* undef, [label %next, label %next2] ; CHECK-NOT: disallowed ; CHECK: Function terminators has disallowed instruction: indirectbr + indirectbr i8* undef, [label %next, label %next2] } define void @binops() { @@ -45,13 +45,14 @@ define void @binops() { define void @vectors() { ; CHECK-NOT: disallowed - %a1 = extractelement <2 x i32> , i32 0 ; CHECK: Function vectors has disallowed instruction: extractelement - %a2 = shufflevector <2 x i32> undef, <2 x i32> undef, <2 x i32> undef + %a1 = extractelement <2 x i32> , i32 0 ; CHECK: Function vectors has disallowed instruction: shufflevector - %a3 = insertelement <2 x i32> undef, i32 1, i32 0 + %a2 = shufflevector <2 x i32> undef, <2 x i32> undef, <2 x i32> undef ; CHECK: Function vectors has disallowed instruction: insertelement ; CHECK: Function vectors has instruction with disallowed type +; CHECK: Function vectors has instruction operand with disallowed type + %a3 = insertelement <2 x i32> undef, i32 1, i32 0 ret void } diff --git a/test/NaCl/PNaClABI/types-function.ll b/test/NaCl/PNaClABI/types-function.ll index 0302d3715d95..eb8aedcef508 100644 --- a/test/NaCl/PNaClABI/types-function.ll +++ b/test/NaCl/PNaClABI/types-function.ll @@ -3,8 +3,27 @@ ; all the rules about the various types, but instead to make sure that types ; stashed in various places in function bodies are caught. +@a2 = private global i17 zeroinitializer +declare void @func(i15 %arg) + define void @types() { ; CHECK: Function types has instruction with disallowed type: half %h1 = fptrunc double undef to half +; CHECK: Function types has instruction operand with disallowed type: half + %h2 = bitcast half 0.0 to i16 +; see below... + %h3 = fadd double 0.0, fpext (half 0.0 to double) +; CHECK: Function types has instruction operand with disallowed type: i17* + store i32 0, i32* bitcast (i17* @a2 to i32*), align 4 +; CHECK: Function types has instruction operand with disallowed type: i15 + call void @func(i15 1) ret void -} \ No newline at end of file +} +; CHECK-NOT: disallowed + + +; TODO: +; the bitcode reader seems to expand some operations inline +; (e.g. fpext, sext, uitofp) such that doing something like +; %h3 = fadd double 0.0, fpext (half 0.0 to double) +; means the verifier pass will never see the fpext or its operands diff --git a/test/NaCl/PNaClABI/types.ll b/test/NaCl/PNaClABI/types.ll index 0c68036a73c5..0447ea07ee96 100644 --- a/test/NaCl/PNaClABI/types.ll +++ b/test/NaCl/PNaClABI/types.ll @@ -89,3 +89,11 @@ ; CHECK: Variable alias1 is an alias (disallowed) @alias1 = alias i32* @i32 + +; CHECK: Function badReturn has disallowed return type +declare half* @badReturn() + +; CHECK: Function badArgType1 argument 1 has disallowed type +declare void @badArgType1(half %a, i32 %b) +; CHECK: Function badArgType2 argument 2 has disallowed type +declare void @badArgType2(i32 %a, half %b) \ No newline at end of file From 986706f37c7627510950237c9fbf13d66eba7303 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Fri, 22 Feb 2013 13:37:16 -0800 Subject: [PATCH 013/295] ABI Verifier: Check types in metadata nodes This should be the last source of type references R=jvoung@chromium.org,eliben@chromium.org BUG= https://code.google.com/p/nativeclient/issues/detail?id=2309 Review URL: https://codereview.chromium.org/12321032 (cherry picked from commit 30f2f338c415a51b59f02bc0e4c7ccfce947248d) --- lib/Analysis/NaCl/PNaClABITypeChecker.cpp | 35 ++++++++++++++----- lib/Analysis/NaCl/PNaClABITypeChecker.h | 8 ++++- lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp | 17 +++++++-- lib/Analysis/NaCl/PNaClABIVerifyModule.cpp | 12 ++++++- test/NaCl/PNaClABI/types-function.ll | 6 +++- test/NaCl/PNaClABI/types.ll | 6 +++- 6 files changed, 69 insertions(+), 15 deletions(-) diff --git a/lib/Analysis/NaCl/PNaClABITypeChecker.cpp b/lib/Analysis/NaCl/PNaClABITypeChecker.cpp index 3a1812958895..dd319a3a9f56 100644 --- a/lib/Analysis/NaCl/PNaClABITypeChecker.cpp +++ b/lib/Analysis/NaCl/PNaClABITypeChecker.cpp @@ -15,6 +15,7 @@ #include "PNaClABITypeChecker.h" #include "llvm/IR/Constant.h" #include "llvm/IR/DerivedTypes.h" +#include "llvm/IR/Metadata.h" using namespace llvm; @@ -74,13 +75,14 @@ bool PNaClABITypeChecker::isValidType(const Type *Ty) { return Valid; } -Type *PNaClABITypeChecker::checkTypesInValue(const Value *V) { - // TODO: Checking types in values probably belongs in its - // own value checker which also handles the various types of - // constexpr (in particular, blockaddr constexprs cause this code - // to assert rather than go off and try to verify the BBs of a function) - // But this code is in a good consistent checkpoint-able state. - assert(isa(V)); +Type *PNaClABITypeChecker::checkTypesInConstant(const Constant *V) { + // TODO: blockaddr constexprs cause this code + // to assert rather than go off and try to verify the BBs of a function + // We may just ban blockaddr constexprs, or maybe just ban them in GVs, which + // is really the thing that's streaming-unfriendly. If we end up with more + // complex non-type-related rules for constexprs, maybe this could get its + // own file. + if (!V) return NULL; if (VisitedConstants.count(V)) return VisitedConstants[V]; @@ -95,7 +97,7 @@ Type *PNaClABITypeChecker::checkTypesInValue(const Value *V) { const User *U = cast(V); for (Constant::const_op_iterator I = U->op_begin(), E = U->op_end(); I != E; ++I) { - Type *Invalid = checkTypesInValue(*I); + Type *Invalid = checkTypesInConstant(cast(*I)); if (Invalid) { VisitedConstants[V] = Invalid; return Invalid; @@ -104,3 +106,20 @@ Type *PNaClABITypeChecker::checkTypesInValue(const Value *V) { VisitedConstants[V] = NULL; return NULL; } + + +// MDNodes don't support the same way of iterating over operands that Users do +Type *PNaClABITypeChecker::checkTypesInMDNode(const MDNode *N) { + if (VisitedConstants.count(N)) + return VisitedConstants[N]; + + for (unsigned i = 0, e = N->getNumOperands(); i != e; i++) { + if (Value *Op = N->getOperand(i)) { + if (Type *Invalid = checkTypesInConstant(dyn_cast(Op))) { + VisitedConstants[N] = Invalid; + return Invalid; + } + } + } + return NULL; +} diff --git a/lib/Analysis/NaCl/PNaClABITypeChecker.h b/lib/Analysis/NaCl/PNaClABITypeChecker.h index 1249bf04cd8c..eed2dc76b6d0 100644 --- a/lib/Analysis/NaCl/PNaClABITypeChecker.h +++ b/lib/Analysis/NaCl/PNaClABITypeChecker.h @@ -20,6 +20,8 @@ #include "llvm/Support/raw_ostream.h" namespace llvm { +class Constant; +class MDNode; class Value; class PNaClABITypeChecker { @@ -29,7 +31,11 @@ class PNaClABITypeChecker { // If the value contains an invalid type, return a pointer to the type. // Return null if there are no invalid types. - Type *checkTypesInValue(const Value *V); + Type *checkTypesInConstant(const Constant *V); + + // If the Metadata node contains an invalid type, return a pointer to the + // type. Return null if there are no invalid types. + Type *checkTypesInMDNode(const MDNode *V); // There's no built-in way to get the name of a type, so use a // string ostream to print it. diff --git a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp index ef1c26e17501..fa94733ef04f 100644 --- a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp +++ b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp @@ -16,6 +16,7 @@ #include "llvm/ADT/Twine.h" #include "llvm/IR/Function.h" #include "llvm/IR/Instructions.h" +#include "llvm/IR/Metadata.h" #include "llvm/Support/raw_ostream.h" #include "llvm/Analysis/NaCl.h" @@ -138,15 +139,25 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) { if (!isa(*BBI)) for (User::const_op_iterator OI = BBI->op_begin(), OE = BBI->op_end(); OI != OE; OI++) { - if (isa(OI) && !isa(OI) && - !isa(OI)) { - Type *T = TC.checkTypesInValue(*OI); + if (isa(OI) && !isa(OI)) { + Type *T = TC.checkTypesInConstant(cast(*OI)); if (T) Errors << "Function " + F.getName() + " has instruction operand with disallowed type: " + PNaClABITypeChecker::getTypeName(T) + "\n"; } } + + // Get types hiding in metadata attached to the instruction + SmallVector, 4> MDForInst; + BBI->getAllMetadataOtherThanDebugLoc(MDForInst); + for (unsigned i = 0, e = MDForInst.size(); i != e; i++) { + Type *T = TC.checkTypesInMDNode(MDForInst[i].second); + if (T) + Errors << "Function " + F.getName() + + " has instruction metadata containing disallowed type: " + + PNaClABITypeChecker::getTypeName(T) + "\n"; + } } } diff --git a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp index f17a6fcf37a3..6473e586272b 100644 --- a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp +++ b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp @@ -81,7 +81,7 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) { + "\n"; } else if (MI->hasInitializer()) { // If the type of the global is bad, no point in checking its initializer - Type *T = TC.checkTypesInValue(MI->getInitializer()); + Type *T = TC.checkTypesInConstant(MI->getInitializer()); if (T) Errors << "Initializer for " + MI->getName() + " has disallowed type: " + @@ -121,6 +121,16 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) { } } + // Check named metadata nodes + for (Module::const_named_metadata_iterator I = M.named_metadata_begin(), + E = M.named_metadata_end(); I != E; ++I) { + for (unsigned i = 0, e = I->getNumOperands(); i != e; i++) { + if (Type *T = TC.checkTypesInMDNode(I->getOperand(i))) + Errors << "Named metadata node " + I->getName() + + " refers to disallowed type: " + + PNaClABITypeChecker::getTypeName(T) + "\n"; + } + } Errors.flush(); return false; } diff --git a/test/NaCl/PNaClABI/types-function.ll b/test/NaCl/PNaClABI/types-function.ll index eb8aedcef508..d020c76499c5 100644 --- a/test/NaCl/PNaClABI/types-function.ll +++ b/test/NaCl/PNaClABI/types-function.ll @@ -6,6 +6,9 @@ @a2 = private global i17 zeroinitializer declare void @func(i15 %arg) +!llvm.foo = !{!0} +!0 = metadata !{ half 0.0} + define void @types() { ; CHECK: Function types has instruction with disallowed type: half %h1 = fptrunc double undef to half @@ -17,7 +20,8 @@ define void @types() { store i32 0, i32* bitcast (i17* @a2 to i32*), align 4 ; CHECK: Function types has instruction operand with disallowed type: i15 call void @func(i15 1) - ret void +; CHECK: Function types has instruction metadata containing disallowed type: half + ret void, !foo !0 } ; CHECK-NOT: disallowed diff --git a/test/NaCl/PNaClABI/types.ll b/test/NaCl/PNaClABI/types.ll index 0447ea07ee96..05248bae06dd 100644 --- a/test/NaCl/PNaClABI/types.ll +++ b/test/NaCl/PNaClABI/types.ll @@ -96,4 +96,8 @@ declare half* @badReturn() ; CHECK: Function badArgType1 argument 1 has disallowed type declare void @badArgType1(half %a, i32 %b) ; CHECK: Function badArgType2 argument 2 has disallowed type -declare void @badArgType2(i32 %a, half %b) \ No newline at end of file +declare void @badArgType2(i32 %a, half %b) + +; CHECK: Named metadata node namedmd refers to disallowed type +!0 = metadata !{ half 0.0} +!namedmd = !{!0} From 164c8e3910bc3628c047b9b34ab690394862ab76 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Wed, 13 Mar 2013 16:04:03 -0700 Subject: [PATCH 014/295] ABI verifier: Add standalone tool pnacl-abicheck This CL adds a standalone tool pnacl-abicheck for checking the ABI validity of a bitcode file. It also adds a flag pnaclabi-verify to llc to enable the same checking in llc. It also improves the mechanism for handling and reporting validation errors and uses it in both tools. R=jvoung@chromium.org,mseaborn@chromium.org BUG= https://code.google.com/p/nativeclient/issues/detail?id=2309 Review URL: https://codereview.chromium.org/12449014 (cherry picked from commit cb0dcd0adf5e43486154d143da30710fee0da301) Conflicts: lib/Analysis/NaCl/PNaClABIVerifyModule.cpp tools/CMakeLists.txt tools/LLVMBuild.txt tools/Makefile tools/llc/llc.cpp --- include/llvm/Analysis/NaCl.h | 35 ++++++- lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp | 65 +++++++------ lib/Analysis/NaCl/PNaClABIVerifyModule.cpp | 92 +++++++++++-------- tools/CMakeLists.txt | 1 + tools/LLVMBuild.txt | 2 +- tools/Makefile | 2 +- tools/llc/llc.cpp | 41 +++++++++ tools/pnacl-abicheck/CMakeLists.txt | 5 + tools/pnacl-abicheck/LLVMBuild.txt | 22 +++++ tools/pnacl-abicheck/Makefile | 16 ++++ tools/pnacl-abicheck/pnacl-abicheck.cpp | 60 ++++++++++++ 11 files changed, 274 insertions(+), 67 deletions(-) create mode 100644 tools/pnacl-abicheck/CMakeLists.txt create mode 100644 tools/pnacl-abicheck/LLVMBuild.txt create mode 100644 tools/pnacl-abicheck/Makefile create mode 100644 tools/pnacl-abicheck/pnacl-abicheck.cpp diff --git a/include/llvm/Analysis/NaCl.h b/include/llvm/Analysis/NaCl.h index 8e599594b271..a8f2c9651e83 100644 --- a/include/llvm/Analysis/NaCl.h +++ b/include/llvm/Analysis/NaCl.h @@ -10,13 +10,44 @@ #ifndef LLVM_ANALYSIS_NACL_H #define LLVM_ANALYSIS_NACL_H +#include "llvm/Support/raw_ostream.h" +#include + namespace llvm { class FunctionPass; class ModulePass; -FunctionPass *createPNaClABIVerifyFunctionsPass(); -ModulePass *createPNaClABIVerifyModulePass(); +class PNaClABIErrorReporter { + public: + PNaClABIErrorReporter() : ErrorCount(0), Errors(ErrorString) {} + // Return the number of verification errors from the last run. + int getErrorCount() { return ErrorCount; } + // Print the error messages to O + void printErrors(llvm::raw_ostream &O) { + Errors.flush(); + O << ErrorString; + } + // Increments the error count and returns an ostream to which the error + // message can be streamed. + raw_ostream &addError() { + ErrorCount++; + return Errors; + } + // Reset the error count and error messages. + void reset() { + ErrorCount = 0; + Errors.flush(); + ErrorString.clear(); + } + private: + int ErrorCount; + std::string ErrorString; + raw_string_ostream Errors; +}; + +FunctionPass *createPNaClABIVerifyFunctionsPass(PNaClABIErrorReporter * Reporter); +ModulePass *createPNaClABIVerifyModulePass(PNaClABIErrorReporter * Reporter); } diff --git a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp index fa94733ef04f..a099817f1e79 100644 --- a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp +++ b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp @@ -14,11 +14,11 @@ #include "llvm/Pass.h" #include "llvm/ADT/Twine.h" +#include "llvm/Analysis/NaCl.h" #include "llvm/IR/Function.h" #include "llvm/IR/Instructions.h" #include "llvm/IR/Metadata.h" #include "llvm/Support/raw_ostream.h" -#include "llvm/Analysis/NaCl.h" #include "PNaClABITypeChecker.h" using namespace llvm; @@ -30,22 +30,29 @@ namespace { class PNaClABIVerifyFunctions : public FunctionPass { public: static char ID; - PNaClABIVerifyFunctions() : FunctionPass(ID), Errors(ErrorsString) {} + PNaClABIVerifyFunctions() : FunctionPass(ID), + Reporter(new PNaClABIErrorReporter), + ReporterIsOwned(true) {} + explicit PNaClABIVerifyFunctions(PNaClABIErrorReporter *Reporter_) : + FunctionPass(ID), + Reporter(Reporter_), + ReporterIsOwned(false) {} + ~PNaClABIVerifyFunctions() { + if (ReporterIsOwned) + delete Reporter; + } bool runOnFunction(Function &F); virtual void print(raw_ostream &O, const Module *M) const; private: PNaClABITypeChecker TC; - std::string ErrorsString; - raw_string_ostream Errors; + PNaClABIErrorReporter *Reporter; + bool ReporterIsOwned; }; } // and anonymous namespace bool PNaClABIVerifyFunctions::runOnFunction(Function &F) { - // For now just start with new errors on each function; this may change - // once we want to do something with them other than just calling print() - ErrorsString.clear(); - // TODO: only report one error per instruction + // TODO: only report one error per instruction? for (Function::const_iterator FI = F.begin(), FE = F.end(); FI != FE; ++FI) { for (BasicBlock::const_iterator BBI = FI->begin(), BBE = FI->end(); @@ -59,9 +66,9 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) { case Instruction::ExtractElement: case Instruction::InsertElement: case Instruction::ShuffleVector: - Errors << "Function " + F.getName() + - " has disallowed instruction: " + - BBI->getOpcodeName() + "\n"; + Reporter->addError() << "Function " << F.getName() << + " has disallowed instruction: " << + BBI->getOpcodeName() << "\n"; break; // Terminator instructions @@ -126,9 +133,9 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) { } // Check the types. First check the type of the instruction. if (!TC.isValidType(BBI->getType())) { - Errors << "Function " + F.getName() + - " has instruction with disallowed type: " + - PNaClABITypeChecker::getTypeName(BBI->getType()) + "\n"; + Reporter->addError() << "Function " << F.getName() << + " has instruction with disallowed type: " << + PNaClABITypeChecker::getTypeName(BBI->getType()) << "\n"; } // Check the instruction operands. Operands which are Instructions will @@ -141,10 +148,11 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) { OI != OE; OI++) { if (isa(OI) && !isa(OI)) { Type *T = TC.checkTypesInConstant(cast(*OI)); - if (T) - Errors << "Function " + F.getName() + - " has instruction operand with disallowed type: " + - PNaClABITypeChecker::getTypeName(T) + "\n"; + if (T) { + Reporter->addError() << "Function " << F.getName() << + " has instruction operand with disallowed type: " << + PNaClABITypeChecker::getTypeName(T) << "\n"; + } } } @@ -153,21 +161,25 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) { BBI->getAllMetadataOtherThanDebugLoc(MDForInst); for (unsigned i = 0, e = MDForInst.size(); i != e; i++) { Type *T = TC.checkTypesInMDNode(MDForInst[i].second); - if (T) - Errors << "Function " + F.getName() + - " has instruction metadata containing disallowed type: " + - PNaClABITypeChecker::getTypeName(T) + "\n"; + if (T) { + Reporter->addError() << "Function " << F.getName() << + " has instruction metadata containing disallowed type: " << + PNaClABITypeChecker::getTypeName(T) << "\n"; + } } } } - Errors.flush(); return false; } +// This method exists so that the passes can easily be run with opt -analyze. +// In this case the default constructor is used and we want to reset the error +// messages after each print. void PNaClABIVerifyFunctions::print(llvm::raw_ostream &O, const Module *M) const { - O << ErrorsString; + Reporter->printErrors(O); + Reporter->reset(); } char PNaClABIVerifyFunctions::ID = 0; @@ -175,6 +187,7 @@ char PNaClABIVerifyFunctions::ID = 0; static RegisterPass X("verify-pnaclabi-functions", "Verify functions for PNaCl", false, false); -FunctionPass *llvm::createPNaClABIVerifyFunctionsPass() { - return new PNaClABIVerifyFunctions(); +FunctionPass *llvm::createPNaClABIVerifyFunctionsPass( + PNaClABIErrorReporter *Reporter) { + return new PNaClABIVerifyFunctions(Reporter); } diff --git a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp index 6473e586272b..6b0243fbee59 100644 --- a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp +++ b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp @@ -13,13 +13,12 @@ // //===----------------------------------------------------------------------===// -#include "llvm/ADT/Twine.h" -#include "llvm/DerivedTypes.h" -#include "llvm/Module.h" #include "llvm/Pass.h" - -#include "llvm/Support/raw_ostream.h" +#include "llvm/ADT/Twine.h" #include "llvm/Analysis/NaCl.h" +#include "llvm/IR/DerivedTypes.h" +#include "llvm/IR/Module.h" +#include "llvm/Support/raw_ostream.h" #include "PNaClABITypeChecker.h" using namespace llvm; @@ -29,13 +28,23 @@ namespace { class PNaClABIVerifyModule : public ModulePass { public: static char ID; - PNaClABIVerifyModule(); + PNaClABIVerifyModule() : ModulePass(ID), + Reporter(new PNaClABIErrorReporter), + ReporterIsOwned(true) {} + explicit PNaClABIVerifyModule(PNaClABIErrorReporter *Reporter_) : + ModulePass(ID), + Reporter(Reporter_), + ReporterIsOwned(false) {} + ~PNaClABIVerifyModule() { + if (ReporterIsOwned) + delete Reporter; + } bool runOnModule(Module &M); virtual void print(raw_ostream &O, const Module *M) const; private: PNaClABITypeChecker TC; - std::string ErrorsString; - raw_string_ostream Errors; + PNaClABIErrorReporter *Reporter; + bool ReporterIsOwned; }; static const char *linkageName(GlobalValue::LinkageTypes LT) { @@ -66,26 +75,24 @@ static const char *linkageName(GlobalValue::LinkageTypes LT) { } // end anonymous namespace -PNaClABIVerifyModule::PNaClABIVerifyModule() : ModulePass(ID), - Errors(ErrorsString) {} - bool PNaClABIVerifyModule::runOnModule(Module &M) { for (Module::const_global_iterator MI = M.global_begin(), ME = M.global_end(); MI != ME; ++MI) { // Check types of global variables and their initializers if (!TC.isValidType(MI->getType())) { // GVs are pointers, so print the pointed-to type for clarity - Errors << "Variable " + MI->getName() + - " has disallowed type: " + + Reporter->addError() << "Variable " << MI->getName() << + " has disallowed type: " << PNaClABITypeChecker::getTypeName(MI->getType()->getContainedType(0)) + "\n"; } else if (MI->hasInitializer()) { // If the type of the global is bad, no point in checking its initializer Type *T = TC.checkTypesInConstant(MI->getInitializer()); - if (T) - Errors << "Initializer for " + MI->getName() + - " has disallowed type: " + - PNaClABITypeChecker::getTypeName(T) + "\n"; + if (T) { + Reporter->addError() << "Initializer for " << MI->getName() << + " has disallowed type: " << + PNaClABITypeChecker::getTypeName(T) << "\n"; + } } // Check GV linkage types @@ -96,28 +103,33 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) { case GlobalValue::PrivateLinkage: break; default: - Errors << "Variable " + MI->getName() + - " has disallowed linkage type: " + - linkageName(MI->getLinkage()) + "\n"; + Reporter->addError() << "Variable " << MI->getName() << + " has disallowed linkage type: " << + linkageName(MI->getLinkage()) << "\n"; } } // No aliases allowed for now. for (Module::alias_iterator MI = M.alias_begin(), - E = M.alias_end(); MI != E; ++MI) - Errors << "Variable " + MI->getName() + " is an alias (disallowed)\n"; + E = M.alias_end(); MI != E; ++MI) { + Reporter->addError() << "Variable " << MI->getName() << + " is an alias (disallowed)\n"; + } for (Module::iterator MI = M.begin(), ME = M.end(); MI != ME; ++MI) { // Check types of functions and their arguments FunctionType *FT = MI->getFunctionType(); - if (!TC.isValidType(FT->getReturnType())) - Errors << "Function " + MI->getName() + " has disallowed return type: " + - PNaClABITypeChecker::getTypeName(FT->getReturnType()) + "\n"; + if (!TC.isValidType(FT->getReturnType())) { + Reporter->addError() << "Function " << MI->getName() << + " has disallowed return type: " << + PNaClABITypeChecker::getTypeName(FT->getReturnType()) << "\n"; + } for (unsigned I = 0, E = FT->getNumParams(); I < E; ++I) { Type *PT = FT->getParamType(I); - if (!TC.isValidType(PT)) - Errors << "Function " << MI->getName() << " argument " << I + 1 << - " has disallowed type: " << - PNaClABITypeChecker::getTypeName(PT) + "\n"; + if (!TC.isValidType(PT)) { + Reporter->addError() << "Function " << MI->getName() << " argument " << + I + 1 << " has disallowed type: " << + PNaClABITypeChecker::getTypeName(PT) << "\n"; + } } } @@ -125,18 +137,23 @@ bool PNaClABIVerifyModule::runOnModule(Module &M) { for (Module::const_named_metadata_iterator I = M.named_metadata_begin(), E = M.named_metadata_end(); I != E; ++I) { for (unsigned i = 0, e = I->getNumOperands(); i != e; i++) { - if (Type *T = TC.checkTypesInMDNode(I->getOperand(i))) - Errors << "Named metadata node " + I->getName() + - " refers to disallowed type: " + - PNaClABITypeChecker::getTypeName(T) + "\n"; + if (Type *T = TC.checkTypesInMDNode(I->getOperand(i))) { + Reporter->addError() << "Named metadata node " << I->getName() << + " refers to disallowed type: " << + PNaClABITypeChecker::getTypeName(T) << "\n"; + } } } - Errors.flush(); return false; } +// This method exists so that the passes can easily be run with opt -analyze. +// In this case the default constructor is used and we want to reset the error +// messages after each print (this is more of an issue for the FunctionPass +// than the ModulePass) void PNaClABIVerifyModule::print(llvm::raw_ostream &O, const Module *M) const { - O << ErrorsString; + Reporter->printErrors(O); + Reporter->reset(); } char PNaClABIVerifyModule::ID = 0; @@ -144,6 +161,7 @@ char PNaClABIVerifyModule::ID = 0; static RegisterPass X("verify-pnaclabi-module", "Verify module for PNaCl", false, false); -ModulePass *llvm::createPNaClABIVerifyModulePass() { - return new PNaClABIVerifyModule(); +ModulePass *llvm::createPNaClABIVerifyModulePass( + PNaClABIErrorReporter *Reporter) { + return new PNaClABIVerifyModule(Reporter); } diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt index 12e10fd0bbd1..d9e8edcb5ede 100644 --- a/tools/CMakeLists.txt +++ b/tools/CMakeLists.txt @@ -38,6 +38,7 @@ add_llvm_tool_subdirectory(llvm-stress) add_llvm_tool_subdirectory(llvm-mcmarkup) add_llvm_tool_subdirectory(llvm-symbolizer) +add_subdirectory(pnacl-abicheck) add_llvm_tool_subdirectory(llvm-c-test) diff --git a/tools/LLVMBuild.txt b/tools/LLVMBuild.txt index 93b8d98dcba4..971b5ef9ed97 100644 --- a/tools/LLVMBuild.txt +++ b/tools/LLVMBuild.txt @@ -16,7 +16,7 @@ ;===------------------------------------------------------------------------===; [common] -subdirectories = bugpoint llc lli llvm-ar llvm-as llvm-bcanalyzer llvm-cov llvm-diff llvm-dis llvm-dwarfdump llvm-extract llvm-jitlistener llvm-link llvm-lto llvm-mc llvm-nm llvm-objdump llvm-rtdyld llvm-size macho-dump opt llvm-mcmarkup +subdirectories = bugpoint llc lli llvm-ar llvm-as llvm-bcanalyzer llvm-cov llvm-diff llvm-dis llvm-dwarfdump llvm-extract llvm-jitlistener llvm-link llvm-lto llvm-mc llvm-nm llvm-objdump llvm-rtdyld llvm-size macho-dump opt llvm-mcmarkup pnacl-abicheck [component_0] type = Group diff --git a/tools/Makefile b/tools/Makefile index be872548e313..5a1899755317 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -31,7 +31,7 @@ PARALLEL_DIRS := opt llvm-as llvm-dis llc llvm-ar llvm-nm llvm-link \ lli llvm-extract llvm-mc bugpoint llvm-bcanalyzer llvm-diff \ macho-dump llvm-objdump llvm-readobj llvm-rtdyld \ llvm-dwarfdump llvm-cov llvm-size llvm-stress llvm-mcmarkup \ - llvm-symbolizer obj2yaml yaml2obj llvm-c-test + llvm-symbolizer obj2yaml yaml2obj llvm-c-test pnacl-abicheck # If Intel JIT Events support is configured, build an extra tool to test it. ifeq ($(USE_INTEL_JITEVENTS), 1) diff --git a/tools/llc/llc.cpp b/tools/llc/llc.cpp index a3bd72347d60..e45d99a71068 100644 --- a/tools/llc/llc.cpp +++ b/tools/llc/llc.cpp @@ -15,6 +15,7 @@ #include "llvm/ADT/Triple.h" +#include "llvm/Analysis/NaCl.h" #include "llvm/CodeGen/CommandFlags.h" #include "llvm/CodeGen/LinkAllAsmWriterComponents.h" #include "llvm/CodeGen/LinkAllCodegenComponents.h" @@ -57,6 +58,14 @@ static cl::opt TimeCompilations("time-compilations", cl::Hidden, cl::init(1u), cl::value_desc("N"), cl::desc("Repeat compilation N times for timing")); +static cl::opt +PNaClABIVerify("pnaclabi-verify", + cl::desc("Verify PNaCl bitcode ABI before translating"), + cl::init(false)); +static cl::opt +PNaClABIVerifyFatalErrors("pnaclabi-verify-fatal-errors", + cl::desc("PNaCl ABI verification errors are fatal"), + cl::init(false)); // Determine optimization level. static cl::opt @@ -199,6 +208,20 @@ int main(int argc, char **argv) { return 0; } +// @LOCALMOD-BEGIN +static void CheckABIVerifyErrors(PNaClABIErrorReporter &Reporter, + const Twine &Name) { + if (PNaClABIVerify && Reporter.getErrorCount() > 0) { + errs() << (PNaClABIVerifyFatalErrors ? "ERROR:" : "WARNING:"); + errs() << Name << " is not valid PNaCl bitcode:\n"; + Reporter.printErrors(errs()); + if (PNaClABIVerifyFatalErrors) + exit(1); + } + Reporter.reset(); +} +// @LOCALMOD-END + static int compileModule(char **argv, LLVMContext &Context) { // Load the module to be compiled... SMDiagnostic Err; @@ -209,6 +232,8 @@ static int compileModule(char **argv, LLVMContext &Context) { bool SkipModule = MCPU == "help" || (!MAttrs.empty() && MAttrs.front() == "help"); + PNaClABIErrorReporter ABIErrorReporter; // @LOCALMOD + // If user just wants to list available options, skip module loading if (!SkipModule) { M.reset(ParseIRFile(InputFilename, Err, Context)); @@ -218,6 +243,12 @@ static int compileModule(char **argv, LLVMContext &Context) { return 1; } + if (PNaClABIVerify) { + // Verify the module (but not the functions yet) + ModulePass *VerifyPass = createPNaClABIVerifyModulePass(&ABIErrorReporter); + VerifyPass->runOnModule(*mod); + CheckABIVerifyErrors(ABIErrorReporter, "Module"); + } // If we are supposed to override the target triple, do so now. if (!TargetTriple.empty()) mod->setTargetTriple(Triple::normalize(TargetTriple)); @@ -305,6 +336,13 @@ static int compileModule(char **argv, LLVMContext &Context) { // Build up all of the passes that we want to do to the module. PassManager PM; + // Add the ABI verifier pass before the analysis and code emission passes. + FunctionPass *FunctionVerifyPass = NULL; + if (PNaClABIVerify) { + FunctionVerifyPass = createPNaClABIVerifyFunctionsPass(&ABIErrorReporter); + PM->add(FunctionVerifyPass); + } + // Add an appropriate TargetLibraryInfo pass for the module's triple. TargetLibraryInfo *TLI = new TargetLibraryInfo(TheTriple); if (DisableSimplifyLibCalls) @@ -328,6 +366,8 @@ static int compileModule(char **argv, LLVMContext &Context) { Target.setMCRelaxAll(true); } + + CheckABIVerifyErrors(ABIErrorReporter, "Function " + I->getName()); { formatted_raw_ostream FOS(Out->os()); @@ -363,6 +403,7 @@ static int compileModule(char **argv, LLVMContext &Context) { cl::PrintOptionValues(); PM.run(*mod); + CheckABIVerifyErrors(ABIErrorReporter, "Function " + I->getName()); } // Declare success. diff --git a/tools/pnacl-abicheck/CMakeLists.txt b/tools/pnacl-abicheck/CMakeLists.txt new file mode 100644 index 000000000000..d3296fa5a54f --- /dev/null +++ b/tools/pnacl-abicheck/CMakeLists.txt @@ -0,0 +1,5 @@ +set(LLVM_LINK_COMPONENTS bitreader asmparser) + +add_llvm_tool(pnacl-abicheck + pnacl-abicheck.cc + ) diff --git a/tools/pnacl-abicheck/LLVMBuild.txt b/tools/pnacl-abicheck/LLVMBuild.txt new file mode 100644 index 000000000000..0af322769c82 --- /dev/null +++ b/tools/pnacl-abicheck/LLVMBuild.txt @@ -0,0 +1,22 @@ +;===- ./tools/pnacl-abicheck/LLVMBuild.txt ---------------------*- Conf -*--===; +; +; The LLVM Compiler Infrastructure +; +; This file is distributed under the University of Illinois Open Source +; License. See LICENSE.TXT for details. +; +;===------------------------------------------------------------------------===; +; +; This is an LLVMBuild description file for the components in this subdirectory. +; +; For more information on the LLVMBuild system, please see: +; +; http://llvm.org/docs/LLVMBuild.html +; +;===------------------------------------------------------------------------===; + +[component_0] +type = Tool +name = pnacl-abicheck +parent = Tools +required_libraries = AsmParser BitReader diff --git a/tools/pnacl-abicheck/Makefile b/tools/pnacl-abicheck/Makefile new file mode 100644 index 000000000000..bd98690dd184 --- /dev/null +++ b/tools/pnacl-abicheck/Makefile @@ -0,0 +1,16 @@ +#===- tools/pnacl-abicheck/Makefile ------------------------*- Makefile -*-===## +# +# The LLVM Compiler Infrastructure +# +# This file is distributed under the University of Illinois Open Source +# License. See LICENSE.TXT for details. +# +##===----------------------------------------------------------------------===## + +LEVEL := ../.. +TOOLNAME := pnacl-abicheck +LINK_COMPONENTS := bitreader asmparser + +include $(LEVEL)/Makefile.common + + diff --git a/tools/pnacl-abicheck/pnacl-abicheck.cpp b/tools/pnacl-abicheck/pnacl-abicheck.cpp new file mode 100644 index 000000000000..2bee78887277 --- /dev/null +++ b/tools/pnacl-abicheck/pnacl-abicheck.cpp @@ -0,0 +1,60 @@ +//===-- pnacl-abicheck.cpp - Check PNaCl bitcode ABI ----------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This tool checks files for compliance with the PNaCl bitcode ABI +// +//===----------------------------------------------------------------------===// + +#include "llvm/ADT/OwningPtr.h" +#include "llvm/Analysis/NaCl.h" +#include "llvm/IR/LLVMContext.h" +#include "llvm/IR/Module.h" +#include "llvm/Pass.h" +#include "llvm/Support/CommandLine.h" +#include "llvm/Support/FormattedStream.h" +#include "llvm/Support/IRReader.h" +#include + +using namespace llvm; + +static cl::opt +InputFilename(cl::Positional, cl::desc(""), cl::init("-")); + +static void CheckABIVerifyErrors(PNaClABIErrorReporter &Reporter, + const Twine &Name) { + if (Reporter.getErrorCount() > 0) { + errs() << "ERROR: " << Name << " is not valid PNaCl bitcode:\n"; + Reporter.printErrors(errs()); + } + Reporter.reset(); +} + +int main(int argc, char **argv) { + LLVMContext &Context = getGlobalContext(); + SMDiagnostic Err; + OwningPtr Mod(ParseIRFile(InputFilename, Err, Context)); + if (Mod.get() == 0) { + Err.print(argv[0], errs()); + return 1; + } + PNaClABIErrorReporter ABIErrorReporter; + // Manually run the passes so we can tell the user which function had the + // error. No need for a pass manager since it's just one pass. + ModulePass *ModuleChecker = createPNaClABIVerifyModulePass(&ABIErrorReporter); + ModuleChecker->runOnModule(*Mod); + CheckABIVerifyErrors(ABIErrorReporter, "Module"); + FunctionPass *FunctionChecker = + createPNaClABIVerifyFunctionsPass(&ABIErrorReporter); + for (Module::iterator MI = Mod->begin(), ME = Mod->end(); MI != ME; ++MI) { + FunctionChecker->runOnFunction(*MI); + CheckABIVerifyErrors(ABIErrorReporter, "Function " + MI->getName()); + } + + return 0; +} From cd2f6d7afcce2aebf3ab1153b9aa392d88395bdc Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Thu, 14 Mar 2013 10:14:47 -0700 Subject: [PATCH 015/295] Cleanup of ABI Checker Fix spacing of error output, use OwningPtr everywhere at top level, fix command line parsing R=jvoung@chromium.org BUG= Review URL: https://codereview.chromium.org/12690007 (cherry picked from commit b3ace3f5ed473e4b19dd1d7f9257218cedd37a96) --- test/NaCl/PNaClABI/instructions.ll | 2 +- test/NaCl/PNaClABI/linkagetypes.ll | 2 +- test/NaCl/PNaClABI/types-function.ll | 2 +- test/NaCl/PNaClABI/types.ll | 2 +- tools/llc/llc.cpp | 2 +- tools/pnacl-abicheck/pnacl-abicheck.cpp | 12 +++++++----- 6 files changed, 12 insertions(+), 10 deletions(-) diff --git a/test/NaCl/PNaClABI/instructions.ll b/test/NaCl/PNaClABI/instructions.ll index 018d66c87634..2f5bbee4dc4e 100644 --- a/test/NaCl/PNaClABI/instructions.ll +++ b/test/NaCl/PNaClABI/instructions.ll @@ -1,4 +1,4 @@ -; RUN: opt -verify-pnaclabi-functions -analyze < %s | FileCheck %s +; RUN: pnacl-abicheck < %s | FileCheck %s ; Test instruction opcodes allowed by PNaCl ABI ; No testing yet of operands, types, attributes, etc diff --git a/test/NaCl/PNaClABI/linkagetypes.ll b/test/NaCl/PNaClABI/linkagetypes.ll index ab6d81f351b7..7aa0ea8122bc 100644 --- a/test/NaCl/PNaClABI/linkagetypes.ll +++ b/test/NaCl/PNaClABI/linkagetypes.ll @@ -1,4 +1,4 @@ -; RUN: opt -verify-pnaclabi-module -analyze < %s | FileCheck %s +; RUN: pnacl-abicheck < %s | FileCheck %s ; Test linkage types allowed by PNaCl ABI target datalayout = "e-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-p:32:32:32-v128:32:32" diff --git a/test/NaCl/PNaClABI/types-function.ll b/test/NaCl/PNaClABI/types-function.ll index d020c76499c5..e781163b8376 100644 --- a/test/NaCl/PNaClABI/types-function.ll +++ b/test/NaCl/PNaClABI/types-function.ll @@ -1,4 +1,4 @@ -; RUN: opt -verify-pnaclabi-functions -analyze < %s | FileCheck %s +; RUN: pnacl-abicheck < %s | FileCheck %s ; Test type-checking in function bodies. This test is not intended to verify ; all the rules about the various types, but instead to make sure that types ; stashed in various places in function bodies are caught. diff --git a/test/NaCl/PNaClABI/types.ll b/test/NaCl/PNaClABI/types.ll index 05248bae06dd..8fdfb4e79465 100644 --- a/test/NaCl/PNaClABI/types.ll +++ b/test/NaCl/PNaClABI/types.ll @@ -1,4 +1,4 @@ -; RUN: opt -verify-pnaclabi-module -analyze < %s | FileCheck %s +; RUN: pnacl-abicheck < %s | FileCheck %s ; Test types allowed by PNaCl ABI ; Basic global types diff --git a/tools/llc/llc.cpp b/tools/llc/llc.cpp index e45d99a71068..4a758ac4a492 100644 --- a/tools/llc/llc.cpp +++ b/tools/llc/llc.cpp @@ -212,7 +212,7 @@ int main(int argc, char **argv) { static void CheckABIVerifyErrors(PNaClABIErrorReporter &Reporter, const Twine &Name) { if (PNaClABIVerify && Reporter.getErrorCount() > 0) { - errs() << (PNaClABIVerifyFatalErrors ? "ERROR:" : "WARNING:"); + errs() << (PNaClABIVerifyFatalErrors ? "ERROR: " : "WARNING: "); errs() << Name << " is not valid PNaCl bitcode:\n"; Reporter.printErrors(errs()); if (PNaClABIVerifyFatalErrors) diff --git a/tools/pnacl-abicheck/pnacl-abicheck.cpp b/tools/pnacl-abicheck/pnacl-abicheck.cpp index 2bee78887277..496a73c2fd40 100644 --- a/tools/pnacl-abicheck/pnacl-abicheck.cpp +++ b/tools/pnacl-abicheck/pnacl-abicheck.cpp @@ -29,8 +29,8 @@ InputFilename(cl::Positional, cl::desc(""), cl::init("-")); static void CheckABIVerifyErrors(PNaClABIErrorReporter &Reporter, const Twine &Name) { if (Reporter.getErrorCount() > 0) { - errs() << "ERROR: " << Name << " is not valid PNaCl bitcode:\n"; - Reporter.printErrors(errs()); + outs() << "ERROR: " << Name << " is not valid PNaCl bitcode:\n"; + Reporter.printErrors(outs()); } Reporter.reset(); } @@ -38,6 +38,8 @@ static void CheckABIVerifyErrors(PNaClABIErrorReporter &Reporter, int main(int argc, char **argv) { LLVMContext &Context = getGlobalContext(); SMDiagnostic Err; + cl::ParseCommandLineOptions(argc, argv, "PNaCl Bitcode ABI checker\n"); + OwningPtr Mod(ParseIRFile(InputFilename, Err, Context)); if (Mod.get() == 0) { Err.print(argv[0], errs()); @@ -46,11 +48,11 @@ int main(int argc, char **argv) { PNaClABIErrorReporter ABIErrorReporter; // Manually run the passes so we can tell the user which function had the // error. No need for a pass manager since it's just one pass. - ModulePass *ModuleChecker = createPNaClABIVerifyModulePass(&ABIErrorReporter); + OwningPtr ModuleChecker(createPNaClABIVerifyModulePass(&ABIErrorReporter)); ModuleChecker->runOnModule(*Mod); CheckABIVerifyErrors(ABIErrorReporter, "Module"); - FunctionPass *FunctionChecker = - createPNaClABIVerifyFunctionsPass(&ABIErrorReporter); + OwningPtr FunctionChecker( + createPNaClABIVerifyFunctionsPass(&ABIErrorReporter)); for (Module::iterator MI = Mod->begin(), ME = Mod->end(); MI != ME; ++MI) { FunctionChecker->runOnFunction(*MI); CheckABIVerifyErrors(ABIErrorReporter, "Function " + MI->getName()); From 1a5d9932ef8d00dc9fcda017b91099658219b8f5 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Thu, 14 Mar 2013 13:53:07 -0700 Subject: [PATCH 016/295] Fix LLVMBuild.txt for NaCl analysis passes R=jvoung@chromium.org BUG= Sandboxed LLC doesn't build correctly anymore without it Review URL: https://codereview.chromium.org/12432018 (cherry picked from commit d4b96b281bef71816410d5eceea44eb3998e2dbb) Conflicts: tools/llc/LLVMBuild.txt tools/llc/Makefile --- lib/Analysis/NaCl/CMakeLists.txt | 4 ++-- lib/Analysis/NaCl/LLVMBuild.txt | 2 +- lib/Analysis/NaCl/Makefile | 2 +- tools/llc/LLVMBuild.txt | 2 +- tools/llc/Makefile | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/Analysis/NaCl/CMakeLists.txt b/lib/Analysis/NaCl/CMakeLists.txt index 7fa3a134ac94..a01a3338022d 100644 --- a/lib/Analysis/NaCl/CMakeLists.txt +++ b/lib/Analysis/NaCl/CMakeLists.txt @@ -1,7 +1,7 @@ -add_llvm_library(LLVMAnalysisNaCl +add_llvm_library(LLVMNaClAnalysis PNaCLABITypeChecker.cpp PNaClABIVerifyFunctions.cpp PNaClABIVerifyModule.cpp ) -add_dependencies(LLVMAnalysisNaCl intrinsics_gen) +add_dependencies(LLVMNaClAnalysis intrinsics_gen) diff --git a/lib/Analysis/NaCl/LLVMBuild.txt b/lib/Analysis/NaCl/LLVMBuild.txt index 997803bb5db7..b5e7c8a5eaf6 100644 --- a/lib/Analysis/NaCl/LLVMBuild.txt +++ b/lib/Analysis/NaCl/LLVMBuild.txt @@ -20,4 +20,4 @@ type = Library name = NaClAnalysis parent = Analysis library_name = NaClAnalysis -required_libraries = Core +required_libraries = Analysis Core Support diff --git a/lib/Analysis/NaCl/Makefile b/lib/Analysis/NaCl/Makefile index 6a2f0ed8d9dc..7d03b1e92eb1 100644 --- a/lib/Analysis/NaCl/Makefile +++ b/lib/Analysis/NaCl/Makefile @@ -8,7 +8,7 @@ ##===----------------------------------------------------------------------===## LEVEL = ../../.. -LIBRARYNAME = LLVMAnalysisNaCl +LIBRARYNAME = LLVMNaClAnalysis BUILD_ARCHIVE = 1 include $(LEVEL)/Makefile.common \ No newline at end of file diff --git a/tools/llc/LLVMBuild.txt b/tools/llc/LLVMBuild.txt index 45cdc6498f86..92f84fec8562 100644 --- a/tools/llc/LLVMBuild.txt +++ b/tools/llc/LLVMBuild.txt @@ -19,4 +19,4 @@ type = Tool name = llc parent = Tools -required_libraries = AsmParser BitReader IRReader all-targets +required_libraries = AsmParser BitReader IRReader all-targets NaClAnalysis diff --git a/tools/llc/Makefile b/tools/llc/Makefile index 71bce4dc1adf..e9d828c3f04b 100644 --- a/tools/llc/Makefile +++ b/tools/llc/Makefile @@ -9,7 +9,7 @@ LEVEL := ../.. TOOLNAME := llc -LINK_COMPONENTS := all-targets bitreader asmparser irreader +LINK_COMPONENTS := all-targets bitreader asmparser irreader NaClAnalysis # Support plugins. NO_DEAD_STRIP := 1 From 419a2f577967df3addd851fc3eb827c065c6aea4 Mon Sep 17 00:00:00 2001 From: Mark Seaborn Date: Fri, 15 Mar 2013 13:44:43 -0700 Subject: [PATCH 017/295] PNaCl: Add ExpandConstantExpr pass for converting ConstantExprs to Instructions Once this pass is enabled, it will simplify the language to reduce the set of constructs that a PNaCl translator needs to handle as part of a stable wire format for PNaCl. BUG=https://code.google.com/p/nativeclient/issues/detail?id=3337 TEST=test/Transforms/NaCl/expand-constantexpr.ll Review URL: https://codereview.chromium.org/12792011 (cherry picked from commit bd7c1e1fa62acc37cf1bed3c1272703bbffd7242) Conflicts: include/llvm/InitializePasses.h --- include/llvm/InitializePasses.h | 2 + include/llvm/Transforms/NaCl.h | 2 + lib/Transforms/NaCl/CMakeLists.txt | 1 + lib/Transforms/NaCl/ExpandConstantExpr.cpp | 112 ++++++++++++++++++++ test/Transforms/NaCl/expand-constantexpr.ll | 109 +++++++++++++++++++ tools/opt/opt.cpp | 1 + 6 files changed, 227 insertions(+) create mode 100644 lib/Transforms/NaCl/ExpandConstantExpr.cpp create mode 100644 test/Transforms/NaCl/expand-constantexpr.ll diff --git a/include/llvm/InitializePasses.h b/include/llvm/InitializePasses.h index 195e5646086d..1595f326d3e7 100644 --- a/include/llvm/InitializePasses.h +++ b/include/llvm/InitializePasses.h @@ -268,9 +268,11 @@ void initializeSLPVectorizerPass(PassRegistry&); void initializeBBVectorizePass(PassRegistry&); void initializeMachineFunctionPrinterPassPass(PassRegistry&); void initializeStackMapLivenessPass(PassRegistry&); +void initializeExpandConstantExprPass(PassRegistry&); // @LOCALMOD void initializeExpandCtorsPass(PassRegistry&); // @LOCALMOD void initializeExpandTlsPass(PassRegistry&); // @LOCALMOD void initializeExpandTlsConstantExprPass(PassRegistry&); // @LOCALMOD +void initializeNaClCcRewritePass(PassRegistry&); // @LOCALMOD } #endif diff --git a/include/llvm/Transforms/NaCl.h b/include/llvm/Transforms/NaCl.h index 79c9b9fe7949..686b45ff747e 100644 --- a/include/llvm/Transforms/NaCl.h +++ b/include/llvm/Transforms/NaCl.h @@ -12,8 +12,10 @@ namespace llvm { +class FunctionPass; class ModulePass; +FunctionPass *createExpandConstantExprPass(); ModulePass *createExpandCtorsPass(); ModulePass *createExpandTlsPass(); ModulePass *createExpandTlsConstantExprPass(); diff --git a/lib/Transforms/NaCl/CMakeLists.txt b/lib/Transforms/NaCl/CMakeLists.txt index 5e24cc7e28f2..acf7656f64b8 100644 --- a/lib/Transforms/NaCl/CMakeLists.txt +++ b/lib/Transforms/NaCl/CMakeLists.txt @@ -1,4 +1,5 @@ add_llvm_library(LLVMTransformsNaCl + ExpandConstantExpr.cpp ExpandCtors.cpp ExpandTls.cpp ExpandTlsConstantExpr.cpp diff --git a/lib/Transforms/NaCl/ExpandConstantExpr.cpp b/lib/Transforms/NaCl/ExpandConstantExpr.cpp new file mode 100644 index 000000000000..5ff47cb00ce0 --- /dev/null +++ b/lib/Transforms/NaCl/ExpandConstantExpr.cpp @@ -0,0 +1,112 @@ +//===- ExpandConstantExpr.cpp - Convert ConstantExprs to Instructions------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This pass expands out ConstantExprs into Instructions. +// +// Note that this only converts ConstantExprs that are referenced by +// Instructions. It does not convert ConstantExprs that are used as +// initializers for global variables. +// +// This simplifies the language so that the PNaCl translator does not +// need to handle ConstantExprs as part of a stable wire format for +// PNaCl. +// +//===----------------------------------------------------------------------===// + +#include + +#include "llvm/IR/Function.h" +#include "llvm/IR/Instructions.h" +#include "llvm/Pass.h" +#include "llvm/Transforms/NaCl.h" + +using namespace llvm; + +static bool expandInstruction(Instruction *Inst); + +namespace { + // This is a FunctionPass because our handling of PHI nodes means + // that our modifications may cross BasicBlocks. + struct ExpandConstantExpr : public FunctionPass { + static char ID; // Pass identification, replacement for typeid + ExpandConstantExpr() : FunctionPass(ID) { + initializeExpandConstantExprPass(*PassRegistry::getPassRegistry()); + } + + virtual bool runOnFunction(Function &Func); + }; +} + +char ExpandConstantExpr::ID = 0; +INITIALIZE_PASS(ExpandConstantExpr, "expand-constant-expr", + "Expand out ConstantExprs into Instructions", + false, false) + +static Value *expandConstantExpr(Instruction *InsertPt, ConstantExpr *Expr) { + Instruction *NewInst = Expr->getAsInstruction(); + NewInst->insertBefore(InsertPt); + NewInst->setName("expanded"); + expandInstruction(NewInst); + return NewInst; +} + +static bool expandInstruction(Instruction *Inst) { + // A landingpad can only accept ConstantExprs, so it should remain + // unmodified. + if (isa(Inst)) + return false; + + bool Modified = false; + if (PHINode *PN = dyn_cast(Inst)) { + // PHI nodes require special treatment. A incoming block may be + // listed twice, but its incoming values must match so they must + // be converted only once. + std::map Converted; + for (unsigned OpNum = 0; OpNum < Inst->getNumOperands(); OpNum++) { + if (ConstantExpr *Expr = + dyn_cast(Inst->getOperand(OpNum))) { + Modified = true; + BasicBlock *Incoming = PN->getIncomingBlock(OpNum); + if (Converted.count(Incoming) == 0) { + Converted[Incoming] = expandConstantExpr(Incoming->getTerminator(), + Expr); + } + Inst->setOperand(OpNum, Converted[Incoming]); + } + } + return Modified; + } + + for (unsigned OpNum = 0; OpNum < Inst->getNumOperands(); OpNum++) { + if (ConstantExpr *Expr = + dyn_cast(Inst->getOperand(OpNum))) { + Modified = true; + Inst->setOperand(OpNum, expandConstantExpr(Inst, Expr)); + } + } + return Modified; +} + +bool ExpandConstantExpr::runOnFunction(Function &Func) { + bool Modified = false; + for (llvm::Function::iterator BB = Func.begin(), E = Func.end(); + BB != E; + ++BB) { + for (BasicBlock::InstListType::iterator Inst = BB->begin(), E = BB->end(); + Inst != E; + ++Inst) { + Modified |= expandInstruction(Inst); + } + } + return Modified; +} + +FunctionPass *llvm::createExpandConstantExprPass() { + return new ExpandConstantExpr(); +} diff --git a/test/Transforms/NaCl/expand-constantexpr.ll b/test/Transforms/NaCl/expand-constantexpr.ll new file mode 100644 index 000000000000..e8786d4cac7a --- /dev/null +++ b/test/Transforms/NaCl/expand-constantexpr.ll @@ -0,0 +1,109 @@ +; RUN: opt < %s -expand-constant-expr -S | FileCheck %s + +@global_var1 = global i32 123 +@global_var2 = global i32 123 + + +define i8* @constantexpr_bitcast() { + ret i8* bitcast (i32* @global_var1 to i8*) +} +; CHECK: @constantexpr_bitcast +; CHECK: %expanded = bitcast i32* @global_var1 to i8* +; CHECK: ret i8* %expanded + + +define i32 @constantexpr_nested() { + ret i32 add (i32 ptrtoint (i32* @global_var1 to i32), + i32 ptrtoint (i32* @global_var2 to i32)) +} +; CHECK: @constantexpr_nested +; CHECK: %expanded1 = ptrtoint i32* @global_var1 to i32 +; CHECK: %expanded2 = ptrtoint i32* @global_var2 to i32 +; CHECK: %expanded = add i32 %expanded1, %expanded2 +; CHECK: ret i32 %expanded + + +define i32 @constantexpr_nested2() { + ret i32 mul (i32 add (i32 ptrtoint (i32* @global_var1 to i32), + i32 ptrtoint (i32* @global_var2 to i32)), i32 2) +} +; CHECK: @constantexpr_nested2 +; CHECK: %expanded2 = ptrtoint i32* @global_var1 to i32 +; CHECK: %expanded3 = ptrtoint i32* @global_var2 to i32 +; CHECK: %expanded1 = add i32 %expanded2, %expanded3 +; CHECK: %expanded = mul i32 %expanded1, 2 +; CHECK: ret i32 %expanded + + +define i32 @constantexpr_phi() { +entry: + br label %label +label: + %result = phi i32 [ ptrtoint (i32* @global_var1 to i32), %entry ] + ret i32 %result +} +; CHECK: @constantexpr_phi +; CHECK: entry: +; CHECK: %expanded = ptrtoint i32* @global_var1 to i32 +; CHECK: br label %label +; CHECK: label: +; CHECK: %result = phi i32 [ %expanded, %entry ] + + +; This tests that ExpandConstantExpr correctly handles a PHI node that +; contains the same ConstantExpr twice. +; Using replaceAllUsesWith() is not correct on a PHI node when the +; new instruction has to be added to an incoming block. +define i32 @constantexpr_phi_twice(i1 %arg) { + br i1 %arg, label %iftrue, label %iffalse +iftrue: + br label %exit +iffalse: + br label %exit +exit: + %result = phi i32 [ ptrtoint (i32* @global_var1 to i32), %iftrue ], + [ ptrtoint (i32* @global_var1 to i32), %iffalse ] + ret i32 %result +} +; CHECK: @constantexpr_phi_twice +; CHECK: iftrue: +; CHECK: %expanded = ptrtoint i32* @global_var1 to i32 +; CHECK: iffalse: +; CHECK: %expanded1 = ptrtoint i32* @global_var1 to i32 +; CHECK: exit: + + +define i32 @constantexpr_phi_multiple_entry(i1 %arg) { +entry: + br i1 %arg, label %done, label %done +done: + %result = phi i32 [ ptrtoint (i32* @global_var1 to i32), %entry ], + [ ptrtoint (i32* @global_var1 to i32), %entry ] + ret i32 %result +} +; CHECK: @constantexpr_phi_multiple_entry +; CHECK: entry: +; CHECK: %expanded = ptrtoint i32* @global_var1 to i32 +; CHECK: br i1 %arg, label %done, label %done +; CHECK: done: +; CHECK: %result = phi i32 [ %expanded, %entry ], [ %expanded, %entry ] + + + +declare void @external_func() +declare void @personality_func() + +define void @test_landingpad() { + invoke void @external_func() to label %ok unwind label %onerror +ok: + ret void +onerror: + %lp = landingpad i32 + personality i8* bitcast (void ()* @personality_func to i8*) + catch i32* null + ret void +} +; landingpad can only accept a ConstantExpr, so this should remain +; unmodified. +; CHECK: @test_landingpad +; CHECK: personality i8* bitcast (void ()* @personality_func to i8*) diff --git a/tools/opt/opt.cpp b/tools/opt/opt.cpp index 8d2a279ff818..40c3874ff222 100644 --- a/tools/opt/opt.cpp +++ b/tools/opt/opt.cpp @@ -421,6 +421,7 @@ int main(int argc, char **argv) { initializeInstCombine(Registry); initializeInstrumentation(Registry); initializeTarget(Registry); + initializeExpandConstantExprPass(Registry); initializeExpandCtorsPass(Registry); initializeExpandTlsPass(Registry); initializeExpandTlsConstantExprPass(Registry); From d023569257fff858ad689a95cf747516757c1158 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Mon, 18 Mar 2013 11:15:22 -0700 Subject: [PATCH 018/295] Switch ABI verifier passes to manual initialization. This lines them up with the rest of the codebase and will make them work with statically-linked opt. R=mseaborn@chromium.org BUG=none TEST=existing tests keep working (we don't actually use static opt) Review URL: https://codereview.chromium.org/12902015 (cherry picked from commit f06c1a6b2bf3772607194f832d0371a7073c1427) Conflicts: include/llvm/InitializePasses.h --- include/llvm/InitializePasses.h | 14 +++++++++----- lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp | 18 +++++++++++------- lib/Analysis/NaCl/PNaClABIVerifyModule.cpp | 18 +++++++++++------- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/include/llvm/InitializePasses.h b/include/llvm/InitializePasses.h index 1595f326d3e7..fe18deb6c2e5 100644 --- a/include/llvm/InitializePasses.h +++ b/include/llvm/InitializePasses.h @@ -268,11 +268,15 @@ void initializeSLPVectorizerPass(PassRegistry&); void initializeBBVectorizePass(PassRegistry&); void initializeMachineFunctionPrinterPassPass(PassRegistry&); void initializeStackMapLivenessPass(PassRegistry&); -void initializeExpandConstantExprPass(PassRegistry&); // @LOCALMOD -void initializeExpandCtorsPass(PassRegistry&); // @LOCALMOD -void initializeExpandTlsPass(PassRegistry&); // @LOCALMOD -void initializeExpandTlsConstantExprPass(PassRegistry&); // @LOCALMOD -void initializeNaClCcRewritePass(PassRegistry&); // @LOCALMOD +// @LOCALMOD-BEGIN +void initializeExpandConstantExprPass(PassRegistry&); +void initializeExpandCtorsPass(PassRegistry&); +void initializeExpandTlsPass(PassRegistry&); +void initializeExpandTlsConstantExprPass(PassRegistry&); +void initializeNaClCcRewritePass(PassRegistry&); +void initializePNaClABIVerifyModulePass(PassRegistry&); +void initializePNaClABIVerifyFunctionsPass(PassRegistry&); +// @LOCALMOD-END } #endif diff --git a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp index a099817f1e79..8404a4cafc8a 100644 --- a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp +++ b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp @@ -30,13 +30,18 @@ namespace { class PNaClABIVerifyFunctions : public FunctionPass { public: static char ID; - PNaClABIVerifyFunctions() : FunctionPass(ID), - Reporter(new PNaClABIErrorReporter), - ReporterIsOwned(true) {} + PNaClABIVerifyFunctions() : + FunctionPass(ID), + Reporter(new PNaClABIErrorReporter), + ReporterIsOwned(true) { + initializePNaClABIVerifyFunctionsPass(*PassRegistry::getPassRegistry()); + } explicit PNaClABIVerifyFunctions(PNaClABIErrorReporter *Reporter_) : FunctionPass(ID), Reporter(Reporter_), - ReporterIsOwned(false) {} + ReporterIsOwned(false) { + initializePNaClABIVerifyFunctionsPass(*PassRegistry::getPassRegistry()); + } ~PNaClABIVerifyFunctions() { if (ReporterIsOwned) delete Reporter; @@ -183,9 +188,8 @@ void PNaClABIVerifyFunctions::print(llvm::raw_ostream &O, const Module *M) } char PNaClABIVerifyFunctions::ID = 0; - -static RegisterPass X("verify-pnaclabi-functions", - "Verify functions for PNaCl", false, false); +INITIALIZE_PASS(PNaClABIVerifyFunctions, "verify-pnaclabi-functions", + "Verify functions for PNaCl", false, true) FunctionPass *llvm::createPNaClABIVerifyFunctionsPass( PNaClABIErrorReporter *Reporter) { diff --git a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp index 6b0243fbee59..660fa9f8fe97 100644 --- a/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp +++ b/lib/Analysis/NaCl/PNaClABIVerifyModule.cpp @@ -28,13 +28,18 @@ namespace { class PNaClABIVerifyModule : public ModulePass { public: static char ID; - PNaClABIVerifyModule() : ModulePass(ID), - Reporter(new PNaClABIErrorReporter), - ReporterIsOwned(true) {} + PNaClABIVerifyModule() : + ModulePass(ID), + Reporter(new PNaClABIErrorReporter), + ReporterIsOwned(true) { + initializePNaClABIVerifyModulePass(*PassRegistry::getPassRegistry()); + } explicit PNaClABIVerifyModule(PNaClABIErrorReporter *Reporter_) : ModulePass(ID), Reporter(Reporter_), - ReporterIsOwned(false) {} + ReporterIsOwned(false) { + initializePNaClABIVerifyModulePass(*PassRegistry::getPassRegistry()); + } ~PNaClABIVerifyModule() { if (ReporterIsOwned) delete Reporter; @@ -157,9 +162,8 @@ void PNaClABIVerifyModule::print(llvm::raw_ostream &O, const Module *M) const { } char PNaClABIVerifyModule::ID = 0; - -static RegisterPass X("verify-pnaclabi-module", - "Verify module for PNaCl", false, false); +INITIALIZE_PASS(PNaClABIVerifyModule, "verify-pnaclabi-module", + "Verify module for PNaCl", false, true) ModulePass *llvm::createPNaClABIVerifyModulePass( PNaClABIErrorReporter *Reporter) { From d69e0b59eeccb0c337ff2a69ebfc45c521c5900f Mon Sep 17 00:00:00 2001 From: Eli Bendersky Date: Mon, 11 Mar 2013 15:38:11 -0700 Subject: [PATCH 019/295] Apply after-merge fixes to return to working state. (cherry picked from commit d41567d2ffd3413600162653c08b2365bd5bcbbf) In an effort to reduce the surface of changes made while cherry picking the PNaCl transformation passes, I, Richard Diamond, removed all changes to all files except lib/Transforms/NaCl/ExpandTls.cpp. Conflicts: lib/MC/MCObjectStreamer.cpp lib/Target/ARM/ARMAsmPrinter.cpp lib/Target/ARM/ARMSubtarget.cpp lib/Target/Mips/MCTargetDesc/MipsELFObjectWriter.cpp lib/Target/Mips/MCTargetDesc/MipsMCTargetDesc.cpp lib/Target/Mips/MipsAsmPrinter.cpp lib/Target/Mips/MipsDelaySlotFiller.cpp lib/Target/Mips/MipsISelDAGToDAG.cpp lib/Target/Mips/MipsInstrFPU.td lib/Target/Mips/MipsNaClRewritePass.cpp lib/Target/X86/MCTargetDesc/X86MCAsmInfo.cpp lib/Target/X86/X86FastISel.cpp lib/Target/X86/X86ISelLowering.cpp lib/Target/X86/X86TargetMachine.cpp lib/Transforms/NaCl/ExpandTls.cpp test/NaCl/ARM/neon-vld1-sandboxing.ll test/NaCl/ARM/neon-vld2-sandboxing.ll test/NaCl/ARM/neon-vld3-sandboxing.ll test/NaCl/ARM/neon-vld4-sandboxing.ll test/NaCl/ARM/neon-vlddup-sandboxing.ll test/NaCl/ARM/neon-vldlane-sandboxing.ll test/NaCl/ARM/neon-vst1-sandboxing.ll test/NaCl/ARM/neon-vst2-sandboxing.ll test/NaCl/ARM/neon-vst3-sandboxing.ll test/NaCl/ARM/neon-vst4-sandboxing.ll test/NaCl/ARM/neon-vstlane-sandboxing.ll --- lib/Transforms/NaCl/ExpandTls.cpp | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/lib/Transforms/NaCl/ExpandTls.cpp b/lib/Transforms/NaCl/ExpandTls.cpp index 8ce439c018e1..929b2e0a15b9 100644 --- a/lib/Transforms/NaCl/ExpandTls.cpp +++ b/lib/Transforms/NaCl/ExpandTls.cpp @@ -24,12 +24,12 @@ #include -#include "llvm/Constants.h" -#include "llvm/DataLayout.h" -#include "llvm/DerivedTypes.h" -#include "llvm/Instructions.h" -#include "llvm/Module.h" #include "llvm/Pass.h" +#include "llvm/IR/Constants.h" +#include "llvm/IR/DataLayout.h" +#include "llvm/IR/DerivedTypes.h" +#include "llvm/IR/Instructions.h" +#include "llvm/IR/Module.h" #include "llvm/Support/raw_ostream.h" #include "llvm/Transforms/NaCl.h" @@ -237,11 +237,10 @@ static void rewriteTlsVars(Module &M, std::vector *TlsVars, FunctionType *ReadTpType = FunctionType::get(PointerType::get(i8, 0), /*isVarArg=*/false); AttrBuilder B; - B.addAttribute(Attributes::ReadOnly); - B.addAttribute(Attributes::NoUnwind); - AttrListPtr ReadTpAttrs = AttrListPtr().addAttr( - M.getContext(), AttrListPtr::FunctionIndex, - Attributes::get(M.getContext(), B)); + B.addAttribute(Attribute::ReadOnly); + B.addAttribute(Attribute::NoUnwind); + AttributeSet ReadTpAttrs = AttributeSet::get( + M.getContext(), AttributeSet::FunctionIndex, B); Constant *ReadTpFunc = M.getOrInsertTargetIntrinsic("llvm.nacl.read.tp", ReadTpType, ReadTpAttrs); From 3a04f8499872f8fafff9f786b6e561df28487db4 Mon Sep 17 00:00:00 2001 From: Mark Seaborn Date: Thu, 21 Mar 2013 14:18:49 -0700 Subject: [PATCH 020/295] PNaCl: Add ExpandVarArgs pass for expanding out variable-args function calls Once this pass is enabled, it will simplify the language to reduce the set of constructs that a PNaCl translator needs to handle as part of a stable wire format for PNaCl. BUG=https://code.google.com/p/nativeclient/issues/detail?id=3338 TEST=test/Transforms/NaCl/expand-varargs.ll Review URL: https://codereview.chromium.org/12481021 (cherry picked from commit 7216560b2f1e66a7c9bb9a1b344ae6e9f0f89b7b) --- include/llvm/InitializePasses.h | 1 + include/llvm/Transforms/NaCl.h | 1 + lib/Transforms/NaCl/CMakeLists.txt | 1 + lib/Transforms/NaCl/ExpandVarArgs.cpp | 327 +++++++++++++++++++++++++ test/Transforms/NaCl/expand-varargs.ll | 129 ++++++++++ tools/opt/opt.cpp | 1 + 6 files changed, 460 insertions(+) create mode 100644 lib/Transforms/NaCl/ExpandVarArgs.cpp create mode 100644 test/Transforms/NaCl/expand-varargs.ll diff --git a/include/llvm/InitializePasses.h b/include/llvm/InitializePasses.h index fe18deb6c2e5..357b96ca94b9 100644 --- a/include/llvm/InitializePasses.h +++ b/include/llvm/InitializePasses.h @@ -273,6 +273,7 @@ void initializeExpandConstantExprPass(PassRegistry&); void initializeExpandCtorsPass(PassRegistry&); void initializeExpandTlsPass(PassRegistry&); void initializeExpandTlsConstantExprPass(PassRegistry&); +void initializeExpandVarArgsPass(PassRegistry&); void initializeNaClCcRewritePass(PassRegistry&); void initializePNaClABIVerifyModulePass(PassRegistry&); void initializePNaClABIVerifyFunctionsPass(PassRegistry&); diff --git a/include/llvm/Transforms/NaCl.h b/include/llvm/Transforms/NaCl.h index 686b45ff747e..6494200f6dd3 100644 --- a/include/llvm/Transforms/NaCl.h +++ b/include/llvm/Transforms/NaCl.h @@ -19,6 +19,7 @@ FunctionPass *createExpandConstantExprPass(); ModulePass *createExpandCtorsPass(); ModulePass *createExpandTlsPass(); ModulePass *createExpandTlsConstantExprPass(); +ModulePass *createExpandVarArgsPass(); } diff --git a/lib/Transforms/NaCl/CMakeLists.txt b/lib/Transforms/NaCl/CMakeLists.txt index acf7656f64b8..6ba9dcc68b5d 100644 --- a/lib/Transforms/NaCl/CMakeLists.txt +++ b/lib/Transforms/NaCl/CMakeLists.txt @@ -3,6 +3,7 @@ add_llvm_library(LLVMTransformsNaCl ExpandCtors.cpp ExpandTls.cpp ExpandTlsConstantExpr.cpp + ExpandVarArgs.cpp ) add_dependencies(LLVMTransformsNaCl intrinsics_gen) diff --git a/lib/Transforms/NaCl/ExpandVarArgs.cpp b/lib/Transforms/NaCl/ExpandVarArgs.cpp new file mode 100644 index 000000000000..e1bd8f93a887 --- /dev/null +++ b/lib/Transforms/NaCl/ExpandVarArgs.cpp @@ -0,0 +1,327 @@ +//===- ExpandVarArgs.cpp - Expand out variable argument function calls-----===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This pass expands out all use of variable argument functions. +// +// This pass replaces a varargs function call with a function call in +// which a pointer to the variable arguments is passed explicitly. +// The callee explicitly allocates space for the variable arguments on +// the stack using "alloca". +// +// Alignment: +// +// This pass does not add any alignment padding between the arguments +// that are copied onto the stack. We assume that the only argument +// types that need to be handled are 32-bit and 64-bit -- i32, i64, +// pointers and double: +// +// * We won't see i1, i8, i16 and float as varargs arguments because +// the C standard requires the compiler to promote these to the +// types "int" and "double". +// +// * We won't see va_arg instructions of struct type because Clang +// does not yet support them in PNaCl mode. See +// https://code.google.com/p/nativeclient/issues/detail?id=2381 +// +// If such arguments do appear in the input, this pass will generate +// correct, working code, but this code might be inefficient due to +// using unaligned memory accesses. +// +//===----------------------------------------------------------------------===// + +#include "llvm/ADT/SmallVector.h" +#include "llvm/IR/DataLayout.h" +#include "llvm/IR/Function.h" +#include "llvm/IR/Instructions.h" +#include "llvm/IR/IntrinsicInst.h" +#include "llvm/IR/Module.h" +#include "llvm/Pass.h" +#include "llvm/Transforms/NaCl.h" + +using namespace llvm; + +namespace { + // This is a ModulePass because the pass recreates functions in + // order to change their argument lists. + class ExpandVarArgs : public ModulePass { + public: + static char ID; // Pass identification, replacement for typeid + ExpandVarArgs() : ModulePass(ID) { + initializeExpandVarArgsPass(*PassRegistry::getPassRegistry()); + } + + virtual bool runOnModule(Module &M); + }; +} + +char ExpandVarArgs::ID = 0; +INITIALIZE_PASS(ExpandVarArgs, "expand-varargs", + "Expand out variable argument function definitions and calls", + false, false) + +static Instruction *CopyDebug(Instruction *NewInst, Instruction *Original) { + NewInst->setDebugLoc(Original->getDebugLoc()); + return NewInst; +} + +static void ExpandVarArgFunc(Function *Func) { + Type *PtrType = Type::getInt8PtrTy(Func->getContext()); + + FunctionType *FTy = Func->getFunctionType(); + SmallVector Params(FTy->param_begin(), FTy->param_end()); + Params.push_back(PtrType); + FunctionType *NFTy = FunctionType::get(FTy->getReturnType(), Params, false); + + // In order to change the function's arguments, we have to recreate + // the function. + Function *NewFunc = Function::Create(NFTy, Func->getLinkage()); + NewFunc->copyAttributesFrom(Func); + Func->getParent()->getFunctionList().insert(Func, NewFunc); + NewFunc->takeName(Func); + NewFunc->getBasicBlockList().splice(NewFunc->begin(), + Func->getBasicBlockList()); + + // Move the arguments across to the new function. + for (Function::arg_iterator Arg = Func->arg_begin(), E = Func->arg_end(), + NewArg = NewFunc->arg_begin(); + Arg != E; ++Arg, ++NewArg) { + Arg->replaceAllUsesWith(NewArg); + NewArg->takeName(Arg); + } + + Func->replaceAllUsesWith( + ConstantExpr::getBitCast(NewFunc, FTy->getPointerTo())); + Func->eraseFromParent(); + + Value *VarArgsArg = --NewFunc->arg_end(); + VarArgsArg->setName("varargs"); + + // Expand out uses of llvm.va_start in this function. + for (Function::iterator BB = NewFunc->begin(), E = NewFunc->end(); + BB != E; + ++BB) { + for (BasicBlock::iterator Iter = BB->begin(), E = BB->end(); + Iter != E; ) { + Instruction *Inst = Iter++; + if (VAStartInst *VAS = dyn_cast(Inst)) { + Value *Cast = CopyDebug(new BitCastInst(VAS->getArgList(), + PtrType->getPointerTo(), + "arglist", VAS), VAS); + CopyDebug(new StoreInst(VarArgsArg, Cast, VAS), VAS); + VAS->eraseFromParent(); + } + } + } +} + +static void ExpandVAArgInst(VAArgInst *Inst) { + // Read the argument. We assume that no realignment of the pointer + // is required. + Value *ArgList = CopyDebug(new BitCastInst( + Inst->getPointerOperand(), + Inst->getType()->getPointerTo()->getPointerTo(), "arglist", Inst), Inst); + Value *CurrentPtr = CopyDebug(new LoadInst(ArgList, "arglist_current", Inst), + Inst); + Value *Result = CopyDebug(new LoadInst(CurrentPtr, "va_arg", Inst), Inst); + Result->takeName(Inst); + + // Update the va_list to point to the next argument. + SmallVector Indexes; + Indexes.push_back(ConstantInt::get(Inst->getContext(), APInt(32, 1))); + Value *Next = CopyDebug(GetElementPtrInst::Create( + CurrentPtr, Indexes, "arglist_next", Inst), Inst); + CopyDebug(new StoreInst(Next, ArgList, Inst), Inst); + + Inst->replaceAllUsesWith(Result); + Inst->eraseFromParent(); +} + +static void ExpandVACopyInst(VACopyInst *Inst) { + // va_list may have more space reserved, but we only need to + // copy a single pointer. + Type *PtrTy = Type::getInt8PtrTy(Inst->getContext())->getPointerTo(); + Value *Src = CopyDebug(new BitCastInst(Inst->getSrc(), PtrTy, "vacopy_src", + Inst), Inst); + Value *Dest = CopyDebug(new BitCastInst(Inst->getDest(), PtrTy, "vacopy_dest", + Inst), Inst); + Value *CurrentPtr = CopyDebug(new LoadInst(Src, "vacopy_currentptr", Inst), + Inst); + CopyDebug(new StoreInst(CurrentPtr, Dest, Inst), Inst); + Inst->eraseFromParent(); +} + +static void LifetimeDecl(Intrinsic::ID id, Value *Ptr, Value *Size, + Instruction *InsertPt) { + Module *M = InsertPt->getParent()->getParent()->getParent(); + Value *Func = Intrinsic::getDeclaration(M, id); + SmallVector Args; + Args.push_back(Size); + Args.push_back(Ptr); + CallInst::Create(Func, Args, "", InsertPt); +} + +// CopyCall() uses argument overloading so that it can be used by the +// template ExpandVarArgCall(). +static Instruction *CopyCall(CallInst *Original, Value *Callee, + ArrayRef Args) { + return CallInst::Create(Callee, Args, "", Original); +} + +static Instruction *CopyCall(InvokeInst *Original, Value *Callee, + ArrayRef Args) { + return InvokeInst::Create(Callee, Original->getNormalDest(), + Original->getUnwindDest(), Args, "", Original); +} + +// ExpandVarArgCall() converts a CallInst or InvokeInst to expand out +// of varargs. It returns whether the module was modified. +template +static bool ExpandVarArgCall(InstType *Call, DataLayout *DL) { + FunctionType *FuncType = cast( + Call->getCalledValue()->getType()->getPointerElementType()); + if (!FuncType->isFunctionVarArg()) + return false; + + LLVMContext *Context = &Call->getContext(); + + // Split argument list into fixed and variable arguments. + SmallVector FixedArgs; + SmallVector VarArgs; + SmallVector VarArgsTypes; + for (unsigned I = 0; I < FuncType->getNumParams(); ++I) + FixedArgs.push_back(Call->getArgOperand(I)); + for (unsigned I = FuncType->getNumParams(); + I < Call->getNumArgOperands(); ++I) { + VarArgs.push_back(Call->getArgOperand(I)); + VarArgsTypes.push_back(Call->getArgOperand(I)->getType()); + } + + StructType *VarArgsTy; + Value *ArgToAdd; + Instruction *BufPtr = NULL; + Value *BufSize = NULL; + if (VarArgs.size() == 0) { + // If there are no variable arguments being passed, we still want + // to add an extra argument to the function call so that the + // number of arguments matches the callee's type. + VarArgsTy = StructType::get(*Context); + ArgToAdd = UndefValue::get(VarArgsTy->getPointerTo()); + } else { + // Create struct type for packing variable arguments into. We + // create this as packed for now and assume that no alignment + // padding is desired. + VarArgsTy = StructType::create(VarArgsTypes, "vararg_call", true); + + // Allocate space for the variable argument buffer. Do this at the + // start of the function so that we don't leak space if the function + // is called in a loop. + Function *Func = Call->getParent()->getParent(); + Instruction *Buf = new AllocaInst(VarArgsTy, "vararg_buffer"); + Func->getEntryBlock().getInstList().push_front(Buf); + ArgToAdd = Buf; + + // Call llvm.lifetime.start/end intrinsics to indicate that Buf is + // only used for the duration of the function call, so that the + // stack space can be reused elsewhere. + Type *I8Ptr = Type::getInt8Ty(*Context)->getPointerTo(); + BufPtr = new BitCastInst(Buf, I8Ptr, "vararg_lifetime_bitcast"); + BufPtr->insertAfter(Buf); + BufSize = ConstantInt::get(*Context, + APInt(64, DL->getTypeAllocSize(VarArgsTy))); + LifetimeDecl(Intrinsic::lifetime_start, BufPtr, BufSize, Call); + + // Copy variable arguments into buffer. + int Index = 0; + for (SmallVector::iterator Iter = VarArgs.begin(); + Iter != VarArgs.end(); + ++Iter, ++Index) { + SmallVector Indexes; + Indexes.push_back(ConstantInt::get(*Context, APInt(32, 0))); + Indexes.push_back(ConstantInt::get(*Context, APInt(32, Index))); + Value *Ptr = CopyDebug(GetElementPtrInst::Create( + Buf, Indexes, "vararg_ptr", Call), Call); + CopyDebug(new StoreInst(*Iter, Ptr, Call), Call); + } + } + + // Cast function to new type to add our extra pointer argument. + SmallVector ArgTypes(FuncType->param_begin(), + FuncType->param_end()); + ArgTypes.push_back(VarArgsTy->getPointerTo()); + FunctionType *NFTy = FunctionType::get(FuncType->getReturnType(), + ArgTypes, false); + Value *CastFunc = + CopyDebug(new BitCastInst(Call->getCalledValue(), NFTy->getPointerTo(), + "vararg_func", Call), Call); + + // Create the converted function call. + FixedArgs.push_back(ArgToAdd); + Value *NewCall = CopyDebug(CopyCall(Call, CastFunc, FixedArgs), Call); + NewCall->takeName(Call); + + if (BufPtr) { + if (isa(Call)) { + LifetimeDecl(Intrinsic::lifetime_end, BufPtr, BufSize, Call); + } else if (InvokeInst *Invoke = dyn_cast(Call)) { + LifetimeDecl(Intrinsic::lifetime_end, BufPtr, BufSize, + Invoke->getNormalDest()->getFirstInsertionPt()); + LifetimeDecl(Intrinsic::lifetime_end, BufPtr, BufSize, + Invoke->getUnwindDest()->getFirstInsertionPt()); + } + } + + Call->replaceAllUsesWith(NewCall); + Call->eraseFromParent(); + + return true; +} + +bool ExpandVarArgs::runOnModule(Module &M) { + bool Changed = false; + DataLayout DL(&M); + + for (Module::iterator Iter = M.begin(), E = M.end(); Iter != E; ) { + Function *Func = Iter++; + + for (Function::iterator BB = Func->begin(), E = Func->end(); + BB != E; + ++BB) { + for (BasicBlock::iterator Iter = BB->begin(), E = BB->end(); + Iter != E; ) { + Instruction *Inst = Iter++; + if (VAArgInst *VI = dyn_cast(Inst)) { + Changed = true; + ExpandVAArgInst(VI); + } else if (isa(Inst)) { + // va_end() is a no-op in this implementation. + Changed = true; + Inst->eraseFromParent(); + } else if (VACopyInst *VAC = dyn_cast(Inst)) { + Changed = true; + ExpandVACopyInst(VAC); + } else if (CallInst *Call = dyn_cast(Inst)) { + Changed |= ExpandVarArgCall(Call, &DL); + } else if (InvokeInst *Call = dyn_cast(Inst)) { + Changed |= ExpandVarArgCall(Call, &DL); + } + } + } + + if (Func->isVarArg()) { + Changed = true; + ExpandVarArgFunc(Func); + } + } + + return Changed; +} + +ModulePass *llvm::createExpandVarArgsPass() { + return new ExpandVarArgs(); +} diff --git a/test/Transforms/NaCl/expand-varargs.ll b/test/Transforms/NaCl/expand-varargs.ll new file mode 100644 index 000000000000..c1da3d8b2d70 --- /dev/null +++ b/test/Transforms/NaCl/expand-varargs.ll @@ -0,0 +1,129 @@ +; RUN: opt < %s -expand-varargs -S | FileCheck %s + +%va_list = type i8* + +declare void @llvm.va_start(i8*) +declare void @llvm.va_end(i8*) +declare void @llvm.va_copy(i8*, i8*) + +declare i32 @outside_func(i32 %arg, %va_list* %args) + + +; Produced by the expansion of @varargs_call1(): +; CHECK: %vararg_call = type <{ i64, i32 }> + + +define i32 @varargs_func(i32 %arg, ...) { + %arglist_alloc = alloca %va_list + %arglist = bitcast %va_list* %arglist_alloc to i8* + + call void @llvm.va_start(i8* %arglist) + %result = call i32 @outside_func(i32 %arg, %va_list* %arglist_alloc) + call void @llvm.va_end(i8* %arglist) + ret i32 %result +} +; CHECK: define i32 @varargs_func(i32 %arg, i8* %varargs) { +; CHECK-NEXT: %arglist_alloc = alloca i8* +; CHECK-NEXT: %arglist = bitcast i8** %arglist_alloc to i8* +; CHECK-NEXT: %arglist1 = bitcast i8* %arglist to i8** +; CHECK-NEXT: store i8* %varargs, i8** %arglist1 +; CHECK-NEXT: %result = call i32 @outside_func(i32 %arg, i8** %arglist_alloc) +; CHECK-NEXT: ret i32 %result + + +define i32 @varargs_call1() { + %result = call i32 (i32, ...)* @varargs_func(i32 111, i64 222, i32 333) + ret i32 %result +} +; CHECK: define i32 @varargs_call1() { +; CHECK-NEXT: %vararg_buffer = alloca %vararg_call +; CHECK-NEXT: %vararg_lifetime_bitcast = bitcast %vararg_call* %vararg_buffer to i8* +; CHECK-NEXT: call void @llvm.lifetime.start(i64 12, i8* %vararg_lifetime_bitcast) +; CHECK-NEXT: %vararg_ptr = getelementptr %vararg_call* %vararg_buffer, i32 0, i32 0 +; CHECK-NEXT: store i64 222, i64* %vararg_ptr +; CHECK-NEXT: %vararg_ptr1 = getelementptr %vararg_call* %vararg_buffer, i32 0, i32 1 +; CHECK-NEXT: store i32 333, i32* %vararg_ptr1 +; CHECK-NEXT: %vararg_func = bitcast i32 (i32, ...)* bitcast (i32 (i32, i8*)* @varargs_func to i32 (i32, ...)*) to i32 (i32, %vararg_call*)* +; CHECK-NEXT: %result = call i32 %vararg_func(i32 111, %vararg_call* %vararg_buffer) +; CHECK-NEXT: call void @llvm.lifetime.end(i64 12, i8* %vararg_lifetime_bitcast) +; CHECK-NEXT: ret i32 %result + + +; Check that the pass works when there are no variable arguments. +define i32 @varargs_call2() { + %result = call i32 (i32, ...)* @varargs_func(i32 111) + ret i32 %result +} +; CHECK: define i32 @varargs_call2() { +; CHECK-NEXT: %vararg_func = bitcast i32 (i32, ...)* bitcast (i32 (i32, i8*)* @varargs_func to i32 (i32, ...)*) to i32 (i32, {}*)* +; CHECK-NEXT: %result = call i32 %vararg_func(i32 111, {}* undef) + + +; Check that "invoke" instructions are expanded out too. +define i32 @varargs_invoke() { + %result = invoke i32 (i32, ...)* @varargs_func(i32 111, i64 222) + to label %cont unwind label %lpad +cont: + ret i32 %result +lpad: + %lp = landingpad { i8*, i32 } personality i8* null cleanup + ret i32 0 +} +; CHECK: @varargs_invoke +; CHECK: %result = invoke i32 %vararg_func(i32 111, %vararg_call.0* %vararg_buffer) +; CHECK-NEXT: to label %cont unwind label %lpad +; CHECK: cont: +; CHECK-NEXT: call void @llvm.lifetime.end(i64 8, i8* %vararg_lifetime_bitcast) +; CHECK: lpad: +; CHECK: call void @llvm.lifetime.end(i64 8, i8* %vararg_lifetime_bitcast) + + +define void @varargs_multiple_calls() { + %call1 = call i32 (i32, ...)* @varargs_func(i32 11, i64 22, i32 33) + %call2 = call i32 (i32, ...)* @varargs_func(i32 44, i64 55, i32 66) + ret void +} +; CHECK: @varargs_multiple_calls() +; The added allocas should appear at the start of the function. +; CHECK: %vararg_buffer{{.*}} = alloca %vararg_call{{.*}} +; CHECK: %vararg_buffer{{.*}} = alloca %vararg_call{{.*}} +; CHECK: %call1 = call i32 %vararg_func{{.*}}(i32 11, %vararg_call{{.*}}* %vararg_buffer{{.*}}) +; CHECK: %call2 = call i32 %vararg_func{{.*}}(i32 44, %vararg_call{{.*}}* %vararg_buffer{{.*}}) + + +define i32 @va_arg_i32(i8* %arglist) { + %result = va_arg i8* %arglist, i32 + ret i32 %result +} +; CHECK: define i32 @va_arg_i32(i8* %arglist) { +; CHECK-NEXT: %arglist1 = bitcast i8* %arglist to i32** +; CHECK-NEXT: %arglist_current = load i32** %arglist1 +; CHECK-NEXT: %result = load i32* %arglist_current +; CHECK-NEXT: %arglist_next = getelementptr i32* %arglist_current, i32 1 +; CHECK-NEXT: store i32* %arglist_next, i32** %arglist1 +; CHECK-NEXT: ret i32 %result + + +define i64 @va_arg_i64(i8* %arglist) { + %result = va_arg i8* %arglist, i64 + ret i64 %result +} +; CHECK: define i64 @va_arg_i64(i8* %arglist) { +; CHECK-NEXT: %arglist1 = bitcast i8* %arglist to i64** +; CHECK-NEXT: %arglist_current = load i64** %arglist1 +; CHECK-NEXT: %result = load i64* %arglist_current +; CHECK-NEXT: %arglist_next = getelementptr i64* %arglist_current, i32 1 +; CHECK-NEXT: store i64* %arglist_next, i64** %arglist1 +; CHECK-NEXT: ret i64 %result + + +define void @do_va_copy(i8* %dest, i8* %src) { + call void @llvm.va_copy(i8* %dest, i8* %src) + ret void +} +; CHECK: define void @do_va_copy(i8* %dest, i8* %src) { +; CHECK-NEXT: %vacopy_src = bitcast i8* %src to i8** +; CHECK-NEXT: %vacopy_dest = bitcast i8* %dest to i8** +; CHECK-NEXT: %vacopy_currentptr = load i8** %vacopy_src +; CHECK-NEXT: store i8* %vacopy_currentptr, i8** %vacopy_dest +; CHECK-NEXT: ret void diff --git a/tools/opt/opt.cpp b/tools/opt/opt.cpp index 40c3874ff222..d60253acb8ca 100644 --- a/tools/opt/opt.cpp +++ b/tools/opt/opt.cpp @@ -425,6 +425,7 @@ int main(int argc, char **argv) { initializeExpandCtorsPass(Registry); initializeExpandTlsPass(Registry); initializeExpandTlsConstantExprPass(Registry); + initializeExpandVarArgsPass(Registry); cl::ParseCommandLineOptions(argc, argv, "llvm .bc -> .bc modular optimizer and analysis printer\n"); From 073e18523ec7fbe4092a743247c0968657fd42af Mon Sep 17 00:00:00 2001 From: Mark Seaborn Date: Mon, 25 Mar 2013 12:08:49 -0700 Subject: [PATCH 021/295] PNaCl: Fix ExpandTls to handle GlobalAliases of thread-local variables Expand out these aliases in the ExpandTlsConstantExpr pass. BUG=https://code.google.com/p/nativeclient/issues/detail?id=3347 TEST=test/Transforms/NaCl/expand-tls-constexpr-alias.ll Review URL: https://codereview.chromium.org/12989011 (cherry picked from commit 14a8889a009e84ba18ab200d590cf76bb47000b1) --- lib/Transforms/NaCl/ExpandTlsConstantExpr.cpp | 8 ++++++ .../NaCl/expand-tls-constexpr-alias.ll | 28 +++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 test/Transforms/NaCl/expand-tls-constexpr-alias.ll diff --git a/lib/Transforms/NaCl/ExpandTlsConstantExpr.cpp b/lib/Transforms/NaCl/ExpandTlsConstantExpr.cpp index 90e007604f6f..01663a0d7d97 100644 --- a/lib/Transforms/NaCl/ExpandTlsConstantExpr.cpp +++ b/lib/Transforms/NaCl/ExpandTlsConstantExpr.cpp @@ -95,6 +95,14 @@ static void expandConstExpr(Constant *Expr) { } bool ExpandTlsConstantExpr::runOnModule(Module &M) { + for (Module::alias_iterator Iter = M.alias_begin(); + Iter != M.alias_end(); ) { + GlobalAlias *GA = Iter++; + if (GA->isThreadDependent()) { + GA->replaceAllUsesWith(GA->getAliasee()); + GA->eraseFromParent(); + } + } for (Module::global_iterator Global = M.global_begin(); Global != M.global_end(); ++Global) { diff --git a/test/Transforms/NaCl/expand-tls-constexpr-alias.ll b/test/Transforms/NaCl/expand-tls-constexpr-alias.ll new file mode 100644 index 000000000000..65daa5eacd4a --- /dev/null +++ b/test/Transforms/NaCl/expand-tls-constexpr-alias.ll @@ -0,0 +1,28 @@ +; RUN: opt < %s -nacl-expand-tls-constant-expr -S | FileCheck %s + +@real_tvar = thread_local global i32 123 +@tvar_alias = alias i32* @real_tvar +@tvar_alias2 = alias i32* getelementptr (i32* @real_tvar, i32 100) + + +define i32* @get_tvar() { + ret i32* @tvar_alias +} +; CHECK: define i32* @get_tvar() +; CHECK: ret i32* @real_tvar + + +define i32* @get_tvar2() { + ret i32* @tvar_alias2 +} +; CHECK: define i32* @get_tvar2() +; CHECK: %expanded = getelementptr i32* @real_tvar, i32 100 +; CHECK: ret i32* %expanded + + +define i32* @get_tvar3() { + ret i32* getelementptr (i32* @tvar_alias2, i32 100) +} +; CHECK: define i32* @get_tvar3() +; CHECK: %expanded = getelementptr i32* @real_tvar, i32 200 +; CHECK: ret i32* %expanded From c45afbdf98419a8c0c568b6071606afe8fd5f984 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Mon, 25 Mar 2013 16:51:51 -0700 Subject: [PATCH 022/295] PNaCl ABI: add passes to cleanup/finalize some linkage types and resolve aliases. R=mseaborn@chromium.org,eliben@chromium.org BUG= https://code.google.com/p/nativeclient/issues/detail?id=3339 Review URL: https://codereview.chromium.org/13036005 (cherry picked from commit 77fc541fc5b17685047aa296f7669a2ddc2bfd89) --- include/llvm/InitializePasses.h | 4 +- include/llvm/Transforms/NaCl.h | 2 + lib/Transforms/NaCl/CMakeLists.txt | 1 + lib/Transforms/NaCl/GlobalCleanup.cpp | 109 ++++++++++++++++++++++++ test/Transforms/NaCl/globalcleanup.ll | 34 ++++++++ test/Transforms/NaCl/resolve-aliases.ll | 36 ++++++++ tools/opt/opt.cpp | 4 + 7 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 lib/Transforms/NaCl/GlobalCleanup.cpp create mode 100644 test/Transforms/NaCl/globalcleanup.ll create mode 100644 test/Transforms/NaCl/resolve-aliases.ll diff --git a/include/llvm/InitializePasses.h b/include/llvm/InitializePasses.h index 357b96ca94b9..d971caa4b896 100644 --- a/include/llvm/InitializePasses.h +++ b/include/llvm/InitializePasses.h @@ -271,12 +271,14 @@ void initializeStackMapLivenessPass(PassRegistry&); // @LOCALMOD-BEGIN void initializeExpandConstantExprPass(PassRegistry&); void initializeExpandCtorsPass(PassRegistry&); -void initializeExpandTlsPass(PassRegistry&); void initializeExpandTlsConstantExprPass(PassRegistry&); +void initializeExpandTlsPass(PassRegistry&); void initializeExpandVarArgsPass(PassRegistry&); +void initializeGlobalCleanupPass(PassRegistry&); void initializeNaClCcRewritePass(PassRegistry&); void initializePNaClABIVerifyModulePass(PassRegistry&); void initializePNaClABIVerifyFunctionsPass(PassRegistry&); +void initializeResolveAliasesPass(PassRegistry&); // @LOCALMOD-END } diff --git a/include/llvm/Transforms/NaCl.h b/include/llvm/Transforms/NaCl.h index 6494200f6dd3..b5108fbf3271 100644 --- a/include/llvm/Transforms/NaCl.h +++ b/include/llvm/Transforms/NaCl.h @@ -20,6 +20,8 @@ ModulePass *createExpandCtorsPass(); ModulePass *createExpandTlsPass(); ModulePass *createExpandTlsConstantExprPass(); ModulePass *createExpandVarArgsPass(); +ModulePass *createGlobalCleanupPass(); +ModulePass *createResolveAliasesPass(); } diff --git a/lib/Transforms/NaCl/CMakeLists.txt b/lib/Transforms/NaCl/CMakeLists.txt index 6ba9dcc68b5d..5b1de88dc489 100644 --- a/lib/Transforms/NaCl/CMakeLists.txt +++ b/lib/Transforms/NaCl/CMakeLists.txt @@ -4,6 +4,7 @@ add_llvm_library(LLVMTransformsNaCl ExpandTls.cpp ExpandTlsConstantExpr.cpp ExpandVarArgs.cpp + GlobalCleanup.cpp ) add_dependencies(LLVMTransformsNaCl intrinsics_gen) diff --git a/lib/Transforms/NaCl/GlobalCleanup.cpp b/lib/Transforms/NaCl/GlobalCleanup.cpp new file mode 100644 index 000000000000..9a28063af67e --- /dev/null +++ b/lib/Transforms/NaCl/GlobalCleanup.cpp @@ -0,0 +1,109 @@ +//===- GlobalCleanup.cpp - Cleanup global symbols post-bitcode-link -------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +// ===---------------------------------------------------------------------===// +// +// PNaCl executables should have no external symbols or aliases. These passes +// internalize (or otherwise remove/resolve) GlobalValues and resolve all +// GlobalAliases. +// +//===----------------------------------------------------------------------===// + +#include "llvm/IR/Constants.h" +#include "llvm/IR/DerivedTypes.h" +#include "llvm/IR/Instructions.h" +#include "llvm/IR/Module.h" +#include "llvm/Pass.h" +#include "llvm/Transforms/NaCl.h" + +using namespace llvm; + +namespace { + class GlobalCleanup : public ModulePass { + public: + static char ID; + GlobalCleanup() : ModulePass(ID) { + initializeGlobalCleanupPass(*PassRegistry::getPassRegistry()); + } + virtual bool runOnModule(Module &M); + }; + + class ResolveAliases : public ModulePass { + public: + static char ID; + ResolveAliases() : ModulePass(ID) { + initializeResolveAliasesPass(*PassRegistry::getPassRegistry()); + } + virtual bool runOnModule(Module &M); + }; +} + +char GlobalCleanup::ID = 0; +INITIALIZE_PASS(GlobalCleanup, "nacl-global-cleanup", + "GlobalValue cleanup for PNaCl", false, false) + +static bool CleanUpLinkage(GlobalValue *GV) { + // TODO(dschuff): handle the rest of the linkage types as necessary without + // running afoul of the IR verifier or breaking the native link + switch (GV->getLinkage()) { + case GlobalValue::ExternalWeakLinkage: { + Constant *NullRef = Constant::getNullValue(GV->getType()); + GV->replaceAllUsesWith(NullRef); + GV->eraseFromParent(); + return true; + } + default: + // default with fall through to avoid compiler warning + return false; + } + return false; +} + +bool GlobalCleanup::runOnModule(Module &M) { + bool Modified = false; + + if (GlobalVariable *GV = M.getNamedGlobal("llvm.compiler.used")) { + GV->eraseFromParent(); + Modified = true; + } + if (GlobalVariable *GV = M.getNamedGlobal("llvm.used")) { + GV->eraseFromParent(); + Modified = true; + } + + for (Module::global_iterator I = M.global_begin(), E = M.global_end(); + I != E; ) { + GlobalVariable *GV = I++; + Modified |= CleanUpLinkage(GV); + } + return Modified; +} + +ModulePass *llvm::createGlobalCleanupPass() { + return new GlobalCleanup(); +} + +char ResolveAliases::ID = 0; +INITIALIZE_PASS(ResolveAliases, "resolve-aliases", + "resolve global variable and function aliases", false, false) + +bool ResolveAliases::runOnModule(Module &M) { + bool Modified = false; + + for (Module::alias_iterator I = M.alias_begin(), E = M.alias_end(); + I != E; ) { + GlobalAlias *Alias = I++; + Alias->replaceAllUsesWith(Alias->getAliasee()); + Alias->eraseFromParent(); + Modified = true; + } + return Modified; +} + +ModulePass *createResolveAliasesPass() { + return new ResolveAliases(); +} diff --git a/test/Transforms/NaCl/globalcleanup.ll b/test/Transforms/NaCl/globalcleanup.ll new file mode 100644 index 000000000000..3af5df003e93 --- /dev/null +++ b/test/Transforms/NaCl/globalcleanup.ll @@ -0,0 +1,34 @@ +; RUN: opt < %s -nacl-global-cleanup -S | FileCheck %s +; RUN: opt < %s -nacl-global-cleanup -S | FileCheck -check-prefix=GV %s + +@llvm.compiler.used = appending global [0 x i8*] zeroinitializer, section "llvm.metadata" +@llvm.used = appending global [0 x i8*] zeroinitializer, section "llvm.metadata" + +; GV-NOT: llvm.used +; GV-NOT: llvm.compiler.used + +@extern_weak_const = extern_weak constant i32 +@extern_weak_gv = extern_weak global i32 + +; GV-NOT: @extern_weak_const +; GV-NOT: @extern_weak_gv + +; CHECK define void @_start +define void @_start() { + ret void +} + +define i32* @ewgv() { +; CHECK: %bc = getelementptr i8* null, i32 0 + %bc = getelementptr i8* bitcast (i32* @extern_weak_gv to i8*), i32 0 +; CHECK: ret i32* null + ret i32* @extern_weak_gv +} + +define i32* @ewc() { +; CHECK: %bc = getelementptr i8* null, i32 0 + %bc = getelementptr i8* bitcast (i32* @extern_weak_const to i8*), i32 0 +; CHECK: ret i32* null + ret i32* @extern_weak_gv +} + diff --git a/test/Transforms/NaCl/resolve-aliases.ll b/test/Transforms/NaCl/resolve-aliases.ll new file mode 100644 index 000000000000..82ad54d74e95 --- /dev/null +++ b/test/Transforms/NaCl/resolve-aliases.ll @@ -0,0 +1,36 @@ +; RUN: opt < %s -resolve-aliases -S | FileCheck %s + +; CHECK-NOT: @alias + +@r1 = internal global i32 zeroinitializer +@a1 = alias i32* @r1 +define i32* @usea1() { +; CHECK: ret i32* @r1 + ret i32* @a1 +} + +@funcalias = alias i32* ()* @usea1 +; CHECK: @usefuncalias +define void @usefuncalias() { +; CHECK: call i32* @usea1 + %1 = call i32* @funcalias() + ret void +} + +@bc1 = global i8* bitcast (i32* @r1 to i8*) +@bcalias = alias i8* bitcast (i32* @r1 to i8*) + +; CHECK: @usebcalias +define i8* @usebcalias() { +; CHECK: ret i8* bitcast (i32* @r1 to i8*) + ret i8* @bcalias +} + + +@fa2 = alias i32* ()* @funcalias +; CHECK: @usefa2 +define void @usefa2() { +; CHECK: call i32* @usea1 + call i32* @fa2() + ret void +} diff --git a/tools/opt/opt.cpp b/tools/opt/opt.cpp index d60253acb8ca..ebc5fd6ca655 100644 --- a/tools/opt/opt.cpp +++ b/tools/opt/opt.cpp @@ -421,11 +421,15 @@ int main(int argc, char **argv) { initializeInstCombine(Registry); initializeInstrumentation(Registry); initializeTarget(Registry); + // @LOCALMOD-BEGIN initializeExpandConstantExprPass(Registry); initializeExpandCtorsPass(Registry); initializeExpandTlsPass(Registry); initializeExpandTlsConstantExprPass(Registry); initializeExpandVarArgsPass(Registry); + initializeGlobalCleanupPass(Registry); + initializeResolveAliasesPass(Registry); + // @LOCALMOD-END cl::ParseCommandLineOptions(argc, argv, "llvm .bc -> .bc modular optimizer and analysis printer\n"); From 70c26492f77503842bfb5691c51b8afde97f9c2f Mon Sep 17 00:00:00 2001 From: Mark Seaborn Date: Tue, 26 Mar 2013 13:49:56 -0700 Subject: [PATCH 023/295] PNaCl: Add ExpandGetElementPtr pass for converting GetElementPtr to arithmetic This is similar to the GEP handling in visitGetElementPtr() in lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp. Once this pass is enabled, it will simplify the language to reduce the set of constructs that a PNaCl translator needs to handle as part of a stable wire format for PNaCl. BUG=https://code.google.com/p/nativeclient/issues/detail?id=3343 TEST=test/Transforms/NaCl/expand-getelementptr.ll Review URL: https://codereview.chromium.org/12849009 (cherry picked from commit 9c7984ea3134c4f7f425bb2e01a5ee8540829fd9) --- include/llvm/InitializePasses.h | 1 + include/llvm/Transforms/NaCl.h | 2 + lib/Transforms/NaCl/CMakeLists.txt | 1 + lib/Transforms/NaCl/ExpandGetElementPtr.cpp | 146 +++++++++++++++++++ test/Transforms/NaCl/expand-getelementptr.ll | 109 ++++++++++++++ tools/opt/opt.cpp | 1 + 6 files changed, 260 insertions(+) create mode 100644 lib/Transforms/NaCl/ExpandGetElementPtr.cpp create mode 100644 test/Transforms/NaCl/expand-getelementptr.ll diff --git a/include/llvm/InitializePasses.h b/include/llvm/InitializePasses.h index d971caa4b896..9db5ecee1817 100644 --- a/include/llvm/InitializePasses.h +++ b/include/llvm/InitializePasses.h @@ -271,6 +271,7 @@ void initializeStackMapLivenessPass(PassRegistry&); // @LOCALMOD-BEGIN void initializeExpandConstantExprPass(PassRegistry&); void initializeExpandCtorsPass(PassRegistry&); +void initializeExpandGetElementPtrPass(PassRegistry&); void initializeExpandTlsConstantExprPass(PassRegistry&); void initializeExpandTlsPass(PassRegistry&); void initializeExpandVarArgsPass(PassRegistry&); diff --git a/include/llvm/Transforms/NaCl.h b/include/llvm/Transforms/NaCl.h index b5108fbf3271..6782f6581f50 100644 --- a/include/llvm/Transforms/NaCl.h +++ b/include/llvm/Transforms/NaCl.h @@ -12,11 +12,13 @@ namespace llvm { +class BasicBlockPass; class FunctionPass; class ModulePass; FunctionPass *createExpandConstantExprPass(); ModulePass *createExpandCtorsPass(); +BasicBlockPass *createExpandGetElementPtrPass(); ModulePass *createExpandTlsPass(); ModulePass *createExpandTlsConstantExprPass(); ModulePass *createExpandVarArgsPass(); diff --git a/lib/Transforms/NaCl/CMakeLists.txt b/lib/Transforms/NaCl/CMakeLists.txt index 5b1de88dc489..4e33abc1a74a 100644 --- a/lib/Transforms/NaCl/CMakeLists.txt +++ b/lib/Transforms/NaCl/CMakeLists.txt @@ -1,6 +1,7 @@ add_llvm_library(LLVMTransformsNaCl ExpandConstantExpr.cpp ExpandCtors.cpp + ExpandGetElementPtr.cpp ExpandTls.cpp ExpandTlsConstantExpr.cpp ExpandVarArgs.cpp diff --git a/lib/Transforms/NaCl/ExpandGetElementPtr.cpp b/lib/Transforms/NaCl/ExpandGetElementPtr.cpp new file mode 100644 index 000000000000..974c3193de92 --- /dev/null +++ b/lib/Transforms/NaCl/ExpandGetElementPtr.cpp @@ -0,0 +1,146 @@ +//===- ExpandGetElementPtr.cpp - Expand GetElementPtr into arithmetic------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This pass expands out GetElementPtr instructions into ptrtoint, +// inttoptr and arithmetic instructions. +// +// This simplifies the language so that the PNaCl translator does not +// need to handle GetElementPtr and struct types as part of a stable +// wire format for PNaCl. +// +// Note that we drop the "inbounds" attribute of GetElementPtr. +// +//===----------------------------------------------------------------------===// + +#include "llvm/IR/BasicBlock.h" +#include "llvm/IR/DataLayout.h" +#include "llvm/IR/Function.h" +#include "llvm/IR/InstrTypes.h" +#include "llvm/IR/Instructions.h" +#include "llvm/IR/Module.h" +#include "llvm/IR/Type.h" +#include "llvm/Pass.h" +#include "llvm/Transforms/NaCl.h" + +using namespace llvm; + +namespace { + class ExpandGetElementPtr : public BasicBlockPass { + public: + static char ID; // Pass identification, replacement for typeid + ExpandGetElementPtr() : BasicBlockPass(ID) { + initializeExpandGetElementPtrPass(*PassRegistry::getPassRegistry()); + } + + virtual bool runOnBasicBlock(BasicBlock &BB); + }; +} + +char ExpandGetElementPtr::ID = 0; +INITIALIZE_PASS(ExpandGetElementPtr, "expand-getelementptr", + "Expand out GetElementPtr instructions into arithmetic", + false, false) + +static Value *CastToPtrSize(Value *Val, Instruction *InsertPt, + const DebugLoc &Debug, Type *PtrType) { + unsigned ValSize = Val->getType()->getIntegerBitWidth(); + unsigned PtrSize = PtrType->getIntegerBitWidth(); + if (ValSize == PtrSize) + return Val; + Instruction *Inst; + if (ValSize > PtrSize) { + Inst = new TruncInst(Val, PtrType, "gep_trunc", InsertPt); + } else { + // GEP indexes must be sign-extended. + Inst = new SExtInst(Val, PtrType, "gep_sext", InsertPt); + } + Inst->setDebugLoc(Debug); + return Inst; +} + +static void FlushOffset(Instruction **Ptr, uint64_t *CurrentOffset, + Instruction *InsertPt, const DebugLoc &Debug, + Type *PtrType) { + if (*CurrentOffset) { + *Ptr = BinaryOperator::Create(Instruction::Add, *Ptr, + ConstantInt::get(PtrType, *CurrentOffset), + "gep", InsertPt); + (*Ptr)->setDebugLoc(Debug); + *CurrentOffset = 0; + } +} + +static void ExpandGEP(GetElementPtrInst *GEP, DataLayout *DL, Type *PtrType) { + const DebugLoc &Debug = GEP->getDebugLoc(); + Instruction *Ptr = new PtrToIntInst(GEP->getPointerOperand(), PtrType, + "gep_int", GEP); + Ptr->setDebugLoc(Debug); + + Type *CurrentTy = GEP->getPointerOperand()->getType(); + // We do some limited constant folding ourselves. An alternative + // would be to generate verbose, unfolded output (e.g. multiple + // adds; adds of zero constants) and use a later pass such as + // "-instcombine" to clean that up. However, "-instcombine" can + // reintroduce GetElementPtr instructions. + uint64_t CurrentOffset = 0; + + for (GetElementPtrInst::op_iterator Op = GEP->op_begin() + 1; + Op != GEP->op_end(); + ++Op) { + Value *Index = *Op; + if (StructType *StTy = dyn_cast(CurrentTy)) { + uint64_t Field = cast(Op)->getZExtValue(); + CurrentTy = StTy->getElementType(Field); + CurrentOffset += DL->getStructLayout(StTy)->getElementOffset(Field); + } else { + CurrentTy = cast(CurrentTy)->getElementType(); + uint64_t ElementSize = DL->getTypeAllocSize(CurrentTy); + if (ConstantInt *C = dyn_cast(Index)) { + CurrentOffset += C->getSExtValue() * ElementSize; + } else { + FlushOffset(&Ptr, &CurrentOffset, GEP, Debug, PtrType); + Index = CastToPtrSize(Index, GEP, Debug, PtrType); + Instruction *Mul = BinaryOperator::Create( + Instruction::Mul, Index, ConstantInt::get(PtrType, ElementSize), + "gep_array", GEP); + Mul->setDebugLoc(Debug); + Ptr = BinaryOperator::Create(Instruction::Add, Ptr, Mul, "gep", GEP); + Ptr->setDebugLoc(Debug); + } + } + } + FlushOffset(&Ptr, &CurrentOffset, GEP, Debug, PtrType); + + assert(CurrentTy == GEP->getType()->getElementType()); + Instruction *Result = new IntToPtrInst(Ptr, GEP->getType(), "", GEP); + Result->setDebugLoc(Debug); + Result->takeName(GEP); + GEP->replaceAllUsesWith(Result); + GEP->eraseFromParent(); +} + +bool ExpandGetElementPtr::runOnBasicBlock(BasicBlock &BB) { + bool Modified = false; + DataLayout DL(BB.getParent()->getParent()); + Type *PtrType = DL.getIntPtrType(BB.getContext()); + + for (BasicBlock::InstListType::iterator Iter = BB.begin(); + Iter != BB.end(); ) { + Instruction *Inst = Iter++; + if (GetElementPtrInst *GEP = dyn_cast(Inst)) { + Modified = true; + ExpandGEP(GEP, &DL, PtrType); + } + } + return Modified; +} + +BasicBlockPass *llvm::createExpandGetElementPtrPass() { + return new ExpandGetElementPtr(); +} diff --git a/test/Transforms/NaCl/expand-getelementptr.ll b/test/Transforms/NaCl/expand-getelementptr.ll new file mode 100644 index 000000000000..2a089f2dc458 --- /dev/null +++ b/test/Transforms/NaCl/expand-getelementptr.ll @@ -0,0 +1,109 @@ +; RUN: opt < %s -expand-getelementptr -S | FileCheck %s + +target datalayout = "p:32:32:32" + +%MyStruct = type { i8, i32, i8 } +%MyArray = type { [100 x i64] } + + +; Test indexing struct field +define i8* @test_struct_field(%MyStruct* %ptr) { + %addr = getelementptr %MyStruct* %ptr, i32 0, i32 2 + ret i8* %addr +} +; CHECK: @test_struct_field +; CHECK-NEXT: %gep_int = ptrtoint %MyStruct* %ptr to i32 +; CHECK-NEXT: %gep = add i32 %gep_int, 8 +; CHECK-NEXT: %addr = inttoptr i32 %gep to i8* +; CHECK-NEXT: ret i8* %addr + + +; Test non-constant index into an array +define i64* @test_array_index(%MyArray* %ptr, i32 %index) { + %addr = getelementptr %MyArray* %ptr, i32 0, i32 0, i32 %index + ret i64* %addr +} +; CHECK: @test_array_index +; CHECK-NEXT: %gep_int = ptrtoint %MyArray* %ptr to i32 +; CHECK-NEXT: %gep_array = mul i32 %index, 8 +; CHECK-NEXT: %gep = add i32 %gep_int, %gep_array +; CHECK-NEXT: %addr = inttoptr i32 %gep to i64* +; CHECK-NEXT: ret i64* %addr + + +; Test constant index into an array (as a pointer) +define %MyStruct* @test_ptr_add(%MyStruct* %ptr) { + %addr = getelementptr %MyStruct* %ptr, i32 2 + ret %MyStruct* %addr +} +; CHECK: @test_ptr_add +; CHECK-NEXT: %gep_int = ptrtoint %MyStruct* %ptr to i32 +; CHECK-NEXT: %gep = add i32 %gep_int, 24 +; CHECK-NEXT: %addr = inttoptr i32 %gep to %MyStruct* +; CHECK-NEXT: ret %MyStruct* %addr + + +; Test that additions and multiplications are combined properly +define i64* @test_add_and_index(%MyArray* %ptr, i32 %index) { + %addr = getelementptr %MyArray* %ptr, i32 1, i32 0, i32 %index + ret i64* %addr +} +; CHECK: @test_add_and_index +; CHECK-NEXT: %gep_int = ptrtoint %MyArray* %ptr to i32 +; CHECK-NEXT: %gep = add i32 %gep_int, 800 +; CHECK-NEXT: %gep_array = mul i32 %index, 8 +; CHECK-NEXT: %gep1 = add i32 %gep, %gep_array +; CHECK-NEXT: %addr = inttoptr i32 %gep1 to i64* +; CHECK-NEXT: ret i64* %addr + + +; Test >32-bit array index +define i64* @test_array_index64(%MyArray* %ptr, i64 %index) { + %addr = getelementptr %MyArray* %ptr, i32 0, i32 0, i64 %index + ret i64* %addr +} +; CHECK: @test_array_index64 +; CHECK-NEXT: %gep_int = ptrtoint %MyArray* %ptr to i32 +; CHECK-NEXT: %gep_trunc = trunc i64 %index to i32 +; CHECK-NEXT: %gep_array = mul i32 %gep_trunc, 8 +; CHECK-NEXT: %gep = add i32 %gep_int, %gep_array +; CHECK-NEXT: %addr = inttoptr i32 %gep to i64* +; CHECK-NEXT: ret i64* %addr + + +; Test <32-bit array index +define i64* @test_array_index16(%MyArray* %ptr, i16 %index) { + %addr = getelementptr %MyArray* %ptr, i32 0, i32 0, i16 %index + ret i64* %addr +} +; CHECK: @test_array_index16 +; CHECK-NEXT: %gep_int = ptrtoint %MyArray* %ptr to i32 +; CHECK-NEXT: %gep_sext = sext i16 %index to i32 +; CHECK-NEXT: %gep_array = mul i32 %gep_sext, 8 +; CHECK-NEXT: %gep = add i32 %gep_int, %gep_array +; CHECK-NEXT: %addr = inttoptr i32 %gep to i64* +; CHECK-NEXT: ret i64* %addr + + +; Test >32-bit constant array index +define i64* @test_array_index64_const(%MyArray* %ptr) { + %addr = getelementptr %MyArray* %ptr, i32 0, i32 0, i64 100 + ret i64* %addr +} +; CHECK: @test_array_index64_const +; CHECK-NEXT: %gep_int = ptrtoint %MyArray* %ptr to i32 +; CHECK-NEXT: %gep = add i32 %gep_int, 800 +; CHECK-NEXT: %addr = inttoptr i32 %gep to i64* +; CHECK-NEXT: ret i64* %addr + + +; Test <32-bit constant array index -- test sign extension +define i64* @test_array_index16_const(%MyArray* %ptr) { + %addr = getelementptr %MyArray* %ptr, i32 0, i32 0, i16 -100 + ret i64* %addr +} +; CHECK: @test_array_index16_const +; CHECK-NEXT: %gep_int = ptrtoint %MyArray* %ptr to i32 +; CHECK-NEXT: %gep = add i32 %gep_int, -800 +; CHECK-NEXT: %addr = inttoptr i32 %gep to i64* +; CHECK-NEXT: ret i64* %addr diff --git a/tools/opt/opt.cpp b/tools/opt/opt.cpp index ebc5fd6ca655..e2c8e2da22a5 100644 --- a/tools/opt/opt.cpp +++ b/tools/opt/opt.cpp @@ -424,6 +424,7 @@ int main(int argc, char **argv) { // @LOCALMOD-BEGIN initializeExpandConstantExprPass(Registry); initializeExpandCtorsPass(Registry); + initializeExpandGetElementPtrPass(Registry); initializeExpandTlsPass(Registry); initializeExpandTlsConstantExprPass(Registry); initializeExpandVarArgsPass(Registry); From 033197011ba804e37e7cf0271d0dfc120311e14c Mon Sep 17 00:00:00 2001 From: Mark Seaborn Date: Wed, 27 Mar 2013 13:28:15 -0700 Subject: [PATCH 024/295] PNaCl: Fix ExpandVarArgs to handle "byval" struct arguments properly * Ensure that the "byval" attribute is preserved for the fixed arguments. Before, it was stripped off from function calls but left in for function definitions, which broke passing struct arguments (which PNaCl Clang generates as "byval"). * Ensure that function and return attributes are preserved too. * Handle "byval" variable arguments in calls too by dereferencing the pointer. These are not yet usable with PNaCl Clang, which does not allow va_arg on structs (see https://code.google.com/p/nativeclient/issues/detail?id=2381), but we might as well make this pass ready to handle this. BUG=https://code.google.com/p/nativeclient/issues/detail?id=3338 TEST=test/Transforms/NaCl/expand-varargs*.ll Review URL: https://codereview.chromium.org/13100002 (cherry picked from commit 66afec2f1d3ebc1909f4acfea836cc4e0332dbce) --- lib/Transforms/NaCl/ExpandVarArgs.cpp | 32 ++++++--- test/Transforms/NaCl/expand-varargs-attrs.ll | 72 +++++++++++++++++++ test/Transforms/NaCl/expand-varargs-struct.ll | 18 +++++ 3 files changed, 114 insertions(+), 8 deletions(-) create mode 100644 test/Transforms/NaCl/expand-varargs-attrs.ll create mode 100644 test/Transforms/NaCl/expand-varargs-struct.ll diff --git a/lib/Transforms/NaCl/ExpandVarArgs.cpp b/lib/Transforms/NaCl/ExpandVarArgs.cpp index e1bd8f93a887..6998001754c7 100644 --- a/lib/Transforms/NaCl/ExpandVarArgs.cpp +++ b/lib/Transforms/NaCl/ExpandVarArgs.cpp @@ -168,13 +168,13 @@ static void LifetimeDecl(Intrinsic::ID id, Value *Ptr, Value *Size, // CopyCall() uses argument overloading so that it can be used by the // template ExpandVarArgCall(). -static Instruction *CopyCall(CallInst *Original, Value *Callee, - ArrayRef Args) { +static CallInst *CopyCall(CallInst *Original, Value *Callee, + ArrayRef Args) { return CallInst::Create(Callee, Args, "", Original); } -static Instruction *CopyCall(InvokeInst *Original, Value *Callee, - ArrayRef Args) { +static InvokeInst *CopyCall(InvokeInst *Original, Value *Callee, + ArrayRef Args) { return InvokeInst::Create(Callee, Original->getNormalDest(), Original->getUnwindDest(), Args, "", Original); } @@ -190,16 +190,30 @@ static bool ExpandVarArgCall(InstType *Call, DataLayout *DL) { LLVMContext *Context = &Call->getContext(); + SmallVector Attrs; + Attrs.push_back(Call->getAttributes().getFnAttributes()); + Attrs.push_back(Call->getAttributes().getRetAttributes()); + // Split argument list into fixed and variable arguments. SmallVector FixedArgs; SmallVector VarArgs; SmallVector VarArgsTypes; - for (unsigned I = 0; I < FuncType->getNumParams(); ++I) + for (unsigned I = 0; I < FuncType->getNumParams(); ++I) { FixedArgs.push_back(Call->getArgOperand(I)); + // AttributeSets use 1-based indexing. + Attrs.push_back(Call->getAttributes().getParamAttributes(I + 1)); + } for (unsigned I = FuncType->getNumParams(); I < Call->getNumArgOperands(); ++I) { - VarArgs.push_back(Call->getArgOperand(I)); - VarArgsTypes.push_back(Call->getArgOperand(I)->getType()); + Value *ArgVal = Call->getArgOperand(I); + if (Call->getAttributes().hasAttribute(I + 1, Attribute::ByVal)) { + // For "byval" arguments we must dereference the pointer and + // make a copy of the struct being passed by value. + ArgVal = CopyDebug(new LoadInst(ArgVal, "vararg_struct_copy", Call), + Call); + } + VarArgs.push_back(ArgVal); + VarArgsTypes.push_back(ArgVal->getType()); } StructType *VarArgsTy; @@ -262,7 +276,9 @@ static bool ExpandVarArgCall(InstType *Call, DataLayout *DL) { // Create the converted function call. FixedArgs.push_back(ArgToAdd); - Value *NewCall = CopyDebug(CopyCall(Call, CastFunc, FixedArgs), Call); + InstType *NewCall = CopyCall(Call, CastFunc, FixedArgs); + CopyDebug(NewCall, Call); + NewCall->setAttributes(AttributeSet::get(Call->getContext(), Attrs)); NewCall->takeName(Call); if (BufPtr) { diff --git a/test/Transforms/NaCl/expand-varargs-attrs.ll b/test/Transforms/NaCl/expand-varargs-attrs.ll new file mode 100644 index 000000000000..f53a8106f014 --- /dev/null +++ b/test/Transforms/NaCl/expand-varargs-attrs.ll @@ -0,0 +1,72 @@ +; RUN: opt < %s -expand-varargs -S | FileCheck %s + +declare i32 @varargs_func(i32 %arg, ...) + + +; Check that attributes such as "byval" are preserved on fixed arguments. + +%MyStruct = type { i64, i64 } + +define void @func_with_arg_attrs(%MyStruct* byval, ...) { + ret void +} +; CHECK: define void @func_with_arg_attrs(%MyStruct* byval, i8* %varargs) { + + +declare void @take_struct_arg(%MyStruct* byval %s, ...) + +define void @call_with_arg_attrs(%MyStruct* %s) { + call void (%MyStruct*, ...)* @take_struct_arg(%MyStruct* byval %s) + ret void +} +; CHECK: define void @call_with_arg_attrs(%MyStruct* %s) { +; CHECK: call void %vararg_func(%MyStruct* byval %s, {}* undef) + + +; The "byval" attribute here should be dropped. +define i32 @pass_struct_via_vararg1(%MyStruct* %s) { + %result = call i32 (i32, ...)* @varargs_func(i32 111, %MyStruct* byval %s) + ret i32 %result +} +; CHECK: define i32 @pass_struct_via_vararg1(%MyStruct* %s) { +; CHECK: %result = call i32 %vararg_func(i32 111, %{{.*}}* %vararg_buffer) + + +; The "byval" attribute here should be dropped. +define i32 @pass_struct_via_vararg2(%MyStruct* %s) { + %result = call i32 (i32, ...)* @varargs_func(i32 111, i32 2, %MyStruct* byval %s) + ret i32 %result +} +; CHECK: define i32 @pass_struct_via_vararg2(%MyStruct* %s) { +; CHECK: %result = call i32 %vararg_func(i32 111, %{{.*}}* %vararg_buffer) + + +; Check that return attributes such as "signext" are preserved. +define i32 @call_with_return_attr() { + %result = call signext i32 (i32, ...)* @varargs_func(i32 111, i64 222) + ret i32 %result +} +; CHECK: define i32 @call_with_return_attr() { +; CHECK: %result = call signext i32 %vararg_func(i32 111 + + +; Check that the "readonly" function attribute is preserved. +define i32 @call_readonly() { + %result = call i32 (i32, ...)* @varargs_func(i32 111, i64 222) readonly + ret i32 %result +} +; CHECK: define i32 @call_readonly() { +; CHECK: %result = call i32 %vararg_func(i32 111, {{.*}}) #1 + + +; Check that the "tail" attribute gets removed, because the callee +; reads space alloca'd by the caller. +define i32 @tail_call() { + %result = tail call i32 (i32, ...)* @varargs_func(i32 111, i64 222) + ret i32 %result +} +; CHECK: define i32 @tail_call() { +; CHECK: %result = call i32 %vararg_func(i32 111 + + +; CHECK: attributes #1 = { readonly } diff --git a/test/Transforms/NaCl/expand-varargs-struct.ll b/test/Transforms/NaCl/expand-varargs-struct.ll new file mode 100644 index 000000000000..f147d7006fe5 --- /dev/null +++ b/test/Transforms/NaCl/expand-varargs-struct.ll @@ -0,0 +1,18 @@ +; RUN: opt < %s -expand-varargs -S | FileCheck %s + +declare i32 @varargs_func(i32 %arg, ...) + + +%MyStruct = type { i64, i64 } + +; CHECK: %vararg_call = type <{ i64, %MyStruct }> + +; Test passing a struct by value. +define i32 @varargs_call_struct(%MyStruct* %ptr) { + %result = call i32 (i32, ...)* @varargs_func(i32 111, i64 222, %MyStruct* byval %ptr) + ret i32 %result +} +; CHECK: define i32 @varargs_call_struct(%MyStruct* %ptr) { +; CHECK: %vararg_struct_copy = load %MyStruct* %ptr +; CHECK: %vararg_ptr1 = getelementptr %vararg_call* %vararg_buffer, i32 0, i32 1 +; CHECK: store %MyStruct %vararg_struct_copy, %MyStruct* %vararg_ptr1 From cb879b0043b837294ef7ffa120ee1f653b438b25 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Wed, 27 Mar 2013 16:59:55 -0700 Subject: [PATCH 025/295] Make pnacl-abicheck return nonzero status if errors are found. Also add -q flag for quiet operation; i.e. don't print the errors, only use the exit status to indicate failure. R=mseaborn@chromium.org,jvoung@chromium.org BUG= https://code.google.com/p/nativeclient/issues/detail?id=2309 Review URL: https://codereview.chromium.org/13117004 (cherry picked from commit aa101e39d60f96e435fbc619558d541c93431ef6) --- tools/pnacl-abicheck/pnacl-abicheck.cpp | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/tools/pnacl-abicheck/pnacl-abicheck.cpp b/tools/pnacl-abicheck/pnacl-abicheck.cpp index 496a73c2fd40..2fefd46c8bdd 100644 --- a/tools/pnacl-abicheck/pnacl-abicheck.cpp +++ b/tools/pnacl-abicheck/pnacl-abicheck.cpp @@ -26,13 +26,22 @@ using namespace llvm; static cl::opt InputFilename(cl::Positional, cl::desc(""), cl::init("-")); -static void CheckABIVerifyErrors(PNaClABIErrorReporter &Reporter, +static cl::opt +Quiet("q", cl::desc("Do not print error messages")); + +// Print any errors collected by the error reporter. Return true if +// there were any. +static bool CheckABIVerifyErrors(PNaClABIErrorReporter &Reporter, const Twine &Name) { - if (Reporter.getErrorCount() > 0) { - outs() << "ERROR: " << Name << " is not valid PNaCl bitcode:\n"; - Reporter.printErrors(outs()); + bool HasErrors = Reporter.getErrorCount() > 0; + if (HasErrors) { + if (!Quiet) { + outs() << "ERROR: " << Name << " is not valid PNaCl bitcode:\n"; + Reporter.printErrors(outs()); + } } Reporter.reset(); + return HasErrors; } int main(int argc, char **argv) { @@ -46,17 +55,19 @@ int main(int argc, char **argv) { return 1; } PNaClABIErrorReporter ABIErrorReporter; + bool ErrorsFound = false; // Manually run the passes so we can tell the user which function had the // error. No need for a pass manager since it's just one pass. OwningPtr ModuleChecker(createPNaClABIVerifyModulePass(&ABIErrorReporter)); ModuleChecker->runOnModule(*Mod); - CheckABIVerifyErrors(ABIErrorReporter, "Module"); + ErrorsFound |= CheckABIVerifyErrors(ABIErrorReporter, "Module"); OwningPtr FunctionChecker( createPNaClABIVerifyFunctionsPass(&ABIErrorReporter)); for (Module::iterator MI = Mod->begin(), ME = Mod->end(); MI != ME; ++MI) { FunctionChecker->runOnFunction(*MI); - CheckABIVerifyErrors(ABIErrorReporter, "Function " + MI->getName()); + ErrorsFound |= CheckABIVerifyErrors(ABIErrorReporter, + "Function " + MI->getName()); } - return 0; + return ErrorsFound; } From b6534274af9143a2ade9bed44e26974951b357fc Mon Sep 17 00:00:00 2001 From: Jan Voung Date: Thu, 28 Mar 2013 16:56:41 -0700 Subject: [PATCH 026/295] Add a pass to strip bitcode metadata. This only works on instruction attachments for now. Since it is a ModulePass we can add something to strip NamedMetadata based on a whitelist, if we want to retain some of that. It does not touch debug metadata, and leaves -strip-debug to handle that. BUG= https://code.google.com/p/nativeclient/issues/detail?id=3348 Review URL: https://codereview.chromium.org/12844027 (cherry picked from commit 946417b9fe7da9334c76182f28020ff4f46e11f8) --- include/llvm/InitializePasses.h | 1 + include/llvm/Transforms/NaCl.h | 1 + lib/Transforms/NaCl/CMakeLists.txt | 1 + lib/Transforms/NaCl/StripMetadata.cpp | 77 +++++++++++++++++++ .../NaCl/strip-branchweight-metadata.ll | 29 +++++++ .../NaCl/strip-meta-leaves-debug.ll | 34 ++++++++ test/Transforms/NaCl/strip-tbaa-metadata.ll | 36 +++++++++ tools/opt/opt.cpp | 1 + 8 files changed, 180 insertions(+) create mode 100644 lib/Transforms/NaCl/StripMetadata.cpp create mode 100644 test/Transforms/NaCl/strip-branchweight-metadata.ll create mode 100644 test/Transforms/NaCl/strip-meta-leaves-debug.ll create mode 100644 test/Transforms/NaCl/strip-tbaa-metadata.ll diff --git a/include/llvm/InitializePasses.h b/include/llvm/InitializePasses.h index 9db5ecee1817..b41fbd5ee638 100644 --- a/include/llvm/InitializePasses.h +++ b/include/llvm/InitializePasses.h @@ -280,6 +280,7 @@ void initializeNaClCcRewritePass(PassRegistry&); void initializePNaClABIVerifyModulePass(PassRegistry&); void initializePNaClABIVerifyFunctionsPass(PassRegistry&); void initializeResolveAliasesPass(PassRegistry&); +void initializeStripMetadataPass(PassRegistry&); // @LOCALMOD-END } diff --git a/include/llvm/Transforms/NaCl.h b/include/llvm/Transforms/NaCl.h index 6782f6581f50..58166bbf2274 100644 --- a/include/llvm/Transforms/NaCl.h +++ b/include/llvm/Transforms/NaCl.h @@ -24,6 +24,7 @@ ModulePass *createExpandTlsConstantExprPass(); ModulePass *createExpandVarArgsPass(); ModulePass *createGlobalCleanupPass(); ModulePass *createResolveAliasesPass(); +ModulePass *createStripMetadataPass(); } diff --git a/lib/Transforms/NaCl/CMakeLists.txt b/lib/Transforms/NaCl/CMakeLists.txt index 4e33abc1a74a..1ca8e3d59ef6 100644 --- a/lib/Transforms/NaCl/CMakeLists.txt +++ b/lib/Transforms/NaCl/CMakeLists.txt @@ -6,6 +6,7 @@ add_llvm_library(LLVMTransformsNaCl ExpandTlsConstantExpr.cpp ExpandVarArgs.cpp GlobalCleanup.cpp + StripMetadata.cpp ) add_dependencies(LLVMTransformsNaCl intrinsics_gen) diff --git a/lib/Transforms/NaCl/StripMetadata.cpp b/lib/Transforms/NaCl/StripMetadata.cpp new file mode 100644 index 000000000000..a7d2e7eb68e7 --- /dev/null +++ b/lib/Transforms/NaCl/StripMetadata.cpp @@ -0,0 +1,77 @@ +//===- StripMetadata.cpp - Strip non-stable non-debug metadata ------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// The StripMetadata transformation strips instruction attachment +// metadata, such as !tbaa and !prof metadata. +// TODO: Strip NamedMetadata too. +// +// It does not strip debug metadata. Debug metadata is used by debug +// intrinsic functions and calls to those intrinsic functions. Use the +// -strip-debug or -strip pass to strip that instead. +// +// The goal of this pass is to reduce bitcode ABI surface area. +// We don't know yet which kind of metadata is considered stable. +//===----------------------------------------------------------------------===// + +#include "llvm/IR/Instructions.h" +#include "llvm/IR/Module.h" +#include "llvm/Pass.h" +#include "llvm/Transforms/NaCl.h" + +using namespace llvm; + +namespace { + class StripMetadata : public ModulePass { + public: + static char ID; + explicit StripMetadata() : ModulePass(ID) { + initializeStripMetadataPass(*PassRegistry::getPassRegistry()); + } + + virtual bool runOnModule(Module &M); + + virtual void getAnalysisUsage(AnalysisUsage &AU) const { + AU.setPreservesCFG(); + } + }; +} + +char StripMetadata::ID = 0; +INITIALIZE_PASS(StripMetadata, "strip-metadata", + "Strip all non-stable non-debug metadata from a module.", + false, false) + +ModulePass *llvm::createStripMetadataPass() { + return new StripMetadata(); +} + +static bool DoStripMetadata(Module &M) { + bool Changed = false; + + for (Module::iterator MI = M.begin(), ME = M.end(); MI != ME; ++MI) { + for (Function::iterator FI = MI->begin(), FE = MI->end(); FI != FE; ++FI) { + for (BasicBlock::iterator BI = FI->begin(), BE = FI->end(); BI != BE; + ++BI) { + SmallVector, 8> InstMeta; + // Let the debug metadata be stripped by the -strip-debug pass. + BI->getAllMetadataOtherThanDebugLoc(InstMeta); + for (size_t i = 0; i < InstMeta.size(); ++i) { + BI->setMetadata(InstMeta[i].first, NULL); + Changed = true; + } + } + } + } + + return Changed; +} + +bool StripMetadata::runOnModule(Module &M) { + return DoStripMetadata(M); +} diff --git a/test/Transforms/NaCl/strip-branchweight-metadata.ll b/test/Transforms/NaCl/strip-branchweight-metadata.ll new file mode 100644 index 000000000000..61d3a6d5af47 --- /dev/null +++ b/test/Transforms/NaCl/strip-branchweight-metadata.ll @@ -0,0 +1,29 @@ +; RUN: opt -S -strip-metadata %s | FileCheck %s + +; Test that !prof metadata is removed from branches +; CHECK: @foo +; CHECK-NOT: !prof +define i32 @foo(i32 %c) { + switch i32 %c, label %3 [ + i32 5, label %4 + i32 0, label %1 + i32 4, label %2 + ], !prof !0 + +;