Last modified: 2014-07-18 00:37:34 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T67796, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 65796 - HHVM segfaults when calling Parser->callParserFunction
HHVM segfaults when calling Parser->callParserFunction
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
Parser (Other open bugs)
1.24rc
All All
: Normal major (vote)
: ---
Assigned To: Tim Starling
: hhvm
: 65792 66205 66936 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-05-27 06:02 UTC by Ori Livneh
Modified: 2014-07-18 00:37 UTC (History)
4 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
Stack trace (6.08 KB, application/octet-stream)
2014-05-27 06:02 UTC, Ori Livneh
Details
stack trace from osmium (30.09 KB, application/octet-stream)
2014-06-09 21:10 UTC, Ori Livneh
Details

Description Ori Livneh 2014-05-27 06:02:20 UTC
Created attachment 15481 [details]
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

# 0  ?? at php:0
# 1  killpg at /lib/x86_64-linux-gnu/libc.so.6:0
# 2  ?? at php:0
# 3  HPHP::JIT::X64::BackEnd::enterTCHelper(unsigned char*, HPHP::JIT::TReqInfo&) at php:0
# 4  HPHP::JIT::MCGenerator::enterTC(unsigned char*, void*) at php:0
# 5  HPHP::ExecutionContext::enterVM(HPHP::ActRec*, HPHP::ExecutionContext::StackArgsState, HPHP::Resumable*, HPHP::ObjectData*) at php:0
# 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
# 7  HPHP::ExecutionContext::invokeUnit(HPHP::TypedValue*, HPHP::Unit*) at php:0
# 8  HPHP::invoke_file(HPHP::String const&, bool, char const*) at php:0
# 9  HPHP::include_impl_invoke(HPHP::String const&, bool, char const*) at php:0
# 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
# 11 HPHP::hphp_invoke_simple(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool) at php:0
# 12 ?? at php:0
# 13 HPHP::execute_program(int, char**) at php:0
# 14 HPHP::emulate_zend(int, char**) at php:0
# 15 main at php:0
# 16 __libc_start_main at /build/buildd/eglibc-2.19/csu/libc-start.c:321
# 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, 3194428) 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(δίκια, 3194428, 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(δίκια, 3194428) 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]
Comment 1 Ori Livneh 2014-05-27 06:03:02 UTC
Sorry, I didn't mean to paste the content of the attachment into the comment body.
Comment 2 Tim Starling 2014-05-28 06:14:00 UTC
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.
Comment 3 Andre Klapper 2014-05-28 13:19:07 UTC
(In reply to Tim Starling from comment #2)
> This is mostly a LuaSandbox bug.

CC'ing anomie
Comment 4 Gerrit Notification Bot 2014-05-28 21:50:46 UTC
Change 135942 had a related patch set uploaded by Ori.livneh:
Fix leak in LuaSandbox::setCPULimit

https://gerrit.wikimedia.org/r/135942
Comment 5 Gerrit Notification Bot 2014-06-02 05:47:27 UTC
Change 135942 merged by jenkins-bot:
Fix leak in LuaSandbox::setCPULimit

https://gerrit.wikimedia.org/r/135942
Comment 6 Ori Livneh 2014-06-09 21:10:11 UTC
Created attachment 15610 [details]
stack trace from osmium

This is still happening..
Comment 7 Tim Starling 2014-06-13 00:08:49 UTC
Slightly reduced test case:

mwscript eval.php --wiki=elwiktionary
$out = $wgParser->parse('{{#invoke:Kleida-el|kleida}}{{#invoke:Kleida-el|reverseit}}', Title::newMainPage(), new ParserOptions);
Comment 8 Brad Jorsch 2014-06-13 19:14:19 UTC
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.
Comment 9 Tim Starling 2014-06-16 06:52:46 UTC
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.
Comment 10 Tim Starling 2014-06-16 23:28:41 UTC
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.
Comment 11 Tim Starling 2014-06-17 04:48:10 UTC
There's a fix in my dev branch, to be submitted as a PR once I've finished testing it: <https://github.com/tstarling/hiphop-php/commit/24005dee6a113e87cbb2bc715274de31f614fcb2>
Comment 12 Tim Starling 2014-06-17 05:41:10 UTC
https://github.com/facebook/hhvm/pull/2959
Comment 13 Tim Starling 2014-06-17 06:20:54 UTC
*** Bug 66205 has been marked as a duplicate of this bug. ***
Comment 14 Ori Livneh 2014-07-15 21:29:01 UTC
*** Bug 66936 has been marked as a duplicate of this bug. ***
Comment 15 Ori Livneh 2014-07-18 00:37:34 UTC
*** Bug 65792 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links