Skip to content

Commit

Permalink
Add type checking to cgCallHelper()
Browse files Browse the repository at this point in the history
Summary:
I've wanted to do this for years, and this should help with the
migration from TypedValue* to tv_lval. CallSpec::direct() now
template-generates a function signature for the callee, using jit::Type
equivalents of the C++ types (CallSpec::method() can be done later). This
requires passing a properly-typed function pointer to CallSpec, so there are a
few related changes to clean up places that were doing some
information-destroying casting (especially irlower-minstr). I punted for
builtin function calls, where there's no great way to get the actual function
pointer right now.

This didn't expose any real mismatches, but it did remind me that we still had
a few helper functions that bitcast between double and int64_t, from before the
JIT knew how to pass and return doubles to/from C++. Those have been cleaned
up. We still use u?int64_t to return bools from a bunch of helpers, which I may
clean up in a followup diff.

Reviewed By: paulbiss

Differential Revision: D8385908

fbshipit-source-id: c424d6ccd83b70ac267247e9244155426ee0053b
  • Loading branch information
swtaarrs authored and hhvm-bot committed Jul 7, 2018
1 parent 6a7bba9 commit 15be1eb
Show file tree
Hide file tree
Showing 21 changed files with 600 additions and 340 deletions.
8 changes: 5 additions & 3 deletions hphp/runtime/vm/jit/arg-group.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,12 @@ ArgGroup& ArgGroup::typedValue(int i) {
}
static_assert(offsetof(TypedValue, m_data) == 0, "");
static_assert(offsetof(TypedValue, m_type) == 8, "");
ssa(i).type(i);
ssa(i, false).type(i);
m_override = nullptr;
return *this;
}

void ArgGroup::push_arg(const ArgDesc& arg) {
void ArgGroup::push_arg(const ArgDesc& arg, Type t) {
// If m_override is set, use it unconditionally. Otherwise, select
// m_gpArgs or m_stkArgs depending on how many args we've already pushed.
ArgVec* args = m_override;
Expand All @@ -107,16 +107,18 @@ void ArgGroup::push_arg(const ArgDesc& arg) {
}
}
args->push_back(arg);
m_argTypes.emplace_back(t);
}

