Skip to content

Commit

Permalink
raise error when calling BuiltinClass::InstanceMethod()
Browse files Browse the repository at this point in the history
Summary:
PHP allows UseClass::InstanceMethod() calls, as long as inside the method,
there is no access to $this. We used o_id == 0 to detect that. But for extension
classes, there is no "implicit static method". If it's declared as instance one,
it cannot be called statically no matter what. Added the checking to
INSTANCE_METHOD_INJECTION_BUILTIN, which was incorrectly injected as
INSTANCE_METHOD_INJECTION without "_BUILTIN" before. So that's fixed now.

Test Plan:
This script gave the same error in HPHPi now:

  <?php DateTimeZone::getName();

DiffCamp Revision: 129751
Reviewed By: qixin
CC: hphp-diffs@lists, qixin, mpal
Tasks:
call

Revert Plan:
OK
  • Loading branch information
haiping authored and macvicar committed Jul 5, 2010
1 parent 0560449 commit ab20856
Show file tree
Hide file tree
Showing 20 changed files with 627 additions and 609 deletions.
4 changes: 2 additions & 2 deletions bin/ext_injection.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@
$pattern = '/c_(\w+)::t_(\w+)([^\{\}]*)\{([\n\t ]+)'.
'(?:\w+INJECTION\([\w:, ]+\);[\n\t ]+)?([^\}])/s';
$replace = "c_\${1}::t_\${2}\${3}{\n ".
"INSTANCE_METHOD_INJECTION(\${1}, \${1}::\${2});\${4}\${5}";
"INSTANCE_METHOD_INJECTION_BUILTIN(\${1}, \${1}::\${2});\${4}\${5}";

$replaced = preg_replace($pattern, $replace, $contents);

$pattern = '/c_(\w+)::ti_(\w+)([^\{\}]*)\{([\n\t ]+)'.
'(?:\w+INJECTION\([\w:, ]+\);[\n\t ]+)?([^\}])/s';
$replace = "c_\${1}::ti_\${2}\${3}{\n ".
"STATIC_METHOD_INJECTION(\${1}, \${1}::\${2});\${4}\${5}";
"STATIC_METHOD_INJECTION_BUILTIN(\${1}, \${1}::\${2});\${4}\${5}";

$replaced = preg_replace($pattern, $replace, $replaced);

