Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Issue 52105 Segfault in pp::Module::GetBrowserInterface
Starred by 2 users Reported by polina@google.com, Aug 13 2010 Back to list
Status: Fixed
Owner:
Closed: Aug 2010
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 0
Type: Bug

Restricted
  • Only users with Commit permission may comment.



Sign in to add a comment
This CL describes the issue and provides a temporary fix for DontManage Vars. http://codereview.chromium.org/3136002/show

To reproduce the problem, remove the fix from Var copy constructor and use ppapi/example.
 
Comment 1 by polina@google.com, Aug 14 2010
Labels: -Pri-2 Pri-0
From an email by Brett:

Albert,

We're having a problem which I think is related to the way you set up
the pp::Module singleton when running the example plugin. This is a
high priority to fix as it's blocking NaCl.

If you use the attached html in example.html adn then click on the
button, it will crash trying to get the browser "Var" interface
because g_module_singleton is NULL. This fails on 64-bit Linux. I
don't know about other platforms.

It seems to be getting confused about which version of the code to use
(plugin versus chrome.so), and ends up with the wrong one in this
case. To repro, build the ppapi_example plugin, and use this for the
html (you need the additional button added in this version).


<body style="background-image:url(http://www.google.com/intl/en_ALL/images/logo.gif);
            background-repeat:repeat">

<script type="text/javascript">
function Test() {
   plugin = document.getElementById('plugin');
   alert(plugin.toString(new Array(10)));
}
</script>

 <button onclick='Test()'>Test</button>
 <div id="fps" style="background-color:white; font-weight:bold;
padding:4px;">FPS GOES HERE</div>
 <object id="plugin" type="application/x-ppapi-example" width="400"
height="400" />
 <hr>
</body>


The thing seems to be whether the copy constructor is called from STL
versus being called directly. One easy way to see a narrowed case is
to add this code to Call at line 99 in scriptable_object.cc:

 if (argc > 0) {
   Var a(Var::DontManage(), argv[0]);
   Var b(a);
   std::vector<Var> c;
   c.push_back(a);
 }

This is invoked when you press the button in the example.html file and
use the ppapi_example plugin. The copy constructor for b gets called
and works. The copy constructor invoked from the vector c uses the
wrong module and crashes.

Could this have anything to do with PIC? Darin noticed a TODO in the
ppapi.gyp file.

Brett
Comment 2 by darin@chromium.org, Aug 15 2010
This sounds like a classic issue of the same symbols existing in both an executable and a shared library.  The shared library will see the version of those symbols defined in the executable because the executable was loaded first.  We need to hide the symbols in the executable to prevent this problem.

It would be interesting to see a stack trace that includes the location (which module, chrome or nacl.so) of the various functions on the call stack.  Perhaps when the copy constructor is called from STL, it is somehow reaching the version of the symbol in the plugin instead of the version of the symbol in chrome.

I think http://codereview.chromium.org/3136002/show should be reverted.
Comment 3 by darin@chromium.org, Aug 15 2010
I wouldn't remove DontManage on account of this bug.  This bug doesn't justify it.  The underlying problem would still bite us elsewhere.

DontManage is a reasonable performance improvement that has very little cost to maintain.  It doesn't cause any problems, so I'd keep it.
Comment 4 by polina@google.com, Aug 15 2010
Here is the stack trace for chrome/nacl at ppapi rev 188:

Program received signal SIGSEGV, Segmentation fault.
0x0000000002712cf0 in pp::Module::GetBrowserInterface (this=0x0, 
    interface_name=0x3d21abd "PPB_Var;1")
    at third_party/ppapi/cpp/module.cc:340
340       return get_browser_interface_(interface_name);
(gdb) bt
#0  0x0000000002712cf0 in pp::Module::GetBrowserInterface (this=0x0, 
    interface_name=0x3d21abd "PPB_Var;1")
    at third_party/ppapi/cpp/module.cc:340
#1  0x0000000002717fa6 in EnsureInit () at third_party/ppapi/cpp/var.cc:34
#2  0x0000000002718faa in Var (this=0x7fffeccd04c0, other=...)
    at third_party/ppapi/cpp/var.cc:90
#3  0x000000000263cb40 in __gnu_cxx::new_allocator<pp::Var>::construct (
    this=0x7fffffffb620, __p=0x7fffeccd04c0, __val=...)
    at /usr/include/c++/4.2/ext/new_allocator.h:107
