Skip to content

Commit 79c81d0

Browse files
lucasly-baphilberty
authored andcommitted
gccrs: add unused unsafe lint
Warn about an unsafe block that does not contain any operation requiring an unsafe context. The unsafe checker now records which unsafe blocks are actually used by an unsafe operation, and reports the rest. The warning is gated behind -frust-unused-check-2.0. gcc/rust/ChangeLog: * checks/errors/rust-unsafe-checker.cc (check_static_mut) (check_extern_static, check_unsafe_call, check_extern_call) (check_target_attr): Return whether the operation requires unsafe and only error outside of an unsafe context. (UnsafeChecker::mark_unsafe_used): New. (UnsafeChecker::check_use_of_static) (UnsafeChecker::check_function_call) (UnsafeChecker::check_function_attr): Mark the unsafe block as used. (UnsafeChecker::visit): Mark used and warn on unnecessary unsafe block. * checks/errors/rust-unsafe-checker.h (UnsafeChecker): Add mark_unsafe_used and used_unsafe_blocks. * rust-lang.cc (grs_langhook_init_options_struct): Enable warn_unused. gcc/testsuite/ChangeLog: * rust/compile/unused-unsafe_0.rs: New test. Signed-off-by: Lucas Ly Ba <lucas.ly-ba@outlook.com>
1 parent e9dac80 commit 79c81d0

3 files changed

Lines changed: 139 additions & 59 deletions

File tree

gcc/rust/checks/errors/rust-unsafe-checker.cc

Lines changed: 123 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "rust-system.h"
2727
#include "rust-finalized-name-resolution-context.h"
2828
#include "rust-intrinsic-values.h"
29+
#include "options.h"
2930

3031
namespace Rust {
3132
namespace HIR {
@@ -43,8 +44,15 @@ UnsafeChecker::go (HIR::Crate &crate)
4344
item->accept_vis (*this);
4445
}
4546

46-
static void
47-
check_static_mut (HIR::Item *maybe_static, location_t locus)
47+
void
48+
UnsafeChecker::mark_unsafe_used ()
49+
{
50+
if (unsafe_context.is_in_context ())
51+
used_unsafe_blocks.insert (unsafe_context.peek ());
52+
}
53+
54+
static bool
55+
check_static_mut (HIR::Item *maybe_static, location_t locus, bool in_context)
4856
{
4957
if (maybe_static->get_hir_kind () == Node::BaseKind::VIS_ITEM)
5058
{
@@ -53,42 +61,62 @@ check_static_mut (HIR::Item *maybe_static, location_t locus)
5361
{
5462
auto static_item = static_cast<StaticItem *> (item);
5563
if (static_item->is_mut ())
56-
rust_error_at (
57-
locus, "use of mutable static requires unsafe function or block");
64+
{
65+
if (!in_context)
66+
rust_error_at (locus, "use of mutable static requires unsafe "
67+
"function or block");
68+
return true;
69+
}
5870
}
5971
}
72+
return false;
6073
}
6174

62-
static void
63-
check_extern_static (HIR::ExternalItem *maybe_static, location_t locus)
75+
static bool
76+
check_extern_static (HIR::ExternalItem *maybe_static, location_t locus,
77+
bool in_context)
6478
{
6579
if (maybe_static->get_extern_kind () == ExternalItem::ExternKind::Static)
66-
rust_error_at (locus,
67-
"use of extern static requires unsafe function or block");
80+
{
81+
if (!in_context)
82+
rust_error_at (
83+
locus, "use of extern static requires unsafe function or block");
84+
return true;
85+
}
86+
return false;
6887
}
6988

7089
void
7190
UnsafeChecker::check_use_of_static (HirId node_id, location_t locus)
7291
{
73-
if (unsafe_context.is_in_context ())
74-
return;
92+
bool in_context = unsafe_context.is_in_context ();
93+
bool unsafe_op = false;
7594

7695
if (auto maybe_static_mut = mappings.lookup_hir_item (node_id))
77-
check_static_mut (*maybe_static_mut, locus);
96+
unsafe_op |= check_static_mut (*maybe_static_mut, locus, in_context);
7897

7998
if (auto maybe_extern_static = mappings.lookup_hir_extern_item (node_id))
80-
check_extern_static (static_cast<ExternalItem *> (
81-
maybe_extern_static->first),
82-
locus);
99+
unsafe_op |= check_extern_static (static_cast<ExternalItem *> (
100+
maybe_extern_static->first),
101+
locus, in_context);
102+
103+
if (unsafe_op && in_context)
104+
mark_unsafe_used ();
83105
}
84106

85-
static void
86-
check_unsafe_call (HIR::Function *fn, location_t locus, const std::string &kind)
107+
static bool
108+
check_unsafe_call (HIR::Function *fn, location_t locus, const std::string &kind,
109+
bool in_context)
87110
{
88111
if (fn->get_qualifiers ().is_unsafe ())
89-
rust_error_at (locus, ErrorCode::E0133,
90-
"call to unsafe %s requires unsafe function or block",
91-
kind.c_str ());
112+
{
113+
if (!in_context)
114+
rust_error_at (locus, ErrorCode::E0133,
115+
"call to unsafe %s requires unsafe function or block",
116+
kind.c_str ());
117+
return true;
118+
}
119+
return false;
92120
}
93121

94122
static bool
@@ -137,9 +165,9 @@ is_safe_intrinsic (const std::string &fn_name)
137165
return safe_intrinsics.find (fn_name) != safe_intrinsics.end ();
138166
}
139167

