Skip to content

Commit

Permalink
fix: Crash caused by UnwindEntryShard on stack
Browse files Browse the repository at this point in the history
  • Loading branch information
@rvql @sharang
rvql authored and sharang committed Sep 12, 2024
1 parent e0218b7 commit 80bcff4
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 63 deletions.
74 changes: 41 additions & 33 deletions agent/Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

71 changes: 56 additions & 15 deletions agent/crates/trace-utils/src/unwind.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@
pub mod dwarf;
pub mod maps;

use std::alloc::{alloc, dealloc, handle_alloc_error, Layout};
use std::collections::{hash_map::Entry, HashMap, HashSet, VecDeque};
use std::fs;
use std::hash::Hasher;
use std::mem;
use std::path::PathBuf;
use std::ptr::NonNull;
use std::slice;

use ahash::AHasher;
Expand Down Expand Up @@ -69,7 +71,7 @@ impl UnwindTable {
trace!("load dwarf entries for process#{pid}");

let mut shard_list = ProcessShardList::default();
let mut shard: Option<UnwindEntryShard> = None;
let mut shard: Option<BoxedShard> = None;
let mut shard_count = 0;

let base_path: PathBuf = ["/proc", &pid.to_string(), "root"].iter().collect();
Expand Down Expand Up @@ -168,7 +170,7 @@ impl UnwindTable {
continue;
}

match shard.as_ref() {
match shard.as_ref().map(|b| b.as_ref()) {
Some(s) if s.len as usize + entries.len() > UNWIND_ENTRIES_PER_SHARD => {
trace!(
"finish shard#{} because {} + {} > {}",
Expand All @@ -177,19 +179,20 @@ impl UnwindTable {
entries.len(),
UNWIND_ENTRIES_PER_SHARD
);
let s = shard.take().unwrap();
self.update_unwind_entry_shard(s.id, &s);
let boxed_shard = shard.take().unwrap();
let s = boxed_shard.as_ref();
self.update_unwind_entry_shard(s.id, s);
}
_ => (),
}
if shard.is_none() {
let shard_id = self.id_gen.acquire();
trace!("create shard#{shard_id} for unwind entries");
shard.replace(UnwindEntryShard::new(shard_id));
shard.replace(BoxedShard::new(shard_id));
shard_count += 1;
}

let shard = shard.as_mut().unwrap();
let shard = shard.as_mut().unwrap().as_mut();
trace!(
"load object {} into shard#{} with {} entries",
path.display(),
Expand Down Expand Up @@ -230,9 +233,10 @@ impl UnwindTable {
);
shard_list.len += 1;
}
if let Some(s) = shard.take() {
if let Some(bs) = shard.take() {
let s = bs.as_ref();
trace!("finish shard#{} with {} entries", s.id, s.len);
self.update_unwind_entry_shard(s.id, &s);
self.update_unwind_entry_shard(s.id, s);
}

if shard_list.len == 0 {
Expand Down Expand Up @@ -346,18 +350,20 @@ impl UnwindTable {
..Default::default()
};
let mut first_shard = true;
let mut boxed_shard = BoxedShard::new(0);
for chunk in entries.chunks(UNWIND_ENTRIES_PER_SHARD) {
let shard_id = self.id_gen.acquire();
trace!(
"load object into shard#{shard_id} with {} entries",
chunk.len()
);

let mut shard = UnwindEntryShard::new(shard_id);
let shard = boxed_shard.as_mut();
shard.id = shard_id;
shard.len = chunk.len() as u32;
(&mut shard.entries[..shard.len as usize]).copy_from_slice(chunk);

self.update_unwind_entry_shard(shard_id, &shard);
self.update_unwind_entry_shard(shard_id, shard);

if shard_list.len as usize >= UNWIND_SHARDS_PER_PROCESS {
warn!(
Expand Down Expand Up @@ -565,6 +571,9 @@ impl Default for ProcessShardList {
}
}

/*
* This struct has size of 1M, do not allocate it on stack
*/
#[repr(C)]
#[derive(Clone, Debug)]
pub struct UnwindEntryShard {
Expand All @@ -573,12 +582,44 @@ pub struct UnwindEntryShard {
pub entries: [UnwindEntry; UNWIND_ENTRIES_PER_SHARD],
}

impl UnwindEntryShard {
struct BoxedShard(NonNull<UnwindEntryShard>);

impl BoxedShard {
fn new(id: u32) -> Self {
Self {
id,
len: 0,
entries: [UnwindEntry::default(); UNWIND_ENTRIES_PER_SHARD],
// creating UnwindEntryShard with `new` or `default` still use stack
// allocate with Allocater API to avoid this
unsafe {
let layout = Layout::new::<UnwindEntryShard>();
let ptr = alloc(layout);
if ptr.is_null() {
handle_alloc_error(layout);
}
let ptr = ptr as *mut UnwindEntryShard;
(*ptr).id = id;
(*ptr).len = 0;
// entries array do not require initializing
Self(NonNull::new_unchecked(ptr as *mut UnwindEntryShard))
}
}
}

impl Drop for BoxedShard {
fn drop(&mut self) {
unsafe {
let layout = Layout::new::<UnwindEntryShard>();
dealloc(self.0.as_ptr() as *mut u8, layout);
}
}
}

impl AsRef<UnwindEntryShard> for BoxedShard {
fn as_ref(&self) -> &UnwindEntryShard {
unsafe { self.0.as_ref() }
}
}

impl AsMut<UnwindEntryShard> for BoxedShard {
fn as_mut(&mut self) -> &mut UnwindEntryShard {
unsafe { self.0.as_mut() }
}
}
24 changes: 9 additions & 15 deletions agent/src/flow_generator/protocol_logs/tls.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

use std::fmt::Display;

use chrono::{prelude::NaiveDateTime, Utc};
use chrono::{DateTime, Utc};
use serde::Serialize;

use super::pb_adapter::{
Expand Down Expand Up @@ -370,20 +370,17 @@ impl From<TlsInfo> for L7ProtocolSendLog {
if !f.client_cert_not_before.is_zero() {
attributes.push(KeyVal {
key: "client_cert_not_before".to_string(),
val: NaiveDateTime::from_timestamp_opt(
f.client_cert_not_before.as_secs() as i64,
0,
)
.unwrap()
.to_string(),
val: DateTime::from_timestamp(f.client_cert_not_before.as_secs() as i64, 0)
.unwrap()
.to_string(),
});
}
if !f.client_cert_not_after.is_zero() {
let valid_days =
(f.client_cert_not_after.as_secs() as i64 - now) as f32 / Self::SECONDS_PER_DAY;
attributes.push(KeyVal {
key: "client_cert_not_after".to_string(),
val: NaiveDateTime::from_timestamp_opt(f.client_cert_not_after.as_secs() as i64, 0)
val: DateTime::from_timestamp(f.client_cert_not_after.as_secs() as i64, 0)
.unwrap()
.to_string(),
});
Expand All @@ -395,20 +392,17 @@ impl From<TlsInfo> for L7ProtocolSendLog {
if !f.server_cert_not_before.is_zero() {
attributes.push(KeyVal {
key: "server_cert_not_before".to_string(),
val: NaiveDateTime::from_timestamp_opt(
f.server_cert_not_before.as_secs() as i64,
0,
)
.unwrap()
.to_string(),
val: DateTime::from_timestamp(f.server_cert_not_before.as_secs() as i64, 0)
.unwrap()
.to_string(),
});
}
if !f.server_cert_not_after.is_zero() {
let valid_days =
(f.server_cert_not_after.as_secs() as i64 - now) as f32 / Self::SECONDS_PER_DAY;
attributes.push(KeyVal {
key: "server_cert_not_after".to_string(),
val: NaiveDateTime::from_timestamp_opt(f.server_cert_not_after.as_secs() as i64, 0)
val: DateTime::from_timestamp(f.server_cert_not_after.as_secs() as i64, 0)
.unwrap()
.to_string(),
});
Expand Down

0 comments on commit 80bcff4

Please sign in to comment.