avoid leaking host details in proc macro metadata decoding #54265

arielb1 · 2018-09-15T20:33:25Z

proc macro crates are essentially implemented as dynamic libraries using
a dlopen-based ABI. They are also Rust crates, so they have 2 worlds -
the "host" world in which they are defined, and the "target" world in
which they are used.

For all the "target" world knows, the proc macro crate might not even
be implemented in Rust, so leaks of details from the host to the target
must be avoided for correctness.

Because the "host" DefId space is different from the "target" DefId
space, any leak involving a DefId will have a nonsensical or
out-of-bounds DefKey, and will cause all sorts of crashes.

This PR fixes all leaks I have found in decoder. In particular, #54059
was caused by host native libraries leaking into the target, which feels
like it might even be a correctness issue if it doesn't cause an ICE.

Fixes #54059

proc macro crates are essentially implemented as dynamic libraries using a dlopen-based ABI. They are also Rust crates, so they have 2 worlds - the "host" world in which they are defined, and the "target" world in which they are used. For all the "target" world knows, the proc macro crate might not even be implemented in Rust, so leaks of details from the host to the target must be avoided for correctness. Because the "host" DefId space is different from the "target" DefId space, any leak involving a DefId will have a nonsensical or out-of-bounds DefKey, and will cause all sorts of crashes. This PR fixes all leaks I have found in `decoder`. In particular, rust-lang#54059 was caused by host native libraries leaking into the target, which feels like it might even be a correctness issue if it doesn't cause an ICE. Fixes rust-lang#54059

rust-highfive · 2018-09-15T20:33:28Z

r? @davidtwco

(rust_highfive has picked a reviewer for you, use r? to override)

arielb1 · 2018-09-15T20:33:52Z

r? @alexcrichton

alexcrichton · 2018-09-17T17:30:12Z

@bors: r+

Thanks for the fix! This all looks right to me yeah

bors · 2018-09-17T17:30:14Z

📌 Commit 0d840e1 has been approved by alexcrichton

pnkfelix · 2018-09-20T14:49:54Z

discussed at T-compiler meeting. beta-accepted.

pietroalbini · 2018-09-20T22:08:35Z

@bors p=1 (needs to be backported)

bors · 2018-09-21T13:00:57Z

⌛ Testing commit 0d840e1 with merge d7371f2...

avoid leaking host details in proc macro metadata decoding proc macro crates are essentially implemented as dynamic libraries using a dlopen-based ABI. They are also Rust crates, so they have 2 worlds - the "host" world in which they are defined, and the "target" world in which they are used. For all the "target" world knows, the proc macro crate might not even be implemented in Rust, so leaks of details from the host to the target must be avoided for correctness. Because the "host" DefId space is different from the "target" DefId space, any leak involving a DefId will have a nonsensical or out-of-bounds DefKey, and will cause all sorts of crashes. This PR fixes all leaks I have found in `decoder`. In particular, #54059 was caused by host native libraries leaking into the target, which feels like it might even be a correctness issue if it doesn't cause an ICE. Fixes #54059

bors · 2018-09-21T14:18:18Z

💔 Test failed - status-appveyor

kennytm · 2018-09-21T14:52:40Z

(i686-pc-windows-msvc)

test [incremental] incremental-fulldeps\issue-54059.rs ... FAILED

...

error in revision `rpass1`: auxiliary build of "C:\\projects\\rust\\src/test\\incremental-fulldeps\\auxiliary\\issue_54059.rs" failed to compile: 

...

error: linking with `link.exe` failed: exit code: 1120

...

error LNK2019: unresolved external symbol __imp__rust_dbg_extern_identity_u64 referenced in function __ZN11issue_5405910base2_impl17hc658a2e925e33bb5E

alexcrichton · 2018-09-21T17:05:53Z

@bors: r+

bors · 2018-09-21T17:05:53Z

📌 Commit 1b93806 has been approved by alexcrichton

bors · 2018-09-22T04:47:40Z

⌛ Testing commit 1b93806 with merge 9002b459ed7d66c010550e223787e2062edd1487...

bors · 2018-09-22T07:06:06Z

💔 Test failed - status-travis

rust-highfive · 2018-09-22T07:06:08Z

