Merge ~paelzer/ubuntu/+source/dovecot:fix-FTFBS-HIRSUTE into ubuntu/+source/dovecot:ubuntu/hirsute-devel

Proposed by Christian Ehrhardt 
Status: Merged
Approved by: Christian Ehrhardt 
Approved revision: e4e8986470372c4e0b81b0116389b5baab1f0c7e
Merge reported by: Bryce Harrington
Merged at revision: e4e8986470372c4e0b81b0116389b5baab1f0c7e
Proposed branch: ~paelzer/ubuntu/+source/dovecot:fix-FTFBS-HIRSUTE
Merge into: ubuntu/+source/dovecot:ubuntu/hirsute-devel
Diff against target: 81 lines (+23/-3) (has conflicts)
4 files modified
debian/changelog (+7/-0)
debian/control (+2/-1)
debian/patches/test-backtrace.patch (+2/-2)
debian/rules (+12/-0)
Conflict in debian/changelog
Conflict in debian/rules
Reviewer Review Type Date Requested Status
Lucas Kanashiro Approve
Canonical Server Team Pending
Bryce Harrington Pending
Review via email: mp+400403@code.launchpad.net
To post a comment you must log in.
Christian Ehrhardt  (paelzer) wrote :

Not yet ready - two fixes good, now hitting a third issue.

test-istream-attachment.c:354: Assert failed: memcmp(data + sizeof(BINARY_TEXT_LONG)-1, BINARY_TEXT_SHORT, strlen(BINARY_TEXT_SHORT)) == 0
istream attachment ................................................... : FAILED
Panic: file test-istream-attachment.c: line 395 (test_istream_attachment_extractor_one): assertion failed: (size >= prefix_len && memcmp(data, mail_broken_input_body_prefix, prefix_len) == 0)
Error: Raw backtrace: ./test-istream-attachment(+0x4cd95) [0x55c0db91bd95] -> ./test-istream-attachment(backtrace_get+0x75) [0x55c0db91bf65] -> ./test-istream-attachment(+0x2a7fb) [0x55c0db8f97fb] -> ./test-istream-attachment(+0x2a837) [0x55c0db8f9837] -> ./test-istream-attachment(+0x13c5c) [0x55c0db8e2c5c] -> ./test-istream-attachment(+0x12d39) [0x55c0db8e1d39] -> ./test-istream-attachment(+0x1cca3) [0x55c0db8ebca3] -> ./test-istream-attachment(+0x2424d) [0x55c0db8f324d] -> ./test-istream-attachment(test_run+0x63) [0x55c0db8f32f3] -> /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xd5) [0x7f60d232d565] -> ./test-istream-attachment(_start+0x2e) [0x55c0db8e7c2e]
/bin/bash: line 1: 1650909 Aborted (core dumped) ./$bin
make[4]: *** [Makefile:1463: check-local] Error 1

Bryce Harrington (bryce) :
Christian Ehrhardt  (paelzer) wrote :

> > + * d/p/test-backtrace.patch: update to fix FTFBS with glibc 2.33 and later
>
> Ah, I'd seen a s/backtrace_append/test_backtrace/ (or maybe vice versa) in my googling. Was that due to a symbol change, or just a mistake? If it was due to a symbol change that might be worth mention here, in case it randomly crops up again.

Not really a symbol change, upstreams original code looks for "main"
but on some architectures that isn't present so debian changed it.
Debian had picked a function that was in the backtrace back then, but
the combination of the new glibc and compiler optimizations have
removed that function :-)
Picking the function name that has to be externally called ensures it
will not be optimized away (as it needs to stay to be callable)

> > + * d/p/avoid-json-parse-misoptimization.patch: new toolchains mis-optimized
> > + json parsing to fail at negative values, avoid by not-optimizing the
> > + function
>
> "at negative values; avoid by not-optimizing the function."

Yeah a ";" might indeed be better - adapting

...

> > +Description: avoid failures due to over-optimization
> > + New gcc reorders clear/set and thereby breaks json parsing.
> > + For now add avoidance to not run unto unexpected issues.
> > +Forwarded: yes - https://dovecot.org/pipermail/dovecot/2021-March/121770.html
>
> Wow, awesome debugging work there. Hopefully that gives upstream a solid path to look down for a solution.

Thanks, they are responsive already - we will see where it leads us.

Christian Ehrhardt  (paelzer) wrote :

Despite my first results being different upstream identified LTO as the bad-guy here.
I was able to confirm this and it also fixes the follow on issue.

Mid-term upstream will fix the underlying issue (they are able to reproduce it now) and short term we will opt out of LTO. Who knows what else in dovecot breaks now that we already have had two cases ...

I'll update this MP and then open it up for full review.

Christian Ehrhardt  (paelzer) wrote :

PPA: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/4513/+packages

Opening this up for full review, it should be good to fix both FTFBS issues.
Since dovecot is seeded but not on the ISO we could even immediately upload.

Utkarsh Gupta (utkarsh) wrote :

I'll let Bryce do the final review here.

But it looks good to me now. And more importantly, whoever reviews, the web UI shows conflict but actually there are none whilst using git :)

Lucas Kanashiro (lucaskanashiro) wrote :

Interesting... the LP web ui is showing conflicts but I was not able to spot any issue with this MP (as Utkarsh mentioned). Am I missing something? Or is it a bug in LP?

About the packaging changes they all LGTM, well documented and patches/issues forwarded to the interested parts. +1 from me.

review: Approve
Christian Ehrhardt  (paelzer) wrote :