Expand Down
6 changes: 4 additions & 2 deletions src/compiler/statement/method_statement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -541,11 +541,13 @@ void MethodStatement::outputCPPImpl(CodeGenerator &cg, AnalysisResultPtr ar) {
funcScope->outputCPPParamsDecl(cg, ar, m_params, false);
cg_indentBegin(") {\n");
if (m_stmt->hasBody()) {
const char *sys =
(cg.getOutput() == CodeGenerator::SystemCPP ? "_BUILTIN" : "");
if (m_modifiers->isStatic()) {
cg_printf("STATIC_METHOD_INJECTION(%s, %s);\n",
cg_printf("STATIC_METHOD_INJECTION%s(%s, %s);\n", sys,
scope->getOriginalName().c_str(), origFuncName.c_str());
} else {
cg_printf("INSTANCE_METHOD_INJECTION(%s, %s);\n",
cg_printf("INSTANCE_METHOD_INJECTION%s(%s, %s);\n", sys,
scope->getOriginalName().c_str(), origFuncName.c_str());
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/runtime/base/builtin_functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,12 @@ Variant o_invoke_failed(const char *cls, const char *meth,
}
}

void throw_instance_method_fatal(const char *name) {
if (!strstr(name, "::__destruct")) {
raise_error("Non-static method %s() cannot be called statically", name);
}
}

Variant throw_missing_arguments(const char *fn, int num, int level /* = 0 */) {
if (level == 2 || RuntimeOption::ThrowMissingArguments) {
raise_error("Missing argument %d for %s()", num, fn);
Expand Down
1 change: 1 addition & 0 deletions src/runtime/base/builtin_functions.h
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,7 @@ inline Variant throw_missing_class(const char *cls) {
inline Variant throw_missing_file(const char *cls) {
throw PhpFileDoesNotExistException(cls);
}
void throw_instance_method_fatal(const char *name);

/**
* Argument count handling.
Expand Down
1 change: 1 addition & 0 deletions src/runtime/base/macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@ do { \
FRAME_INJECTION(c, n) \

#define INSTANCE_METHOD_INJECTION_BUILTIN(c, n) \
if (!o_id) throw_instance_method_fatal(#n); \
DECLARE_THREAD_INFO \
RECURSION_INJECTION \
REQUEST_TIMEOUT_INJECTION \
Expand Down
34 changes: 17 additions & 17 deletions src/runtime/ext/ext_datetime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,25 +60,25 @@ c_datetime::~c_datetime() {

void c_datetime::t___construct(CStrRef time /*= "now"*/,
CObjRef timezone /*= null_object*/) {
INSTANCE_METHOD_INJECTION(datetime, datetime::__construct);
INSTANCE_METHOD_INJECTION_BUILTIN(datetime, datetime::__construct);
m_dt = NEW(DateTime)(TimeStamp::Current());
if (!time.empty()) {
m_dt->fromString(time, c_datetimezone::unwrap(timezone));
}
}

String c_datetime::t_format(CStrRef format) {
INSTANCE_METHOD_INJECTION(datetime, datetime::format);
INSTANCE_METHOD_INJECTION_BUILTIN(datetime, datetime::format);
return m_dt->toString(format, false);
}

int64 c_datetime::t_getoffset() {
INSTANCE_METHOD_INJECTION(datetime, datetime::getoffset);
INSTANCE_METHOD_INJECTION_BUILTIN(datetime, datetime::getoffset);
return m_dt->offset();
}

Variant c_datetime::t_gettimezone() {
INSTANCE_METHOD_INJECTION(datetime, datetime::gettimezone);
INSTANCE_METHOD_INJECTION_BUILTIN(datetime, datetime::gettimezone);
SmartObject<TimeZone> tz = m_dt->timezone();
if (tz->isValid()) {
return c_datetimezone::wrap(tz);
Expand All @@ -87,37 +87,37 @@ Variant c_datetime::t_gettimezone() {
}

Object c_datetime::t_modify(CStrRef modify) {
INSTANCE_METHOD_INJECTION(datetime, datetime::modify);
INSTANCE_METHOD_INJECTION_BUILTIN(datetime, datetime::modify);
m_dt->modify(modify);
return this;
}

Object c_datetime::t_setdate(int64 year, int64 month, int64 day) {
INSTANCE_METHOD_INJECTION(datetime, datetime::setdate);
INSTANCE_METHOD_INJECTION_BUILTIN(datetime, datetime::setdate);
m_dt->setDate(year, month, day);
return this;
}

Object c_datetime::t_setisodate(int64 year, int64 week, int64 day /*= 1*/) {
INSTANCE_METHOD_INJECTION(datetime, datetime::setisodate);
INSTANCE_METHOD_INJECTION_BUILTIN(datetime, datetime::setisodate);
m_dt->setISODate(year, week, day);
return this;
}

Object c_datetime::t_settime(int64 hour, int64 minute, int64 second /*= 0*/) {
INSTANCE_METHOD_INJECTION(datetime, datetime::settime);
INSTANCE_METHOD_INJECTION_BUILTIN(datetime, datetime::settime);
m_dt->setTime(hour, minute, second);
return this;
}

Object c_datetime::t_settimezone(CObjRef timezone) {
INSTANCE_METHOD_INJECTION(datetime, datetime::settimezone);
INSTANCE_METHOD_INJECTION_BUILTIN(datetime, datetime::settimezone);
m_dt->setTimezone(c_datetimezone::unwrap(timezone));
return this;
}

Variant c_datetime::t___destruct() {
INSTANCE_METHOD_INJECTION(datetime, datetime::__destruct);
INSTANCE_METHOD_INJECTION_BUILTIN(datetime, datetime::__destruct);
return null;
}

Expand All @@ -128,39 +128,39 @@ c_datetimezone::~c_datetimezone() {
}

void c_datetimezone::t___construct(CStrRef timezone) {
INSTANCE_METHOD_INJECTION(datetimezone, datetimezone::__construct);
INSTANCE_METHOD_INJECTION_BUILTIN(datetimezone, datetimezone::__construct);
m_tz = NEW(TimeZone)(timezone);
}

String c_datetimezone::t_getname() {
INSTANCE_METHOD_INJECTION(datetimezone, datetimezone::getname);
INSTANCE_METHOD_INJECTION_BUILTIN(datetimezone, datetimezone::getname);
return m_tz->name();
}

int64 c_datetimezone::t_getoffset(CObjRef datetime) {
INSTANCE_METHOD_INJECTION(datetimezone, datetimezone::getoffset);
INSTANCE_METHOD_INJECTION_BUILTIN(datetimezone, datetimezone::getoffset);
bool error;
int64 ts = c_datetime::unwrap(datetime)->toTimeStamp(error);
return m_tz->offset(ts);
}

Array c_datetimezone::t_gettransitions() {
INSTANCE_METHOD_INJECTION(datetimezone, datetimezone::gettransitions);
INSTANCE_METHOD_INJECTION_BUILTIN(datetimezone, datetimezone::gettransitions);
return m_tz->transitions();
}

Array c_datetimezone::ti_listabbreviations(const char* cls) {
STATIC_METHOD_INJECTION(datetimezone, datetimezone::listabbreviations);
STATIC_METHOD_INJECTION_BUILTIN(datetimezone, datetimezone::listabbreviations);
return TimeZone::GetAbbreviations();
}

Array c_datetimezone::ti_listidentifiers(const char* cls) {
STATIC_METHOD_INJECTION(datetimezone, datetimezone::listidentifiers);
STATIC_METHOD_INJECTION_BUILTIN(datetimezone, datetimezone::listidentifiers);
return TimeZone::GetNames();
}

Variant c_datetimezone::t___destruct() {
INSTANCE_METHOD_INJECTION(datetimezone, datetimezone::__destruct);
INSTANCE_METHOD_INJECTION_BUILTIN(datetimezone, datetimezone::__destruct);
return null;
}

Expand Down
Loading

0 comments on commit ab20856

Please sign in to comment.