Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[glibc] Fix redefinition of _FORTIFY_SOURCE #1865

Merged
merged 1 commit into from
May 3, 2024

Conversation

oshogbo
Copy link
Contributor

@oshogbo oshogbo commented May 1, 2024

Description of the changes

The new Ubuntu release, Noble, includes a version of GCC with a spec that defines _FORTIFY_SOURCE when optimization is enabled:

$ gcc -dumpspecs | grep _FORTIFY_SOURCE
 %{!O0:%{O*:%{!D_FORTIFY_SOURCE=*:%{!U_FORTIFY_SOURCE:-D_FORTIFY_SOURCE=3}}}

However, some files in glibc need to be built without _FORTIFY_SOURCE, such as vprintf.c. The patch from Ubuntu Noble changes the method of undefining _FORTIFY_SOURCE to -U_FORTIFY_SOURCE. Previously it was limited only to the preprocessor (-Wp,-U_FORTIFY_SOURCE). The new method also undefines the definition from the spec.
The patch addresses some other non-standard redefinitions of _FORTIFY_SOURCE as well.

Some useful things

Get logs from the ubuntu/debian build (depends on the machine)

getbuildlog glibc last amd64

Patch obtained from

Ubuntu change log

https://changelogs.ubuntu.com/changelogs/pool/main/g/glibc/glibc_2.39-0ubuntu8.1/changelog

glibc (2.39-0ubuntu7) noble; urgency=medium

[...]
  * d/p/fix-fortify-source.patch: Fix FTBFS on Noble
[...]
 -- Simon Chopin <schopin@ubuntu.com>  Thu, 28 Mar 2024 15:16:51 +0100

How to test this PR?

Test suite.


This change is Reviewable

@oshogbo oshogbo marked this pull request as ready for review May 2, 2024 06:53
Copy link
Contributor

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (3 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @oshogbo)

a discussion (no related file):
Please also mention this patch in this list:



subprojects/packagefiles/glibc-2.39/fix-fortify-source.patch line 1 at r1 (raw file):

From 73302ea9454b816d5e0a4f02bbd68df1bf2bb53d Mon Sep 17 00:00:00 2001

I verified this patch to be an exact copy of the one in the Ubuntu Noble patches (bar the differently laid out spaces).

Copy link
Contributor Author

@oshogbo oshogbo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (3 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @dimakuv)

a discussion (no related file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Please also mention this patch in this list:

Done.


Copy link
Contributor

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners

Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required), "fixup! " found in commit messages' one-liners

Copy link
Contributor

@kailun-qin kailun-qin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, all discussions resolved, "fixup! " found in commit messages' one-liners

The new Ubuntu release, Noble, includes a version of GCC with a spec that
defines `_FORTIFY_SOURCE` when optimization is enabled:
```
$ gcc -dumpspecs | grep _FORTIFY_SOURCE
 %{!O0:%{O*:%{!D_FORTIFY_SOURCE=*:%{!U_FORTIFY_SOURCE:-D_FORTIFY_SOURCE=3}}}
```

However, some files in glibc need to be built without `_FORTIFY_SOURCE`,
such as `vprintf.c`. The patch from Ubuntu Noble changes the method of
undefining _FORTIFY_SOURCE to -U_FORTIFY_SOURCE. Previously it was limited
only to the preprocessor (-Wp,-U_FORTIFY_SOURCE). The new method also
undefines the definition from the spec.
The patch addresses some other non-standard redefinitions of
_FORTIFY_SOURCE as well.

Signed-off-by: Mariusz Zaborski <oshogbo@invisiblethingslab.com>
Copy link
Contributor

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Contributor

@kailun-qin kailun-qin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@mkow mkow merged commit 438ca16 into gramineproject:master May 3, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants