Skip to content

Commit 1333fd3

Browse files
committed
Use MaybeUninit<u8> instead of mem::uninitialized.
Fixes #35. Also contains a small stacked borrows soundness fix for a `ptr::copy` invocation, it reorders the creation of the src and dst pointers. The code now passes miri with `-Zmiri-tag-raw-pointers`.
1 parent 0566b16 commit 1333fd3

File tree

4 files changed

+58
-39
lines changed

4 files changed

+58
-39
lines changed

src/lib.rs

Lines changed: 55 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -72,15 +72,15 @@ extern crate log;
7272

7373
extern crate env_logger;
7474

75+
use log::{Level, Log, Metadata, Record};
7576
#[cfg(target_os = "android")]
7677
use log_ffi::LogPriority;
77-
use log::{Level, Log, Metadata, Record};
7878
use std::ffi::{CStr, CString};
79-
use std::mem;
8079
use std::fmt;
80+
use std::mem::{self, MaybeUninit};
8181
use std::ptr;
8282

83-
pub use env_logger::filter::{Filter, Builder as FilterBuilder};
83+
pub use env_logger::filter::{Builder as FilterBuilder, Filter};
8484
pub use env_logger::fmt::Formatter;
8585

8686
pub(crate) type FormatFn = Box<dyn Fn(&mut dyn fmt::Write, &Record) -> fmt::Result + Sync + Send>;
@@ -115,7 +115,6 @@ impl AndroidLogger {
115115
}
116116
}
117117

118-
119118
static ANDROID_LOGGER: OnceCell<AndroidLogger> = OnceCell::new();
120119

121120
const LOGGING_TAG_MAX_LEN: usize = 23;
@@ -136,22 +135,23 @@ impl Log for AndroidLogger {
136135
}
137136

138137
fn log(&self, record: &Record) {
139-
let config = self.config
140-
.get_or_init(Config::default);
138+
let config = self.config.get_or_init(Config::default);
141139

142140
if !config.filter_matches(record) {
143141
return;
144142
}
145143

146144
// tag must not exceed LOGGING_TAG_MAX_LEN
147-
#[allow(deprecated)] // created an issue #35 for this
148-
let mut tag_bytes: [u8; LOGGING_TAG_MAX_LEN + 1] = unsafe { mem::uninitialized() };
145+
let mut tag_bytes: [MaybeUninit<u8>; LOGGING_TAG_MAX_LEN + 1] = uninit_array();
149146

150147
let module_path = record.module_path().unwrap_or_default().to_owned();
151148

152149
// If no tag was specified, use module name
153150
let custom_tag = &config.tag;
154-
let tag = custom_tag.as_ref().map(|s| s.as_bytes()).unwrap_or_else(|| module_path.as_bytes());
151+
let tag = custom_tag
152+
.as_ref()
153+
.map(|s| s.as_bytes())
154+
.unwrap_or_else(|| module_path.as_bytes());
155155

156156
// truncate the tag here to fit into LOGGING_TAG_MAX_LEN
157157
self.fill_tag_bytes(&mut tag_bytes, tag);
@@ -181,21 +181,19 @@ impl Log for AndroidLogger {
181181
}
182182

183183
impl AndroidLogger {
184-
fn fill_tag_bytes(&self, array: &mut [u8], tag: &[u8]) {
184+
fn fill_tag_bytes(&self, array: &mut [MaybeUninit<u8>], tag: &[u8]) {
185185
if tag.len() > LOGGING_TAG_MAX_LEN {
186-
for (input, output) in tag.iter()
186+
for (input, output) in tag
187+
.iter()
187188
.take(LOGGING_TAG_MAX_LEN - 2)
188189
.chain(b"..\0".iter())
189190
.zip(array.iter_mut())
190191
{
191-
*output = *input;
192+
output.write(*input);
192193
}
193194
} else {
194-
for (input, output) in tag.iter()
195-
.chain(b"\0".iter())
196-
.zip(array.iter_mut())
197-
{
198-
*output = *input;
195+
for (input, output) in tag.iter().chain(b"\0".iter()).zip(array.iter_mut()) {
196+
output.write(*input);
199197
}
200198
}
201199
}
@@ -257,12 +255,14 @@ impl Config {
257255
}
258256

259257
pub struct PlatformLogWriter<'a> {
260-
#[cfg(target_os = "android")] priority: LogPriority,
261-
#[cfg(not(target_os = "android"))] priority: Level,
258+
#[cfg(target_os = "android")]
259+
priority: LogPriority,
260+
#[cfg(not(target_os = "android"))]
261+
priority: Level,
262262
len: usize,
263263
last_newline_index: usize,
264264
tag: &'a CStr,
265-
buffer: [u8; LOGGING_MSG_MAX_LEN + 1],
265+
buffer: [MaybeUninit<u8>; LOGGING_MSG_MAX_LEN + 1],
266266
}
267267

268268
impl<'a> PlatformLogWriter<'a> {
@@ -280,7 +280,7 @@ impl<'a> PlatformLogWriter<'a> {
280280
len: 0,
281281
last_newline_index: 0,
282282
tag,
283-
buffer: unsafe { mem::uninitialized() },
283+
buffer: uninit_array(),
284284
}
285285
}
286286

@@ -292,7 +292,7 @@ impl<'a> PlatformLogWriter<'a> {
292292
len: 0,
293293
last_newline_index: 0,
294294
tag,
295-
buffer: unsafe { mem::uninitialized() },
295+
buffer: uninit_array(),
296296
}
297297
}
298298

@@ -338,21 +338,22 @@ impl<'a> PlatformLogWriter<'a> {
338338

339339
/// Output buffer up until the \0 which will be placed at `len` position.
340340
fn output_specified_len(&mut self, len: usize) {
341-
let mut last_byte: u8 = b'\0';
341+
let mut last_byte = MaybeUninit::new(b'\0');
342+
342343
mem::swap(&mut last_byte, unsafe {
343344
self.buffer.get_unchecked_mut(len)
344345
});
345346

346-
let msg: &CStr = unsafe { CStr::from_ptr(mem::transmute(self.buffer.as_ptr())) };
347+
let msg: &CStr = unsafe { CStr::from_ptr(self.buffer.as_ptr().cast()) };
347348
android_log(self.priority, self.tag, msg);
348349

349-
*unsafe { self.buffer.get_unchecked_mut(len) } = last_byte;
350+
unsafe { *self.buffer.get_unchecked_mut(len) = last_byte };
350351
}
351352

352353
/// Copy `len` bytes from `index` position to starting position.
353354
fn copy_bytes_to_start(&mut self, index: usize, len: usize) {
354-
let src = unsafe { self.buffer.as_ptr().add(index) };
355355
let dst = self.buffer.as_mut_ptr();
356+
let src = unsafe { self.buffer.as_ptr().add(index) };
356357
unsafe { ptr::copy(src, dst, len) };
357358
}
358359
}
@@ -371,7 +372,7 @@ impl<'a> fmt::Write for PlatformLogWriter<'a> {
371372
.zip(incomming_bytes)
372373
.enumerate()
373374
.fold(None, |acc, (i, (output, input))| {
374-
*output = *input;
375+
output.write(*input);
375376
if *input == b'\n' {
376377
Some(i)
377378
} else {
@@ -409,7 +410,9 @@ impl<'a> fmt::Write for PlatformLogWriter<'a> {
409410
/// This action does not require initialization. However, without initialization it
410411
/// will use the default filter, which allows all logs.
411412
pub fn log(record: &Record) {
412-
ANDROID_LOGGER.get_or_init(AndroidLogger::default).log(record)
413+
ANDROID_LOGGER
414+
.get_or_init(AndroidLogger::default)
415+
.log(record)
413416
}
414417

415418
/// Initializes the global logger with an android logger.
@@ -430,6 +433,12 @@ pub fn init_once(config: Config) {
430433
}
431434
}
432435

436+
// FIXME: When `maybe_uninit_uninit_array ` is stabilized, use it instead of this helper
437+
fn uninit_array<const N: usize, T>() -> [MaybeUninit<T>; N] {
438+
// SAFETY: Array contains MaybeUninit, which is fine to be uninit
439+
unsafe { MaybeUninit::uninit().assume_init() }
440+
}
441+
433442
#[cfg(test)]
434443
mod tests {
435444
use super::*;
@@ -489,25 +498,25 @@ mod tests {
489498
let logger = AndroidLogger::new(Config::default());
490499
let too_long_tag: [u8; LOGGING_TAG_MAX_LEN + 20] = [b'a'; LOGGING_TAG_MAX_LEN + 20];
491500

492-
let mut result: [u8; LOGGING_TAG_MAX_LEN + 1] = Default::default();
501+
let mut result: [MaybeUninit<u8>; LOGGING_TAG_MAX_LEN + 1] = uninit_array();
493502
logger.fill_tag_bytes(&mut result, &too_long_tag);
494503

495504
let mut expected_result = [b'a'; LOGGING_TAG_MAX_LEN - 2].to_vec();
496505
expected_result.extend("..\0".as_bytes());
497-
assert_eq!(result.to_vec(), expected_result);
506+
assert_eq!(unsafe { assume_init_slice(&result) }, expected_result);
498507
}
499508

500509
#[test]
501510
fn fill_tag_bytes_keeps_short_tag() {
502511
let logger = AndroidLogger::new(Config::default());
503512
let short_tag: [u8; 3] = [b'a'; 3];
504513

505-
let mut result: [u8; LOGGING_TAG_MAX_LEN + 1] = Default::default();
514+
let mut result: [MaybeUninit<u8>; LOGGING_TAG_MAX_LEN + 1] = uninit_array();
506515
logger.fill_tag_bytes(&mut result, &short_tag);
507516

508517
let mut expected_result = short_tag.to_vec();
509518
expected_result.push(0);
510-
assert_eq!(result.to_vec()[..4], expected_result);
519+
assert_eq!(unsafe { assume_init_slice(&result[..4]) }, expected_result);
511520
}
512521

513522
#[test]
@@ -535,7 +544,10 @@ mod tests {
535544
// Should have flushed up until the last newline.
536545
assert_eq!(writer.len, 3);
537546
assert_eq!(writer.last_newline_index, 0);
538-
assert_eq!(&writer.buffer.to_vec()[..writer.len], "\n90".as_bytes());
547+
assert_eq!(
548+
unsafe { assume_init_slice(&writer.buffer[..writer.len]) },
549+
"\n90".as_bytes()
550+
);
539551

540552
writer.temporal_flush();
541553
// Should have flushed all remaining bytes.
@@ -578,7 +590,7 @@ mod tests {
578590
writer.output_specified_len(5);
579591

580592
assert_eq!(
581-
writer.buffer[..log_string.len()].to_vec(),
593+
unsafe { assume_init_slice(&writer.buffer[..log_string.len()]) },
582594
log_string.as_bytes()
583595
);
584596
}
@@ -592,7 +604,10 @@ mod tests {
592604

593605
writer.copy_bytes_to_start(3, 2);
594606

595-
assert_eq!(writer.buffer[..10].to_vec(), "3423456789".as_bytes());
607+
assert_eq!(
608+
unsafe { assume_init_slice(&writer.buffer[..10]) },
609+
"3423456789".as_bytes()
610+
);
596611
}
597612

598613
#[test]
@@ -607,12 +622,16 @@ mod tests {
607622
writer.copy_bytes_to_start(10, 0);
608623

609624
assert_eq!(
610-
writer.buffer[..test_string.len()].to_vec(),
625+
unsafe { assume_init_slice(&writer.buffer[..test_string.len()]) },
611626
test_string.as_bytes()
612627
);
613628
}
614629

615630
fn get_tag_writer() -> PlatformLogWriter<'static> {
616631
PlatformLogWriter::new(Level::Warn, CStr::from_bytes_with_nul(b"tag\0").unwrap())
617632
}
633+
634+
unsafe fn assume_init_slice<T>(slice: &[MaybeUninit<T>]) -> &[T] {
635+
&*(slice as *const [MaybeUninit<T>] as *const [T])
636+
}
618637
}

tests/config_log_level.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@ fn config_log_level() {
66
android_logger::init_once(android_logger::Config::default().with_min_level(log::Level::Trace));
77

88
assert_eq!(log::max_level(), log::LevelFilter::Trace);
9-
}
9+
}

tests/default_init.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,4 @@ fn default_init() {
77

88
// android_logger has default log level "off"
99
assert_eq!(log::max_level(), log::LevelFilter::Off);
10-
}
10+
}

tests/multiple_init.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,4 @@ fn multiple_init() {
99
android_logger::init_once(android_logger::Config::default().with_min_level(log::Level::Error));
1010

1111
assert_eq!(log::max_level(), log::LevelFilter::Trace);
12-
}
12+
}

0 commit comments

Comments
 (0)