diff options
author | Jay Somedon <[email protected]> | 2020-12-23 13:24:53 +0000 |
---|---|---|
committer | Edwin Cheng <[email protected]> | 2021-03-04 01:11:33 +0000 |
commit | 55d73bc6754c351dead6ab4d4b57ddaa347734d6 (patch) | |
tree | eb6236274e385dfb0e8c74b621506fa9d57add52 | |
parent | 8fd7cd74067445484fbbd3f78e715c4fe24b004c (diff) |
Fix multiple issues from code review
* check metadata version
* use memmap
* use Result instead of unwrap
with Jay Somedon <[email protected]>
-rw-r--r-- | Cargo.lock | 33 | ||||
-rw-r--r-- | crates/proc_macro_api/Cargo.toml | 1 | ||||
-rw-r--r-- | crates/proc_macro_api/src/lib.rs | 44 |
3 files changed, 44 insertions, 34 deletions
diff --git a/Cargo.lock b/Cargo.lock index b61fd2e98..39f43ba17 100644 --- a/Cargo.lock +++ b/Cargo.lock | |||
@@ -80,7 +80,7 @@ dependencies = [ | |||
80 | "cfg-if", | 80 | "cfg-if", |
81 | "libc", | 81 | "libc", |
82 | "miniz_oxide", | 82 | "miniz_oxide", |
83 | "object 0.23.0", | 83 | "object", |
84 | "rustc-demangle", | 84 | "rustc-demangle", |
85 | ] | 85 | ] |
86 | 86 | ||
@@ -892,6 +892,16 @@ source = "registry+https://github.com/rust-lang/crates.io-index" | |||
892 | checksum = "0ee1c47aaa256ecabcaea351eae4a9b01ef39ed810004e298d2511ed284b1525" | 892 | checksum = "0ee1c47aaa256ecabcaea351eae4a9b01ef39ed810004e298d2511ed284b1525" |
893 | 893 | ||
894 | [[package]] | 894 | [[package]] |
895 | name = "memmap" | ||
896 | version = "0.7.0" | ||
897 | source = "registry+https://github.com/rust-lang/crates.io-index" | ||
898 | checksum = "6585fd95e7bb50d6cc31e20d4cf9afb4e2ba16c5846fc76793f11218da9c475b" | ||
899 | dependencies = [ | ||
900 | "libc", | ||
901 | "winapi", | ||
902 | ] | ||
903 | |||
904 | [[package]] | ||
895 | name = "memmap2" | 905 | name = "memmap2" |
896 | version = "0.2.1" | 906 | version = "0.2.1" |
897 | source = "registry+https://github.com/rust-lang/crates.io-index" | 907 | source = "registry+https://github.com/rust-lang/crates.io-index" |
@@ -1010,16 +1020,6 @@ dependencies = [ | |||
1010 | 1020 | ||
1011 | [[package]] | 1021 | [[package]] |
1012 | name = "object" | 1022 | name = "object" |
1013 | version = "0.22.0" | ||
1014 | source = "registry+https://github.com/rust-lang/crates.io-index" | ||
1015 | checksum = "8d3b63360ec3cb337817c2dbd47ab4a0f170d285d8e5a2064600f3def1402397" | ||
1016 | dependencies = [ | ||
1017 | "flate2", | ||
1018 | "wasmparser", | ||
1019 | ] | ||
1020 | |||
1021 | [[package]] | ||
1022 | name = "object" | ||
1023 | version = "0.23.0" | 1023 | version = "0.23.0" |
1024 | source = "registry+https://github.com/rust-lang/crates.io-index" | 1024 | source = "registry+https://github.com/rust-lang/crates.io-index" |
1025 | checksum = "a9a7ab5d64814df0fe4a4b5ead45ed6c5f181ee3ff04ba344313a6c80446c5d4" | 1025 | checksum = "a9a7ab5d64814df0fe4a4b5ead45ed6c5f181ee3ff04ba344313a6c80446c5d4" |
@@ -1164,7 +1164,8 @@ dependencies = [ | |||
1164 | "crossbeam-channel", | 1164 | "crossbeam-channel", |
1165 | "jod-thread", | 1165 | "jod-thread", |
1166 | "log", | 1166 | "log", |
1167 | "object 0.22.0", | 1167 | "memmap", |
1168 | "object", | ||
1168 | "serde", | 1169 | "serde", |
1169 | "serde_json", | 1170 | "serde_json", |
1170 | "snap", | 1171 | "snap", |
@@ -1180,7 +1181,7 @@ dependencies = [ | |||
1180 | "libloading", | 1181 | "libloading", |
1181 | "mbe", | 1182 | "mbe", |
1182 | "memmap2", | 1183 | "memmap2", |
1183 | "object 0.23.0", | 1184 | "object", |
1184 | "proc_macro_api", | 1185 | "proc_macro_api", |
1185 | "proc_macro_test", | 1186 | "proc_macro_test", |
1186 | "serde_derive", | 1187 | "serde_derive", |
@@ -1899,12 +1900,6 @@ dependencies = [ | |||
1899 | ] | 1900 | ] |
1900 | 1901 | ||
1901 | [[package]] | 1902 | [[package]] |
1902 | name = "wasmparser" | ||
1903 | version = "0.57.0" | ||
1904 | source = "registry+https://github.com/rust-lang/crates.io-index" | ||
1905 | checksum = "32fddd575d477c6e9702484139cf9f23dcd554b06d185ed0f56c857dd3a47aa6" | ||
1906 | |||
1907 | [[package]] | ||
1908 | name = "winapi" | 1903 | name = "winapi" |
1909 | version = "0.3.9" | 1904 | version = "0.3.9" |
1910 | source = "registry+https://github.com/rust-lang/crates.io-index" | 1905 | source = "registry+https://github.com/rust-lang/crates.io-index" |
diff --git a/crates/proc_macro_api/Cargo.toml b/crates/proc_macro_api/Cargo.toml index 5b0a05dff..16fd56c7e 100644 --- a/crates/proc_macro_api/Cargo.toml +++ b/crates/proc_macro_api/Cargo.toml | |||
@@ -21,3 +21,4 @@ base_db = { path = "../base_db", version = "0.0.0" } | |||
21 | stdx = { path = "../stdx", version = "0.0.0" } | 21 | stdx = { path = "../stdx", version = "0.0.0" } |
22 | snap = "1" | 22 | snap = "1" |
23 | object = { version = "0.23.0", default-features = false, features = ["std", "read_core", "elf", "macho", "pe", "unaligned"] } | 23 | object = { version = "0.23.0", default-features = false, features = ["std", "read_core", "elf", "macho", "pe", "unaligned"] } |
24 | memmap = "0.7.0" | ||
diff --git a/crates/proc_macro_api/src/lib.rs b/crates/proc_macro_api/src/lib.rs index cec854746..a58e39de0 100644 --- a/crates/proc_macro_api/src/lib.rs +++ b/crates/proc_macro_api/src/lib.rs | |||
@@ -9,15 +9,22 @@ pub mod msg; | |||
9 | mod process; | 9 | mod process; |
10 | mod rpc; | 10 | mod rpc; |
11 | 11 | ||
12 | use std::{ffi::OsStr, fs::read as fsread, io::{self, Read}, path::{Path, PathBuf}, sync::Arc}; | ||
13 | |||
14 | use base_db::{Env, ProcMacro}; | 12 | use base_db::{Env, ProcMacro}; |
13 | use std::{ | ||
14 | ffi::OsStr, | ||
15 | fs::File, | ||
16 | io::{self, Read}, | ||
17 | path::{Path, PathBuf}, | ||
18 | sync::Arc, | ||
19 | }; | ||
20 | |||
15 | use tt::{SmolStr, Subtree}; | 21 | use tt::{SmolStr, Subtree}; |
16 | 22 | ||
17 | use crate::process::{ProcMacroProcessSrv, ProcMacroProcessThread}; | 23 | use crate::process::{ProcMacroProcessSrv, ProcMacroProcessThread}; |
18 | 24 | ||
19 | pub use rpc::{ExpansionResult, ExpansionTask, ListMacrosResult, ListMacrosTask, ProcMacroKind}; | 25 | pub use rpc::{ExpansionResult, ExpansionTask, ListMacrosResult, ListMacrosTask, ProcMacroKind}; |
20 | 26 | ||
27 | use memmap::Mmap; | ||
21 | use object::read::{File as BinaryFile, Object, ObjectSection}; | 28 | use object::read::{File as BinaryFile, Object, ObjectSection}; |
22 | use snap::read::FrameDecoder as SnapDecoder; | 29 | use snap::read::FrameDecoder as SnapDecoder; |
23 | 30 | ||
@@ -110,13 +117,13 @@ impl ProcMacroClient { | |||
110 | 117 | ||
111 | // This is used inside self.read_version() to locate the ".rustc" section | 118 | // This is used inside self.read_version() to locate the ".rustc" section |
112 | // from a proc macro crate's binary file. | 119 | // from a proc macro crate's binary file. |
113 | fn read_section<'a>(&self, dylib_binary: &'a [u8], section_name: &str) -> &'a [u8] { | 120 | fn read_section<'a>(&self, dylib_binary: &'a [u8], section_name: &str) -> io::Result<&'a [u8]> { |
114 | BinaryFile::parse(dylib_binary) | 121 | BinaryFile::parse(dylib_binary) |
115 | .unwrap() | 122 | .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e))? |
116 | .section_by_name(section_name) | 123 | .section_by_name(section_name) |
117 | .unwrap() | 124 | .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "section read error"))? |
118 | .data() | 125 | .data() |
119 | .unwrap() | 126 | .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e)) |
120 | } | 127 | } |
121 | 128 | ||
122 | // Check the version of rustc that was used to compile a proc macro crate's | 129 | // Check the version of rustc that was used to compile a proc macro crate's |
@@ -138,10 +145,19 @@ impl ProcMacroClient { | |||
138 | // * [some more bytes that we don really care but still there] :-) | 145 | // * [some more bytes that we don really care but still there] :-) |
139 | // Check this issue for more about the bytes layout: | 146 | // Check this issue for more about the bytes layout: |
140 | // https://github.com/rust-analyzer/rust-analyzer/issues/6174 | 147 | // https://github.com/rust-analyzer/rust-analyzer/issues/6174 |
141 | fn read_version(&self, dylib_path: &Path) -> String { | 148 | #[allow(unused)] |
142 | let dylib_binary = fsread(dylib_path).unwrap(); | 149 | fn read_version(&self, dylib_path: &Path) -> io::Result<String> { |
150 | let dylib_file = File::open(dylib_path)?; | ||
151 | let dylib_mmaped = unsafe { Mmap::map(&dylib_file) }?; | ||
152 | |||
153 | let dot_rustc = self.read_section(&dylib_mmaped, ".rustc")?; | ||
143 | 154 | ||
144 | let dot_rustc = self.read_section(&dylib_binary, ".rustc"); | 155 | let header = &dot_rustc[..8]; |
156 | const EXPECTED_HEADER: [u8; 8] = [b'r', b'u', b's', b't', 0, 0, 0, 5]; | ||
157 | // check if header is valid | ||
158 | if !(header == EXPECTED_HEADER) { | ||
159 | return Err(io::Error::new(io::ErrorKind::InvalidData, format!(".rustc section should start with header {:?}; header {:?} is actually presented.",EXPECTED_HEADER ,header))); | ||
160 | } | ||
145 | 161 | ||
146 | let snappy_portion = &dot_rustc[8..]; | 162 | let snappy_portion = &dot_rustc[8..]; |
147 | 163 | ||
@@ -154,14 +170,12 @@ impl ProcMacroClient { | |||
154 | // so 13 bytes in total, and we should check the 13th byte | 170 | // so 13 bytes in total, and we should check the 13th byte |
155 | // to know the length | 171 | // to know the length |
156 | let mut bytes_before_version = [0u8; 13]; | 172 | let mut bytes_before_version = [0u8; 13]; |
157 | snappy_decoder | 173 | snappy_decoder.read_exact(&mut bytes_before_version)?; |
158 | .read_exact(&mut bytes_before_version) | ||
159 | .unwrap(); | ||
160 | let length = bytes_before_version[12]; // what? can't use -1 indexing? | 174 | let length = bytes_before_version[12]; // what? can't use -1 indexing? |
161 | 175 | ||
162 | let mut version_string_utf8 = vec![0u8; length as usize]; | 176 | let mut version_string_utf8 = vec![0u8; length as usize]; |
163 | snappy_decoder.read_exact(&mut version_string_utf8).unwrap(); | 177 | snappy_decoder.read_exact(&mut version_string_utf8)?; |
164 | let version_string = String::from_utf8(version_string_utf8).unwrap(); | 178 | let version_string = String::from_utf8(version_string_utf8); |
165 | version_string | 179 | version_string.map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e)) |
166 | } | 180 | } |
167 | } | 181 | } |