Skip to content

Commit

Permalink
[Rust-GCC#3045] #[may_dangle] in safe impl
Browse files Browse the repository at this point in the history
gcc/rust/ChangeLog:
	* ast/rust-ast.cc:
	Fix Attribute constructors to copy inner_attribute
	* checks/errors/rust-unsafe-checker.cc:
	Add pass for #[may_dangle] in safe impl's
	* hir/rust-ast-lower-item.cc:
	Add support for unsafe impl's
	* hir/rust-ast-lower-type.cc:
	Lower attributes in impl's from AST to HIR
	* hir/rust-hir-dump.cc:
	Change single attribute to AttrVec
	* hir/tree/rust-hir-item.h:
	Add unsafe support to Impl blocks in HIR
	* hir/tree/rust-hir.cc:
	Change single attribute to AttrVec
	* hir/tree/rust-hir.h:
	Add has/get_outer_attribute to GenericParam

gcc/testsuite/ChangeLog:
	* rust/compile/issue-3045-1.rs:
	Add test for #[may_dangle] Generic Type triggering error
	* rust/compile/issue-3045-2.rs:
	Add test for #[may_dangle] Lifetime triggering error

Signed-off-by: Liam Naddell <liam.naddell@mail.utoronto.ca>
  • Loading branch information
liamnaddell authored and P-E-P committed Jul 25, 2024
1 parent 47c947c commit a218b03
Show file tree
Hide file tree
Showing 10 changed files with 122 additions and 37 deletions.
4 changes: 3 additions & 1 deletion gcc/rust/ast/rust-ast.cc
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,8 @@ Attribute::get_traits_to_derive ()

// Copy constructor must deep copy attr_input as unique pointer
Attribute::Attribute (Attribute const &other)
: path (other.path), locus (other.locus)
: path (other.path), locus (other.locus),
inner_attribute (other.inner_attribute)
{
// guard to protect from null pointer dereference
if (other.attr_input != nullptr)
Expand All @@ -324,6 +325,7 @@ Attribute::operator= (Attribute const &other)
{
path = other.path;
locus = other.locus;
inner_attribute = other.inner_attribute;
// guard to protect from null pointer dereference
if (other.attr_input != nullptr)
attr_input = other.attr_input->clone_attr_input ();
Expand Down
18 changes: 17 additions & 1 deletion gcc/rust/checks/errors/rust-unsafe-checker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,23 @@ UnsafeChecker::visit (Trait &trait)
void
UnsafeChecker::visit (ImplBlock &impl)
{
// FIXME: Handle unsafe impls
bool safe = !impl.is_unsafe ();
// Check for unsafe-only attributes on generics and lifetimes
if (safe)
for (auto &parm : impl.get_generic_params ())
{
for (auto o_attr : parm->get_outer_attrs ())
{
rust_assert (!o_attr.is_inner_attribute ());

Rust::AST::SimplePath path = o_attr.get_path ();
if (path == Values::Attributes::MAY_DANGLE)
rust_error_at (
o_attr.get_locus (), ErrorCode::E0569,
"use of %<may_dangle%> is unsafe and requires unsafe impl");
}
}

for (auto &item : impl.get_impl_items ())
item->accept_vis (*this);
}
Expand Down
5 changes: 3 additions & 2 deletions gcc/rust/hir/rust-ast-lower-item.cc
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ ASTLoweringItem::visit (AST::InherentImpl &impl_block)
mapping, std::move (impl_items), std::move (generic_params),
std::unique_ptr<HIR::Type> (impl_type), nullptr, where_clause, polarity,
vis, impl_block.get_inner_attrs (), impl_block.get_outer_attrs (),
impl_block.get_locus ());
impl_block.get_locus (), false);
translated = hir_impl_block;

mappings.insert_hir_impl_block (hir_impl_block);
Expand Down Expand Up @@ -623,6 +623,7 @@ void
ASTLoweringItem::visit (AST::TraitImpl &impl_block)
{
std::vector<std::unique_ptr<HIR::WhereClauseItem>> where_clause_items;
bool unsafe = impl_block.is_unsafe ();
for (auto &item : impl_block.get_where_clause ().get_items ())
{
HIR::WhereClauseItem *i = ASTLowerWhereClauseItem::translate (*item);
Expand Down Expand Up @@ -696,7 +697,7 @@ ASTLoweringItem::visit (AST::TraitImpl &impl_block)
std::unique_ptr<HIR::Type> (impl_type),
std::unique_ptr<HIR::TypePath> (trait_ref), where_clause, polarity, vis,
impl_block.get_inner_attrs (), impl_block.get_outer_attrs (),
impl_block.get_locus ());
impl_block.get_locus (), unsafe);
translated = hir_impl_block;

mappings.insert_hir_impl_block (hir_impl_block);
Expand Down
14 changes: 8 additions & 6 deletions gcc/rust/hir/rust-ast-lower-type.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
// <http://www.gnu.org/licenses/>.

#include "rust-ast-lower-type.h"
#include "rust-attribute-values.h"

namespace Rust {
namespace HIR {
Expand Down Expand Up @@ -446,16 +447,17 @@ void
ASTLowerGenericParam::visit (AST::LifetimeParam &param)
{
auto crate_num = mappings.get_current_crate ();
AST::Lifetime lifetime = param.get_lifetime ();
Analysis::NodeMapping mapping (crate_num, param.get_node_id (),
mappings.get_next_hir_id (crate_num),
mappings.get_next_localdef_id (crate_num));

HIR::Lifetime lt (mapping, param.get_lifetime ().get_lifetime_type (),
param.get_lifetime ().get_lifetime_name (),
param.get_lifetime ().get_locus ());
HIR::Lifetime lt (mapping, lifetime.get_lifetime_type (),
lifetime.get_lifetime_name (), lifetime.get_locus ());

translated = new HIR::LifetimeParam (mapping, lt, param.get_locus (),
std::vector<Lifetime> ());
std::vector<Lifetime> (),
param.get_outer_attrs ());
}

void
Expand All @@ -482,7 +484,6 @@ ASTLowerGenericParam::visit (AST::ConstGenericParam &param)
void
ASTLowerGenericParam::visit (AST::TypeParam &param)
{
AST::Attribute outer_attr = AST::Attribute::create_empty ();
std::vector<std::unique_ptr<HIR::TypeParamBound>> type_param_bounds;
if (param.has_type_param_bounds ())
{
Expand All @@ -506,7 +507,8 @@ ASTLowerGenericParam::visit (AST::TypeParam &param)
translated
= new HIR::TypeParam (mapping, param.get_type_representation (),
param.get_locus (), std::move (type_param_bounds),
std::unique_ptr<Type> (type), std::move (outer_attr));
std::unique_ptr<Type> (type),
param.get_outer_attrs ());
}

HIR::TypeParamBound *
Expand Down
3 changes: 2 additions & 1 deletion gcc/rust/hir/rust-hir-dump.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1508,7 +1508,8 @@ void
Dump::visit (TypeParam &e)
{
begin ("TypeParam");
put_field ("outer_attr", e.get_outer_attribute ().as_string ());
auto &outer_attrs = e.get_outer_attrs ();
do_outer_attrs (outer_attrs);

put_field ("type_representation", e.get_type_representation ().as_string ());

Expand Down
26 changes: 14 additions & 12 deletions gcc/rust/hir/tree/rust-hir-item.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ class TypePath;
// A type generic parameter (as opposed to a lifetime generic parameter)
class TypeParam : public GenericParam
{
// bool has_outer_attribute;
// std::unique_ptr<Attribute> outer_attr;
AST::Attribute outer_attr;
AST::AttrVec outer_attrs;

Identifier type_representation;

Expand All @@ -59,24 +57,24 @@ class TypeParam : public GenericParam
bool has_type_param_bounds () const { return !type_param_bounds.empty (); }

// Returns whether the type param has an outer attribute.
bool has_outer_attribute () const { return !outer_attr.is_empty (); }
AST::Attribute &get_outer_attribute () { return outer_attr; }
bool has_outer_attribute () const override { return outer_attrs.size () > 0; }
AST::AttrVec &get_outer_attrs () override { return outer_attrs; }

TypeParam (Analysis::NodeMapping mappings, Identifier type_representation,
location_t locus = UNDEF_LOCATION,
std::vector<std::unique_ptr<TypeParamBound>> type_param_bounds
= std::vector<std::unique_ptr<TypeParamBound>> (),
std::unique_ptr<Type> type = nullptr,
AST::Attribute outer_attr = AST::Attribute::create_empty ())
: GenericParam (mappings), outer_attr (std::move (outer_attr)),
AST::AttrVec outer_attrs = std::vector<AST::Attribute> ())
: GenericParam (mappings), outer_attrs (std::move (outer_attrs)),
type_representation (std::move (type_representation)),
type_param_bounds (std::move (type_param_bounds)),
type (std::move (type)), locus (locus)
{}

// Copy constructor uses clone
TypeParam (TypeParam const &other)
: GenericParam (other.mappings), outer_attr (other.outer_attr),
: GenericParam (other.mappings), outer_attrs (other.outer_attrs),
type_representation (other.type_representation), locus (other.locus)
{
// guard to prevent null pointer dereference
Expand All @@ -92,7 +90,7 @@ class TypeParam : public GenericParam
TypeParam &operator= (TypeParam const &other)
{
type_representation = other.type_representation;
outer_attr = other.outer_attr;
outer_attrs = other.outer_attrs;
locus = other.locus;
mappings = other.mappings;

Expand Down Expand Up @@ -2741,6 +2739,7 @@ class ImplBlock : public VisItem, public WithInnerAttrs
BoundPolarity polarity;
location_t locus;
std::vector<std::unique_ptr<ImplItem>> impl_items;
bool unsafe;

public:
ImplBlock (Analysis::NodeMapping mappings,
Expand All @@ -2749,20 +2748,20 @@ class ImplBlock : public VisItem, public WithInnerAttrs
std::unique_ptr<Type> impl_type,
std::unique_ptr<TypePath> trait_ref, WhereClause where_clause,
BoundPolarity polarity, Visibility vis, AST::AttrVec inner_attrs,
AST::AttrVec outer_attrs, location_t locus)
AST::AttrVec outer_attrs, location_t locus, bool unsafe = false)
: VisItem (std::move (mappings), std::move (vis), std::move (outer_attrs)),
WithInnerAttrs (std::move (inner_attrs)),
generic_params (std::move (generic_params)),
impl_type (std::move (impl_type)), trait_ref (std::move (trait_ref)),
where_clause (std::move (where_clause)), polarity (polarity),
locus (locus), impl_items (std::move (impl_items))
locus (locus), impl_items (std::move (impl_items)), unsafe (unsafe)
{}

ImplBlock (ImplBlock const &other)
: VisItem (other), WithInnerAttrs (other.inner_attrs),
impl_type (other.impl_type->clone_type ()),
where_clause (other.where_clause), polarity (other.polarity),
locus (other.locus)
locus (other.locus), unsafe (other.unsafe)
{
generic_params.reserve (other.generic_params.size ());
for (const auto &e : other.generic_params)
Expand All @@ -2781,6 +2780,7 @@ class ImplBlock : public VisItem, public WithInnerAttrs
polarity = other.polarity;
inner_attrs = other.inner_attrs;
locus = other.locus;
unsafe = other.unsafe;

generic_params.reserve (other.generic_params.size ());
for (const auto &e : other.generic_params)
Expand All @@ -2801,6 +2801,8 @@ class ImplBlock : public VisItem, public WithInnerAttrs
// Returns whether inherent impl block has inherent impl items.
bool has_impl_items () const { return !impl_items.empty (); }

bool is_unsafe () const { return unsafe; }

void accept_vis (HIRFullVisitor &vis) override;
void accept_vis (HIRStmtVisitor &vis) override;
void accept_vis (HIRVisItemVisitor &vis) override;
Expand Down
22 changes: 16 additions & 6 deletions gcc/rust/hir/tree/rust-hir.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2017,14 +2017,19 @@ LifetimeParam::as_string () const
{
std::string str ("LifetimeParam: ");

str += "\n Outer attribute: ";
if (!has_outer_attribute ())
str += "\n Outer attributes: ";
if (outer_attrs.empty ())
{
str += "none";
}
else
{
str += outer_attr.as_string ();
/* note that this does not print them with "outer attribute" syntax -
* just the body */
for (const auto &attr : outer_attrs)
{
str += "\n " + attr.as_string ();
}
}

str += "\n Lifetime: " + lifetime.as_string ();
Expand Down Expand Up @@ -2106,14 +2111,19 @@ TypeParam::as_string () const
{
std::string str ("TypeParam: ");

str += "\n Outer attribute: ";
if (!has_outer_attribute ())
str += "\n Outer attributes: ";
if (outer_attrs.empty ())
{
str += "none";
}
else
{
str += outer_attr.as_string ();
/* note that this does not print them with "outer attribute" syntax -
* just the body */
for (const auto &attr : outer_attrs)
{
str += "\n " + attr.as_string ();
}
}

str += "\n Identifier: " + type_representation.as_string ();
Expand Down
26 changes: 18 additions & 8 deletions gcc/rust/hir/tree/rust-hir.h
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,9 @@ class GenericParam : public FullVisitable
CONST,
};

virtual AST::AttrVec &get_outer_attrs () = 0;
virtual bool has_outer_attribute () const = 0;

// Unique pointer custom clone function
std::unique_ptr<GenericParam> clone_generic_param () const
{
Expand Down Expand Up @@ -654,9 +657,7 @@ class LifetimeParam : public GenericParam
// LifetimeBounds lifetime_bounds;
std::vector<Lifetime> lifetime_bounds; // inlined LifetimeBounds

// bool has_outer_attribute;
// std::unique_ptr<Attribute> outer_attr;
AST::Attribute outer_attr;
AST::AttrVec outer_attrs;

location_t locus;

Expand All @@ -669,7 +670,9 @@ class LifetimeParam : public GenericParam
std::vector<Lifetime> &get_lifetime_bounds () { return lifetime_bounds; }

// Returns whether the lifetime param has an outer attribute.
bool has_outer_attribute () const { return !outer_attr.is_empty (); }
bool has_outer_attribute () const override { return outer_attrs.size () > 1; }

AST::AttrVec &get_outer_attrs () { return outer_attrs; }

// Returns whether the lifetime param is in an error state.
bool is_error () const { return lifetime.is_error (); }
Expand All @@ -679,11 +682,11 @@ class LifetimeParam : public GenericParam
location_t locus = UNDEF_LOCATION,
std::vector<Lifetime> lifetime_bounds
= std::vector<Lifetime> (),
AST::Attribute outer_attr = AST::Attribute::create_empty ())
AST::AttrVec outer_attrs = std::vector<AST::Attribute> ())
: GenericParam (mappings, GenericKind::LIFETIME),
lifetime (std::move (lifetime)),
lifetime_bounds (std::move (lifetime_bounds)),
outer_attr (std::move (outer_attr)), locus (locus)
outer_attrs (std::move (outer_attrs)), locus (locus)
{}

// TODO: remove copy and assignment operator definitions - not required
Expand All @@ -692,15 +695,15 @@ class LifetimeParam : public GenericParam
LifetimeParam (LifetimeParam const &other)
: GenericParam (other.mappings, GenericKind::LIFETIME),
lifetime (other.lifetime), lifetime_bounds (other.lifetime_bounds),
outer_attr (other.outer_attr), locus (other.locus)
outer_attrs (other.outer_attrs), locus (other.locus)
{}

// Overloaded assignment operator to clone attribute
LifetimeParam &operator= (LifetimeParam const &other)
{
lifetime = other.lifetime;
lifetime_bounds = other.lifetime_bounds;
outer_attr = other.outer_attr;
outer_attrs = other.outer_attrs;
locus = other.locus;
mappings = other.mappings;

Expand Down Expand Up @@ -748,6 +751,10 @@ class ConstGenericParam : public GenericParam
default_expression = other.default_expression->clone_expr ();
}

bool has_outer_attribute () const override { return false; }

AST::AttrVec &get_outer_attrs () override { return outer_attrs; }

std::string as_string () const override final;

void accept_vis (HIRFullVisitor &vis) override final;
Expand Down Expand Up @@ -775,6 +782,9 @@ class ConstGenericParam : public GenericParam
std::string name;
std::unique_ptr<Type> type;

/* const params have no outer attrs, should be empty */
AST::AttrVec outer_attrs = std::vector<AST::Attribute> ();

/* Optional - can be a null pointer if there is no default expression */
std::unique_ptr<Expr> default_expression;

Expand Down
21 changes: 21 additions & 0 deletions gcc/testsuite/rust/compile/issue-3045-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#![feature(dropck_eyepatch)]
#[allow(dead_code)]

#[lang = "sized"]
trait Sized {}

struct Test<T> {
_inner: T,
}

struct Test2<T> {
_inner: T,
}

trait Action {}

impl<#[may_dangle] T> Action for Test<T> {} // { dg-error "use of 'may_dangle' is unsafe and requires unsafe impl" "" { target *-*-* } 0 }

unsafe impl<#[may_dangle] T> Action for Test2<T> {}

fn main() {}
20 changes: 20 additions & 0 deletions gcc/testsuite/rust/compile/issue-3045-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#![feature(dropck_eyepatch)]
#[allow(dead_code)]

#[lang = "sized"]
trait Sized {}


trait Action {}

struct Inspector<'a>(&'a u8);
struct Inspector2<'a>(&'a u8);

impl<#[may_dangle] 'a> Action for Inspector<'a> {} // { dg-error "use of 'may_dangle' is unsafe and requires unsafe impl" "" { target *-*-* } 0 }

unsafe impl<#[may_dangle] 'a> Action for Inspector2<'a> {}


fn main() {

}

0 comments on commit a218b03

Please sign in to comment.