Skip to content

Commit

Permalink
Auto merge of #55611 - alexcrichton:beta-next, r=alexcrichton
Browse files Browse the repository at this point in the history
[beta] Fixes #46775 -- don't mutate the process's environment in Command::exec

This is a backport of the following PRs:

* #55359
* #55569
* #55304
  • Loading branch information
bors committed Nov 7, 2018
2 parents 04da282 + 39fa89b commit 00b9ddb
Show file tree
Hide file tree
Showing 11 changed files with 179 additions and 129 deletions.
110 changes: 20 additions & 90 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -197,23 +197,10 @@ matrix:
. src/ci/docker/x86_64-gnu-tools/repo.sh;
commit_toolstate_change "$MESSAGE_FILE" "$TRAVIS_BUILD_DIR/src/tools/publish_toolstate.py" "$(git rev-parse HEAD)" "$(git log --format=%s -n1 HEAD)" "$MESSAGE_FILE" "$TOOLSTATE_REPO_ACCESS_TOKEN";

env:
global:
- SCCACHE_BUCKET=rust-lang-ci-sccache2
- SCCACHE_REGION=us-west-1
- AWS_ACCESS_KEY_ID=AKIAJAMV3QAMMA6AXHFQ
# AWS_SECRET_ACCESS_KEY=...
- secure: "j96XxTVOSUf4s4r4htIxn/fvIa5DWbMgLqWl7r8z2QfgUwscmkMXAwXuFNc7s7bGTpV/+CgDiMFFM6BAFLGKutytIF6oA02s9b+usQYnM0th7YQ2AIgm9GtMTJCJp4AoyfFmh8F2faUICBZlfVLUJ34udHEe35vOklix+0k4WDo="
# TOOLSTATE_REPO_ACCESS_TOKEN=...
- secure: "ESfcXqv4N2VMhqi2iIyw6da9VrsA78I4iR1asouCaq4hzTTrkB4WNRrfURy6xg72gQ4nMhtRJbB0/2jmc9Cu1+g2CzXtyiL223aJ5CKrXdcvbitopQSDfp07dMWm+UED+hNFEanpErKAeU/6FM3A+J+60PMk8MCF1h9tqNRISJw="

before_install:
# We'll use the AWS cli to download/upload cached docker layers, so install
# that here.
- if [ "$TRAVIS_OS_NAME" = linux ]; then
pip install --user awscli;
export PATH=$PATH:$HOME/.local/bin;
fi
# We'll use the AWS cli to download/upload cached docker layers as well as
# push our deployments, so download that here.
- pip install --user awscli; export PATH=$PATH:$HOME/.local/bin
- mkdir -p $HOME/rustsrc
# FIXME(#46924): these two commands are required to enable IPv6,
# they shouldn't exist, please revert once more official solutions appeared.
Expand Down Expand Up @@ -276,6 +263,23 @@ after_success:
echo "#### Build successful; Disk usage after running script:";
df -h;
du . | sort -nr | head -n100
- >
if [ "$DEPLOY$DEPLOY_ALT" == "1" ]; then
mkdir -p deploy/$TRAVIS_COMMIT;
if [ "$TRAVIS_OS_NAME" == "osx" ]; then
rm -rf build/dist/doc &&
cp -r build/dist/* deploy/$TRAVIS_COMMIT;
else
rm -rf obj/build/dist/doc &&
cp -r obj/build/dist/* deploy/$TRAVIS_COMMIT;
fi;
ls -la deploy/$TRAVIS_COMMIT;
deploy_dir=rustc-builds;
if [ "$DEPLOY_ALT" == "1" ]; then
deploy_dir=rustc-builds-alt;
fi;
travis_retry aws s3 cp --no-progress --recursive --acl public-read ./deploy s3://rust-lang-ci2/$deploy_dir
fi
after_failure:
- >
Expand Down Expand Up @@ -322,77 +326,3 @@ after_failure:

notifications:
email: false

before_deploy:
- mkdir -p deploy/$TRAVIS_COMMIT
- >
if [ "$TRAVIS_OS_NAME" == "osx" ]; then
rm -rf build/dist/doc &&
cp -r build/dist/* deploy/$TRAVIS_COMMIT;
else
rm -rf obj/build/dist/doc &&
cp -r obj/build/dist/* deploy/$TRAVIS_COMMIT;
fi
- ls -la deploy/$TRAVIS_COMMIT

deploy:
- provider: s3
bucket: rust-lang-ci2
skip_cleanup: true
local_dir: deploy
upload_dir: rustc-builds
acl: public_read
region: us-west-1
access_key_id: AKIAJVBODR3IA4O72THQ
secret_access_key:
secure: "kUGd3t7JcVWFESgIlzvsM8viZgCA9Encs3creW0xLJaLSeI1iVjlJK4h/2/nO6y224AFrh/GUfsNr4/4AlxPuYb8OU5oC5Lv+Ff2JiRDYtuNpyQSKAQp+bRYytWMtrmhja91h118Mbm90cUfcLPwkdiINgJNTXhPKg5Cqu3VYn0="
on:
branch: auto
condition: $DEPLOY = 1

# this is the same as the above deployment provider except that it uploads to
# a slightly different directory and has a different trigger
- provider: s3
bucket: rust-lang-ci2
skip_cleanup: true
local_dir: deploy
upload_dir: rustc-builds-alt
acl: public_read
region: us-west-1
access_key_id: AKIAJVBODR3IA4O72THQ
secret_access_key:
secure: "kUGd3t7JcVWFESgIlzvsM8viZgCA9Encs3creW0xLJaLSeI1iVjlJK4h/2/nO6y224AFrh/GUfsNr4/4AlxPuYb8OU5oC5Lv+Ff2JiRDYtuNpyQSKAQp+bRYytWMtrmhja91h118Mbm90cUfcLPwkdiINgJNTXhPKg5Cqu3VYn0="
on:
branch: auto
condition: $DEPLOY_ALT = 1

# These two providers are the same as the two above, except deploy on the
# try branch. Travis does not appear to provide a way to use "or" in these
# conditions.
- provider: s3
bucket: rust-lang-ci2
skip_cleanup: true
local_dir: deploy
upload_dir: rustc-builds
acl: public_read
region: us-west-1
access_key_id: AKIAJVBODR3IA4O72THQ
secret_access_key:
secure: "kUGd3t7JcVWFESgIlzvsM8viZgCA9Encs3creW0xLJaLSeI1iVjlJK4h/2/nO6y224AFrh/GUfsNr4/4AlxPuYb8OU5oC5Lv+Ff2JiRDYtuNpyQSKAQp+bRYytWMtrmhja91h118Mbm90cUfcLPwkdiINgJNTXhPKg5Cqu3VYn0="
on:
branch: try
condition: $DEPLOY = 1

- provider: s3
bucket: rust-lang-ci2
skip_cleanup: true
local_dir: deploy
upload_dir: rustc-builds-alt
acl: public_read
region: us-west-1
access_key_id: AKIAJVBODR3IA4O72THQ
secret_access_key:
secure: "kUGd3t7JcVWFESgIlzvsM8viZgCA9Encs3creW0xLJaLSeI1iVjlJK4h/2/nO6y224AFrh/GUfsNr4/4AlxPuYb8OU5oC5Lv+Ff2JiRDYtuNpyQSKAQp+bRYytWMtrmhja91h118Mbm90cUfcLPwkdiINgJNTXhPKg5Cqu3VYn0="
on:
branch: try
condition: $DEPLOY_ALT = 1
19 changes: 4 additions & 15 deletions appveyor.yml
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
environment:
SCCACHE_BUCKET: rust-lang-ci-sccache2
SCCACHE_REGION: us-west-1
AWS_ACCESS_KEY_ID: AKIAJAMV3QAMMA6AXHFQ
AWS_SECRET_ACCESS_KEY:
secure: 7Y+JiquYedOAgnUU26uL0DPzrxmTtR+qIwG6rNKSuWDffqU3vVZxbGXim9QpTO80
SCCACHE_DIGEST: f808afabb4a4eb1d7112bcb3fa6be03b61e93412890c88e177c667eb37f46353d7ec294e559b16f9f4b5e894f2185fe7670a0df15fd064889ecbd80f0c34166c
TOOLSTATE_REPO_ACCESS_TOKEN:
secure: gKGlVktr7iuqCoYSxHxDE9ltLOKU0nYDEuQxvWbNxUIW7ri5ppn8L06jQzN0GGzN

# By default schannel checks revocation of certificates unlike some other SSL
# backends, but we've historically had problems on CI where a revocation
Expand Down Expand Up @@ -235,10 +228,8 @@ before_deploy:
deploy:
- provider: S3
skip_cleanup: true
access_key_id: AKIAJVBODR3IA4O72THQ
secret_access_key:
secure: tQWIE+DJHjXaV4np/3YeETkEmXngtIuIgAO/LYKQaUshGLgN8cBCFGG3cHx5lKLt
access_key_id: $(AWS_ACCESS_KEY_ID)
secret_access_key: $(AWS_SECRET_ACCESS_KEY)
bucket: rust-lang-ci2
set_public: true
region: us-west-1
Expand All @@ -252,10 +243,8 @@ deploy:
# This provider is the same as the one above except that it has a slightly
# different upload directory and a slightly different trigger
- provider: S3
skip_cleanup: true
access_key_id: AKIAJVBODR3IA4O72THQ
secret_access_key:
secure: tQWIE+DJHjXaV4np/3YeETkEmXngtIuIgAO/LYKQaUshGLgN8cBCFGG3cHx5lKLt
access_key_id: $(AWS_ACCESS_KEY_ID)
secret_access_key: $(AWS_SECRET_ACCESS_KEY)
bucket: rust-lang-ci2
set_public: true
region: us-west-1
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_lint/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ use rustc::hir;
declare_lint! {
pub UNUSED_MUST_USE,
Warn,
"unused result of a type flagged as #[must_use]"
"unused result of a type flagged as #[must_use]",
report_in_external_macro: true
}

declare_lint! {
Expand Down
12 changes: 4 additions & 8 deletions src/librustc_resolve/resolve_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ use syntax_pos::{MultiSpan, Span};

use std::cell::{Cell, RefCell};
use std::collections::BTreeMap;
use std::fmt::Write;
use std::{mem, ptr};

/// Contains data for specific types of import directives.
Expand Down Expand Up @@ -778,17 +777,14 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {

let msg = format!("`{}` import is ambiguous", name);
let mut err = self.session.struct_span_err(span, &msg);
let mut suggestion_choices = String::new();
let mut suggestion_choices = vec![];
if external_crate.is_some() {
write!(suggestion_choices, "`::{}`", name);
suggestion_choices.push(format!("`::{}`", name));
err.span_label(span,
format!("can refer to external crate `::{}`", name));
}
if let Some(result) = results.module_scope {
if !suggestion_choices.is_empty() {
suggestion_choices.push_str(" or ");
}
write!(suggestion_choices, "`self::{}`", name);
suggestion_choices.push(format!("`self::{}`", name));
if uniform_paths_feature {
err.span_label(result.span,
format!("can refer to `self::{}`", name));
Expand All @@ -801,7 +797,7 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
err.span_label(result.span,
format!("shadowed by block-scoped `{}`", name));
}
err.help(&format!("write {} explicitly instead", suggestion_choices));
err.help(&format!("write {} explicitly instead", suggestion_choices.join(" or ")));
if uniform_paths_feature {
err.note("relative `use` paths enabled by `#![feature(uniform_paths)]`");
} else {
Expand Down
8 changes: 8 additions & 0 deletions src/libstd/sys/unix/process/process_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,10 @@ impl Command {
pub fn get_argv(&self) -> &Vec<*const c_char> {
&self.argv.0
}
#[cfg(not(target_os = "fuchsia"))]
pub fn get_program(&self) -> &CString {
return &self.program;
}

#[allow(dead_code)]
pub fn get_cwd(&self) -> &Option<CString> {
Expand Down Expand Up @@ -244,6 +248,10 @@ impl CStringArray {
pub fn as_ptr(&self) -> *const *const c_char {
self.ptrs.as_ptr()
}
#[cfg(not(target_os = "fuchsia"))]
pub fn get_items(&self) -> &[CString] {
return &self.items;
}
}

fn construct_envp(env: BTreeMap<DefaultEnvKey, OsString>, saw_nul: &mut bool) -> CStringArray {
Expand Down
99 changes: 91 additions & 8 deletions src/libstd/sys/unix/process/process_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use env;
use ffi::CString;
use io::{self, Error, ErrorKind};
use libc::{self, c_int, gid_t, pid_t, uid_t};
use ptr;
Expand Down Expand Up @@ -39,13 +41,15 @@ impl Command {
return Ok((ret, ours))
}

let possible_paths = self.compute_possible_paths(envp.as_ref());

let (input, output) = sys::pipe::anon_pipe()?;

let pid = unsafe {
match cvt(libc::fork())? {
0 => {
drop(input);
let err = self.do_exec(theirs, envp.as_ref());
let err = self.do_exec(theirs, envp.as_ref(), possible_paths);
let errno = err.raw_os_error().unwrap_or(libc::EINVAL) as u32;
let bytes = [
(errno >> 24) as u8,
Expand Down Expand Up @@ -113,12 +117,48 @@ impl Command {
"nul byte found in provided data")
}

let possible_paths = self.compute_possible_paths(envp.as_ref());
match self.setup_io(default, true) {
Ok((_, theirs)) => unsafe { self.do_exec(theirs, envp.as_ref()) },
Ok((_, theirs)) => unsafe { self.do_exec(theirs, envp.as_ref(), possible_paths) },
Err(e) => e,
}
}

fn compute_possible_paths(&self, maybe_envp: Option<&CStringArray>) -> Option<Vec<CString>> {
let program = self.get_program().as_bytes();
if program.contains(&b'/') {
return None;
}
// Outside the match so we can borrow it for the lifetime of the function.
let parent_path = env::var("PATH").ok();
let paths = match maybe_envp {
Some(envp) => {
match envp.get_items().iter().find(|var| var.as_bytes().starts_with(b"PATH=")) {
Some(p) => &p.as_bytes()[5..],
None => return None,
}
},
// maybe_envp is None if the process isn't changing the parent's env at all.
None => {
match parent_path.as_ref() {
Some(p) => p.as_bytes(),
None => return None,
}
},
};

let mut possible_paths = vec![];
for path in paths.split(|p| *p == b':') {
let mut binary_path = Vec::with_capacity(program.len() + path.len() + 1);
binary_path.extend_from_slice(path);
binary_path.push(b'/');
binary_path.extend_from_slice(program);
let c_binary_path = CString::new(binary_path).unwrap();
possible_paths.push(c_binary_path);
}
return Some(possible_paths);
}

// And at this point we've reached a special time in the life of the
// child. The child must now be considered hamstrung and unable to
// do anything other than syscalls really. Consider the following
Expand Down Expand Up @@ -152,7 +192,8 @@ impl Command {
unsafe fn do_exec(
&mut self,
stdio: ChildPipes,
maybe_envp: Option<&CStringArray>
maybe_envp: Option<&CStringArray>,
maybe_possible_paths: Option<Vec<CString>>,
) -> io::Error {
use sys::{self, cvt_r};

Expand Down Expand Up @@ -193,9 +234,6 @@ impl Command {
if let Some(ref cwd) = *self.get_cwd() {
t!(cvt(libc::chdir(cwd.as_ptr())));
}
if let Some(envp) = maybe_envp {
*sys::os::environ() = envp.as_ptr();
}

// emscripten has no signal support.
#[cfg(not(any(target_os = "emscripten")))]
Expand Down Expand Up @@ -231,8 +269,53 @@ impl Command {
t!(callback());
}

libc::execvp(self.get_argv()[0], self.get_argv().as_ptr());
io::Error::last_os_error()
// If the program isn't an absolute path, and our environment contains a PATH var, then we
// implement the PATH traversal ourselves so that it honors the child's PATH instead of the
// parent's. This mirrors the logic that exists in glibc's execvpe, except using the
// child's env to fetch PATH.
match maybe_possible_paths {
Some(possible_paths) => {
let mut pending_error = None;
for path in possible_paths {
libc::execve(
path.as_ptr(),
self.get_argv().as_ptr(),
maybe_envp.map(|envp| envp.as_ptr()).unwrap_or_else(|| *sys::os::environ())
);
let err = io::Error::last_os_error();
match err.kind() {
io::ErrorKind::PermissionDenied => {
// If we saw a PermissionDenied, and none of the other entries in
// $PATH are successful, then we'll return the first EACCESS we see.
if pending_error.is_none() {
pending_error = Some(err);
}
},
// Errors which indicate we failed to find a file are ignored and we try
// the next entry in the path.
io::ErrorKind::NotFound | io::ErrorKind::TimedOut => {
continue
},
// Any other error means we found a file and couldn't execute it.
_ => {
return err;
}
}
}
if let Some(err) = pending_error {
return err;
}
return io::Error::from_raw_os_error(libc::ENOENT);
},
_ => {
libc::execve(
self.get_argv()[0],
self.get_argv().as_ptr(),
maybe_envp.map(|envp| envp.as_ptr()).unwrap_or_else(|| *sys::os::environ())
);
return io::Error::last_os_error()
}
}
}

#[cfg(not(any(target_os = "macos", target_os = "freebsd",
Expand Down
Loading

0 comments on commit 00b9ddb

Please sign in to comment.