flypig.co.uk

It was a huge relief to finally wrap up the WebRTC changes yesterday. When I started this process I had my suspicions that it would be the hardest part, hence I left it to the end. It turned out not to be as hard as fixing the rendering pipeline. Actually it turned out to be surprisingly straightforward, notwithstanding a few bumps along the way, and either way it's now done. So I'm happy.

During the process, back on Day 285, we did see that the colours for the WebRTC video feed looked wrong. After playing around for a bit this morning I've concluded that the issue is with the red and blue channels being swapped over. I'm not planning to spend the time fixing this just yet; it needs fixing for sure, but there are other more important tasks to complete. In the meantime, I've lodged an issue on the browser bug tracker so that it doesn't get forgotten about.
 
I was happy to rapidly receive replies back from Frajo (krnlyng) and Björn (Thaodan). They suggest that the issue is happening in gecko-camera. If that's the case, then I'm satisfied that it doesn't have to be fixed within the gecko code.

That brings me on nicely to my next task, which is to work my way through the remaining bugs on this tracker to see what really needs addressing. I have three other important tasks as well, which in totality gives me the following on my to-do list:

1. Work through the outstanding issues tagged with ESR 91 in the bug tracker.
2. Respond to Simon Schmeisser and Raine's comments on various PRs.
3. Package up versions of all the projects so that brave members of the community can test them out.
4. Convert all of the commits on the FIREFOX_ESR_91_9_X_RELBRANCH_patches branch into patches.

The last of these is more than just a mechanical process. Not only do I need to try to align the patches with those in the existing ESR 78 build, I also want to try to rationalise the remaining patches to minimise overlaps between them.

So that's my four-point plan. And as has been the case throughout this process, there's no point delaying, so I'm going to get straight on to working through the issues on the bug tracker.

First up is bug #1055, an apparent crash when visiting amazon.de. When I try a few pages on the site I don't experience any issues, but there's an example page given in my earlier bug report that, according to what I've written, consistently causes the ESR 91 browser to crash. And when I try this I do indeed still get a crash. There's a backtrace in the bug report, but here's the latest one that I got today.

This is the most enormous backtrace with depth of 177 frames, so I'm not going to include all of them here. But I want to flag up it's surprising depth as it may turn out to be relevant.

Thread 10 "GeckoWorkerThre" received signal SIGSEGV, Segmentation 
    fault.
[Switching to LWP 31398]
0x0000007ff57c9eb4 in mapped_hyph::Hyphenator::level () from /usr/lib64/
    xulrunner-qt5-91.9.1/libxul.so
(gdb)
(gdb) bt
#0  0x0000007ff57c9eb4 in mapped_hyph::Hyphenator::level () from /usr/lib64/
    xulrunner-qt5-91.9.1/libxul.so
#1  0x0000007ff57ca0a0 in mapped_hyph::Hyphenator::find_hyphen_values () from /
    usr/lib64/xulrunner-qt5-91.9.1/libxul.so
#2  0x0000007ff57caed8 in mapped_hyph_find_hyphen_values_raw () from /usr/lib64/
    xulrunner-qt5-91.9.1/libxul.so
#3  0x0000007ff1a2856c in nsHyphenator::<lambda(mozilla::UniquePtr<base::
    SharedMemory, mozilla::DefaultDelete<base::SharedMemory> >&)>::operator() (
    shm=..., __closure=<optimized out>)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/nsTArray.h:1183
#4  mozilla::detail::VariantImplementation<unsigned char, 1, mozilla::
    UniquePtr<base::SharedMemory, mozilla::DefaultDelete<base::SharedMemory> >, 
    mozilla::UniquePtr<HyphDic const, mozilla::DefaultDelete<HyphDic const> > >:
    :matchN<mozilla::Variant<void const*, mozilla::UniquePtr<base::
    SharedMemory, mozilla::DefaultDelete<base::SharedMemory> >, mozilla::
    UniquePtr<const HyphDic, mozilla::DefaultDelete<const HyphDic> > >&, 
    nsHyphenator::HyphenateWord(const nsAString&, uint32_t, uint32_t, 
    nsTArray<bool>&)::<lambda(mozilla::UniquePtr<base::SharedMemory>&)>, 
    nsHyphenator::HyphenateWord(const nsAString&, uint32_t, uint32_t, 
    nsTArray<bool>&)::<lambda(mozilla::UniquePtr<const HyphDic, mozilla::
    DefaultDelete<const HyphDic> >&)> > (aMi=..., aV=...)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/Variant.h:309