140-
static void
168+
static bool
141169
check_extern_call (HIR::ExternalItem *maybe_fn, HIR::ExternBlock *parent_block,
142-
location_t locus)
170+
location_t locus, bool in_context)
143171
{
144172
// We have multiple operations to perform here
145173
// 1. Is the item an actual function we're calling
@@ -151,60 +179,75 @@ check_extern_call (HIR::ExternalItem *maybe_fn, HIR::ExternBlock *parent_block,
151179
// "Rust"` blocks) is unsafe to call
152180

153181
if (maybe_fn->get_extern_kind () != ExternalItem::ExternKind::Function)
154-
return;
182+
return false;
155183

156184
// Some intrinsics are safe to call
157185
if (parent_block->get_abi () == Rust::ABI::INTRINSIC
158186
&& is_safe_intrinsic (maybe_fn->get_item_name ().as_string ()))
159-
return;
187+
return false;
160188

161-
rust_error_at (locus,
162-
"call to extern function requires unsafe function or block");
189+
if (!in_context)
190+
rust_error_at (locus,
191+
"call to extern function requires unsafe function or block");
192+
return true;
163193
}
164194

165195
void
166196
UnsafeChecker::check_function_call (HirId node_id, location_t locus)
167197
{
168-
if (unsafe_context.is_in_context ())
169-
return;
198+
bool in_context = unsafe_context.is_in_context ();
199+
bool unsafe_op = false;
170200

171201
auto maybe_fn = mappings.lookup_hir_item (node_id);
172202

173203
if (maybe_fn
174204
&& maybe_fn.value ()->get_item_kind () == Item::ItemKind::Function)
175-
check_unsafe_call (static_cast<Function *> (*maybe_fn), locus, "function");
205+
unsafe_op |= check_unsafe_call (static_cast<Function *> (*maybe_fn), locus,
206+
"function", in_context);
176207

177208
if (auto maybe_extern = mappings.lookup_hir_extern_item (node_id))
178-
check_extern_call (static_cast<ExternalItem *> (maybe_extern->first),
179-
*mappings.lookup_hir_extern_block (maybe_extern->second),
180-
locus);
209+
unsafe_op
210+
|= check_extern_call (static_cast<ExternalItem *> (maybe_extern->first),
211+
*mappings.lookup_hir_extern_block (
212+
maybe_extern->second),
213+
locus, in_context);
214+
215+
if (unsafe_op && in_context)
216+
mark_unsafe_used ();
181217
}
182218

183-
static void
184-
check_target_attr (HIR::Function *fn, location_t locus)
219+
static bool
220+
check_target_attr (HIR::Function *fn, location_t locus, bool in_context)
185221
{
186222
if (std::any_of (fn->get_outer_attrs ().begin (),
187223
fn->get_outer_attrs ().end (),
188224
[] (const AST::Attribute &attr) {
189225
return attr.get_path ().as_string ()
190226
== Values::Attributes::TARGET_FEATURE;
191227
}))
192-
rust_error_at (locus,
193-
"call to function with %<#[target_feature]%> requires "
194-
"unsafe function or block");
228+
{
229+
if (!in_context)
230+
rust_error_at (locus,
231+
"call to function with %<#[target_feature]%> requires "
232+
"unsafe function or block");
233+
return true;
234+
}
235+
return false;
195236
}
196237

197238
void
198239
UnsafeChecker::check_function_attr (HirId node_id, location_t locus)
199240
{
200-
if (unsafe_context.is_in_context ())
201-
return;
241+
bool in_context = unsafe_context.is_in_context ();
202242

203243
auto maybe_fn = mappings.lookup_hir_item (node_id);
204244

205245
if (maybe_fn
206-
&& maybe_fn.value ()->get_item_kind () == Item::ItemKind::Function)
207-
check_target_attr (static_cast<Function *> (*maybe_fn), locus);
246+
&& maybe_fn.value ()->get_item_kind () == Item::ItemKind::Function
247+
&& check_target_attr (static_cast<Function *> (*maybe_fn), locus,
248+
in_context)
249+
&& in_context)
250+
mark_unsafe_used ();
208251
}
209252

210253
void
@@ -280,10 +323,14 @@ UnsafeChecker::visit (DereferenceExpr &expr)
280323

281324
rust_assert (context.lookup_type (to_deref, &to_deref_type));
282325

283-
if (to_deref_type->get_kind () == TyTy::TypeKind::POINTER
284-
&& !unsafe_context.is_in_context ())
285-
rust_error_at (expr.get_locus (), "dereference of raw pointer requires "
286-
"unsafe function or block");
326+
if (to_deref_type->get_kind () == TyTy::TypeKind::POINTER)
327+
{
328+
if (unsafe_context.is_in_context ())
329+
mark_unsafe_used ();
330+
else
331+
rust_error_at (expr.get_locus (), "dereference of raw pointer requires "
332+
"unsafe function or block");
333+
}
287334
}
288335

