Re: Non-header-only headers shared between SpiderMonkey and the rest of Gecko

[Gabriele Svelto]

On 05/08/19 12:04, Henri Sivonen wrote:
> I has come to my attention that that putting non-header-only code
> under mfbt/ is something we're trying to get away from:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1554062
> 
> Do we have an appropriate place for headers that declare entry points
> for non-header-only functionality (in my case, backed by Rust code)
> and that depend on types declared in headers that live under mfbt/ and
> that need to be available both to SpiderMonkey and the rest of Gecko?

IIRC we have some stuff like that under mozglue/misc. The TimeStamp
class for example is used in both Gecko and SpiderMonky, has
platform-dependent C++ implementations (also linking to external
libraries) and uses MFBT headers.

 Gabriele



signature.asc
Description: OpenPGP digital signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: NotNull and pointer parameters that may or may not be null

[Henri Sivonen]

On Mon, Jul 22, 2019 at 10:00 AM Karl Tomlinson  wrote:
> Google style requires pointers for parameters that may be mutated
> by the callee, which provides that the potential mutation is
> visible at the call site.  Pointers to `const` types are
> permitted, but recommended when "input is somehow treated
> differently" [1], such as when a null value may be passed.
>
> Comments at function declarations should mention
> "Whether any of the arguments can be a null pointer." [2]

I understand that there's value in adopting Google rules without
modification in order to avoid having to discuss which parts to adopt
and to avoid having to adapt tooling in accordance to the results of
such discussion. However, what should we think of Google disagreeing
with the C++ Core Guidelines, which can be expected to also have
larger ecosystem value as a tooling target and that in some sense
could be considered to come from higher C++ authority than Google's
rules?

On this particular topic, the Core Guidelines have:
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-inout
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f60-prefer-t-over-t-when-no-argument-is-a-valid-option
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f23-use-a-not_nullt-to-indicate-that-null-is-not-a-valid-value

(FWIW, I have relatively recently r+ed code that used non-const
references to indicate non-nullable modifiable arguments on the
grounds that it was endorsed by the Core Guidelines.)

-- 
Henri Sivonen
hsivo...@mozilla.com
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Non-header-only headers shared between SpiderMonkey and the rest of Gecko

[Henri Sivonen]

I has come to my attention that that putting non-header-only code
under mfbt/ is something we're trying to get away from:
https://bugzilla.mozilla.org/show_bug.cgi?id=1554062

Do we have an appropriate place for headers that declare entry points
for non-header-only functionality (in my case, backed by Rust code)
and that depend on types declared in headers that live under mfbt/ and
that need to be available both to SpiderMonkey and the rest of Gecko?

(So far, shipping headers that depend on types that come from mfbt/
inside the related crates.io crate has been suggested, but it seems
weird to ship Gecko-specific code via crates.io and Gecko developers
probably aren't looking for mfbt type-aware C++ API headers under
third_party/rust/.)

-- 
Henri Sivonen
hsivo...@mozilla.com
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

[desktop] Bugs logged by Desktop Release QA in the last 7 days

[Mihai Boldan]

Hello,

Here's the list of new issues found and filed by the Desktop Release QA 
team in the last 7 days.
Additional details on the team's priorities last week, as well as the 
plans for the current week are available at: https://tinyurl.com/y6g925jx.

Bugs logged by Desktop Release QA in the last 7 days:

Firefox: Distributions
NEW - https://bugzil.la/1569983 - [WIN] [Acer] Language pack is not 
installed after updating a win32 Beta build to latest Beta on Windows x64


Firefox: Downloads Panel
NEW    - https://bugzil.la/1569905 - [Ubuntu] Overflow Menu is not 
closed after opening "Show All Downloads" from Downloads Panel (Private 
window)


Firefox: Enterprise Policies
NEW - https://bugzil.la/1569623 - The user can bypass the popup blocking 
policy from the banner section


Firefox: Preferences
RESOLVED FIXED - https://bugzil.la/1569963 - Add Credit Card field texts 
are not displayed properly


Firefox: Tracking Protection
NEW - https://bugzil.la/1570968 - First-party tracker is not blocked 
correctly on certain machines


Core: Audio/Video: Playback
NEW - https://bugzil.la/1570632 - A playback error message will appear 
on Hulu after turning the computer on from sleep mode


Core: Graphics
* NEW - https://bugzil.la/1569971 - [mac] layers.acceleration.disabled 
set to true causes video controls on youtube to flicker
* NEW - https://bugzil.la/1569978 - Page goes blank when moving the node 
back in the Fx window


Core: Canvas: WebGL
* NEW - https://bugzil.la/1569933 - [Photo Wrapping] Parts of the border 
disappear while the image is twisted
* NEW - https://bugzil.la/1569976 - Games not working on 
https://www.crazygames.com/t/webgl after disabling hardware acceleration


Core: Layout
NEW - https://bugzil.la/1570277 - Clicking on an image to zoom it will 
close the image preview on Wikipedia


