10000 rune: Avoid clobbering assigned addresses (fixes #838) · rune-rs/rune@18169b4 · GitHub
[go: up one dir, main page]
Skip to content

Commit 18169b4

Browse files
udoprogudoprog
committed
rune: Avoid clobbering assigned addresses (fixes #838)
1 parent 9e5f3c3 commit 18169b4

File tree

4 files changed

+144
-38
lines changed

4 files changed

+144
-38
lines changed

‎crates/rune/src/compile/v1/assemble.rs

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -351,16 +351,9 @@ fn return_<'a, 'hir, T>(
351351
hir: T,
352352
asm: impl FnOnce(&mut Ctxt<'a, 'hir, '_>, T, &mut dyn Needs<'a, 'hir>) -> compile::Result<Asm<'hir>>,
353353
) -> compile::Result<Asm<'hir>> {
354-
let mut needs = cx.scopes.defer(span).with_name("return value");
354+
let mut needs = cx.scopes.alloc(span)?.with_name("return value");
355355
converge!(asm(cx, hir, &mut needs)?, free(needs));
356-
357-
cx.asm.push(
358-
Inst::Return {
359-
addr: needs.addr()?.addr(),
360-
},
361-
span,
362-
)?;
363-
356+
cx.asm.push(Inst::Return { addr: needs.addr() }, span)?;
364357
needs.free()?;
365358
Ok(Asm::new(span, ()))
366359
}
@@ -640,10 +633,10 @@ fn condition<'a, 'hir>(
640633
match *hir {
641634
hir::Condition::Expr(hir) => {
642635
let scope = cx.scopes.child(hir)?;
643-
let mut addr = cx.scopes.alloc(hir)?.with_name("expression condition");
636+
let mut addr = cx.scopes.defer(hir).with_name("expression condition");
644637

645638
let asm = if expr(cx, hir, &mut addr)?.converging() {
646-
cx.asm.jump_if(addr.addr(), then_label, hir)?;
639+
cx.asm.jump_if(addr.addr()?.addr(), then_label, hir)?;
647640
addr.free()?;
648641
Asm::new(hir, (scope, Pattern::Irrefutable))
649642
} else {
@@ -2922,25 +2915,21 @@ fn expr_unary<'a, 'hir>(
29222915
) -> compile::Result<Asm<'hir>> {
29232916
converge!(expr(cx, &hir.expr, needs)?);
29242917

2925-
let Some(addr) = needs.try_as_addr()? else {
2926-
return Ok(Asm::new(span, ()));
2927-
};
2928-
29292918
match hir.op {
29302919
ast::UnOp::Not(..) => {
29312920
cx.asm.push(
29322921
Inst::Not {
2933-
addr: addr.addr(),
2934-
out: addr.output(),
2922+
addr: needs.addr()?.addr(),
2923+
out: needs.alloc_output()?,
29352924
},
29362925
span,
29372926
)?;
29382927
}
29392928
ast::UnOp::Neg(..) => {
29402929
cx.asm.push(
29412930
Inst::Neg {
2942-
addr: addr.addr(),
2943-
out: addr.output(),
2931+
addr: needs.addr()?.addr(),
2932+
out: needs.alloc_output()?,
29442933
},
29452934
span,
29462935
)?;

‎crates/rune/src/compile/v1/needs.rs

Lines changed: 65 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ pub(super) trait Needs<'a, 'hir> {
1818

1919
/// Get the need as an output.
2020
///
21-
/// Returns `None` if `Needs::None` is set.
21+
/// Returns `None` if not value is required.
2222
fn try_alloc_output(&mut self) -> compile::Result<Option<Output>>;
2323

2424
fn assign_addr(
@@ -118,12 +118,12 @@ impl<'a, 'hir> Needs<'a, 'hir> for Address<'a, 'hir> {
118118

119119
#[inline]
120120
fn try_alloc_output(&mut self) -> compile::Result<Option<Output>> {
121-
Ok(Some(Address::alloc_output(self)?))
121+
Address::try_alloc_output(self)
122122
}
123123

124124
#[inline]
125125
fn try_alloc_addr(&mut self) -> compile::Result<Option<&mut Address<'a, 'hir>>> {
126-
Ok(Some(Address::alloc_addr(self)?))
126+
Address::try_alloc_addr(self)
127127
}
128128

129129
#[inline]
@@ -139,8 +139,16 @@ pub(super) enum AddressKind {
139139
Local,
140140
/// The slot has been reserved, but has not been assigned to anything yet.
141141
Dangling,
142-
/// The address is assigned from elsewhere and *should not* be touched.
143-
Assigned,
142+
/// The address is assigned from elsewhere and *should not* be touched. This
143+
/// is used for assignments.
144+
Assign,
145+
/// The address is assigned from elsewhere and *should not* be touched. Once
146+
/// it is allocated as output it will be translated into a scoped address
147+
/// with the specified scope. So any reading from this address must be done
148+
/// before output is allocated for it.
149+
///
150+
/// The provided scope is where the assigned address was deferred from.
151+
DeferAssign(ScopeId),
144152
/// The address is allocated on behalf of the given scope, and we should
145153
/// defer deallocating it until the given scope is deallocated.
146154
Scope(ScopeId),
@@ -153,7 +161,8 @@ impl fmt::Display for AddressKind {
153161
match self {
154162
AddressKind::Local => write!(f, "local"),
155163
AddressKind::Dangling => write!(f, "dangling"),
156-
AddressKind::Assigned => write!(f, "assigned"),
164+
AddressKind::Assign => write!(f, "assign"),
165+
AddressKind::DeferAssign(scope) => write!(f, "defer-assign({scope})"),
157166
AddressKind::Scope(scope) => write!(f, "scope({scope})"),
158167
AddressKind::Freed => write!(f, "freed"),
159168
}
@@ -203,7 +212,7 @@ impl<'a, 'hir> Address<'a, 'hir> {
203212
span,
204213
scopes,
205214
address: addr,
206-
kind: AddressKind::Assigned,
215+
kind: AddressKind::Assign,
207216
name: None,
208217
backtrace: Backtrace::capture(),
209218
}
@@ -241,25 +250,56 @@ impl<'a, 'hir> Address<'a, 'hir> {
241250

242251
#[inline]
243252
pub(super) fn alloc_addr(&mut self) -> compile::Result<&mut Self> {
244-
if matches!(self.kind, AddressKind::Dangling) {
245-
self.kind = AddressKind::Local;
253+
match self.kind {
254+
AddressKind::Dangling => {
255+
self.kind = AddressKind::Local;
256+
}
257+
AddressKind::DeferAssign(scope) => {
258+
self.address = self.scopes.alloc_in(self.span, scope)?;
259+
self.kind = AddressKind::Scope(scope);
260+
}
261+
_ => {}
246262
}
247263

248264
Ok(self)
249265
}
250266

267+
#[inline]
268+
pub(super) fn < 17AE span class=pl-en>try_alloc_addr(&mut self) -> compile::Result<Option<&mut Self>> {
269+
match self.kind {
270+
AddressKind::Dangling => {
271+
self.kind = AddressKind::Local;
272+
}
273+
AddressKind::DeferAssign(..) => {
274+
return Ok(None);
275+
}
276+
_ => {}
277+
}
278+
279+
Ok(Some(self))
280+
}
281+
251282
#[inline]
252283
pub(super) fn alloc_output(&mut self) -> compile::Result<Output> {
253284
Ok(self.alloc_addr()?.output())
254285
}
255286

287+
#[inline]
288+
pub(super) fn try_alloc_output(&mut self) -> compile::Result<Option<Output>> {
289+
let Some(addr) = self.try_alloc_addr()? else {
290+
return Ok(None);
291+
};
292+
293+
Ok(Some(addr.output()))
294+
}
295+
256296
#[inline]
257297
pub(super) fn output(&self) -> Output {
258298
self.address.output()
259299
}
260300

261301
pub(super) fn assign_addr(
262-
&self,
302+
&mut self,
263303
cx: &mut Ctxt<'_, '_, '_>,
264304
from: InstAddress,
265305
) -> compile::Result<()> {
@@ -459,20 +499,25 @@ impl<'a, 'hir> Any<'a, 'hir> {
459499
cx: &mut Ctxt<'_, 'hir, '_>,
460500
from: InstAddress,
461501
) -> compile::Result<()> {
462-
match &self.kind {
463-
AnyKind::Defer { scopes, name, .. } => {
502+
match self.kind {
503+
AnyKind::Defer {
504+
scopes,
505+
scope,
506+
name,
507+
..
508+
} => {
464509
self.kind = AnyKind::Address {
465510
address: Address {
466511
span: self.span,
467512
scopes,
468513
address: from,
469-
kind: AddressKind::Assigned,
470-
name: *name,
514+
kind: AddressKind::DeferAssign(scope),
515+
name,
471516
backtrace: Backtrace::capture(),
472517
},
473518
};
474519
}
475-
AnyKind::Address { address } => {
520+
AnyKind::Address { ref mut address } => {
476521
address.assign_addr(cx, from)?;
477522
}
478523
_ => {}
@@ -502,10 +547,11 @@ impl<'a, 'hir> Any<'a, 'hir> {
502547
self.kind = AnyKind::Address { address };
503548
}
504549

505-
match &mut self.kind {
506-
AnyKind::Address { address } => Ok(Some(address.alloc_addr()?)),
507-
_ => Ok(None),
508-
}
550+
let AnyKind::Address { address } = &mut self.kind else {
551+
return Ok(None);
552+
};
553+
554+
address.try_alloc_addr()
509555
}
510556

511557
/// Get the needs as an output.

‎crates/rune/tests/bug_838.rn

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// This ensures that unary operations do not clobber a slot.
2+
// https://github.com/rune-rs/rune/issues/838
3+
4+
#[test]
5+
fn negation_simple() {
6+
let b = -1;
7+
let d = 0 + -b;
8+
assert_eq!(d, 1);
9+
assert_eq!(b, -1);
10+
}
11+
12+
#[test]
13+
fn negation() {
14+
let a = 1;
15+
let b = -2;
16+
let c = -b;
17+
let d = 1 < -b;
18+
let e = 1 < -b;
19+
20+
assert!(d);
21+
assert!(e);
22+
assert_eq!(a, 1);
23+
assert_eq!(b, -2);
24+
assert_eq!(c, 2);
25+
}

‎crates/rune/tests/matching.rn

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,3 +168,49 @@ fn match_enum2() {
168168

169169
assert_eq!(out, Enum::Right);
170170
}
171+
172+
#[test]
173+
fn match_extraction_vec() {
174+
let v = [42];
175+
176+
fn inner(v) {
177+
match v {
178+
[a] => a,
179+
_ => 0,
180+
}
181+
}
182+
183+
assert_eq!(inner(v), 42);
184+
}
185+
186+
#[test]
187+
fn match_extraction_tuple() {
188+
let v = (42,);
189+
190+
fn inner(v) {
191+
match v {
192+
(a,) => a,
193+
_ => 0,
194+
}
195+
}
196+
197+
assert_eq!(inner(v), 42);
198+
}
199+
200+
#[test]
201+
fn match_extraction_struct() {
202+
struct Struct {
203+
a,
204+
}
205+
206+
fn inner(v) {
207+
match v {
208+
Struct { a, .. } => a,
209+
_ => 0,
210+
}
211+
}
212+
213+
let v = Struct { a: 42 };
214+
215+
assert_eq!(inner(v), 42);
216+
}

0 commit comments

Comments
 (0)
0