From 3a68e5b1510acc4e682cdcd23625097cef591c1b Mon Sep 17 00:00:00 2001 From: Zach Tatlock Date: Tue, 10 May 2016 18:08:42 -0700 Subject: [PATCH 1/4] crude fixes to "potentially uninitialized variable" errors from clang --- herbgrind/runtime/hg_mathreplace.c | 2 +- herbgrind/runtime/hg_shadowop.c | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/herbgrind/runtime/hg_mathreplace.c b/herbgrind/runtime/hg_mathreplace.c index 72711ae..2b0f75c 100644 --- a/herbgrind/runtime/hg_mathreplace.c +++ b/herbgrind/runtime/hg_mathreplace.c @@ -151,7 +151,7 @@ void performOp(OpType op, double* result, double* args){ // Get the actual value from the pointer they gave us. mpfr_set_d(args_m[i], args[i], MPFR_RNDN); // Get the location of the arg source slot in the op structure. - Op_Info** src_loc_slot; + Op_Info** src_loc_slot = NULL; // Get the slot in the op info structure for the value source // structure cooresponding to this argument. switch(nargs){ diff --git a/herbgrind/runtime/hg_shadowop.c b/herbgrind/runtime/hg_shadowop.c index af21391..d9d88fd 100644 --- a/herbgrind/runtime/hg_shadowop.c +++ b/herbgrind/runtime/hg_shadowop.c @@ -38,6 +38,8 @@ #include "../types/hg_ast.h" #include "hg_runtime.h" +#include "../include/pub_tool_libcassert.h" + // Execute a shadow operation, storing the result of the high // precision operation applied to the shadow value at the arg_tmp // offset, into the shadow value at the dest_tmp offset. Depending on @@ -827,6 +829,9 @@ VG_REGPARM(1) void executeBinaryShadowOp(Op_Info* opInfo){ opInfo->args.bargs.arg1_value, &(opInfo->args.bargs.arg1_src)); argType = Lt_Floatx4; + } else { + /* impossible? */ + tl_assert(0); } // Now we'll allocate memory for the shadowed result of this From ebcef334279ad09ccfb710f05226a0f3d595232b Mon Sep 17 00:00:00 2001 From: Alex Sanchez-Stern Date: Tue, 17 May 2016 15:57:24 -0700 Subject: [PATCH 2/4] Turned off libm wrapping --- herbgrind/hg_mathwrap.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/herbgrind/hg_mathwrap.c b/herbgrind/hg_mathwrap.c index fb4f494..a47dd20 100644 --- a/herbgrind/hg_mathwrap.c +++ b/herbgrind/hg_mathwrap.c @@ -64,7 +64,7 @@ // This macro is defined in include/hg_mathreplace_funcs.h, and // invokes the above macro for each unary operation that needs to be // wrapped. -WRAP_UNARY_OPS +/* WRAP_UNARY_OPS */ /*---------------------------- ====== Binary Ops ============ @@ -84,7 +84,7 @@ WRAP_UNARY_OPS // This macro is defined in include/hg_mathreplace_funcs.h, and // invokes the above macro for each binary operation that needs to be // wrapped. -WRAP_BINARY_OPS +/* WRAP_BINARY_OPS */ /*---------------------------- ====== Ternary Ops =========== @@ -105,4 +105,4 @@ WRAP_BINARY_OPS // This macro is defined in include/hg_mathreplace_funcs.h, and // invokes the above macro for each ternary operation that needs to be // wrapped. -WRAP_TERNARY_OPS +/* WRAP_TERNARY_OPS */ From 2475ec83d63877397e353be3c2bf8e67b4b55265 Mon Sep 17 00:00:00 2001 From: Alex Sanchez-Stern Date: Tue, 17 May 2016 15:59:29 -0700 Subject: [PATCH 3/4] removed herbgrind runtime starters. Try this? --- bench/tiny.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bench/tiny.c b/bench/tiny.c index 926c1e2..ca9c2ed 100644 --- a/bench/tiny.c +++ b/bench/tiny.c @@ -3,9 +3,9 @@ int main(int argc, char** argv){ int x; - HERBGRIND_BEGIN(); + /* HERBGRIND_BEGIN(); */ x = 1 + 2; - HERBGRIND_END(); + /* HERBGRIND_END(); */ printf("x\n"); return 0; } From 906f5813bcdc46a5a34f76e73d14d3634a15bea1 Mon Sep 17 00:00:00 2001 From: Zach Tatlock Date: Fri, 27 May 2016 13:23:20 -0700 Subject: [PATCH 4/4] wip on os x debugging --- Makefile | 6 ++-- herbgrind/hg_instrument.c | 58 ++++++++++++++++++++++++++++++++++++++- herbgrind/hg_main.c | 1 + 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 37f0708..7369790 100644 --- a/Makefile +++ b/Makefile @@ -68,7 +68,7 @@ valgrind/herbgrind/Makefile: valgrind/README herbgrind/Makefile.am # real makefile. cd valgrind && ./autogen.sh cd valgrind && \ - CFLAGS="-fno-stack-protector" \ + CFLAGS="-fno-stack-protector -O0" \ ./configure --prefix=$(shell pwd)/valgrind/$(HG_LOCAL_INSTALL_NAME) \ --enable-only64bit \ --build=$(TARGET_PLAT) @@ -113,7 +113,7 @@ deps/gmp-%/README: setup/gmp-$(GMP_VERSION).tar.xz setup/patch_gmp.sh # locally instead of in a global location, so it doesn't conflict with # other versions of gmp. cd deps/gmp-$*/ && \ - CFLAGS="-fno-stack-protector" \ + CFLAGS="-fno-stack-protector -O0" \ ABI=$* \ ./configure --prefix=$(shell pwd)/deps/gmp-$*/$(HG_LOCAL_INSTALL_NAME) $(MAKE) -C deps/gmp-$* @@ -130,7 +130,7 @@ MPFR_CONFIGURE_FLAGS = --disable-thread-safe configure-mpfr-32: cd deps/mpfr-32/ && \ - CFLAGS="-fno-stack-protector" \ + CFLAGS="-fno-stack-protector -O0" \ ./configure --prefix=$(shell pwd)/deps/mpfr-32/$(HG_LOCAL_INSTALL_NAME) \ --with-gmp-build=$(shell pwd)/deps/gmp-32 \ --build=i386 \ diff --git a/herbgrind/hg_instrument.c b/herbgrind/hg_instrument.c index b41c176..adbad55 100644 --- a/herbgrind/hg_instrument.c +++ b/herbgrind/hg_instrument.c @@ -32,6 +32,7 @@ #include "include/hg_helper.h" #include "include/hg_macros.h" #include "runtime/hg_mathreplace.h" +#include "include/pub_tool_libcassert.h" void instrumentStatement(IRStmt* st, IRSB* sbOut, Addr stAddr){ IRExpr* expr; @@ -131,6 +132,7 @@ That doesn't seem flattened...\n"); // Here we'll instrument moving Shadow values into temps. See // above. addStmtToIRSB(sbOut, st); + if (!isFloat(sbOut->tyenv, st->Ist.WrTmp.tmp)) break; expr = st->Ist.WrTmp.data; switch(expr->tag) { @@ -143,6 +145,7 @@ That doesn't seem flattened...\n"); mkU64(expr->Iex.Get.ty), mkU64(st->Ist.WrTmp.tmp))); addStmtToIRSB(sbOut, IRStmt_Dirty(copyShadowLocation)); + break; case Iex_GetI: // See comments above on PutI to make sense of this thing. @@ -158,6 +161,7 @@ That doesn't seem flattened...\n"); mkU64(expr->Iex.Get.ty), mkU64(st->Ist.WrTmp.tmp))); addStmtToIRSB(sbOut, IRStmt_Dirty(copyShadowLocation)); + break; case Iex_RdTmp: copyShadowLocation = @@ -202,6 +206,7 @@ That doesn't seem flattened...\n"); mkIRExprVec_2(mkU64(condTmp), mkU64(st->Ist.WrTmp.tmp))); addStmtToIRSB(sbOut, IRStmt_Dirty(copyShadowLocation)); + } break; case Iex_Load: @@ -214,6 +219,7 @@ That doesn't seem flattened...\n"); mkU64(expr->Iex.Load.ty), mkU64(st->Ist.WrTmp.tmp))); addStmtToIRSB(sbOut, IRStmt_Dirty(copyShadowLocation)); + } break; case Iex_Qop: @@ -239,6 +245,7 @@ That doesn't seem flattened...\n"); // Here we'll instrument moving Shadow values into memory, // unconditionally. addStmtToIRSB(sbOut, st); + expr = st->Ist.Store.data; switch (expr->tag) { case Iex_RdTmp: @@ -250,6 +257,7 @@ That doesn't seem flattened...\n"); mkIRExprVec_2(mkU64(expr->Iex.RdTmp.tmp), st->Ist.Store.addr)); addStmtToIRSB(sbOut, IRStmt_Dirty(copyShadowLocation)); + } break; case Iex_Const: @@ -266,10 +274,27 @@ That doesn't seem flattened...\n"); // Same as above, but only assigns the value to memory if a // guard returns true. addStmtToIRSB(sbOut, st); + + // TEST TROUBLE HERE + // break; + // + expr = st->Ist.Store.data; + switch(expr->tag) { + case Iex_RdTmp: + /* + + VG_(dmsg)("\ +100\n"); + VG_(exit)(100); + if (isFloat(sbOut->tyenv, expr->Iex.RdTmp.tmp)){ + + VG_(dmsg)("\ +101\n"); + VG_(exit)(101); copyShadowLocation = unsafeIRDirty_0_N(3, "copyShadowTmptoMemG", @@ -277,16 +302,32 @@ That doesn't seem flattened...\n"); mkIRExprVec_3(st->Ist.StoreG.details->guard, mkU64(expr->Iex.RdTmp.tmp), st->Ist.StoreG.details->addr)); + VG_(dmsg)("\ +110\n"); + VG_(exit)(110); addStmtToIRSB(sbOut, IRStmt_Dirty(copyShadowLocation)); + + VG_(dmsg)("\ +111\n"); + VG_(exit)(111); + // TEST + break; } + */ break; + /* case Iex_Const: + VG_(dmsg)("\ +102\n"); + VG_(exit)(102); break; + */ default: // This shouldn't happen in flattened IR. VG_(dmsg)("\ A non-constant or temp is being placed into memory in a single IR statement! \ That doesn't seem flattened...\n"); + VG_(exit)(109); break; } break; @@ -294,6 +335,9 @@ That doesn't seem flattened...\n"); // Guarded load. This will load a value from memory, and write // it to a temp, but only if a condition returns true. addStmtToIRSB(sbOut, st); + + // TEST + break; if (isFloat(sbOut->tyenv, st->Ist.LoadG.details->dst)){ ALLOC(loadGInfo, "hg.loadGmalloc.1", 1, sizeof(LoadG_Info)); // These are the lines we'd like to write. Unfortunately, we @@ -308,7 +352,7 @@ That doesn't seem flattened...\n"); addStmtToIRSB(sbOut, IRStmt_Store(ENDIAN, mkU64((uintptr_t)&(loadGInfo->cond)), st->Ist.LoadG.details->guard)); addStmtToIRSB(sbOut, IRStmt_Store(ENDIAN, mkU64((uintptr_t)&(loadGInfo->src_mem)), - st->Ist.LoadG.details->addr)); + st->Ist.LoadG.details->addr)); addStmtToIRSB(sbOut, IRStmt_Store(ENDIAN, mkU64((uintptr_t)&(loadGInfo->alt_tmp)), st->Ist.LoadG.details->alt)); loadGInfo->dest_tmp = st->Ist.LoadG.details->dst; @@ -320,6 +364,9 @@ That doesn't seem flattened...\n"); VG_(fnptr_to_fnentry)(©ShadowMemtoTmpIf), mkIRExprVec_1(mkU64((uintptr_t)loadGInfo))); addStmtToIRSB(sbOut, IRStmt_Dirty(copyShadowLocation)); + + // TEST + break; } break; case Ist_CAS: @@ -332,6 +379,9 @@ That doesn't seem flattened...\n"); // TODO: Add something here if we ever want to support multithreading. addStmtToIRSB(sbOut, st); + + // TEST + break; VG_(dmsg)("\ Warning! Herbgrind does not currently support the Compare and Swap instruction, \ because we don't support multithreaded programs.\n"); @@ -342,6 +392,9 @@ because we don't support multithreaded programs.\n"); // TODO: Add something here if we ever want to support multithreading. addStmtToIRSB(sbOut, st); + + // TEST + break; VG_(dmsg)("\ Warning! Herbgrind does not currently support the Load Linked / Store Conditional \ set of instructions, because we don't support multithreaded programs.\n"); @@ -351,6 +404,9 @@ set of instructions, because we don't support multithreaded programs.\n"); // side effects should be denoted in the attributes of this // instruction. addStmtToIRSB(sbOut, st); + + // TEST + break; break; } } diff --git a/herbgrind/hg_main.c b/herbgrind/hg_main.c index 3405dac..2590e7b 100644 --- a/herbgrind/hg_main.c +++ b/herbgrind/hg_main.c @@ -89,6 +89,7 @@ IRSB* hg_instrument ( VgCallbackClosure* closure, if (st->tag == Ist_IMark) cur_addr = st->Ist.IMark.addr; // Take a look at hg_instrument.c to see what's going on here. + instrumentStatement(st, sbOut, cur_addr); }