#4  0x000000000263d383 in std::vector<pp::Var, std::allocator<pp::Var> >::push_back (this=0x7fffffffb620, __x=...)
    at /usr/include/c++/4.2/bits/stl_vector.h:601
#5  0x00007fffedca6ce7 in ArgListToVector (argc=1, argv=0x7fffecc27430, 
    output=0x7fffffffb620)
    at /home/polina/src-nacl/ppapi/cpp/scriptable_object.cc:38
#6  0x00007fffedca6f0a in Call (object=0x7fffc8a7edc0, method_name=..., 
    argc=1, argv=0x7fffecc27430, exception=0x7fffffffb6e0)
    at /home/polina/src-nacl/ppapi/cpp/scriptable_object.cc:102
#7  0x0000000001736872 in WrapperClass_Invoke (object=0x7fffc8a7e500, 
    method_name=0x7fffc8a7ef00, argv=0x7fffeccd0500, argc=1, 
    result=0x7fffffffb770) at webkit/glue/plugins/pepper_var.cc:272
#8  0x0000000001af9cfe in npObjectInvokeImpl (args=..., 
    functionId=WebCore::InvokeMethod)
    at third_party/WebKit/WebCore/bindings/v8/V8NPObject.cpp:118
#9  0x0000000001af9e82 in WebCore::npObjectMethodHandler (args=...)
    at third_party/WebKit/WebCore/bindings/v8/V8NPObject.cpp:149
#10 0x000000000138cdbf in HandleApiCallHelper<false> (args=...)
    at v8/src/builtins.cc:972
#11 0x000000000138ce83 in Builtin_Impl_HandleApiCall (args=...)
    at v8/src/builtins.cc:989
#12 0x000000000138ceaa in Builtin_HandleApiCall (args=...)
    at v8/src/builtins.cc:988
#13 0x00007fffc8af01aa in ?? ()
#14 0x00007fffc8af56f4 in ?? ()

The version from Chrome is pulled in. When I was looking for a fix, I had to rebuild Chrome to make the crash go away.

As for DontManage, Brett mentioned that he introduced it before he realized how cheap reference counting actually was. I think he wants to remove it independently of this bug.
Comment 5 by darin@chromium.org, Aug 16 2010
That stack trace gives the memory address of the functions, and it resolves them to source, but it doesn't say which module (which shared library or executable) the functions reside in.  For that you probably need to look in /proc/<pid>/maps (IIRC) to see which module those addresses fall into.  This is easier to do while you have it stuck in the debugger.  I can help you look at this tomorrow if you catch me at a good time :-)
Comment 6 by darin@chromium.org, Aug 16 2010
> As for DontManage, Brett mentioned that he introduced it before he realized how 
> cheap reference counting actually was. I think he wants to remove it independently 
> of this bug.

OK, but this bug isn't justification.  That's all I wanted to point out :)
Comment 7 by polina@google.com, Aug 16 2010
The stack trace shows the file path. When the function comes from Chrome, the path starts with third_party/ppapi/cpp. When the function comes from plugin, the path starts with /home/polina/src-nacl/ppapi/cpp. 
Comment 8 by polina@google.com, Aug 18 2010
A way to reproduce this failure coming from ~Var() in a Chrome client (as of lkgr 56556)
- make chrome
- make ppapi_example
- open example/example.html in chrome
- click on the cyan rectangle
- observe the crash

The stack trace is:
Program received signal SIGSEGV, Segmentation fault.
0x00000000027f8cac in pp::Module::GetBrowserInterface (this=0x0, 
    interface_name=0x3e0cb80 "PPB_Var;1")
    at third_party/ppapi/cpp/module.cc:359
359       return get_browser_interface_(interface_name);
(gdb) bt
#0  0x00000000027f8cac in pp::Module::GetBrowserInterface (this=0x0, 
    interface_name=0x3e0cb80 "PPB_Var;1")
    at third_party/ppapi/cpp/module.cc:359
#1  0x00000000027fe207 in operator _ppb_Var const* (this=0x5950980)
    at third_party/ppapi/cpp/module_impl.h:18
#2  0x00000000027ff29d in ~Var (this=0x7fffc93a7000, 
    __in_chrg=<value optimized out>) at third_party/ppapi/cpp/var.cc:108
#3  0x0000000002721f78 in std::_Destroy<pp::Var> (__pointer=0x7fffc93a7000)
    at /usr/include/c++/4.2/bits/stl_construct.h:107