void ArgGroup::push_SIMDarg(const ArgDesc& arg) {
void ArgGroup::push_SIMDarg(const ArgDesc& arg, Type t) {
// See push_arg above
ArgVec* args = m_override;
if (!args) {
args = m_simdArgs.size() < num_arg_regs_simd()
? &m_simdArgs : &m_stkArgs;
}
args->push_back(arg);
m_argTypes.emplace_back(t);
}

///////////////////////////////////////////////////////////////////////////////
Expand Down
29 changes: 21 additions & 8 deletions hphp/runtime/vm/jit/arg-group.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,17 @@ enum class DestType : uint8_t {
const char* destTypeName(DestType);

struct CallDest {
DestType type;
CallDest(DestType t, Type vt, Vreg r0 = Vreg{}, Vreg r1 = Vreg{})
: valueType{vt}, reg0{r0}, reg1{r1}, type{t}
{}

CallDest(DestType t, Vreg r0 = Vreg{}, Vreg r1 = Vreg{})
: CallDest(t, TTop, r0, r1)
{}

Type valueType;
Vreg reg0, reg1;
DestType type;
};
UNUSED const CallDest kVoidDest { DestType::None };
UNUSED const CallDest kIndirectDest { DestType::Indirect };
Expand Down Expand Up @@ -86,7 +95,7 @@ struct ArgDesc {
}
Immed disp() const {
assertx(m_kind == Kind::Addr ||
m_kind == Kind::IndRet);
m_kind == Kind::IndRet);
return m_disp32;
}
bool isZeroExtend() const { return m_zeroExtend; }
Expand Down Expand Up @@ -153,6 +162,9 @@ struct ArgGroup {
size_t numStackArgs() const { return m_stkArgs.size(); }
size_t numIndRetArgs() const { return m_indRetArgs.size(); }

const std::vector<Type>& argTypes() const {
return m_argTypes;
}
ArgDesc& gpArg(size_t i) {
assertx(i < m_gpArgs.size());
return m_gpArgs[i];
Expand Down Expand Up @@ -211,17 +223,17 @@ struct ArgGroup {
return *this;
}

ArgGroup& ssa(int i, bool isFP = false) {
ArgGroup& ssa(int i, bool allowFP = true) {
auto s = m_inst->src(i);
ArgDesc arg(s, m_locs[s]);
if (isFP) {
push_SIMDarg(arg);
if (s->isA(TDbl) && allowFP) {
push_SIMDarg(arg, s->type());
if (arch() == Arch::PPC64) {
// PPC64 ABIv2 compliant: reserve the aligned GP if FP is used
push_arg(ArgDesc(ArgDesc::Kind::Imm, 0)); // Push a dummy parameter
}
} else {
push_arg(arg);
push_arg(arg, s->type());
}
return *this;
}
Expand All @@ -244,8 +256,8 @@ struct ArgGroup {
}

private:
void push_arg(const ArgDesc& arg);
void push_SIMDarg(const ArgDesc& arg);
void push_arg(const ArgDesc& arg, Type t = TBottom);
void push_SIMDarg(const ArgDesc& arg, Type t = TBottom);

/*
* For passing the m_type field of a TypedValue.
Expand All @@ -272,6 +284,7 @@ struct ArgGroup {
ArgVec m_gpArgs; // INTEGER class args
ArgVec m_simdArgs; // SSE class args
ArgVec m_stkArgs; // Overflow
std::vector<Type> m_argTypes;
};

ArgGroup toArgGroup(const NativeCalls::CallInfo&,
Expand Down
134 changes: 134 additions & 0 deletions hphp/runtime/vm/jit/call-spec.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
/*
+----------------------------------------------------------------------+
| HipHop for PHP |
+----------------------------------------------------------------------+
| Copyright (c) 2010-present 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. |
+----------------------------------------------------------------------+
*/

#include "hphp/runtime/vm/jit/call-spec.h"

#include "hphp/runtime/vm/jit/arg-group.h"

#include "hphp/util/abi-cxx.h"

#include <folly/Range.h>

namespace HPHP { namespace jit {

namespace {
std::string show_types(const std::vector<Type>& ts) {
std::string ret = "{";
auto sep = "";
for (auto& t : ts) {
folly::format(&ret, "{}{}", sep, t);
sep = ", ";
}
return ret + "}";
}

template<typename F>
void verify_return_type(Type ret, const CallDest& dest, F fail) {
if (dest.type == DestType::TV) {
// We really want equality here: TGen corresponds to a full TypedValue
// being returned.
if (ret == TGen) return;
} else {
if (ret <= dest.valueType) return;

// Back before the JIT knew how to deal with byte-sized registers, we held
// TBool in a zero-extended, 8-byte register. A number of runtime helpers
// still return TBool as int64_t or uint64_t, so allow that mismatch here.
if (dest.valueType <= TBool && ret <= TInt) return;

// Some JIT types are much more specific than what we can express in C++,
// so treat certain classes of types as equivalent.
static auto constexpr special_types = {
TPtrToGen,
TLvalToGen,
TBoxedInitCell,
TObj,
TStr,
TArrLike,
};
for (auto t : special_types) {
if (ret <= t && dest.valueType.maybe(t)) return;
}

// SetElem's helper returns a StringData* that is sometimes statically
// known to be TNullptr. Pointers in hhir aren't allowed to be null, so the
// normal Type for StringData* is Str. Check for this special case here
// rather than making all users of StringData* suboptimal.
if (ret <= TStr && dest.valueType <= TNullptr) return;
}

fail("Return type mismatch");
}
}

bool CallSpec::verifySignature(const CallDest& dest,
const std::vector<Type>& args) const {
auto const type = m_typeKind.ptr();
if (!type) return true;
if (kind() != Kind::Direct && kind() != Kind::Smashable) return true;

auto fail = [&](auto&&... fmt) {
auto const why = folly::sformat(std::forward<decltype(fmt)>(fmt)...);
auto const func = getNativeFunctionName(this->address());
auto msg = folly::sformat(
"Failed to verify signature for call to {}: {}\n\n", func, why
);
folly::format(
&msg, "Arguments: {} -> {}\nSignature: {} -> {}",
show_types(args), dest.valueType, show_types(type->params), type->ret
);
always_assert_flog(false, "{}", msg);
};

verify_return_type(type->ret, dest, fail);

size_t argi = 0, parami = 0;
for (; parami < type->params.size() && argi < args.size();
++parami, ++argi) {
auto const param = type->params[parami];
// TGen (for a TypedValue parameter) special: the value and type are passed
// as two separate entries. Make sure both are present.
if (param == TGen) {
if (!(args[argi] <= TGen)) {
fail("Incompatible type {} for Value half of TypedValue parameter {}",
args[argi], parami);
}
if (++argi == args.size()) break;
if (args[argi] != TBottom) {
fail(
"Incompatible type {} for DataType half of TypedValue parameter {}",
args[argi], parami
);
}
} else if (!(args[argi] <= param)) {
// A few instructions pass Cls|Nullptr to helpers that take
// Class*. Handle that special case here.
if (!(param <= TCls && args[argi].maybe(TNullptr))) {
fail(
"Incompatible type {} for {} parameter {}", args[argi], param, parami
);
}
}
}

if (parami != type->params.size() || argi != args.size()) {
fail("Mismatch between argument and parameter counts");
}

return true;
}

}}
Loading

0 comments on commit 15be1eb

Please sign in to comment.