On win32, NS_InvokeByIndex is implemented with inline assembly. This
inline assembly assumes that it is wrapped by the compiler with the
standard x86 prologue and epilogue:
push ebp
mov ebp, esp
[inline assembly that manipulates the stack pointer]
pop ebp
ret
In particular, the last instruction of the inline assembly is:
mov esp, ebp
which cancels out the effects of the stack manipulation performed by all
the inline assembly that proceeds the instruction.
When compiling with clang-cl, however, the above assumption does not
hold, as clang-cl inserts a more complex prologue and epilogue,
something like:
push ebp
mov ebp, esp
sub esp, frame_size
[save registers into stack frame]
[inline assembly that manipulates the stack pointer]
[restore registers from stack frame]
add esp, frame_size
mov esp, ebp
pop ebp
ret
Combining this more extensive prologue and epilogue with the assumptions
of the inline assembly leads to interesting crashes when
NS_InvokeByIndex is called: the inline assembly effectively deallocates
the stack allocated by the inline assembly *and* the stack frame
allocated by the compiler itself. The compiler-generated code then
attemptes to deallocate the stack frame, leading to the crash, as the
code now returns to an unspecified address.
To avoid these sorts of problems in clang-cl and make the code more
robust generally, let's move the NS_InvokeByIndex implementation to a
separate assembly file. We can then write exactly what we need to have
happen, safe from any manipulations of the compiler.
Since we don't compile much (any?) code in Gecko with MASM, we need to
add the /SAFESEH flag to the assembler invocation so that the object
file with be appropriately marked as not containing exception handlers;
the linker (which is invoked with the /SAFESEH flag itself) will then
consent to link it into libxul.
A bunch of threads have their wait time set to PR_INTERVAL_NO_TIMEOUT and so
we divide this by 4 and set waitTime to that. This causes us to wait a very
long but not PR_INTERVAL_NO_TIMEOUT amount of time and so we still update
mIntervalNow and think that we've been hung for a long time when comparing
to the mInterval in the current thread which is set to the previous value of
mIntervalNow.
The JS engine does not export symbols outside of XUL, so having these
defined inside mozglue apparently causes linking errors on some
platforms with the patches in bug 1120016.
This patch moves enough methods outside of mozglue that the patch in
that other bug will still link on all platforms, without moving so
much out that there are other linking errors.
The patch changes all uses of SizeOfIncludingThisMustBeUnshared() to
SizeOfIncludingThisIfUnshared(). This incurs the (tiny) cost of an unnecessary
IsReadonly() check for guaranteed-unshared strings, but avoids the possible
assertion failures that would occur when MustBeUnshared() was used incorrectly
on shared strings, which is an easy mistake to make.
When people write:
array.AppendElement(nsDependentString(...));
(resp. nsDependentCString), it's not clear whether they expect the newly
constructed dependent string to live in the array, or whether they're
just making a nsString-like holder whose contents can be freely copied
into the array's newly-created nsString. Sometimes the latter is what
you prefer, and sometimes the former. In all cases, however, the latter
behavior is what you get.
Let's try to make that behavior more explicit by pre-constructing
nsString elements and then using Assign to show that copying is taking
place. This patch involves no functional change in behavior (it ought
to be epsilon faster due to using AppendElements, rather than repeatedly
calling AppendElement).
This patch moves the logic for selecting MOZ_WINCONSOLE out of individual
Makefile.in files and into configure. It also changes config.mk to only
pass -SUBSYSTEM:CONSOLE if MOZ_WINCONSOLE=1. The MSDN docs state that
in the absence of -SUBSYSTEM, the linker will select the proper subsystem
based on whether the program contains [w]main or [w]WinMain, so let it
do that.
One program (windbgdlg) needed a tweak to add a wmain for when MOZ_WINCONSOLE
is defined.
This patch leaves one instance in security/sandbox/win/wow_helper/Makefile.in,
that Makefile has its own separate bug.
Dehydra/Treehydra is unmaintained, broken (iirc), and obsoleted by clang
static analysis. We've removed parts of the build system support for it, but
not all. This is meant to remove the remains.
Previously we were only logging if we accumulated 50ms of lag. Start logging
all lag so we can use this measure to compare smaller changes in UI
responsiveness.
xpcom/glue/PLDHashTable.cpp:471:10 [-Wunreachable-code-return] 'return' will never be executed
xpcom/tests/TestAutoPtr.cpp:324:9 [-Wunreachable-code] code will never be executed
xpcom/tests/TestBlockingProcess.cpp:6:11 [-Wunreachable-code-return] 'return' will never be executed
The logging interface is moved to xpcom/base, a LogModule wrapper for PR_Log is
added, a thread-safe LogModuleManager is added, and a LazyLogModule class used
to lazily load log modules in a thread-safe manner is added.
The bulk of this commit was generated with a script, executed at the top
level of a typical source code checkout. The only non-machine-generated
part was modifying MFBT's moz.build to reflect the new naming.
CLOSED TREE makes big refactorings like this a piece of cake.
# The main substitution.
find . -name '*.cpp' -o -name '*.cc' -o -name '*.h' -o -name '*.mm' -o -name '*.idl'| \
xargs perl -p -i -e '
s/nsRefPtr\.h/RefPtr\.h/g; # handle includes
s/nsRefPtr ?</RefPtr</g; # handle declarations and variables
'
# Handle a special friend declaration in gfx/layers/AtomicRefCountedWithFinalize.h.
perl -p -i -e 's/::nsRefPtr;/::RefPtr;/' gfx/layers/AtomicRefCountedWithFinalize.h
# Handle nsRefPtr.h itself, a couple places that define constructors
# from nsRefPtr, and code generators specially. We do this here, rather
# than indiscriminantly s/nsRefPtr/RefPtr/, because that would rename
# things like nsRefPtrHashtable.
perl -p -i -e 's/nsRefPtr/RefPtr/g' \
mfbt/nsRefPtr.h \
xpcom/glue/nsCOMPtr.h \
xpcom/base/OwningNonNull.h \
ipc/ipdl/ipdl/lower.py \
ipc/ipdl/ipdl/builtin.py \
dom/bindings/Codegen.py \
python/lldbutils/lldbutils/utils.py
# In our indiscriminate substitution above, we renamed
# nsRefPtrGetterAddRefs, the class behind getter_AddRefs. Fix that up.
find . -name '*.cpp' -o -name '*.h' -o -name '*.idl' | \
xargs perl -p -i -e 's/nsRefPtrGetterAddRefs/RefPtrGetterAddRefs/g'
if [ -d .git ]; then
git mv mfbt/nsRefPtr.h mfbt/RefPtr.h
else
hg mv mfbt/nsRefPtr.h mfbt/RefPtr.h
fi
This commit was generated using the following script, executed at the
top level of a typical source code checkout.
# Don't modify select files in mfbt/ because it's not worth trying to
# tease out the dependencies currently.
#
# Don't modify anything in media/gmp-clearkey/0.1/ because those files
# use their own RefPtr, defined in their own RefCounted.h.
find . -name '*.cpp' -o -name '*.h' -o -name '*.mm' -o -name '*.idl'| \
grep -v 'mfbt/RefPtr.h' | \
grep -v 'mfbt/nsRefPtr.h' | \
grep -v 'mfbt/RefCounted.h' | \
grep -v 'media/gmp-clearkey/0.1/' | \
xargs perl -p -i -e '
s/mozilla::RefPtr/nsRefPtr/g; # handle declarations in headers
s/\bRefPtr</nsRefPtr</g; # handle local variables in functions
s#mozilla/RefPtr.h#mozilla/nsRefPtr.h#; # handle #includes
s#mfbt/RefPtr.h#mfbt/nsRefPtr.h#; # handle strange #includes
'
# |using mozilla::RefPtr;| is OK; |using nsRefPtr;| is invalid syntax.
find . -name '*.cpp' -o -name '*.mm' | xargs sed -i -e '/using nsRefPtr/d'
# RefPtr.h used |byRef| for dealing with COM-style outparams.
# nsRefPtr.h uses |getter_AddRefs|.
# Fixup that mismatch.
find . -name '*.cpp' -o -name '*.h'| \
xargs perl -p -i -e 's/byRef/getter_AddRefs/g'
It's a little more convenient than checking Count(), and also gives
nsTHashtable the same interface as nsTArray (for this operation, at
least), which seems worthwhile.
|getReportsForThisProcess| differs from |getReports| in that it is limited to current process and is synchronous. When asynchronous memory reporters are added the function will no longer be able tobe synchronous. There isn't much utility in only measuring the current process, so we can remove the function and switch existing users to |getReports|.
The calculation of |explicit| relies on the synchronous |getReportsForThisProcess|, once we have asynchronous reporters this will no longer work. As it is currently referenced in the about::memory tests we can just remove it.
The configure option has explicitly thrown an error for more than a year now,
and it happens that the remaining way to still forcefully use it has been
broken for more than 8 months.
Having a template parameter conflict with a global name is terribly
inconvenient, so let's try to avoid that by renaming the 'RefPtr'
template parameter to something else.
Leave a typedef for compatibility. nsVariant will be defined as a
separate class in the next patch.
Also, remove an obsolete comment and fix some whitespace.
|getReportsForThisProcess| differs from |getReports| in that it is limited to current process and is synchronous. When asynchronous memory reporters are added the function will no longer be able tobe synchronous. There isn't much utility in only measuring the current process, so we can remove the function and switch existing users to |getReports|.
The calculation of |explicit| relies on the synchronous |getReportsForThisProcess|, once we have asynchronous reporters this will no longer work. As it is currently referenced in the about::memory tests we can just remove it.
This class can be used instead of raw pointer for a sound leaking-by-default
behavior. Also it could take advantage of move semantic check in the future.
Using forget() to extract mMessage from MessageElement ends up going
from nsCOMPtr<T> to already_AddRefed<T> to nsCOMPtr<T>. For the second
step, the compiler can't tell that the already_AddRefed<T> came from a
canonical nsCOMPtr, so it calls Assert_NoQueryNeeded() in debug
builds. This in turn causes a QI, which does an AddRef. That is bad
because we're not on the main thread, and mMessage is
main-thread-only, so we get an assertion.
This patch works around that by using swap directly between two
nsCOMPtr<>, which avoids the Assert_NoQueryNeeded().
I called the method "swapMessage" rather than "swap" to emphasize that
we are not swapping the whole MessageElement, but just one part of
it. I find the existing forget() name to be confusing.
We don't need to reinterpret_cast when casting from void*, and we don't
need to reinterpret_cast when we're casting up and down a class
hierarchy. static_cast takes care of those cases just fine, and doesn't
scare the reader into thinking that nsTHashtable is doing something
unusual.
