1654807 - [big endian] initializing nsTArray's header in thin_vec sets mIsAutoArray instead of mCapacity

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Jan Horak [:jhorak]]

The LocaleService class member variables gets corrupted when the fluent_langneg_negotiate_languages returns nsAutoTArray instead of nsTArray. This leads to later crash because mRequestedLocales array header is overwritten because mAppLocales is considered as nsAutoTArray (for example during SetLength(0) call).

This happens only on s390x. I really don't know to what is this related to.

Surprisingly the mCapacity and mIsAutoArray seems to be swapped:
S390X: (both there [1] )

(gdb) p *aRetVal.mHdr
$95 = {mLength = 1, mCapacity = 0, mIsAutoArray = 1}

x86_64:

(gdb) p *aRetVal.mHdr
$8 = {mLength = 1, mCapacity = 1, mIsAutoArray = 0}

[1] https://searchfox.org/mozilla-central/source/intl/locale/LocaleService.cpp#118

Comment 1

[Jan Horak [:jhorak]]

Guys, did you noticed that in 78 ESR?

Comment 2

[Jan Horak [:jhorak]]

With some help I've been able to track this down, looks like bug in thin_vec:
nsTArray->mHdr:

type = struct nsTArrayHeader {
uint32_t mLength;
uint32_t mCapacity : 31;
uint32_t mIsAutoArray : 1;
} 

thin_vec:

struct Header {
    _len: SizeType,
    _cap: SizeType,
}

(no sign of mIsAutoArray)

x/8x aRetVal->mHdr
before fluent_langneg_negotiate_languages call:
0x3fff9d6b310 <sEmptyTArrayHeader>:     0x00    0x00    0x00    0x00    0x00    0x00    0x00    0x00
after fluent_langneg_negotiate_languages call:
0x2aa00319dc0:                          0x00    0x00    0x00    0x01    0x00    0x00    0x00    0x01
(when manually setting mCapacity to 1 and resetting mIsAutoArray to 0 by using gdb)
0x2aa00319dc0:                          0x00    0x00    0x00    0x01    0x00    0x00    0x00    0x02

Comment 3

[Aria Beingessner [:Gankra]]

thin-vec does handle the mIsAutoArray bit, but evidently we need to properly redefine AUTO_MASK/CAP_MASK on big endian platforms: https://docs.rs/thin-vec/0.1.0/src/thin_vec/lib.rs.html#27

Comment 4

[Aria Beingessner [:Gankra]]

Potential fix here: https://github.com/Gankra/thin-vec/pull/18

Comment 5

[Aria Beingessner [:Gankra]]

thin-vec 0.2.0 has been published with a tentative fix, but I'm not sure how to test a big endian configuration

Comment 6

[Aria Beingessner [:Gankra]]

Attachment: Bug 1654807 - update thin-vec to 0.2.1 for potential endianess fix. r?jhorak

Comment 7

[Jan Horak [:jhorak]]

I finally had a chance to test the fix on s390x and it seems to be working fine. I'm not sure I'm the right person to do the review but I can definitely give a positive feedback. If that's enough for you I'll give you the review.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:Gankra, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b0fabcd60430
update thin-vec to 0.2.1 for potential endianess fix. r=froydnj

Comment 10

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/b0fabcd60430