1619231 - [s390x] Startup crash in sqlite3VdbeExec: Assertion `pIn3->flags & MEM_Blob' failed.

Status: RESOLVED FIXED

(Toolkit :: Storage, defect, P3)

Description

[msirringhaus]

Attachment: s390x-sqlite.patch

Current mozilla-central crashes shortly after startup on s390x (big endian), with:

mozilla-central/third_party/sqlite3/src/sqlite3.c:89376: sqlite3VdbeExec: Assertion `pIn3->flags & MEM_Blob' failed.

This is a sqlite-bug, fixed with this upstream checkin: https://www.sqlite.org/src/info/04885763c4cd00cb

I've attached an in-tree patch as well.

Once a new sqlite-version is released, please update.

Comment 1

[msirringhaus]

Should have added. The affected sqilte-version is 3.31.1
Anything later should contain the fix.

Comment 2

[Marco Bonardo [:mak]]

Considered this has been merged upstream, we could also take the patch for Firefox 75, though I suppose distribution maintainers can pick it as well, so I wonder if it's effectively useful to patch now, or we can just wait the next Sqlite release. Thoughts?

Comment 3

[msirringhaus]

I'd be fine with waiting for the next sqlite release, if that happens before the next ESR-release of Firefox. I very much would like to have that fix in there.
In principle, I created this patch mostly to be found by others, who need a fix right now.

Comment 4

[Marco Bonardo [:mak]]

The next ESR will be Firefox 78 that is quite far in the future (2020-09-22), if you want this fix in the next ESR minor version (68.7, since I think it's too late for 68.6) then we need to push the patch to that branch.

Comment 5

[msirringhaus]

I don't think the 68-branch is affected. At least 68.6 build 1 still has sqlite 3.28.0. And that worked so far.

Comment 6

[Marco Bonardo [:mak]]

you are right, I didn't think it through.

Comment 7

[Petr Sumbera]

Thank you very much for addressing this and cross linking it with:

[Bug 1602836] [meta] Support for big endian

It saved my day on Solaris SPARC.

Comment 8

[msirringhaus]

Any news on this with regard to having this fixed in FF 78 esr?

Comment 9

[Marco Bonardo [:mak]]

Unfortunately the sqlite update didn't make 78 due to timing (there was an additional .1 release that caused skipping).
I'd be ok taking the sqlite diff pointed by comment 0 into firefox 78, if it applies cleanly, since it has already been merged upstream. Since we don't use anymore system Sqlite from 75 on, that would solve the problem.
Ryan, wdyt?

Comment 10

[Marco Bonardo [:mak]]

Comment on attachment 9130104 [details] [diff] [review]
s390x-sqlite.patch

Beta/Release Uplift Approval Request

• User impact if declined: Ths fixes a compatibility problem with s390x (BigEndian), without this patch the system is not supported.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch has been applied to Sqlite upstream just after the 3.31.1 release, see https://www.sqlite.org/src/info/04885763c4cd00cb. The change looks safe.
• String changes made/needed:

Comment 11

[Julien Cristau [:jcristau] (back April 22)]

I can take this on beta, but the patch is missing authorship information and a commit message.

Comment 12

[Marco Bonardo [:mak]]

That's an interesting question, the patch is pretty much from https://www.sqlite.org/src/info/04885763c4cd00cb, I guess we can use drh info and the original commit message. let me prepare it.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Otherwise, I often create a "Backport upstream XYZ fix" patch with my name on it and a link to the upstream commit in question in these situations.

Comment 14

[Marco Bonardo [:mak]]

Attachment: Upstream patch backport

Beta/Release Uplift Approval Request

• User impact if declined: Ths fixes a compatibility problem with s390x (BigEndian), without this patch the system is not supported.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch has been applied to Sqlite upstream just after the 3.31.1 release, see https://www.sqlite.org/src/info/04885763c4cd00cb. The change looks safe.
• String changes made/needed:

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9157112 [details] [diff] [review]
Upstream patch backport

Approved for 78.0b8.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/80025f9399df

Comment 17

[Marco Bonardo [:mak]]

Do we already need a separate approval/landing for ESR? I see you didn't set its flag.

Comment 18

[Julien Cristau [:jcristau] (back April 22)]

We're good, beta gets merged to esr78 for now.