Skip to content

Remove new field from ucontext_t for compatibility with earlier glibc versions #1411

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jun 27, 2019
Merged

Remove new field from ucontext_t for compatibility with earlier glibc versions #1411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dba4138
remove newer `__ssp` field from `ucontext_t` for earlier glib compat
acfoltzer Jun 26, 2019
886bb3f
filter out the struct test rather than removing the header
acfoltzer Jun 26, 2019
339fe22
add fixmes for the ucontext_t shadow stack field
acfoltzer Jun 27, 2019
e94fffc
replace deprecated string functions in style script
acfoltzer Jun 27, 2019
f6e48fc
fix line length
acfoltzer Jun 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ci/style.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ fn check_style(file: &str, path: &Path, err: &mut Errors) {
} else {
prev_blank = false;
}
if line != line.trim_right() {
if line != line.trim_end() {
err.error(path, i, "trailing whitespace");
}
if line.contains("\t") {
Expand All @@ -139,7 +139,7 @@ fn check_style(file: &str, path: &Path, err: &mut Errors) {
}
}

let line = line.trim_left();
let line = line.trim_start();
let is_pub = line.starts_with("pub ");
let line = if is_pub {&line[4..]} else {line};

Expand Down
6 changes: 6 additions & 0 deletions libc-test/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2098,6 +2098,12 @@ fn test_linux(target: &str) {
// FIXME: musl version using by mips build jobs 1.0.15 is ancient:
"ifmap" | "ifreq" | "ifconf" if mips32_musl => true,

// FIXME: remove once Ubuntu 20.04 LTS is released, somewhere in 2020.
// ucontext_t added a new field as of glibc 2.28; our struct definition is
// conservative and omits the field, but that means the size doesn't match for newer
// glibcs (see https://github.com/rust-lang/libc/issues/1410)
"ucontext_t" if gnu => true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add the following FIXME here?

FIXME: remove once Ubuntu 20.04 LTS is released, somewhere in 2020.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, though I'm a little concerned about the timeline given that 18.04 LTS is supported until 2023. Is there a stated policy for how long this crate supports a given glibc version?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not really. We tend to support a "sufficiently" old version, adding features of newer glibc versions on top. The glibc version supported differs per target, it's all a huge mess.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hah, fair enough. It would be nice if we could trigger a load-time failure for too-old versions of glibc, as the original bug that triggered my report was pretty difficult to track down. I've seen programs fail to dynamically load before, asking for a particular glibc version, but I don't know how we could introduce that behavior. Either way, something for another PR probably.


_ => false,
}
});
Expand Down
6 changes: 5 additions & 1 deletion src/unix/linux_like/linux/gnu/b64/x86_64/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,11 @@ s_no_extra_traits! {
pub uc_mcontext: mcontext_t,
pub uc_sigmask: ::sigset_t,
__private: [u8; 512],
__ssp: [::c_ulonglong; 4],
Copy link
Contributor

@gnzlbg gnzlbg Jun 27, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Instead of removing the field, can you comment it out, and add a comment about why it is commented out ? For example:

// FIXME: the shadow stack field requires glibc >= 2.28.
// Re-add once we drop compatibility with glibc versions older than 2.28.
// __ssp: [::c_ulonglong; 4],

// FIXME: the shadow stack field requires glibc >= 2.28.
// Re-add once we drop compatibility with glibc versions older than
// 2.28.
//
// __ssp: [::c_ulonglong; 4],
}
}

Expand Down