Page MenuHomePhabricator

HHVM segfaults when calling Parser->callParserFunction
Closed, ResolvedPublic

Description

Stack trace

Host: osmium
ProcessID: 32178
ThreadID: 7f335b102540
ThreadPID: 32178
Name: /usr/bin/nice
Type: Segmentation fault
Runtime: hhvm
Version: heads/master-0-g298b3b71908c101e158dcb917f5f34f47b4e1675
DebuggerCount: 0

Arguments: MWScript.php runJobs.php --wiki=elwiktionary --procs=1 --maxtime=60 --memory-limit=300M
ThreadType: CLI

  1. 0 ?? at php:0
  2. 1 killpg at /lib/x86_64-linux-gnu/libc.so.6:0
  3. 2 ?? at php:0
  4. 3 HPHP::JIT::X64::BackEnd::enterTCHelper(unsigned char*, HPHP::JIT::TReqInfo&) at php:0
  5. 4 HPHP::JIT::MCGenerator::enterTC(unsigned char*, void*) at php:0
  6. 5 HPHP::ExecutionContext::enterVM(HPHP::ActRec*, HPHP::ExecutionContext::StackArgsState, HPHP::Resumable*, HPHP::ObjectData*) at php:0
  7. 6 HPHP::ExecutionContext::invokeFunc(HPHP::TypedValue*, HPHP::Func const*, HPHP::Variant const&, HPHP::ObjectData*, HPHP::Class*, HPHP::VarEnv*, HPHP::StringData*, HPHP::ExecutionContext::InvokeFlags) at php:0
  8. 7 HPHP::ExecutionContext::invokeUnit(HPHP::TypedValue*, HPHP::Unit*) at php:0
  9. 8 HPHP::invoke_file(HPHP::String const&, bool, char const*) at php:0
  10. 9 HPHP::include_impl_invoke(HPHP::String const&, bool, char const*) at php:0
  11. 10 HPHP::hphp_invoke(HPHP::ExecutionContext*, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool, HPHP::Array const&, HPHP::VRefParamValue const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool&, std::basic_string<char, std::char_traits<char>, std::allocator<char> >&, bool, bool, bool) at php:0
  12. 11 HPHP::hphp_invoke_simple(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool) at php:0
  13. 12 ?? at php:0
  14. 13 HPHP::execute_program(int, char**) at php:0
  15. 14 HPHP::emulate_zend(int, char**) at php:0
  16. 15 main at php:0
  17. 16 __libc_start_main at /build/buildd/eglibc-2.19/csu/libc-start.c:321
  18. 17 ?? at php:0

PHP Stacktrace:

#0 Parser->callParserFunction(tplframe{}, invoke, Array) called at [/usr/local/apache/common-local/php-1.24wmf5/includes/parser/Parser.php:3416]
#1 Parser->braceSubstitution(Array, tplframe{}) called at [/usr/local/apache/common-local/php-1.24wmf5/includes/parser/Preprocessor_DOM.php:1153]
#2 PPFrame_DOM->expand(Object of class DOMElement could not be converted to string) called at [/usr/local/apache/common-local/php-1.24wmf5/includes/parser/Parser.php:3329]
#3 Parser->braceSubstitution(Array, tplframe{}) called at [/usr/local/apache/common-local/php-1.24wmf5/includes/parser/Preprocessor_DOM.php:1153]
#4 PPFrame_DOM->expand(Object of class DOMElement could not be converted to string) called at [/usr/local/apache/common-local/php-1.24wmf5/includes/parser/Parser.php:3568]
#5 Parser->braceSubstitution(Array, frame{}) called at [/usr/local/apache/common-local/php-1.24wmf5/includes/parser/Preprocessor_DOM.php:1153]
#6 PPFrame_DOM->expand(Object of class DOMElement could not be converted to string, 0) called at [/usr/local/apache/common-local/php-1.24wmf5/includes/parser/Parser.php:3226]
#7 Parser->replaceVariables({{δείτε|δικιά|-δικία}}

{{-el-}}

{{μορφή ουσιαστικού|el}}

'''{{PAGENAME}}''' {{ο}}

{{πτώσειςΟΑΚπλ|δίκιο}}

{{κλείδα-ελλ}}

[[en:δίκια]]) called at [/usr/local/apache/common-local/php-1.24wmf5/includes/parser/Parser.php:1238]
#8 Parser->internalParse({{δείτε|δικιά|-δικία}}

{{-el-}}

{{μορφή ουσιαστικού|el}}

'''{{PAGENAME}}''' {{ο}}

{{πτώσειςΟΑΚπλ|δίκιο}}

{{κλείδα-ελλ}}

[[en:δίκια]]) called at [/usr/local/apache/common-local/php-1.24wmf5/includes/parser/Parser.php:406]
#9 Parser->parse({{δείτε|δικιά|-δικία}}

{{-el-}}

{{μορφή ουσιαστικού|el}}

'''{{PAGENAME}}''' {{ο}}

{{πτώσειςΟΑΚπλ|δίκιο}}

{{κλείδα-ελλ}}

[[en:δίκια]], δίκια, Object of class ParserOptions could not be converted to string, 1, 1, rEWLE319442866aa0) called at [/usr/local/apache/common-local/php-1.24wmf5/includes/StubObject.php:105]
#10 StubObject->_call(parse, Array) called at [/usr/local/apache/common-local/php-1.24wmf5/includes/StubObject.php:125]
#11 StubObject->__call(parse, Array) called at [/usr/local/apache/common-local/php-1.24wmf5/includes/content/WikitextContent.php:327]
#12 WikitextContent->fillParserOutput(δίκια, rEWLE319442866aa0, Object of class ParserOptions could not be converted to string, 1, Object of class ParserOutput could not be converted to string) called at [/usr/local/apache/common-local/php-1.24wmf5/includes/content/AbstractContent.php:486]
#13 AbstractContent->getParserOutput(δίκια, rEWLE319442866aa0) called at [/usr/local/apache/common-local/php-1.24wmf5/extensions/CirrusSearch/includes/Updater.php:334]
#14 CirrusSearch\Updater->getContentAndParserOutput(Object of class WikiPage could not be converted to string) called at [/usr/local/apache/common-local/php-1.24wmf5/extensions/CirrusSearch/includes/Updater.php:286]
#15 CirrusSearch\Updater->buildDocumentsForPages(Array, 0) called at [/usr/local/apache/common-local/php-1.24wmf5/extensions/CirrusSearch/includes/Updater.php:161]
#16 CirrusSearch\Updater->updatePages(Array, 1ms, 5, 0) called at [/usr/local/apache/common-local/php-1.24wmf5/extensions/CirrusSearch/includes/Updater.php:67]
#17 CirrusSearch\Updater->updateFromTitle(δίκια) called at [/usr/local/apache/common-local/php-1.24wmf5/extensions/CirrusSearch/includes/LinksUpdateJob.php:47]
#18 CirrusSearch\LinksUpdateJob->doJob() called at [/usr/local/apache/common-local/php-1.24wmf5/extensions/CirrusSearch/includes/Job.php:52]
#19 CirrusSearch\Job->run() called at [/usr/local/apache/common-local/php-1.24wmf5/maintenance/runJobs.php:110]
#20 RunJobs->execute() called at [/usr/local/apache/common-local/php-1.24wmf5/maintenance/doMaintenance.php:109]
#21 include(/usr/local/apache/common-local/php-1.24wmf5/maintenance/doMaintenance.php) called at [/usr/local/apache/common-local/php-1.24wmf5/maintenance/runJobs.php:281]
#22 include(/usr/local/apache/common-local/php-1.24wmf5/maintenance/runJobs.php) called at [/usr/local/apache/common-local/multiversion/MWScript.php:97]


Version: 1.24rc
Severity: major

Attached:

Details

Reference
bz65796

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 3:25 AM
bzimport added a project: MediaWiki-Parser.
bzimport set Reference to bz65796.