#4  0x0000000002721f95 in std::__destroy_aux<pp::Var*> (
    __first=0x7fffc93a7000, __last=0x7fffc93aa9a0)
    at /usr/include/c++/4.2/bits/stl_construct.h:122
#5  0x0000000002721fca in std::_Destroy<pp::Var*> (__first=0x7fffc93a7000, 
    __last=0x7fffc93aa9a0) at /usr/include/c++/4.2/bits/stl_construct.h:155
#6  0x0000000002721fed in std::_Destroy<pp::Var*, pp::Var> (
    __first=0x7fffc93a7000, __last=0x7fffc93aa9a0)
    at /usr/include/c++/4.2/bits/stl_construct.h:182
#7  0x00000000027fc051 in ~vector (this=0x7fffffffa320, 
    __in_chrg=<value optimized out>)
    at /usr/include/c++/4.2/bits/stl_vector.h:268
#8  0x00007fffedcbf533 in MyInstance::SayHello (this=0x7ffff7e456c0)
    at third_party/ppapi/example/example.cc:372
#9  0x00007fffedcbf5ab in MyInstance::HandleEvent (this=0x7ffff7e456c0, 
---Type <return> to continue, or q <return> to quit---
    event=...) at third_party/ppapi/example/example.cc:182
#10 0x00007fffedcc2008 in pp::Instance_HandleEvent (
    pp_instance=140737352411776, event=0x7fffedb368d0)
    at third_party/ppapi/cpp/module.cc:108
#11 0x00000000017b76e6 in pepper::PluginInstance::HandleInputEvent (
    this=0x7ffff7e5aa80, event=..., cursor_info=0x7fffffffa470)
    at webkit/glue/plugins/pepper_plugin_instance.cc:390
#12 0x00000000017ca237 in pepper::WebPluginImpl::handleInputEvent (
    this=0x7fffeda6ee00, event=..., cursor_info=...)
    at webkit/glue/plugins/pepper_webplugin_impl.cc:105
#13 0x0000000001afb573 in WebKit::WebPluginContainerImpl::handleMouseEvent (
    this=0x7fffedb16d90, event=0x7ffff7e46be0)
    at third_party/WebKit/WebKit/chromium/src/WebPluginContainerImpl.cpp:420
#14 0x0000000001afc1ac in WebKit::WebPluginContainerImpl::handleEvent (
    this=0x7fffedb16d90, event=0x7ffff7e46be0)
    at third_party/WebKit/WebKit/chromium/src/WebPluginContainerImpl.cpp:170
#15 0x0000000001d2be34 in WebCore::HTMLPlugInElement::defaultEventHandler (
    this=0x7ffff7eb1a80, event=0x7ffff7e46be0)
    at third_party/WebKit/WebCore/html/HTMLPlugInElement.cpp:170
#16 0x0000000001c22f5f in WebCore::Node::dispatchGenericEvent (
    this=0x7ffff7eb1a80, prpEvent=...)
    at third_party/WebKit/WebCore/dom/Node.cpp:2760
#17 0x0000000001c231b3 in WebCore::Node::dispatchEvent (this=0x7ffff7eb1a80,

Comment 9 by ajwong@chromium.org, Aug 19 2010
I just tried building and am unable to repro the crash.  Do I need to pass any flags into chrome?  I built at r56616 on Hardy for x86-64.


ajwong@ajwong:~/src/git-chrome/src$ file out/Debug/chrome
out/Debug/chrome: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), for GNU/Linux 2.6.8, dynamically linked (uses shared libs), not stripped


ajwong@ajwong:~/src/git-chrome/src$ gcc -v
Using built-in specs.
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --enable-languages=c,c++,fortran,objc,obj-c++,treelang --prefix=/usr --enable-shared --with-system-zlib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --enable-nls --with-gxx-include-dir=/usr/include/c++/4.2 --program-suffix=-4.2 --enable-clocale=gnu --enable-libstdcxx-debug --enable-objc-gc --enable-mpfr --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 4.2.4 (Ubuntu 4.2.4-1ubuntu4)

Comment 10 by polina@google.com, Aug 19 2010
Here is what I did

src$ gclient update --revision=56556 # this was last known good revision at the time
src$ make -j30 chrome # GYP_DEFINES="target_arch=x64"
src$ make ppapi_example
src$ out/Debug/chrome --register-pepper-plugins="out/Debug/lib.target/libppapi_example.so;application/x-ppapi-example" http://www/~polina/example.html
# click on the plugin area
# crash