#5  mozilla::detail::VariantImplementation<unsigned char, 0, void const*, 
    mozilla::UniquePtr<base::SharedMemory, mozilla::DefaultDelete<base::
    SharedMemory> >, mozilla::UniquePtr<HyphDic const, mozilla::
    DefaultDelete<HyphDic const> > >::matchN<mozilla::Variant<void const*, 
    mozilla::UniquePtr<base::SharedMemory, mozilla::DefaultDelete<base::
    SharedMemory> >, mozilla::UniquePtr<const HyphDic, mozilla::
    DefaultDelete<const HyphDic> > >&, nsHyphenator::HyphenateWord(const 
    nsAString&, uint32_t, uint32_t, nsTArray<bool>&)::<lambda(void const*&)>, 
    nsHyphenator::HyphenateWord(const nsAString&, uint32_t, uint32_t, 
    nsTArray<bool>&)::<lambda(mozilla::UniquePtr<base::SharedMemory, mozilla::
    DefaultDelete<base::SharedMemory> >&)>, nsHyphenator::HyphenateWord(const 
    nsAString&, uint32_t, uint32_t, nsTArray<bool>&)::<lambda(mozilla::
    UniquePtr<const HyphDic, mozilla::DefaultDelete<const HyphDic> >&)> > (
    aMi=..., aV=...)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/Variant.h:318
#6  mozilla::Variant<void const*, mozilla::UniquePtr<base::SharedMemory, 
    mozilla::DefaultDelete<base::SharedMemory> >, mozilla::UniquePtr<HyphDic 
    const, mozilla::DefaultDelete<HyphDic const> > >::matchN<mozilla::
    Variant<void const*, mozilla::UniquePtr<base::SharedMemory, mozilla::
    DefaultDelete<base::SharedMemory> >, mozilla::UniquePtr<const HyphDic, 
    mozilla::DefaultDelete<const HyphDic> > >&, nsHyphenator::HyphenateWord(
    const nsAString&, uint32_t, uint32_t, nsTArray<bool>&)::<lambda(void 
    const*&)>, nsHyphenator::HyphenateWord(const nsAString&, uint32_t, 
    uint32_t, nsTArray<bool>&)::<lambda(mozilla::UniquePtr<base::
    SharedMemory>&)>, nsHyphenator::HyphenateWord(const nsAString&, uint32_t, 
    uint32_t, nsTArray<bool>&)::<lambda(mozilla::UniquePtr<const HyphDic, 
    mozilla::DefaultDelete<const HyphDic> >&)> > (aM1=..., aM0=..., 
    aVariant=...)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/Variant.h:902
#7  mozilla::Variant<void const*, mozilla::UniquePtr<base::SharedMemory, 
    mozilla::DefaultDelete<base::SharedMemory> >, mozilla::UniquePtr<HyphDic 
    const, mozilla::DefaultDelete<HyphDic const> > >::match<nsHyphenator::
    HyphenateWord(const nsAString&, uint32_t, uint32_t, nsTArray<bool>&)::
    <lambda(void const*&)>, nsHyphenator::HyphenateWord(const nsAString&, 
    uint32_t, uint32_t, nsTArray<bool>&)::<lambda(mozilla::UniquePtr<base::
    SharedMemory>&)>, nsHyphenator::HyphenateWord(const nsAString&, uint32_t, 
    uint32_t, nsTArray<bool>&)::<lambda(mozilla::UniquePtr<const HyphDic, 
    mozilla::DefaultDelete<const HyphDic> >&)> > (
    aM1=..., aM0=..., this=0x7e512e3188)
    at ${PROJECT}/obj-build-mer-qt-xr/dist/include/mozilla/Variant.h:857
#8  nsHyphenator::HyphenateWord (this=this@entry=0x7e512e3180, aString=..., 
    aStart=aStart@entry=0, aLimit=aLimit@entry=7, aHyphens=...)
    at intl/hyphenation/glue/nsHyphenator.cpp:444
#9  0x0000007ff1a2884c in nsHyphenator::Hyphenate (
    this=this@entry=0x7e512e3180, aString=..., aHyphens=...)
    at intl/hyphenation/glue/nsHyphenator.cpp:378
#10 0x0000007ff28e3890 in nsLineBreaker::FindHyphenationPoints (
    this=this@entry=0x7fde7c6e90, aHyphenator=aHyphenator@entry=0x7e512e3180,
    aTextStart=<optimized out>, aTextLimit=<optimized out>, 
    aBreakState=0x7fde7c4a08 &quot;&quot;)
    at dom/base/nsLineBreaker.cpp:322
[...]
#177 0x0000007fef54989c in ?? () from /lib64/libc.so.6
(gdb)

