diff --git a/hphp/compiler/compiler.cpp b/hphp/compiler/compiler.cpp index 5b71197c8..fac86e0ca 100644 --- a/hphp/compiler/compiler.cpp +++ b/hphp/compiler/compiler.cpp @@ -195,6 +195,10 @@ int compiler_main(int argc, char **argv) { return ret; } catch (Exception &e) { Logger::Error("Exception: %s\n", e.getMessage().c_str()); + } catch (const FailedAssertion& fa) { + fa.print(); + StackTraceNoHeap::AddExtraLogging("Assertion failure", fa.summary); + abort(); } catch (std::exception &e) { Logger::Error("std::exception: %s\n", e.what()); } catch (...) { diff --git a/hphp/runtime/base/crash_reporter.cpp b/hphp/runtime/base/crash_reporter.cpp index 65610de56..a393d5dd0 100644 --- a/hphp/runtime/base/crash_reporter.cpp +++ b/hphp/runtime/base/crash_reporter.cpp @@ -97,6 +97,8 @@ void install_crash_reporter() { signal(SIGSEGV, bt_handler); signal(SIGBUS, bt_handler); signal(SIGABRT, bt_handler); + + register_assert_fail_logger(&StackTraceNoHeap::AddExtraLogging); } ////////////////////////////////////////////////////////////////////// diff --git a/hphp/runtime/base/program_functions.cpp b/hphp/runtime/base/program_functions.cpp index d3e70a5e3..7575fa4c8 100644 --- a/hphp/runtime/base/program_functions.cpp +++ b/hphp/runtime/base/program_functions.cpp @@ -683,6 +683,10 @@ int execute_program(int argc, char **argv) { ret_code = execute_program_impl(argc, argv); } catch (const Exception &e) { Logger::Error("Uncaught exception: %s", e.what()); + } catch (const FailedAssertion& fa) { + fa.print(); + StackTraceNoHeap::AddExtraLogging("Assertion failure", fa.summary); + abort(); } catch (const std::exception &e) { Logger::Error("Uncaught exception: %s", e.what()); } catch (...) { diff --git a/hphp/runtime/vm/translator/hopt/cse.h b/hphp/runtime/vm/translator/hopt/cse.h index 85c83e230..8a0b3acea 100644 --- a/hphp/runtime/vm/translator/hopt/cse.h +++ b/hphp/runtime/vm/translator/hopt/cse.h @@ -21,6 +21,8 @@ #include "folly/Hash.h" +#include "runtime/vm/translator/hopt/ir.h" + namespace HPHP { namespace VM { namespace JIT { diff --git a/hphp/runtime/vm/translator/hopt/dce.cpp b/hphp/runtime/vm/translator/hopt/dce.cpp index 20c6e371e..da8e7f0d5 100644 --- a/hphp/runtime/vm/translator/hopt/dce.cpp +++ b/hphp/runtime/vm/translator/hopt/dce.cpp @@ -182,7 +182,12 @@ void optimizeRefCount(Trace* trace, DceState& state) { forEachInst(trace, [&](IRInstruction* inst) { if (inst->getOpcode() == IncRef && !state[inst].countConsumedAny()) { auto& s = state[inst]; - always_assert(s.decRefNZed()); + always_assert_log(s.decRefNZed(), [&]{ + Trace* mainTrace = trace->isMain() ? trace : trace->getMain(); + return folly::format("\n{} has state {} in trace:\n{}{}\n", + inst->toString(), s.toString(), mainTrace->toString(), + trace == mainTrace ? "" : trace->toString()).str(); + }); inst->setOpcode(Mov); s.setDead(); } diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp index b0542a90e..afac1b649 100644 --- a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp @@ -24,6 +24,8 @@ #include "runtime/vm/runtime.h" #include "runtime/vm/translator/hopt/irfactory.h" +// Include last to localize effects to this file +#include "util/assert_throw.h" namespace HPHP { namespace VM { diff --git a/hphp/runtime/vm/translator/hopt/ir.cpp b/hphp/runtime/vm/translator/hopt/ir.cpp index 8fd88a9ab..545c3a174 100644 --- a/hphp/runtime/vm/translator/hopt/ir.cpp +++ b/hphp/runtime/vm/translator/hopt/ir.cpp @@ -35,6 +35,9 @@ #include "runtime/vm/translator/hopt/cse.h" #include "runtime/vm/translator/hopt/simplifier.h" +// Include last to localize effects to this file +#include "util/assert_throw.h" + namespace HPHP { namespace VM { namespace JIT { #define IRT(name, ...) const Type Type::name(Type::k##name); @@ -977,6 +980,12 @@ TCA SSATmp::getTCA() const { return getInstruction()->getTCA(); } +std::string SSATmp::toString() const { + std::ostringstream out; + print(out); + return out.str(); +} + void SSATmp::print(std::ostream& os, bool printLastUse) const { if (m_inst->getOpcode() == DefConst) { printConst(os, m_inst); @@ -1015,6 +1024,10 @@ std::string Trace::toString() const { return out.str(); } +void Trace::print() const { + print(std::cout, nullptr); +} + void Trace::print(std::ostream& os, const AsmInfo* asmInfo) const { static const int kIndent = 4; Disasm disasm(kIndent + 4, RuntimeOption::EvalDumpIR > 5); @@ -1278,6 +1291,7 @@ bool checkCfg(Trace* trace, const IRFactory& factory) { int idom_id = idom[block->postId()]; if (idom_id != -1) children[idom_id].push_front(block); } + // visit dom tree in preorder, checking all tmps boost::dynamic_bitset<> defined0(factory.numTmps()); forPreorderDoms(blocks.front(), children, defined0, @@ -1285,8 +1299,14 @@ bool checkCfg(Trace* trace, const IRFactory& factory) { for (IRInstruction& inst : *block) { for (SSATmp* UNUSED src : inst.getSrcs()) { assert(src->getInstruction() != &inst); - assert(src->getInstruction()->getOpcode() == DefConst || - defined[src->getId()]); + assert_log(src->getInstruction()->getOpcode() == DefConst || + defined[src->getId()], + [&]{ return folly::format( + "src '{}' in '{}' came from '{}', which is not a " + "DefConst and is not defined at this use site", + src->toString(), inst.toString(), + src->getInstruction()->toString()).str(); + }); } for (SSATmp& dst : inst.getDsts()) { assert(dst.getInstruction() == &inst && diff --git a/hphp/runtime/vm/translator/hopt/ir.h b/hphp/runtime/vm/translator/hopt/ir.h index fbcd19893..e4944c13e 100644 --- a/hphp/runtime/vm/translator/hopt/ir.h +++ b/hphp/runtime/vm/translator/hopt/ir.h @@ -1627,6 +1627,7 @@ public: bool isBoxed() const { return getType().isBoxed(); } bool isString() const { return isA(Type::Str); } bool isArray() const { return isA(Type::Arr); } + std::string toString() const; void print(std::ostream& ostream, bool printLastUse = false) const; void print() const; @@ -2012,6 +2013,7 @@ public: ExitList& getExitTraces() { return m_exitTraces; } std::string toString() const; void print(std::ostream& ostream, const AsmInfo* asmInfo = nullptr) const; + void print() const; private: // offset of the first bytecode in this trace; 0 if this trace doesn't diff --git a/hphp/runtime/vm/translator/hopt/irtranslator.cpp b/hphp/runtime/vm/translator/hopt/irtranslator.cpp index 8b2ceeea1..8bc263d24 100644 --- a/hphp/runtime/vm/translator/hopt/irtranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/irtranslator.cpp @@ -18,6 +18,7 @@ #include "folly/Format.h" #include "util/trace.h" +#include "util/stack_trace.h" #include "util/util.h" #include "runtime/vm/bytecode.h" @@ -36,6 +37,9 @@ #include "runtime/vm/translator/hopt/codegen.h" #include "runtime/vm/translator/hopt/hhbctranslator.h" +// Include last to localize effects to this file +#include "util/assert_throw.h" + namespace HPHP { namespace VM { namespace Transl { @@ -1706,6 +1710,13 @@ TranslatorX64::irTranslateTracelet(Tracelet& t, x.file, x.line, x.func); } catch (TranslationFailedExc& tfe) { not_reached(); + } catch (const FailedAssertion& fa) { + fa.print(); + StackTraceNoHeap::AddExtraLogging( + "Assertion failure", + folly::format("{}\n\nActive Trace:\n{}\n", + fa.summary, m_hhbcTrans->getTrace()->toString()).str()); + abort(); } catch (const std::exception& e) { transResult = Failure; FTRACE(1, "HHIR: FAILED with exception: {}\n", e.what()); diff --git a/hphp/runtime/vm/translator/hopt/simplifier.h b/hphp/runtime/vm/translator/hopt/simplifier.h index b2e551dd0..5208bff52 100644 --- a/hphp/runtime/vm/translator/hopt/simplifier.h +++ b/hphp/runtime/vm/translator/hopt/simplifier.h @@ -17,8 +17,8 @@ #ifndef incl_HHVM_HHIR_SIMPLIFIER_H_ #define incl_HHVM_HHIR_SIMPLIFIER_H_ -#include "ir.h" -#include "cse.h" +#include "runtime/vm/translator/hopt/cse.h" +#include "runtime/vm/translator/hopt/ir.h" namespace HPHP { namespace VM { diff --git a/hphp/runtime/vm/translator/hopt/vectortranslator.cpp b/hphp/runtime/vm/translator/hopt/vectortranslator.cpp index e2a814191..e83ee2c08 100644 --- a/hphp/runtime/vm/translator/hopt/vectortranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/vectortranslator.cpp @@ -20,7 +20,8 @@ #include "runtime/vm/translator/hopt/hhbctranslator.h" #include "runtime/vm/translator/translator-x64.h" -// This file does ugly things with macros so include it last +// These files do ugly things with macros so include them last +#include "util/assert_throw.h" #include "runtime/vm/translator/hopt/vectortranslator-internal.h" namespace HPHP { namespace VM { namespace JIT { diff --git a/hphp/util/assert_throw.h b/hphp/util/assert_throw.h new file mode 100644 index 000000000..e1f217010 --- /dev/null +++ b/hphp/util/assert_throw.h @@ -0,0 +1,53 @@ +/* + +----------------------------------------------------------------------+ + | HipHop for PHP | + +----------------------------------------------------------------------+ + | Copyright (c) 2010- Facebook, Inc. (http://www.facebook.com) | + +----------------------------------------------------------------------+ + | This source file is subject to version 3.01 of the PHP license, | + | that is bundled with this package in the file LICENSE, and is | + | available through the world-wide-web at the following url: | + | http://www.php.net/license/3_01.txt | + | If you did not receive a copy of the PHP license and are unable to | + | obtain it through the world-wide-web, please send a note to | + | license@php.net so we can mail you a copy immediately. | + +----------------------------------------------------------------------+ +*/ + +#ifndef incl_HPHP_ASSERT_THROW_H_ +#define incl_HPHP_ASSERT_THROW_H_ + +#include "util/assertions.h" + +/* + * This file replaces assert and always_assert with assert_throw and + * always_assert_throw, respectively. You probably don't want to + * include it from a header. + */ + +// This can make debugging in gdb a pain, so make it two levels of +// opt-in for now. Uncomment the next line to enable it for files that +// have included this header. + +//#define DO_ASSERT_THROW +#ifdef DO_ASSERT_THROW + +#undef assert +#undef assert_log +#undef always_assert +#undef always_assert_log + +#ifndef NDEBUG +#define assert(e) assert_throw(e) +#define assert_log(e, l) assert_throw_log(e, l) +#else +#define assert(e) +#define assert_log(e, l) +#endif + +#define always_assert(e) always_assert_throw(e) +#define always_assert_log(e, l) always_assert_throw_log(e, l) + +#endif + +#endif diff --git a/hphp/util/assertions.cpp b/hphp/util/assertions.cpp index f80615085..0aaa2e622 100644 --- a/hphp/util/assertions.cpp +++ b/hphp/util/assertions.cpp @@ -17,6 +17,8 @@ namespace HPHP { +static AssertFailLogger s_logger; + // In builds without NDEBUG, we don't have __assert_fail from the GNU // library, so we implement it here for always_assert(). void impl_assert_fail(const char* e, const char* file, @@ -25,4 +27,14 @@ void impl_assert_fail(const char* e, const char* file, std::abort(); } +void assert_fail_log(const char* title, const std::string& msg) { + if (s_logger) { + s_logger(title, msg); + } +} + +void register_assert_fail_logger(AssertFailLogger l) { + s_logger = l; +} + } diff --git a/hphp/util/assertions.h b/hphp/util/assertions.h index cb80804c8..742c6c798 100644 --- a/hphp/util/assertions.h +++ b/hphp/util/assertions.h @@ -17,8 +17,11 @@ #define incl_HPHP_ASSERTIONS_H_ #include -#include +#include #include +#include +#include +#include ////////////////////////////////////////////////////////////////////// @@ -89,12 +92,78 @@ inline void assert_fail(const char* e, #endif } +void assert_fail_log(const char* title, const std::string& msg); +typedef std::function AssertFailLogger; +void register_assert_fail_logger(AssertFailLogger); + +class FailedAssertion : public std::exception { + public: + FailedAssertion(const char* msg, const char* file, unsigned line, + const char* func) + : msg(msg) + , file(file) + , line(line) + , func(func) + , summary(makeSummary()) {} + + ~FailedAssertion() throw() {} + + const char* what() const throw() { + return summary.c_str(); + } + + void print() const { + fputs(summary.c_str(), stderr); + fputc('\n', stderr); + } + + const char* const msg; + const char* const file; + unsigned const line; + const char* const func; + const std::string summary; + + private: + std::string makeSummary() const { + char buf[4096]; + if (snprintf(buf, sizeof(buf), "Failed assertion '%s' in %s at %s:%u", + msg, func, file, line) >= sizeof(buf)) { + buf[sizeof(buf)-1] = '\0'; + } + return std::string(buf); + } +}; + } -#define always_assert(e) \ - ((e) ? static_cast(0) \ - : (::HPHP::assert_fail(#e, __FILE__, __LINE__, __PRETTY_FUNCTION__), \ - static_cast(0))) +#define assert_impl(cond, fail) \ + ((cond) ? static_cast(0) : ((fail), static_cast(0))) + +#define assert_log_impl(cond, fail, func) assert_impl( \ + cond, \ + (::HPHP::assert_fail_log(#cond, func()), (fail))) \ + +#define assert_fail_impl(e) \ + ::HPHP::assert_fail(#e, __FILE__, __LINE__, __PRETTY_FUNCTION__) +#define always_assert(e) assert_impl(e, assert_fail_impl(e)) +#define always_assert_log(e, l) assert_log_impl(e, assert_fail_impl(e), l) + +#define assert_throw_fail_impl(e) \ + throw ::HPHP::FailedAssertion(#e, __FILE__, __LINE__, __PRETTY_FUNCTION__) +#define always_assert_throw(e) \ + assert_impl(e, assert_throw_fail_impl(e)) +#define always_assert_throw_log(e, l) \ + assert_log_impl(e, assert_throw_fail_impl(e), l) + +#ifndef NDEBUG +#define assert_log(e, l) always_assert_log(e, l) +#define assert_throw(e) always_assert_throw(e) +#define assert_throw_log(e, l) always_assert_throw_log(e, l) +#else +#define assert_log(e, l) +#define assert_throw(e) +#define assert_throw_log(e, l) +#endif ////////////////////////////////////////////////////////////////////// diff --git a/hphp/util/stack_trace.cpp b/hphp/util/stack_trace.cpp index 8cb10620d..7b0754b37 100644 --- a/hphp/util/stack_trace.cpp +++ b/hphp/util/stack_trace.cpp @@ -216,10 +216,9 @@ public: }; IMPLEMENT_THREAD_LOCAL(StackTraceLog, StackTraceLog::s_logData); -void StackTraceNoHeap::AddExtraLogging(const char *name, const char *value) { +void StackTraceNoHeap::AddExtraLogging(const char *name, + const std::string &value) { assert(name && *name); - assert(value); - StackTraceLog::s_logData->data[name] = value; } diff --git a/hphp/util/stack_trace.h b/hphp/util/stack_trace.h index 627fdefcd..e38eb6b14 100644 --- a/hphp/util/stack_trace.h +++ b/hphp/util/stack_trace.h @@ -151,7 +151,7 @@ public: /** * Add extra information to log together with a crash stacktrace log. */ - static void AddExtraLogging(const char *name, const char *value); + static void AddExtraLogging(const char *name, const std::string &value); static void ClearAllExtraLogging(); class ExtraLoggingClearer {