289336
void
@@ -465,9 +512,12 @@ UnsafeChecker::visit (MethodCallExpr &expr)
465512
// should probably use the defid lookup instead
466513
// tl::optional<HIR::Item *> lookup_defid (DefId id);
467514
auto method = mappings.lookup_hir_implitem (fn.get_ref ());
468-
if (!unsafe_context.is_in_context () && method)
469-
check_unsafe_call (static_cast<Function *> (method->first),
470-
expr.get_locus (), "method");
515+
if (method
516+
&& check_unsafe_call (static_cast<Function *> (method->first),
517+
expr.get_locus (), "method",
518+
unsafe_context.is_in_context ())
519+
&& unsafe_context.is_in_context ())
520+
mark_unsafe_used ();
471521

472522
expr.get_receiver ().accept_vis (*this);
473523

@@ -480,21 +530,23 @@ UnsafeChecker::visit (FieldAccessExpr &expr)
480530
{
481531
expr.get_receiver_expr ().accept_vis (*this);
482532

483-
if (unsafe_context.is_in_context ())
484-
return;
485-
486533
TyTy::BaseType *receiver_ty;
487-
auto ok = context.lookup_type (
488-
expr.get_receiver_expr ().get_mappings ().get_hirid (), &receiver_ty);
489-
rust_assert (ok);
534+
if (!context.lookup_type (
535+
expr.get_receiver_expr ().get_mappings ().get_hirid (), &receiver_ty))
536+
return;
490537

491538
if (receiver_ty->get_kind () == TyTy::TypeKind::ADT)
492539
{
493540
auto maybe_union = static_cast<TyTy::ADTType *> (receiver_ty);
494541
if (maybe_union->is_union ())
495-
rust_error_at (
496-
expr.get_locus (),
497-
"access to union field requires unsafe function or block");
542+
{
543+
if (unsafe_context.is_in_context ())
544+
mark_unsafe_used ();
545+
else
546+
rust_error_at (
547+
expr.get_locus (),
548+
"access to union field requires unsafe function or block");
549+
}
498550
}
499551
}
500552

@@ -582,11 +634,17 @@ UnsafeChecker::visit (ReturnExpr &expr)
582634
void
583635
UnsafeChecker::visit (UnsafeBlockExpr &expr)
584636
{
585-
unsafe_context.enter (expr.get_mappings ().get_hirid ());
637+
auto id = expr.get_mappings ().get_hirid ();
638+
unsafe_context.enter (id);
586639

587640
expr.get_block_expr ().accept_vis (*this);
588641

589642
unsafe_context.exit ();
643+
644+
if (flag_unused_check_2_0
645+
&& used_unsafe_blocks.find (id) == used_unsafe_blocks.end ())
646+
rust_warning_at (expr.get_locus (), OPT_Wunused,
647+
"unnecessary %<unsafe%> block");
590648
}
591649

592650
void
@@ -649,7 +707,10 @@ void
649707
UnsafeChecker::visit (InlineAsm &expr)
650708
{
651709
if (unsafe_context.is_in_context ())
652-
return;
710+
{
711+
mark_unsafe_used ();
712+
return;
713+
}
653714

654715
rust_error_at (
655716
expr.get_locus (), ErrorCode::E0133,
@@ -660,7 +721,10 @@ void
660721
UnsafeChecker::visit (LlvmInlineAsm &expr)
661722
{
662723
if (unsafe_context.is_in_context ())
663-
return;
724+
{
725+
mark_unsafe_used ();
726+
return;
727+
}
664728

665729
rust_error_at (
666730
expr.get_locus (), ErrorCode::E0133,

gcc/rust/checks/errors/rust-unsafe-checker.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
#include "rust-hir-type-check.h"
2525
#include "rust-stacked-contexts.h"
2626

27+
#include <unordered_set>
28+
2729
namespace Rust {
2830
namespace HIR {
2931
class UnsafeChecker : public HIRFullVisitor
@@ -51,7 +53,13 @@ class UnsafeChecker : public HIRFullVisitor
5153
*/
5254
void check_function_attr (HirId node_id, location_t locus);
5355

56+
/**
57+
* Mark the current unsafe block as required by an unsafe operation
58+
*/
59+
void mark_unsafe_used ();
60+
5461
StackedContexts<HirId> unsafe_context;
62+
std::unordered_set<HirId> used_unsafe_blocks;
5563

5664
Resolver::TypeCheckContext &context;
5765
const Resolver2_0::FinalizedNameResolutionContext &resolver;
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// { dg-additional-options "-frust-unused-check-2.0" }
2+
#![feature(no_core)]
3+
#![no_core]
4+
5+
pub fn foo() {
6+
unsafe {}
7+
// { dg-warning "unnecessary .unsafe. block" "" { target *-*-* } .-1 }
8+
}

0 commit comments

Comments
 (0)