Reviewers: Yury Semikhatsky, Kasper Lund, iposva,

Message:
You guys have some pretty creative uses of inlining and static variables
in here...

There are really two bugs here.

I wound up tracing most of this back to static variables within inlined
code.  When the inlined code comes from a header and is included and
used in multiple compilation units (.cc files), each unit gets its own
distinct definition of the static.  It doesn't seem like that's what
anyone intended.  I've de-inlined the cases that I caught causing
problems, ensuring uniform definitions of the statics across the entire
program.  It's possible that more of these are waiting to bite us.

That part's not a toolchain problem.  The next part, from parser.cc, is.

I caught v8::internal::AstBuildingParserFactory::EmptyStatement crashing
with a stripped symbol table (not dead-stripped sections) when compiled
in certain ways.  The crashes had nothing to do with our code, they were
occurring when trying to locate the guard variable used to protect the
static.  (We're not using -fno-threadsafe-statics on the Mac.)  It
appears that in certain rare circumstances, it's possible to get Apple
gcc and the Apple linker to strip something out of the symbol table that
dyld (the Apple loader) needs to perform a proper relocation to be able
to access the guard variable.  In this case, it was happening with a
static in an inlined virtual method when symbol visibility was left at
the default.  I'm going to try to reduce it to a good testcase and file
a bug with Apple.  The workaround here, too, was to de-inline the
function containing the static, but it was done for a different reason.
This inline function is only used within the parser.cc compilation unit,
and would ordinarily be safe to leave a static in, even with the Apple
compiler.

Disabling dead code stripping was an incomplete fix and dead-stripping
really wound up not being the actual culprit here.  Switching symbol
visibility settings while debugging this really wound up helping, and
showed what was happening almost immediately.  This had to do with the
way the linker resolves symbols when different visibility modes are
used, and subsequently, what's available for it to dead-strip.

Again, it's possible that there may be other inlined statics, I've only
fixed the cases here that were necessary to run mksnapshot without it
crashing.

Description:
Don't put static variables inline.

BUG=404

Please review this at http://codereview.chromium.org/149768

SVN Base: http://v8.googlecode.com/svn/branches/1.2/

Affected files:
   M     src/frame-element.h
   A     src/frame-element.cc
   M     src/ia32/register-allocator-ia32-inl.h
   M     src/parser.cc
   M     src/register-allocator.h
   M     src/register-allocator.cc
   M     tools/gyp/v8.gyp



--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to