The job x86_64-gnu-distcheck of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[02:15:11] warning: spurious network error (2 tries remaining): curl error: Could not resolve host: github.com
[02:15:11] ; class=Net (12)
[02:15:32] warning: spurious network error (1 tries remaining): curl error: Could not resolve host: github.com
[02:15:32] ; class=Net (12)
[02:15:53] error: failed to load source for a dependency on `rand`
[02:15:53] Caused by:
[02:15:53]   Unable to update registry `https://github.com/rust-lang/crates.io-index`
[02:15:53] 
[02:15:53] Caused by:
[02:15:53] Caused by:
[02:15:53]   failed to fetch `https://github.com/rust-lang/crates.io-index`
[02:15:53] 
[02:15:53] Caused by:
[02:15:53]   curl error: Could not resolve host: github.com
[02:15:53] ; class=Net (12)
[02:15:53] 
[02:15:53] 
[02:15:53] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "generate-lockfile" "--manifest-path" "/checkout/obj/build/tmp/distcheck-src/rust-src/lib/rustlib/src/rust/src/libstd/Cargo.toml"
[02:15:53] 
[02:15:53] 
[02:15:53] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test distcheck
[02:15:53] Build completed unsuccessfully in 2:12:36
---
travis_time:end:2d151da0:start=1537599934202507937,finish=1537599934214495755,duration=11987818
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:07d234ab
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0194eac2
travis_time:start:0194eac2
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:0f2d207c
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

alexcrichton · 2018-09-22T07:21:56Z

@bors: retry

bors · 2018-09-22T11:56:15Z

⌛ Testing commit 1b93806 with merge e7b5ba8...

avoid leaking host details in proc macro metadata decoding proc macro crates are essentially implemented as dynamic libraries using a dlopen-based ABI. They are also Rust crates, so they have 2 worlds - the "host" world in which they are defined, and the "target" world in which they are used. For all the "target" world knows, the proc macro crate might not even be implemented in Rust, so leaks of details from the host to the target must be avoided for correctness. Because the "host" DefId space is different from the "target" DefId space, any leak involving a DefId will have a nonsensical or out-of-bounds DefKey, and will cause all sorts of crashes. This PR fixes all leaks I have found in `decoder`. In particular, #54059 was caused by host native libraries leaking into the target, which feels like it might even be a correctness issue if it doesn't cause an ICE. Fixes #54059

bors · 2018-09-22T14:26:11Z

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing e7b5ba8 to master...

@ghost

[beta] Rollup backports Merged and approved: * #54323: rustbuild: drop color handling * #54265: avoid leaking host details in proc macro metadata decoding r? @ghost

rust-highfive assigned davidtwco Sep 15, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 15, 2018

rust-highfive assigned alexcrichton and unassigned davidtwco Sep 15, 2018

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 17, 2018

pietroalbini added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Sep 20, 2018

pnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Sep 20, 2018

pnkfelix mentioned this pull request Sep 20, 2018

ICE compiling the objrs crate #54059

Closed

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 21, 2018

kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 21, 2018

Ignore new test on Windows

1b93806

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 21, 2018

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 22, 2018

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 22, 2018

bors merged commit 1b93806 into rust-lang:master Sep 22, 2018

pietroalbini mentioned this pull request Sep 22, 2018

[beta] Rollup backports #54484

Merged

pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Sep 22, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

avoid leaking host details in proc macro metadata decoding #54265

avoid leaking host details in proc macro metadata decoding #54265

arielb1 commented Sep 15, 2018

rust-highfive commented Sep 15, 2018

arielb1 commented Sep 15, 2018

alexcrichton commented Sep 17, 2018

bors commented Sep 17, 2018

pnkfelix commented Sep 20, 2018

pietroalbini commented Sep 20, 2018

bors commented Sep 21, 2018

bors commented Sep 21, 2018

kennytm commented Sep 21, 2018

alexcrichton commented Sep 21, 2018

bors commented Sep 21, 2018

bors commented Sep 22, 2018

bors commented Sep 22, 2018

rust-highfive commented Sep 22, 2018

alexcrichton commented Sep 22, 2018

bors commented Sep 22, 2018

bors commented Sep 22, 2018

avoid leaking host details in proc macro metadata decoding #54265

avoid leaking host details in proc macro metadata decoding #54265

Conversation

arielb1 commented Sep 15, 2018

rust-highfive commented Sep 15, 2018

arielb1 commented Sep 15, 2018

alexcrichton commented Sep 17, 2018

bors commented Sep 17, 2018

pnkfelix commented Sep 20, 2018

pietroalbini commented Sep 20, 2018

bors commented Sep 21, 2018

bors commented Sep 21, 2018

kennytm commented Sep 21, 2018

alexcrichton commented Sep 21, 2018

bors commented Sep 21, 2018

bors commented Sep 22, 2018

bors commented Sep 22, 2018

rust-highfive commented Sep 22, 2018

alexcrichton commented Sep 22, 2018

bors commented Sep 22, 2018

bors commented Sep 22, 2018