================================================================================

src$ file out/Debug/chrome
out/Debug/chrome: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), for GNU/Linux 2.6.8, dynamically linked (uses shared libs), not stripped

src$ gcc -v
Using built-in specs.
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --enable-languages=c,c++,fortran,objc,obj-c++,treelang --prefix=/usr --enable-shared --with-system-zlib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --enable-nls --with-gxx-include-dir=/usr/include/c++/4.2 --program-suffix=-4.2 --enable-clocale=gnu --enable-libstdcxx-debug --enable-objc-gc --enable-mpfr --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 4.2.4 (Ubuntu 4.2.4-1ubuntu4)

Ah...I didn't register the plugin.  I've got a repro now.  Trying to debug.
Status: Assigned
The problem is that in linux, we are dumping all the symbols into the dynamic symbol table during link.  This is being caused by the -rdynamic flag being passed to the final link line for the chrome binary, which seems to effectively override all the -fvisibility* happiness that is given in the compile line for the individual objects.

If I remove -rdynamic from the final link line, then I can run the plugin, and click on it without breaking things.  This isn't a 100% clean test since I also added -fvisibility-hidden-inlines into ppapi.gyp's cflags, but removing rdynamic is necessary, even if it isn't sufficient, to fix this.

So who put this dastardly flag into common.gypi?  Me. On my first change to chrome trying to get stack traces working in linux. :(  It is needed in order for backtrace_symbols() to work. I love my life.

Since then, satorux added in code for a custom symbolizer that I think was supposed to work w/o needing to have the symbols exported into the dynamic symbol table.

I'll play with this tomorrow to verify.

Really, we should probably become more disciplined with what symbols are actually exported from the chrome binary at all, probably by using a linker version script or something.

Evan, Satoru, Mark, any thoughts on switching over completely to our custom symbolizer and removing -rdynamic from the build?  Or on explicitly whitelisting the symbols exported into hhe dynamic symbol table from the chrome binary?
The symbolizer I added should work without -rdyamic. I think it's safe to remove -rdynamic, if the main purpose of the flag was to get backtrace_symbols() to work.

http://src.chromium.org/viewvc/chrome/trunk/src/base/third_party/symbolize/

Patch to fix this is here:

  http://codereview.chromium.org/3195010/show

Polina, if you get a sec, can you try patching it down and seeing if it fixes things?  Alternately, you could try a release build.  That should also make things work...
Comment 15 by polina@google.com, Aug 20 2010
With the patch I am no longer getting a crash with ppapi_example clicking or NaCl.
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=56924 

------------------------------------------------------------------------
r56924 | ajwong@chromium.org | 2010-08-20 15:55:34 -0700 (Fri, 20 Aug 2010) | 12 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/build/common.gypi?r1=56924&r2=56923

Remove -rdynamic from the linux debug build.

Having -rdynamic breaks some plugins since it exports more symbols into
the dynamic symbol table than wanted. In particular, this breaks users of
ppapi's C++ wrappers. It was added to make StaceTraces resolve to symbols
on linux. But now we use the symbolize library from glog so it isn't needed
anymore.

BUG= 52105 
TEST=Compiles and StackTrace.* test in base_unittests will work.

Review URL: http://codereview.chromium.org/3119033
------------------------------------------------------------------------

Comment 17 by evan@chromium.org, Aug 20 2010
Should we cherry-pick this to M6?
Status: Fixed
It's debug build only, so I think it's unnecessary.

Marking bug as fixed.
Labels: Feature-Plugins-Pepper
Project Member Comment 20 by bugdroid1@chromium.org, Oct 13 2012
Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member Comment 21 by bugdroid1@chromium.org, Mar 10 2013
Labels: -Area-Internals -Feature-Plugins-Pepper Cr-Content-Plugins-Pepper Cr-Internals
Project Member Comment 22 by bugdroid1@chromium.org, Apr 6 2013
Labels: Cr-Blink
Project Member Comment 23 by bugdroid1@chromium.org, Apr 6 2013
Labels: Cr-Internals-Plugins
Project Member Comment 24 by bugdroid1@chromium.org, Apr 6 2013
Labels: -Cr-Content-Plugins-Pepper Cr-Internals-Plugins-Pepper
Sign in to add a comment