DevTools: Storage Inspector
NEW - https://bugzil.la/1569577 - Storage - increase min_width for 
cookies HttpOnly and SameSite columns


Web Compatibility: Desktop
NEW - https://bugzil.la/1569589 - Fullscreen button not working on 
https://www.justareflektor.com/


This is available as a Bugzilla bug list as well: 
https://tinyurl.com/y6et2tre.


Regards,
Mihai Boldan












___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: PHC, a probabilistic heap checker, will soon be enabled on Linux64 Nightly

[Nicholas Nethercote]

Hi again,

The enabling of PHC on Linux64 Nightly builds went smoothly, so I just
enabled it for Win64 Nightly builds (bug 1569864). This will increase the
number of users by roughly 10x.

If you have minidump access on crash-stats you can search for PHC crash
reports by searching for the presence of the `phc kind` field. The `phc
alloc stack` and `phc free stack` fields currently are not
auto-symbolicated; Christian Holler has a script that can do the
symbolication.

Nick


On Fri, 19 Jul 2019 at 13:32, Nicholas Nethercote 
wrote:

> Greetings,
>
> PHC is a probabilistic heap checker I have been working on. It has landed
> and I am planning to enable it on Monday morning AEST (which is Sunday
> afternoon/evening in many parts of the world). For now it will only be
> enabled on Linux64 Nightly builds (and also local developer builds) but I
> hope to also enable it on Win64 Nightly builds soon.
>
> Here is a short description of PHC that comes from the comment at the top
> of memory/replace/phc/PHC.cpp:
>
> // PHC is a probabilistic heap checker. A tiny fraction of randomly chosen
> heap
> // allocations are subject to some expensive checking via the use of OS
> page
> // access protection. A failed check triggers a crash, whereupon useful
> // information about the failure is put into the crash report. The cost and
> // coverage for each user is minimal, but spread over the entire user base
> the
> // coverage becomes significant.
>
> I have included the rest of that comment, which describes the
> implementation in more detail, at the bottom of this email. Also see bug
> 1523276  for
> additional details.
>
> This is a tool that will hopefully detect use-after-free errors in the
> wild. It is a somewhat complex wrapper of our heap allocator, and
> everything depends on the heap allocator. This means that any bugs in PHC
> have the potential to cause a wide variety of problems. Please keep an eye
> out for any such problems. It's hard to say exactly what those problems
> might be... during testing I saw and fixed some deadlocks and some crashes
> within arena_dalloc(), but other effects may be possible. Also, it's
> completely non-deterministic (by design) which further complicates the
> detection of problems.
>
> Please let me know if you have any questions or concerns.
>
> Nick
>
>
> // The idea comes from Chromium, where it is called GWP-ASAN. (Firefox
> uses PHC
> // as the name because GWP-ASAN is long, awkward, and doesn't have any
> // particular meaning.)
> //
> // In the current implementation up to 64 allocations per process can
> become
> // PHC allocations. These allocations must be page-sized or smaller. Each
> PHC
> // allocation gets its own page, and when the allocation is freed its page
> is
> // marked inaccessible until the page is reused for another allocation.
> This
> // means that a use-after-free defect (which includes double-frees) will be
> // caught if the use occurs before the page is reused for another
> allocation.
> // The crash report will contain stack traces for the allocation site, the
> free
> // site, and the use-after-free site, which is often enough to diagnose the
> // defect.
> //
> // The design space for the randomization strategy is large. The current
> // implementation has a large random delay before it starts operating, and
> a
> // small random delay between each PHC allocation attempt. Each freed PHC
> // allocation is quarantined for a medium random delay before being
> reused, in
> // order to increase the chance of catching UAFs.
> //
> // The basic cost of PHC's operation is as follows.
> //
> // - The memory cost is 64 * 4 KiB = 256 KiB per process (assuming 4 KiB
> //   pages) plus some metadata (including stack traces) for each page.
> //
> // - Every allocation requires a size check and a decrement-and-check of an
> //   atomic counter. When the counter reaches zero a PHC allocation can
> occur,
> //   which involves marking a page as accessible and getting a stack trace
> for
> //   the allocation site. Otherwise, mozjemalloc performs the allocation.
> //
> // - Every deallocation requires a range check on the pointer to see if it
> //   involves a PHC allocation. (The choice to only do PHC allocations
> that are
> //   a page or smaller enables this range check, because the 64 pages are
> //   contiguous. Allowing larger allocations would make this more
> complicated,
> //   and we definitely don't want something as slow as a hash table lookup
> on
> //   every deallocation.) PHC deallocations involve marking a page as
> //   inaccessible and getting a stack trace for the deallocation site.
> //
> // In the future, we may add guard pages between the used pages in order
> // to detect buffer overflows/underflows. This would change the memory
> cost to
> // (64 * 2 + 1) * 4 KiB = 516 KiB per process and complicate the machinery
> // somewhat.
>
___
dev-pl