Sorry, I didn't mean to paste the content of the attachment into the comment body.

This is mostly a LuaSandbox bug.

LuaSandbox::setCPULimit() calls convert_to_double_ex() on its argument, which decrefs the original zval and creates a new one that is a double. convert_to_double_ex() callers in the PHP source tree invariably use the "Z" type character in zend_parse_parameters(), which is what LuaSandbox used to do, but I changed it to "z" to support HHVM. With "Z", a zval** is returned, and so convert_to_double_ex() will leave the newly-allocated zval* in the stack, which will be decref'd on return. But with "z", the newly-allocated zval leaks, since the pointer is only stored in a local variable, and the argument zval is decref'd, which apparently breaks HHVM's frame cleanup.

(In reply to Tim Starling from comment #2)

This is mostly a LuaSandbox bug.

CC'ing anomie

Change 135942 had a related patch set uploaded by Ori.livneh:
Fix leak in LuaSandbox::setCPULimit

https://gerrit.wikimedia.org/r/135942

Change 135942 merged by jenkins-bot:
Fix leak in LuaSandbox::setCPULimit

https://gerrit.wikimedia.org/r/135942

Created attachment 15610
stack trace from osmium

This is still happening..

Attached:

Slightly reduced test case:

mwscript eval.php --wiki=elwiktionary
$out = $wgParser->parse('{{#invoke:Kleida-el|kleida}}{{#invoke:Kleida-el|reverseit}}', Title::newMainPage(), new ParserOptions);

I note the C stack trace reported in Tim's test case doesn't really match what Ori reported in comment 6. Not sure if that matters.

I managed to reduce Tim's test case a bit further, though:

<?php
$sandbox = new LuaSandbox;
$ret = $sandbox->loadString( 'return function() end', "=test" )->call();
$ret[0];
$ret[0];
?>

Then I did this:

<?php
$s = new LuaSandbox;
$f = $s->loadString( "return function() end", "=x" )->call();
debug_zval_dump( $f );

On zend PHP this says that the LuaSandboxFunction has a refcount of 1, while in HHVM it says it has a refcount of 0. The same happens with "return { function() end }", the LuaSandboxFunction has a refcount of 0.

At that point I ran out of luck in trying to figure out why it's coming out with a 0 refcount in HHVM and 1 in zend PHP.

I'm at the "how can this possibly work at all" stage now, which is usually a sign of progress. _object_and_properties_init() has:

Z_OBJVAL_P(arg) = HPHP::ObjectData::newInstance(cls);

at this point, the ObjectData's refcount is 0, which is apparently broken. Then it does:

// Zend doesn't have this, but I think we need it or else new objects have a
// refcount of 0
Z_ADDREF_P(arg);

The comment is apparently incorrect -- the RefData's refcount is 1 already and apparently doesn't need to be incremented, and it fails to increment the ObjectData's refcount.

The two bugs apparently sometimes cancel each other out, since the RefData stays live indefinitely and keeps the ObjectData alive. I'm still sorting through the exact chronology, but it seems some sequence of boxing and unboxing exposes the incorrect reference count in the ObjectData.

The reason it works at all is because when you pass the result of _object_and_properties_init() as the EZC function return value, the tvUnbox() at the end of zend_wrap_func() fixes the broken ObjectData refcount, because the RefData is leaked, not freed, so the decref of the RefData in tvUnbox() does not cause the ObjectData refcount to be decremented like it normally would.

If you return the result of _object_and_properties_init() to userspace any other way -- say by putting it into an array where it will be protected from tvUnbox() -- then the broken ObjectData refcount is exposed to userspace. In the first snippet of comment 8, the first $ret[0] causes the ObjectData's refcount to go up to 1, so that the ObjectData is freed when the result of the array access is freed. Then the second $ret[0] is a use-after-free.

  • Bug 66205 has been marked as a duplicate of this bug. ***
  • Bug 66936 has been marked as a duplicate of this bug. ***
  • Bug 65792 has been marked as a duplicate of this bug. ***