MOZ_THREAD_LOCAL currently assumes its invocations live in the global
namespace, which may not always be true, e.g. when declaring a static
class member whose enclosing class lives in `namespace mozilla` or
similar. We should qualify the name lookups required to always start
from the global namespace to avoid such problems.
With `ac_add_options --enable-project=tools/crashreporter` in a
mozconfig, `./mach build` builds minidump_stackwalk, dump_syms
and fileid.
One caveat is that due to limitation in how the build system works
currently, it's cumbersome to keep dump_syms as a host program for
Gecko, and to make it a target program for this project. For now,
keep it as a host program. We're not going to use it on automation,
but it's still convenient to have for quick local builds (I've had
to resort to awful hacks downstream).
Differential Revision: https://phabricator.services.mozilla.com/D16299
--HG--
extra : moz-landing-system : lando
Fails with clang trunk:
"type of UTF-8 string literal will change from array of const char to array of const char8_t in C++2a"
otherwise
Differential Revision: https://phabricator.services.mozilla.com/D14696
--HG--
extra : moz-landing-system : lando
As discussed in the previous commit message, HashTableEntry wastes space
for common entry types. This commit reorganizes the internal storage to
store all the hashes packed together, followed by all the entries, which
eliminates the aforementioned waste.
HashTableEntry's data layout currently wastes a fair amount of space due
to ABI-mandated padding. For instance, HashTableEntry<T*> on a 64-bit
platform looks like:
class HashTableEntry {
HashNumber mKeyHash;
// Four bytes of wasted space here to pad mValueData to the correct place.
unsigned char mValueData[sizeof(T*)];
};
This wasted space means that sets of pointers backed by
mozilla::HashTable waste a quarter of their entry storage space. Maps
of pointers to pointers waste a sixth of their entry storage space.
We'd like to fix this by packing all the cached hashes together,
followed by all the hash table entries.
As a first step to laying out the hash table storage differently, we
have to make HashTable not access entries directly, but go through an
abstraction that represents the key and the entry. We call this
abstraction "slots". This commit is similar to the change done for
PLDHashTable previously.
Parts of HashTable still work in terms of Entry; the creation and
destruction of tables was not worth changing here. We'll address that
in the next commit.
We do this to make our lives easier in later patches; this check
guarantees that we don't need padding between the block of cached hash
values and the block of entries immediately after it.
As discussed in the previous commit message, HashTableEntry wastes space
for common entry types. This commit reorganizes the internal storage to
store all the hashes packed together, followed by all the entries, which
eliminates the aforementioned waste.
HashTableEntry's data layout currently wastes a fair amount of space due
to ABI-mandated padding. For instance, HashTableEntry<T*> on a 64-bit
platform looks like:
class HashTableEntry {
HashNumber mKeyHash;
// Four bytes of wasted space here to pad mValueData to the correct place.
unsigned char mValueData[sizeof(T*)];
};
This wasted space means that sets of pointers backed by
mozilla::HashTable waste a quarter of their entry storage space. Maps
of pointers to pointers waste a sixth of their entry storage space.
We'd like to fix this by packing all the cached hashes together,
followed by all the hash table entries.
As a first step to laying out the hash table storage differently, we
have to make HashTable not access entries directly, but go through an
abstraction that represents the key and the entry. We call this
abstraction "slots". This commit is similar to the change done for
PLDHashTable previously.
Parts of HashTable still work in terms of Entry; the creation and
destruction of tables was not worth changing here. We'll address that
in the next commit.
We do this to make our lives easier in later patches; this check
guarantees that we don't need padding between the block of cached hash
values and the block of entries immediately after it.
- modify line wrap up to 80 chars; (tw=80)
- modify size of tab to 2 chars everywhere; (sts=2, sw=2)
--HG--
extra : rebase_source : 7eedce0311b340c9a5a1265dc42d3121cc0f32a0
extra : amend_source : 9cb4ffdd5005f5c4c14172390dd00b04b2066cd7
This is a best effort attempt at ensuring that the adverse impact of
reformatting the entire tree over the comments would be minimal. I've used a
combination of strategies including disabling of formatting, some manual
formatting and some changes to formatting to work around some clang-format
limitations.
Differential Revision: https://phabricator.services.mozilla.com/D13193
--HG--
extra : moz-landing-system : lando
These macros tend to be handled quite poorly since the clang-format
tokenizer cannot figure out how to handle them.
Differential Revision: https://phabricator.services.mozilla.com/D13179
--HG--
extra : moz-landing-system : lando
HasFreeLSB currently fails for any types that map to void*. Unless we fix this,
we can't use certain types as template parameters for mozilla::Result.
Differential Revision: https://phabricator.services.mozilla.com/D12358
--HG--
extra : moz-landing-system : lando
Rename MOZ_MakeRecordReplayWrapper to MOZ_MAKE_RECORD_REPLAY_WRAPPER so
that clang-format doesn't mangle code.
Differential Revision: https://phabricator.services.mozilla.com/D12233
--HG--
extra : moz-landing-system : lando
The current rust panic hook keeps a string for the crash reporter, and
goes on calling the default rust panic hook, which prints out a crash
stack... when RUST_BOOTSTRAP is set *and* when that works. Notably, on
both mac and Windows, it only really works for local builds, but fails
for debug builds from automation, although on automation itself, we also
do stackwalk from crash minidumps, which alleviates the problem.
Artifact debug builds are affected, though.
More importantly, C++ calls to e.g. MOZ_CRASH have a similar but
different behavior, in that they dump a stack trace on debug builds, by
default (with exceptions, see below for one). The format of those stack
traces is understood by the various fix*stack*py scripts under
tools/rb/, that are used by the various test harnesses both on
automation and locally.
Additionally, the current rust panic hook, as it calls the default rust
panic hook, ends up calling abort() on non-Windows platforms, which ends
up being verbosely redirected to mozalloc_abort per
https://dxr.mozilla.org/mozilla-central/rev/237e4c0633fda8e227b2ab3ab57e417c980a2811/memory/mozalloc/mozalloc_abort.cpp#79
which then calls MOZ_CRASH. Theoretically, /that/ would also print a
stack trace, but doesn't because currently the stack trace printing code
lives in libxul, and MOZ_CRASH only calls it when compiled from
libxul-code, which mozalloc_abort is not part of.
With this change, we make the rust panic handler call back into
MOZ_CRASH directly. This has multiple advantages:
- This is more consistent cross-platforms (Windows is not special
anymore).
- This is more consistent between C++ and rust (stack traces all look
the same, and can all be post-processed by fix*stack*py if need be)
- This is more consistent in behavior, where debug builds will show
those stack traces without caring about environment variables.
- It demangles C++ symbols in rust-initiated stack traces (for some
reason that didn't happen with the rust panic handler)
A few downsides:
- the loss of demangling for some rust symbols.
- the loss of addresses in the stacks, although they're not entirely
useful
- extra empty lines.
The first should be fixable later one. The latter two are arguably
something that should be consistent across C++ and rust, and should be
changed if necessary, independently of this patch.
Depends on D11719
Depends on D11719
Differential Revision: https://phabricator.services.mozilla.com/D11720
--HG--
extra : moz-landing-system : lando
Because we don't really need all this repetition.
Depends on D11718
Depends on D11718
Differential Revision: https://phabricator.services.mozilla.com/D11719
--HG--
extra : moz-landing-system : lando
Ideally, we'd want the function to stay in Assertions.cpp, but that's
only part of MFBT proper, and that doesn't have access to WalkTheStack
like MOZ_CRASH has from being in Assertion.h, when included from Gecko
code. Moving WalkTheStack to mozglue, putting it close together with
MozStackWalk would be prefered, but that causes problems linking MFBT
tests (which don't have access to mozglue), and other things.
Overall, this was too deep a rabbit hole, and moving MOZ_CrashOOL to
Assertions.h is much simpler. Since it's essentially the same as
MOZ_CRASH, except it allows non-literal strings, we can make it inlined,
and leave it to the compiler to drop the filename argument when it's not
used.
Differential Revision: https://phabricator.services.mozilla.com/D11718
--HG--
extra : moz-landing-system : lando
The current rust panic hook keeps a string for the crash reporter, and
goes on calling the default rust panic hook, which prints out a crash
stack... when RUST_BOOTSTRAP is set *and* when that works. Notably, on
both mac and Windows, it only really works for local builds, but fails
for debug builds from automation, although on automation itself, we also
do stackwalk from crash minidumps, which alleviates the problem.
Artifact debug builds are affected, though.
More importantly, C++ calls to e.g. MOZ_CRASH have a similar but
different behavior, in that they dump a stack trace on debug builds, by
default (with exceptions, see below for one). The format of those stack
traces is understood by the various fix*stack*py scripts under
tools/rb/, that are used by the various test harnesses both on
automation and locally.
Additionally, the current rust panic hook, as it calls the default rust
panic hook, ends up calling abort() on non-Windows platforms, which ends
up being verbosely redirected to mozalloc_abort per
https://dxr.mozilla.org/mozilla-central/rev/237e4c0633fda8e227b2ab3ab57e417c980a2811/memory/mozalloc/mozalloc_abort.cpp#79
which then calls MOZ_CRASH. Theoretically, /that/ would also print a
stack trace, but doesn't because currently the stack trace printing code
lives in libxul, and MOZ_CRASH only calls it when compiled from
libxul-code, which mozalloc_abort is not part of.
With this change, we make the rust panic handler call back into
MOZ_CRASH directly. This has multiple advantages:
- This is more consistent cross-platforms (Windows is not special
anymore).
- This is more consistent between C++ and rust (stack traces all look
the same, and can all be post-processed by fix*stack*py if need be)
- This is more consistent in behavior, where debug builds will show
those stack traces without caring about environment variables.
- It demangles C++ symbols in rust-initiated stack traces (for some
reason that didn't happen with the rust panic handler)
A few downsides:
- the loss of demangling for some rust symbols.
- the loss of addresses in the stacks, although they're not entirely
useful
- extra empty lines.
The first should be fixable later one. The latter two are arguably
something that should be consistent across C++ and rust, and should be
changed if necessary, independently of this patch.
Depends on D11719
Differential Revision: https://phabricator.services.mozilla.com/D11720
--HG--
extra : moz-landing-system : lando
Ideally, we'd want the function to stay in Assertions.cpp, but that's
only part of MFBT proper, and that doesn't have access to WalkTheStack
like MOZ_CRASH has from being in Assertion.h, when included from Gecko
code. Moving WalkTheStack to mozglue, putting it close together with
MozStackWalk would be prefered, but that causes problems linking MFBT
tests (which don't have access to mozglue), and other things.
Overall, this was too deep a rabbit hole, and moving MOZ_CrashOOL to
Assertions.h is much simpler. Since it's essentially the same as
MOZ_CRASH, except it allows non-literal strings, we can make it inlined,
and leave it to the compiler to drop the filename argument when it's not
used.
Differential Revision: https://phabricator.services.mozilla.com/D11718
--HG--
extra : moz-landing-system : lando
Ideally, we'd want the function to stay in Assertions.cpp, but that's
only part of MFBT proper, and that doesn't have access to WalkTheStack
like MOZ_CRASH has from being in Assertion.h, when included from Gecko
code. Moving WalkTheStack to mozglue, putting it close together with
MozStackWalk would be prefered, but that causes problems linking MFBT
tests (which don't have access to mozglue), and other things.
Overall, this was too deep a rabbit hole, and moving MOZ_CrashOOL to
Assertions.h is much simpler. Since it's essentially the same as
MOZ_CRASH, except it allows non-literal strings, we can make it inlined,
and leave it to the compiler to drop the filename argument when it's not
used.
Differential Revision: https://phabricator.services.mozilla.com/D11718
--HG--
extra : moz-landing-system : lando
The logic in JSMath for generating cryptographically-secure
pseudorandom numbers without NSS is independently useful, and so
it's been moved to a common area.
It will eventually be used for generated random arena ids.
Differential Revision: https://phabricator.services.mozilla.com/D8597
--HG--
extra : moz-landing-system : lando
Sylvestre Ledru
Nathan Froyd
Cosmin Sabou
Andrew McCreight
David Major
Mike Hommey
Ehsan Akhgari
Olli Pettay
Henri Sivonen
Tooru Fujisawa
Benjamin Bouvier
Jeff Walden
Brian Hackett
Aaron Klotz
Ted Campbell
arthur.iakab
Dorel Luca
Coroiu Cristina
Steve Fink
Chris Martin