Enabled more aggressive compiler warnings. This caught one bug in COND_BR.h. Added some pragmas into the types and state structure related files to make sure that extra padding added to structures is reported as a fatal error. The rest of the changes are downcasts from 64-bit values to 32-bit values.

docs/GOALS.md

@@ -1,5 +1,9 @@
 # What problem Remill solves
 
+Remill is designed first and foremost for dynamic program analysis. The two motivating use cases are [taint tracking](https://en.wikipedia.org/wiki/Taint_checking), and [symbolic execution](https://en.wikipedia.org/wiki/Symbolic_execution). 
+
+
+
 Remill was designed with the following goals in mind.
 
 - It should be easy to add new instruction implementations. Instruction semantics are implemented using C++. Instruction implementations should be thoroughly tested.

remill/Arch/Runtime/Operators.h

@@ -478,13 +478,13 @@ auto Signed(T val) -> typename IntegerType<T>::ST {
 
 // Return the largest possible value assignable to `val`.
 template <typename T>
-ALWAYS_INLINE static T Maximize(T val) {
+ALWAYS_INLINE static T Maximize(T) {
   return std::numeric_limits<T>::max();
 }
 
 // Return the smallest possible value assignable to `val`.
 template <typename T>
-ALWAYS_INLINE static T Minimize(T val) {
+ALWAYS_INLINE static T Minimize(T) {
   return std::numeric_limits<T>::min();
 }
 

remill/Arch/Runtime/Types.h

@@ -8,6 +8,9 @@
 #include <limits>
 #include <type_traits>
 
+#pragma clang diagnostic push
+#pragma clang diagnostic fatal "-Wpadded"
+
 #include "remill/Arch/Runtime/Definitions.h"
 
 struct State;
@@ -625,4 +628,6 @@ inline int128_t operator "" _s128(unsigned long long value) {
 
 #define auto_t(T) typename BaseType<T>::BT
 
+#pragma clang diagnostic pop
+
 #endif  // REMILL_ARCH_RUNTIME_TYPES_H_

remill/Arch/X86/Arch.cpp

@@ -205,7 +205,7 @@ static void DecodeXED(xed_decoded_inst_t *xedd,
   auto bytes = reinterpret_cast<const uint8_t *>(instr_bytes.data());
   xed_decoded_inst_zero_set_mode(xedd, mode);
   xed_decoded_inst_set_input_chip(xedd, XED_CHIP_INVALID);
-  auto err = xed_decode(xedd, bytes, num_bytes);
+  auto err = xed_decode(xedd, bytes, static_cast<uint32_t>(num_bytes));
 
   CHECK(XED_ERROR_NONE == err)
       << "Unable to decode instruction at " << std::hex << address

remill/Arch/X86/Runtime/State.h

@@ -18,6 +18,9 @@
 //      to bitcode for one architecture, then change its `DataLayout` to
 //      match another architecture.
 
+#pragma clang diagnostic push
+#pragma clang diagnostic fatal "-Wpadded"
+
 #include "remill/Arch/Runtime/Runtime.h"
 
 #ifndef HAS_FEATURE_AVX
@@ -423,4 +426,6 @@ static_assert(3146 == __builtin_offsetof(State, interrupt_taken),
 
 static_assert(3200 == sizeof(State), "Invalid packing of `State`.");
 
+#pragma clang diagnostic pop
+
 #endif  // REMILL_ARCH_X86_RUNTIME_STATE_H_

remill/Arch/X86/Semantics/COND_BR.h

@@ -113,7 +113,7 @@ DEF_SEM(JB, R8W cond, PC taken_pc, PC not_taken_pc) {
 DEF_SEM(JLE, R8W cond, PC taken_pc, PC not_taken_pc) {
   auto take_branch = BOr(FLAG_ZF, BXor(FLAG_SF, FLAG_OF));
   Write(cond, take_branch);
-  Write(REG_PC, Select<addr_t>(take_branch, taken_pc, REG_PC));
+  Write(REG_PC, Select<addr_t>(take_branch, taken_pc, not_taken_pc));
 }
 
 } // namespace

remill/Arch/X86/Semantics/FLAGS.h

@@ -149,6 +149,7 @@ struct Overflow<tag_mul> {
   NEVER_INLINE static bool Flag(
       T lhs, T rhs, T res,
       typename std::enable_if<std::is_signed<T>::value,int>::type=0) {
+    (void) res;
     __remill_defer_inlining();
     auto lhs_wide = SExt(lhs);
     auto rhs_wide = SExt(rhs);

remill/BC/Translator.cpp

@@ -89,6 +89,7 @@ static void DisableInlining(llvm::Function *function) {
 //
 // TODO(pag): What about on Windows?
 static std::string CanonicalName(OSName os_name, const std::string &name) {
+  (void) os_name;
   return name;
 //  if (kOSmacOS == os_name && name.length() && '_' == name[0]) {
 //    return name.substr(1);
@@ -859,9 +860,6 @@ llvm::BasicBlock *Translator::LiftInstruction(llvm::Function *block_func,
   auto pc_ptr = ir.CreateLoad(FindVarInFunction(block, "PC"));
   auto next_pc_ptr = ir.CreateLoad(FindVarInFunction(block, "NEXT_PC"));
 
-  // Machine word-sized type. Think of this as `intptr_t`.
-  auto word_type = llvm::Type::getIntNTy(context, arch->address_size);
-
   // Update the next program counter.
   ir.CreateStore(
       ir.CreateAdd(
@@ -1030,12 +1028,11 @@ llvm::Value *Translator::LiftRegisterOperand(
 }
 
 // Lift an immediate operand.
-llvm::Value *Translator::LiftImmediateOperand(llvm::BasicBlock *block,
-                                              llvm::Type *arg_type,
+llvm::Value *Translator::LiftImmediateOperand(llvm::Type *arg_type,
                                               const Operand &arch_op) {
 
   if (arch_op.size > word_type->getBitWidth()) {
-    CHECK(arg_type->isIntegerTy(arch_op.size))
+    CHECK(arg_type->isIntegerTy(static_cast<uint32_t>(arch_op.size)))
         << "Argument to semantics function is not an integer. This may "
         << "not be surprising because the immediate operand is " <<
         arch_op.size << " bits, but the machine word size is "
@@ -1100,7 +1097,7 @@ llvm::Value *Translator::LiftAddressOperand(
   // used in 64-bit).
   if (arch_addr.address_size < word_size) {
     auto addr_type = llvm::Type::getIntNTy(
-        block->getContext(), arch_addr.address_size);
+        block->getContext(), static_cast<unsigned>(arch_addr.address_size));
 
     addr = ir.CreateZExt(
         ir.CreateTrunc(addr, addr_type),
@@ -1135,7 +1132,7 @@ llvm::Value *Translator::LiftOperand(llvm::BasicBlock *block,
       return LiftRegisterOperand(block, arg_type, arch_op.reg);
 
     case Operand::kTypeImmediate:
-      return LiftImmediateOperand(block, arg_type, arch_op);
+      return LiftImmediateOperand(arg_type, arch_op);
 
     case Operand::kTypeAddress:
       CHECK(arg_type == word_type)

remill/BC/Translator.h

@@ -98,8 +98,7 @@ class Translator {
                                    const Operand::Register &reg);
 
   // Lift an immediate operand.
-  llvm::Value *LiftImmediateOperand(llvm::BasicBlock *block,
-                                    llvm::Type *arg_type,
+  llvm::Value *LiftImmediateOperand(llvm::Type *arg_type,
                                     const Operand &op);
 
   // Lift an indirect memory operand to a value.

remill/CFG/CFG.cpp

@@ -22,4 +22,7 @@ const cfg::Module *ReadCFG(std::string cfg_file_name) {
 
 }  // namespace remill
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wshorten-64-to-32"
 #include "generated/CFG/CFG.pb.cc"
+#pragma clang diagnostic pop

remill/CFG/CFG.h

@@ -5,8 +5,10 @@
 
 #include <string>
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wshorten-64-to-32"
 #include "generated/CFG/CFG.pb.h"
-
+#pragma clang diagnostic pop
 class Module;
 
 namespace llvm {

scripts/buildsystem.py

@@ -112,10 +112,16 @@ except:
 CXX_FLAGS = [
   # Enable warnings.
   "-Wall",
+  "-Wextra",
+  "-Wshadow",
   "-Werror",
   "-pedantic",
 
+  "-Wshorten-64-to-32",
+
   # Disable specific warnings.
+  "-Wno-c++98-compat",
+  "-Wno-unreachable-code-return",
   "-Wno-nested-anon-types",
   "-Wno-extended-offsetof",
   "-Wno-gnu-anonymous-struct",

scripts/compile_semantics.sh

@@ -11,7 +11,7 @@ CXXFLAGS+=" -isystem ${DIR}/third_party/include -I${DIR}"
 CXXFLAGS+=" -std=gnu++11 -g0 -O0"
 CXXFLAGS+=" -fno-exceptions -fno-rtti -fno-asynchronous-unwind-tables"
 CXXFLAGS+=" -ffreestanding -fno-common -fno-builtin"
-CXXFLAGS+=" -Wall -Werror -Wconversion -pedantic"
+CXXFLAGS+=" -Wall -Werror -Wshadow -Wconversion -Wshorten-64-to-32 -pedantic"
 CXXFLAGS+=" -Wno-gnu-anonymous-struct -Wno-return-type-c-linkage"
 CXXFLAGS+=" -Wno-gnu-zero-variadic-macro-arguments -Wno-nested-anon-types"
 CXXFLAGS+=" -Wno-extended-offsetof -Wno-gnu-statement-expression"

tests/X86/Generate.cpp

@@ -83,7 +83,9 @@ static void AddFunctionToModule(remill::cfg::Module *module,
   while (addr < test.test_end) {
     auto bytes = reinterpret_cast<const uint8_t *>(addr);
     auto ilen = InstructionLength(
-        bytes, std::min<unsigned>(test::kMaxInstrLen, test.test_end - addr));
+        bytes, std::min<unsigned>(
+            test::kMaxInstrLen,
+            static_cast<unsigned>(test.test_end - addr)));
 
     auto instr = block->add_instructions();
     instr->set_bytes(bytes, ilen);

tests/X86/Run.cpp

@@ -410,7 +410,8 @@ static void RunWithFlags(const test::TestInfo *info,
   // swapping execution to operate on `gStack`.
   if (!sigsetjmp(gJmpBuf, true)) {
     gInNativeTest = false;
-    lifted_func(*lifted_state, nullptr, lifted_state->gpr.rip.qword);
+    lifted_func(*lifted_state, nullptr,
+                static_cast<addr_t>(lifted_state->gpr.rip.qword));
   } else {
     EXPECT_TRUE(native_test_faulted);
   }