Thanks, I think the web UI can go crazy if you ever force-push - which we did here to exchange the first approach to json fixing to the upstream preferred one.

Christian Ehrhardt  (paelzer) wrote :

To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/dovecot
 * [new tag] upload/1%2.3.13+dfsg1-1ubuntu1 -> upload/1%2.3.13+dfsg1-1ubuntu1

Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading dovecot_2.3.13+dfsg1-1ubuntu1.dsc: done.
  Uploading dovecot_2.3.13+dfsg1-1ubuntu1.debian.tar.xz: done.
  Uploading dovecot_2.3.13+dfsg1-1ubuntu1_source.buildinfo: done.
  Uploading dovecot_2.3.13+dfsg1-1ubuntu1_source.changes: done.
Successfully uploaded packages.

Christian Ehrhardt  (paelzer) wrote :

This was raced by https://launchpad.net/ubuntu/+source/dovecot/1:2.3.13+dfsg1-1ubuntu1 having the same fix (disable LTO).

Therefore this MP is superseded

Christian Ehrhardt  (paelzer) wrote :

I updated the Debian MP that LTO also would fix the test-patch but there it makes sense to adapt it still.

Bryce Harrington (bryce) wrote :

* This has migrated

  - Current Version: 1:2.3.13+dfsg1-1ubuntu1
  - Proposed Version: None
  - Debian Version: 1:2.3.13+dfsg1-1ubuntu1
  - New Version: 1:2.3.13+dfsg1-1ubuntu1

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 6627bea..b984e89 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,9 +1,16 @@
6 dovecot (1:2.3.13+dfsg1-1ubuntu1) hirsute; urgency=medium
7
8+<<<<<<< debian/changelog
9 * Package references hidden symbols during an LTO link. This needs further
10 investigation. Until then, disable LTO.
11
12 -- Matthias Klose <doko@ubuntu.com> Tue, 30 Mar 2021 17:23:55 +0200
13+=======
14+ * d/p/test-backtrace.patch: update to fix FTFBS with glibc 2.33 and later
15+ * d/rules: disable LTO as it breaks various self-tests
16+
17+ -- Christian Ehrhardt <christian.ehrhardt@canonical.com> Tue, 30 Mar 2021 11:11:00 +0200
18+>>>>>>> debian/changelog
19
20 dovecot (1:2.3.13+dfsg1-1build1) hirsute; urgency=high
21
22diff --git a/debian/control b/debian/control
23index 7db1c94..93b2117 100644
24--- a/debian/control
25+++ b/debian/control
26@@ -1,7 +1,8 @@
27 Source: dovecot
28 Section: mail
29 Priority: optional
30-Maintainer: Dovecot Maintainers <dovecot@packages.debian.org>
31+Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
32+XSBC-Original-Maintainer: Dovecot Maintainers <dovecot@packages.debian.org>
33 Uploaders: Jaldhar H. Vyas <jaldhar@debian.org>,
34 Jelmer Vernooij <jelmer@debian.org>,
35 Apollon Oikonomopoulos <apoikos@debian.org>,
36diff --git a/debian/patches/test-backtrace.patch b/debian/patches/test-backtrace.patch
37index 66c48b8..ee649c8 100644
38--- a/debian/patches/test-backtrace.patch
39+++ b/debian/patches/test-backtrace.patch
40@@ -18,7 +18,7 @@ index fdebe0f..78bd096 100644
41 test_assert(backtrace_append(bt) == 0);
42 - /* it should have some kind of main in it */
43 - test_assert(strstr(str_c(bt), "main") != NULL);
44-+ test_assert(strstr(str_c(bt), "backtrace_append") != NULL);
45++ test_assert(strstr(str_c(bt), "test_backtrace") != NULL);
46 #else
47 /* should not work in this context */
48 test_assert(backtrace_append(bt) == -1);
49@@ -28,7 +28,7 @@ index fdebe0f..78bd096 100644
50 test_assert(backtrace_get(&bt) == 0);
51 - /* it should have some kind of main in it */
52 - test_assert(strstr(bt, "main") != NULL);
53-+ test_assert(strstr(bt, "backtrace_append") != NULL);
54++ test_assert(strstr(bt, "test_backtrace") != NULL);
55 #else
56 /* should not work in this context */
57 test_assert(backtrace_get(&bt) == -1);
58diff --git a/debian/rules b/debian/rules
59index 85435dd..d116ad3 100755
60--- a/debian/rules
61+++ b/debian/rules
62@@ -6,7 +6,19 @@
63 SHELL=/bin/bash -O extglob
64 PIGEONHOLE_DIR=$(CURDIR)/pigeonhole
65
66+<<<<<<< debian/rules
67 export DEB_BUILD_MAINT_OPTIONS=hardening=+all optimize=-lto
68+=======
69+# Ubuntu enabled LTO by default https://wiki.ubuntu.com/ToolChain/LTO
70+# This breaks dovecot in multiple ways, disable it until the root cause
71+# is fixed in a new upstream version.
72+# See https://dovecot.org/pipermail/dovecot/2021-March/121770.html
73+ifeq ($(shell dpkg-vendor --derives-from Ubuntu && echo yes),yes)
74+ export DEB_BUILD_MAINT_OPTIONS=hardening=+all optimize=-lto
75+else
76+ export DEB_BUILD_MAINT_OPTIONS=hardening=+all
77+endif
78+>>>>>>> debian/rules
79
80 # LP: 1636781 - strip incompatible default linker option
81 ifeq ($(shell dpkg-vendor --derives-from Ubuntu && echo yes),yes)

Subscribers

People subscribed via source and target branches