As I mentioned, this has 177 frames, which makes me think it could be a stack overflow that's the problem. It's not clear to me whether this is therefore a bug (in the sense of an error in the code) or just a mismatch between the memory available and the complexity of this particular page. The error is happening during an attempt to hyphenate a word, which doesn't seem like an especially atypical or extraordinary activity.

I'm not planning to fix all of the remaining issues in the bug tracker, but having a crashing browser is no good so I do feel I need to get to the bottom of this.

The crash is happening inside some Rust code, which is interesting since, as far as I'm aware, this is the first time there's been a problem with the Rust code outside of the build process. We're not getting full debugging information as a result, but it looks like the error is happening with one of the calls to level() in mapped_hyph/src/lib.rs here:

    /// Identify acceptable hyphenation positions in the given `word`.
    ///
    /// The caller-supplied `values` must be at least as long as the `word`.
    ///
    /// On return, any elements with an odd value indicate positions in the word
    /// after which a hyphen could be inserted.
    ///
    /// Returns the number of possible hyphenation positions that were found.
    ///
    /// # Panics
    /// If the given `values` slice is too small to hold the results.
    ///
    /// If the block of memory represented by `self.0` is not in fact a valid
    /// hyphenation dictionary, this function may panic with an overflow or
    /// array bounds violation.
    pub fn find_hyphen_values(&self, word: &str, values: &mut [u8]) -> isize {
        assert!(values.len() >= word.len());
        values.iter_mut().for_each(|x| *x = 0);
        let top_level = self.level(0);
        let (lh_min, rh_min, clh_min, crh_min) = top_level.word_boundary_mins();
        if word.len() < lh_min + rh_min {
            return 0;
        }
        let mut hyph_count = top_level.find_hyphen_values(word, values, lh_min, 
    rh_min);
        let compound = hyph_count > 0;
        // Subsequent levels are applied to fragments between potential breaks
        // already found:
        for l in 1 .. self.num_levels() {
            let level = self.level(l);
            if hyph_count > 0 {
[...]

I've included the comment here in case the part about it potentially panicking in two different situations turns out to be important. Could one of these panics be the cause of the crash?

The first panic about the values slide being too small looks like the easiest one to follow back through the code. The length of this buffer seems to be determined by the length of the hyphenValues array created in the nsHyphenator::HyphenateWord() method in the nsHyphenator.cpp file:

  AutoTArray<uint8_t, 200> hyphenValues;
  hyphenValues.SetLength(utf8.Length());
  int32_t result = mDict.match(
      [&](const void*& ptr) {
        return mapped_hyph_find_hyphen_values_raw(
            static_cast<const uint8_t*>(ptr), mDictSize, utf8.BeginReading(),
            utf8.Length(), hyphenValues.Elements(), hyphenValues.Length());
      },
      [&](UniquePtr<base::SharedMemory>& shm) {
        return mapped_hyph_find_hyphen_values_raw(
            static_cast<const uint8_t*>(shm->memory()), mDictSize,
            utf8.BeginReading(), utf8.Length(), hyphenValues.Elements(),
            hyphenValues.Length());
      },
      [&](UniquePtr<const HyphDic>& hyph) {
        return mapped_hyph_find_hyphen_values_dic(
            hyph.get(), utf8.BeginReading(), utf8.Length(),
            hyphenValues.Elements(), hyphenValues.Length());
      });

This is then getting passed on to this method in mapped_hyph/src/ffi.rs:

pub unsafe extern "C" fn mapped_hyph_find_hyphen_values_raw(dic_buf: 
    *const u8, dic_len: u32,
                                                            word: *const 
    c_char, word_len: u32,
                                                            hyphens: *mut u8, 
    hyphens_len: u32) -> i32 {
    if word_len > hyphens_len {
        return -1;
    }
    let (word_str, hyphen_buf) = params_from_c(word, word_len, hyphens, 
    hyphens_len);
    if word_str.is_err() {
        return -1;
    }
    Hyphenator::new(slice::from_raw_parts(dic_buf, dic_len as usize))
        .find_hyphen_values(word_str.unwrap(), hyphen_buf) as i32
}

I'm going to need a bit more time and mental energy to figure out what's happening here, so I'm going to have to leave the rest of the analysis until the morning. I'm thinking that I could do a couple of things: increase the size of the return buffer; add a check to the Rust code; remove the call to the Rust code entirely. The aim wouldn't be to fix the problem, but rather to explore whether we're in the right place or not.

I'll sleep on it and see how these ideas feel in the morning.

If you'd like to read any of my other gecko diary entries, they're all available on my Gecko-dev Diary page.