Open dviererbe opened 6 days ago
CC: @mateusrodrigues as you worked with Miriam on this. See this (Canonical internal) mattermost thread: https://chat.canonical.com/canonical/pl/aokmic4z77dmfmjdhguc63ue5r
From what I read it looks like
export DEB_CFLAGS_MAINT_STRIP = -fstack-clash-protection -mbranch-protection=standard
export DEB_CXXFLAGS_MAINT_STRIP = -fstack-clash-protection -mbranch-protection=standard
branch-protection
to bti
, because stripping would remove all branch protections and standard
is equivalent to bti+pac-ret
(see the ARM developer documentation), therefore setting branch-protection
to bti
effectively would just strip the pac-ret
protection.export DEB_CFLAGS_MAINT_STRIP = -fstack-clash-protection
export DEB_CXXFLAGS_MAINT_STRIP = -fstack-clash-protection
export DEB_CFLAGS_MAINT_SET = -mbranch-protection=bti
export DEB_CXXFLAGS_MAINT_SET = -mbranch-protection=bti
Note: Apparently other libraries using libunwind also failed to build from source on arm64 with branch-protection=standard
set at the time. Setting branch-protection
to bti
was a common solution. I will ask Miriam if she had to use DEB_CFLAGS_MAINT_SET
and DEB_CXXFLAGS_MAINT_SET
.
Miriam agreed that DEB_flag_MAINT_SET
should be avoided and using DEB_flag_MAINT_APPEND
would be better. She shared how it was implemented in dovcot: https://code.launchpad.net/~mirespace/ubuntu/+source/dovecot/+git/dovecot/+merge/452825
Applied to this situation we would like to change it to:
export DEB_CFLAGS_MAINT_STRIP += -fstack-clash-protection -mbranch-protection=standard
export DEB_CXXFLAGS_MAINT_STRIP += -fstack-clash-protection -mbranch-protection=standard
export DEB_CFLAGS_MAINT_APPEND += -mbranch-protection=bti
export DEB_CXXFLAGS_MAINT_APPEND += -mbranch-protection=bti
I also want to try out to apply this to all Ubunt/.NET versions. I think that previous build errors are caused by the DEB_flag_MAINT_SET
overwrite.
Nope, this will fail on jammy with "Cannot find mkstemps nor mkstemp on this platform.", but the good message is that it works for mantic, noble and oracular.
See the full build logs for:
So we have to continue doing this for mantic+
I don't recall exactly why I went with DEB_flag_MAINT_SET
, but DEB_flag_MAINT_APPEND
should suffice.
What I do recall is that "Cannot find mkstemps nor mkstemp on this platform." is not representative of the actual error. Since it failed in Jammy, you should look at the configure logs to see what the actual problem was.
I wish I could remember the actual name of the file, but if you build locally with sbuild, let it fail, then find . -type f -name CMakeConfigureLog*
within the sbuild build directory, you should get to it (based on the name I got from the docs).
I just had an unrelated discussion with Simon about the
DEB_flag_MAINT_SET
dpkg-buildflags(1). They should not be used, because they overwrite all flags and if they are used a comment should explain why it had to be used.Our rules file contains the
DEB_CFLAGS_MAINT_SET
&DEB_CXXFLAGS_MAINT_SET
flags for arm64 on mantic (Ubuntu 23.10) or later: https://github.com/canonical/dotnet-source-build/blob/29a2a8c248451a9c5edc7eaa5999103f0e264e52/src/rules#L34-L35In the archive it is further just limited to dotnet6 & dotnet7. dotnet8 does not set these flags.
The commit that adds it to this repo is 450bf967b5b2711a55c62816936edc8eac9e24a3, but that is just a sync from the pre-existing package state. One change it does is apply it consistently to all .NET versions.
The reason for this flag was to fix "Cannot find mkstemps nor mkstemp on this platform." build errors and was added in git-ubuntu with commit 031db21dcde4c97ee01d1e6ba66e59e29e78c660.
See also this (Canonical internal) mattermost thread: https://chat.canonical.com/canonical/pl/xrrbchjfgbnaxkajo4jxgf7iyy
Question 1: Why is this just in the dotnet6 and dotnet7 packages? Question 2: Is the
DEB_*_MAINT_SET
style strictly needed? Can it be replaced with something differently likeDEB_*_MAINT_APPEND
orDEB_*_MAINT_PREPEND
?If it turns out that the
DEB_*_MAINT_SET
style is strictly needed, than we should add a comment that explains why, as Simon suggested.