Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Mark several functions as unsafe. #217

Merged
merged 2 commits into from
Sep 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 16 additions & 16 deletions pgx-macros/src/rewriter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ impl PgGuardRewriter {
let result = match pgx::PgMemoryContexts::For(funcctx.multi_call_memory_ctx).switch_to(|_| { #func_call result }) {
Some(result) => result,
None => {
pgx::srf_return_done(fcinfo, &mut funcctx);
unsafe { pgx::srf_return_done(fcinfo, &mut funcctx); }
return pgx::pg_return_null(fcinfo)
}
};
Expand All @@ -274,19 +274,19 @@ impl PgGuardRewriter {
let mut funcctx: pgx::PgBox<pg_sys::FuncCallContext>;
let mut iterator_holder: pgx::PgBox<IteratorHolder<#generic_type>>;

if srf_is_first_call(fcinfo) {
funcctx = pgx::srf_first_call_init(fcinfo);
if unsafe { srf_is_first_call(fcinfo) } {
funcctx = unsafe { pgx::srf_first_call_init(fcinfo) };
funcctx.user_fctx = pgx::PgMemoryContexts::For(funcctx.multi_call_memory_ctx).palloc_struct::<IteratorHolder<#generic_type>>() as void_mut_ptr;

iterator_holder = pgx::PgBox::from_pg(funcctx.user_fctx as *mut IteratorHolder<#generic_type>);
iterator_holder = unsafe { pgx::PgBox::from_pg(funcctx.user_fctx as *mut IteratorHolder<#generic_type>) };

#result_handler

iterator_holder.iter = pgx::PgMemoryContexts::For(funcctx.multi_call_memory_ctx).leak_and_drop_on_delete(result);
}

funcctx = pgx::srf_per_call_setup(fcinfo);
iterator_holder = pgx::PgBox::from_pg(funcctx.user_fctx as *mut IteratorHolder<#generic_type>);
funcctx = unsafe { pgx::srf_per_call_setup(fcinfo) };
iterator_holder = unsafe { pgx::PgBox::from_pg(funcctx.user_fctx as *mut IteratorHolder<#generic_type>) };

let mut iter = unsafe { Box::from_raw(iterator_holder.iter) };
match iter.next() {
Expand All @@ -295,7 +295,7 @@ impl PgGuardRewriter {
// continue to use it
Box::leak(iter);

pgx::srf_return_next(fcinfo, &mut funcctx);
unsafe { pgx::srf_return_next(fcinfo, &mut funcctx) };
match result.into_datum() {
Some(datum) => datum,
None => pgx::pg_return_null(fcinfo),
Expand All @@ -306,7 +306,7 @@ impl PgGuardRewriter {
// function is going to properly drop it for us
Box::leak(iter);

pgx::srf_return_done(fcinfo, &mut funcctx);
unsafe { pgx::srf_return_done(fcinfo, &mut funcctx) };
pgx::pg_return_null(fcinfo)
},
}
Expand Down Expand Up @@ -354,7 +354,7 @@ impl PgGuardRewriter {
let result = match pgx::PgMemoryContexts::For(funcctx.multi_call_memory_ctx).switch_to(|_| { #func_call result }) {
Some(result) => result,
None => {
pgx::srf_return_done(fcinfo, &mut funcctx);
unsafe { pgx::srf_return_done(fcinfo, &mut funcctx); }
return pgx::pg_return_null(fcinfo)
}
};
Expand All @@ -378,8 +378,8 @@ impl PgGuardRewriter {
let mut funcctx: pgx::PgBox<pg_sys::FuncCallContext>;
let mut iterator_holder: pgx::PgBox<IteratorHolder<#generic_type>>;

if srf_is_first_call(fcinfo) {
funcctx = pgx::srf_first_call_init(fcinfo);
if unsafe { srf_is_first_call(fcinfo) } {
funcctx = unsafe { pgx::srf_first_call_init(fcinfo) };
funcctx.user_fctx = pgx::PgMemoryContexts::For(funcctx.multi_call_memory_ctx).palloc_struct::<IteratorHolder<#generic_type>>() as void_mut_ptr;
funcctx.tuple_desc = pgx::PgMemoryContexts::For(funcctx.multi_call_memory_ctx).switch_to(|_| {
let mut tupdesc: *mut pgx::pg_sys::TupleDescData = std::ptr::null_mut();
Expand All @@ -393,15 +393,15 @@ impl PgGuardRewriter {
pgx::pg_sys::BlessTupleDesc(tupdesc)
}
});
iterator_holder = pgx::PgBox::from_pg(funcctx.user_fctx as *mut IteratorHolder<#generic_type>);
iterator_holder = unsafe { pgx::PgBox::from_pg(funcctx.user_fctx as *mut IteratorHolder<#generic_type>) };

#result_handler

iterator_holder.iter = pgx::PgMemoryContexts::For(funcctx.multi_call_memory_ctx).leak_and_drop_on_delete(result);
}

funcctx = pgx::srf_per_call_setup(fcinfo);
iterator_holder = pgx::PgBox::from_pg(funcctx.user_fctx as *mut IteratorHolder<#generic_type>);
funcctx = unsafe { pgx::srf_per_call_setup(fcinfo) };
iterator_holder = unsafe { pgx::PgBox::from_pg(funcctx.user_fctx as *mut IteratorHolder<#generic_type>) };

let mut iter = unsafe { Box::from_raw(iterator_holder.iter) };
match iter.next() {
Expand All @@ -413,15 +413,15 @@ impl PgGuardRewriter {
#create_heap_tuple

let datum = pgx::heap_tuple_get_datum(heap_tuple);
pgx::srf_return_next(fcinfo, &mut funcctx);
unsafe { pgx::srf_return_next(fcinfo, &mut funcctx); }
datum as pgx::pg_sys::Datum
},
None => {
// leak the iterator here too, even tho we're done, b/c our MemoryContextCallback
// function is going to properly drop it for us
Box::leak(iter);

pgx::srf_return_done(fcinfo, &mut funcctx);
unsafe { pgx::srf_return_done(fcinfo, &mut funcctx); }
pgx::pg_return_null(fcinfo)
},
}
Expand Down
6 changes: 3 additions & 3 deletions pgx/src/datum/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ impl<'a, T: FromDatum> Array<'a, T> {
}
}

fn from_pg(
unsafe fn from_pg(
ptr: *mut pg_sys::varlena,
array_type: *mut pg_sys::ArrayType,
elements: *mut pg_sys::Datum,
Expand All @@ -80,8 +80,8 @@ impl<'a, T: FromDatum> Array<'a, T> {
nulls,
typoid,
nelems,
elem_slice: unsafe { std::slice::from_raw_parts(elements, nelems) },
null_slice: unsafe { std::slice::from_raw_parts(nulls, nelems) },
elem_slice: std::slice::from_raw_parts(elements, nelems),
null_slice: std::slice::from_raw_parts(nulls, nelems),
_marker: PhantomData,
}
}
Expand Down
7 changes: 5 additions & 2 deletions pgx/src/datum/varlena.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@ struct PallocdVarlena {
impl Clone for PallocdVarlena {
fn clone(&self) -> Self {
let len = self.len;
let ptr = PgMemoryContexts::Of(self.ptr as void_mut_ptr)
.copy_ptr_into(self.ptr as void_mut_ptr, len) as *mut pg_sys::varlena;

// SAFETY: we know that `self.ptr` is valid as the only way we could have gotten one
// is internally via Postgres
let ptr = unsafe { PgMemoryContexts::Of(self.ptr as void_mut_ptr)
.copy_ptr_into(self.ptr as void_mut_ptr, len) as *mut pg_sys::varlena };

PallocdVarlena { ptr, len }
}
Expand Down
17 changes: 8 additions & 9 deletions pgx/src/enum_helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,27 +73,26 @@ pub fn lookup_enum_by_label(typname: &str, label: &str) -> pg_sys::Datum {
);
}

let oid = extract_enum_oid(tup);

// SAFETY: we know that `tup` is valid because we just got it from Postgres above
unsafe {
let oid = extract_enum_oid(tup);
pg_sys::ReleaseSysCache(tup);
oid as pg_sys::Datum
}

oid as pg_sys::Datum
}

#[cfg(any(feature = "pg10", feature = "pg11"))]
fn extract_enum_oid(tup: *mut pg_sys::HeapTupleData) -> pg_sys::Oid {
unsafe fn extract_enum_oid(tup: *mut pg_sys::HeapTupleData) -> pg_sys::Oid {
extern "C" {
fn pgx_HeapTupleHeaderGetOid(htup_header: pg_sys::HeapTupleHeader) -> pg_sys::Oid;
}

unsafe { pgx_HeapTupleHeaderGetOid(tup.as_ref().unwrap().t_data) }
pgx_HeapTupleHeaderGetOid(tup.as_ref().unwrap().t_data)
}

#[cfg(any(feature = "pg12", feature = "pg13"))]
fn extract_enum_oid(tup: *mut pg_sys::HeapTupleData) -> pg_sys::Oid {
let en = unsafe { pgx_GETSTRUCT(tup) } as pg_sys::Form_pg_enum;
let en = unsafe { en.as_ref() }.unwrap();
unsafe fn extract_enum_oid(tup: *mut pg_sys::HeapTupleData) -> pg_sys::Oid {
let en = pgx_GETSTRUCT(tup) as pg_sys::Form_pg_enum;
let en = en.as_ref().unwrap();
en.oid
}
31 changes: 18 additions & 13 deletions pgx/src/fcinfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,10 @@ pub fn pg_return_void() -> pg_sys::Datum {
0 as pg_sys::Datum
}

pub fn pg_func_extra<ReturnType, DefaultValue: FnOnce() -> ReturnType>(
/// Retrieve the `.flinfo.fn_extra` pointer (as a PgBox'd type) from [pg_sys::FunctionCallInfo].
///
/// This function is unsafe as we cannot guarantee the provided [fcinfo] pointer is valid
pub unsafe fn pg_func_extra<ReturnType, DefaultValue: FnOnce() -> ReturnType>(
fcinfo: pg_sys::FunctionCallInfo,
default: DefaultValue,
) -> PgBox<ReturnType> {
Expand Down Expand Up @@ -343,7 +346,8 @@ fn make_function_call_info(
) as *mut pg_sys::FunctionCallInfoBaseData
};

let mut fcinfo_boxed = PgBox::<pg_sys::FunctionCallInfoBaseData>::from_rust(fcid);
// SAFETY: we know fcid is valid as we just created it
let mut fcinfo_boxed = unsafe { PgBox::<pg_sys::FunctionCallInfoBaseData>::from_rust(fcid) };
let fcinfo = fcinfo_boxed.deref_mut();

fcinfo.nargs = nargs as i16;
Expand All @@ -360,27 +364,31 @@ fn make_function_call_info(
}

#[inline]
pub fn srf_is_first_call(fcinfo: pg_sys::FunctionCallInfo) -> bool {
pub unsafe fn srf_is_first_call(fcinfo: pg_sys::FunctionCallInfo) -> bool {
let fcinfo = PgBox::from_pg(fcinfo);
let flinfo = PgBox::from_pg(fcinfo.flinfo);

flinfo.fn_extra.is_null()
}

#[inline]
pub fn srf_first_call_init(fcinfo: pg_sys::FunctionCallInfo) -> PgBox<pg_sys::FuncCallContext> {
let funcctx = unsafe { pg_sys::init_MultiFuncCall(fcinfo) };
pub unsafe fn srf_first_call_init(
fcinfo: pg_sys::FunctionCallInfo,
) -> PgBox<pg_sys::FuncCallContext> {
let funcctx = pg_sys::init_MultiFuncCall(fcinfo);
PgBox::from_pg(funcctx)
}

#[inline]
pub fn srf_per_call_setup(fcinfo: pg_sys::FunctionCallInfo) -> PgBox<pg_sys::FuncCallContext> {
let funcctx = unsafe { pg_sys::per_MultiFuncCall(fcinfo) };
pub unsafe fn srf_per_call_setup(
fcinfo: pg_sys::FunctionCallInfo,
) -> PgBox<pg_sys::FuncCallContext> {
let funcctx = pg_sys::per_MultiFuncCall(fcinfo);
PgBox::from_pg(funcctx)
}

#[inline]
pub fn srf_return_next(
pub unsafe fn srf_return_next(
fcinfo: pg_sys::FunctionCallInfo,
funcctx: &mut PgBox<pg_sys::FuncCallContext>,
) {
Expand All @@ -392,14 +400,11 @@ pub fn srf_return_next(
}

#[inline]
pub fn srf_return_done(
pub unsafe fn srf_return_done(
fcinfo: pg_sys::FunctionCallInfo,
funcctx: &mut PgBox<pg_sys::FuncCallContext>,
) {
unsafe {
pg_sys::end_MultiFuncCall(fcinfo, funcctx.as_ptr());
}

pg_sys::end_MultiFuncCall(fcinfo, funcctx.as_ptr());
let fcinfo = PgBox::from_pg(fcinfo);
let mut rsi = PgBox::from_pg(fcinfo.resultinfo as *mut pg_sys::ReturnSetInfo);
rsi.isDone = pg_sys::ExprDoneCond_ExprEndResult;
Expand Down
38 changes: 22 additions & 16 deletions pgx/src/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl<T> PgList<T> {
}
}

pub fn from_pg(list: *mut pg_sys::List) -> Self {
pub unsafe fn from_pg(list: *mut pg_sys::List) -> Self {
PgList {
list,
allocated_by_pg: true,
Expand Down Expand Up @@ -80,7 +80,9 @@ impl<T> PgList<T> {

#[inline]
pub fn get_ptr(&self, i: usize) -> Option<*mut T> {
if !self.is_empty() && unsafe { !is_a(self.list as *mut pg_sys::Node, pg_sys::NodeTag_T_List) } {
if !self.is_empty()
&& unsafe { !is_a(self.list as *mut pg_sys::Node, pg_sys::NodeTag_T_List) }
{
panic!("PgList does not contain pointers")
}
if self.list.is_null() || i >= self.len() {
Expand All @@ -92,7 +94,9 @@ impl<T> PgList<T> {

#[inline]
pub fn get_int(&self, i: usize) -> Option<i32> {
if !self.is_empty() && unsafe { !is_a(self.list as *mut pg_sys::Node, pg_sys::NodeTag_T_IntList) } {
if !self.is_empty()
&& unsafe { !is_a(self.list as *mut pg_sys::Node, pg_sys::NodeTag_T_IntList) }
{
panic!("PgList does not contain ints")
}

Expand All @@ -105,7 +109,9 @@ impl<T> PgList<T> {

#[inline]
pub fn get_oid(&self, i: usize) -> Option<pg_sys::Oid> {
if !self.is_empty() && unsafe { !is_a(self.list as *mut pg_sys::Node, pg_sys::NodeTag_T_OidList) } {
if !self.is_empty()
&& unsafe { !is_a(self.list as *mut pg_sys::Node, pg_sys::NodeTag_T_OidList) }
{
panic!("PgList does not contain oids")
}

Expand All @@ -118,22 +124,16 @@ impl<T> PgList<T> {

#[cfg(not(feature = "pg13"))]
#[inline]
pub fn replace_ptr(&mut self, i: usize, with: *mut T) {
unsafe {
let cell = pg_sys::pgx_list_nth_cell(self.list, i as i32);

cell.as_mut().expect("cell is null").data.ptr_value = with as void_mut_ptr;
}
pub unsafe fn replace_ptr(&mut self, i: usize, with: *mut T) {
let cell = pg_sys::pgx_list_nth_cell(self.list, i as i32);
cell.as_mut().expect("cell is null").data.ptr_value = with as void_mut_ptr;
}

#[cfg(feature = "pg13")]
#[inline]
pub fn replace_ptr(&mut self, i: usize, with: *mut T) {
unsafe {
let cell = pg_sys::pgx_list_nth_cell(self.list, i as i32);

cell.as_mut().expect("cell is null").ptr_value = with as void_mut_ptr;
}
pub unsafe fn replace_ptr(&mut self, i: usize, with: *mut T) {
let cell = pg_sys::pgx_list_nth_cell(self.list, i as i32);
cell.as_mut().expect("cell is null").ptr_value = with as void_mut_ptr;
}

#[cfg(not(feature = "pg13"))]
Expand Down Expand Up @@ -196,6 +196,12 @@ impl<T> PgList<T> {
}
}

/// Add a pointer value to the end of this list
///
/// ## Safety
///
/// We cannot guarantee the specified pointer is valid, but we assume it is as we only store it,
/// we don't dereference it
#[inline]
pub fn push(&mut self, ptr: *mut T) {
self.list = unsafe { pg_sys::lappend(self.list, ptr as void_mut_ptr) };
Expand Down
14 changes: 7 additions & 7 deletions pgx/src/memcxt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,15 +308,14 @@ impl PgMemoryContexts {

/// Copies `len` bytes, starting at `src` into this memory context and
/// returns a raw `*mut T` pointer to the newly allocated location
pub fn copy_ptr_into<T>(&mut self, src: *mut T, len: usize) -> *mut T {
pub unsafe fn copy_ptr_into<T>(&mut self, src: *mut T, len: usize) -> *mut T {
if src.is_null() {
panic!("attempt to copy a null pointer");
}
unsafe {
let dest = pg_sys::MemoryContextAlloc(self.value(), len);
pg_sys::memcpy(dest, src as void_mut_ptr, len as u64);
dest as *mut T
}

let dest = pg_sys::MemoryContextAlloc(self.value(), len);
pg_sys::memcpy(dest, src as void_mut_ptr, len as u64);
dest as *mut T
}

/// Allocate memory in this context, which will be free'd whenever Postgres deletes this MemoryContext
Expand Down Expand Up @@ -358,8 +357,9 @@ impl PgMemoryContexts {
}

let leaked_ptr = Box::leak(Box::new(v));
// SAFETY: we know the result of `self.palloc_struct()` is a valid pointer
let mut memcxt_callback =
PgBox::from_pg(self.palloc_struct::<pg_sys::MemoryContextCallback>());
unsafe { PgBox::from_pg(self.palloc_struct::<pg_sys::MemoryContextCallback>()) };
memcxt_callback.func = Some(drop_on_delete::<T>);
memcxt_callback.arg = leaked_ptr as *mut T as void_mut_ptr;
unsafe {
Expand Down
8 changes: 6 additions & 2 deletions pgx/src/namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,12 @@ impl PgQualifiedNameBuilder {
}

pub fn push(mut self, value: &str) -> PgQualifiedNameBuilder {
self.list
.push(unsafe { pg_sys::makeString(std::ffi::CString::new(value).unwrap().into_raw()) });
unsafe {
// SAFETY: the result of pg_sys::makeString is always a valid pointer
self.list.push(pg_sys::makeString(
std::ffi::CString::new(value).unwrap().into_raw(),
));
}
self
}

Expand Down
Loading