Skip to content

Commit 8f9cadf

Browse files
yvtyvt
committed
fix(port_riscv): augment symbol names of NumTy<_>::value to make them unique
This generic function uses the provided integral value as its symbol name (e.g., `#[export_name = "42"]` for `NumTy<42>`), but this is prone to symbol name collision. This commit addresses this problem by augmenting the symbol names with crate-specific metadata (e.g., `#[export_name = "r3_port_riscv-1.2.3*/ 42 /*"]`). The parentheses in the definition of `csrexpr!` had to be removed owing to `unstringify!`'s quirk that tokenizes each input of `concat!` separately, thus rejecting an input like `concat!("(", "PRIV", "+4)")`.
1 parent e45499f commit 8f9cadf

File tree

6 files changed

+110
-35
lines changed

6 files changed

+110
-35
lines changed

‎Cargo.lock

Lines changed: 64 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/r3_port_riscv/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
1111

1212
- **Breaking (semver-exempt):** Change the target compiler version to `nightly-2022-03-30`
1313

14+
### Fixed
15+
16+
- This crate no longer exports symbol names like `0` and `1`, which are prone to name collision.
17+
1418
## [0.2.1] - 2022-03-19
1519

1620
### Fixed

‎src/r3_port_riscv/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,12 @@ r3_portkit = { version = "0.2.0", path = "../r3_portkit" }
2121
r3_kernel = { version = "0.1.0", path = "../r3_kernel" }
2222
r3_core = { version = "0.1.0", path = "../r3_core" }
2323

24+
macro_find_and_replace = { version = "1.0.0" }
2425
tock-registers = { version = "0.7.0" }
2526
unstringify = { version = "0.1.4" }
2627
seq-macro = { version = "0.3.0" }
2728
svgbobdoc = { version = "0.3.0" }
29+
macropol = { version = "0.1.3" }
2830
riscv-rt = { version = ">= 0.6.0, < 0.9.0" }
2931
riscv = { version = ">= 0.5.0, < 0.8.0" }
3032

‎src/r3_port_riscv/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#![feature(generic_const_exprs)]
2+
#![feature(macro_metavar_expr)]
23
#![feature(const_trait_impl)]
34
#![feature(naked_functions)]
45
#![feature(const_mut_refs)]

‎src/r3_port_riscv/src/threading/imp.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -723,21 +723,21 @@ impl State {
723723
724724
# xstatus.XPP := X (if `PRIVILEGE_LEVEL != U`)
725725
# xstatus.XPIE := 1 (if `maintain-mpie` is enabled)
726-
.if {PRIV} == 3
726+
.if /*{PRIV}*/ == 3
727727
" if cfg!(feature = "maintain-pie") { "
728728
li a0, {MPP_M} | " crate::threading::imp::csr::csrexpr!(XSTATUS_XPIE) "
729729
" } else { "
730730
li a0, {MPP_M}
731731
" } "
732732
csrs " crate::threading::imp::csr::csrexpr!(XSTATUS) ", a0
733-
.elseif {PRIV} == 1
733+
.elseif /*{PRIV}*/ == 1
734734
" if cfg!(feature = "maintain-pie") { "
735735
li a0, {SPP_S} | " crate::threading::imp::csr::csrexpr!(XSTATUS_XPIE) "
736736
" } else { "
737737
li a0, {SPP_S}
738738
" } "
739739
csrs " crate::threading::imp::csr::csrexpr!(XSTATUS) ", a0
740-
.elseif {PRIV} == 0
740+
.elseif /*{PRIV}*/ == 0
741741
" if cfg!(feature = "maintain-pie") { "
742742
csrsi " crate::threading::imp::csr::csrexpr!(XSTATUS) ", "
743743
crate::threading::imp::csr::csrexpr!(XSTATUS_XPIE) "
@@ -779,11 +779,11 @@ impl State {
779779
LOAD t5, ({X_SIZE} * 14)(sp)
780780
LOAD t6, ({X_SIZE} * 15)(sp)
781781
addi sp, sp, ({X_SIZE} * 17)
782-
.if {PRIV} == 0
782+
.if /*{PRIV}*/ == 0
783783
uret
784-
.elseif {PRIV} == 1
784+
.elseif /*{PRIV}*/ == 1
785785
sret
786-
.elseif {PRIV} == 3
786+
.elseif /*{PRIV}*/ == 3
787787
mret
788788
.else
789789
.error \"unsupported `PRIVILEGE_LEVEL`\"

‎src/r3_port_riscv/src/threading/imp/csr.rs

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,14 @@
5656
//! *Import constants as `sym` operands*. Interestingly, the `#[export_name]`
5757
//! attributes can be used to give symbol names entirely comprised of numbers,
5858
//! and when such symbols are used in `sym` operands, they are indeed
59-
//! interpreted as the intended numbers. The catch is that we have to specify
59+
//! interpreted as the intended numbers. A catch is that we have to specify
6060
//! `#[export_name]` for each possible value (i.e., we can't easily cover the
61-
//! whole range of `usize`).
61+
//! whole range of `usize`). Another catch is that the compiler enforces that
62+
//! unique symbol names are used in a whole compiled binary. To avoid the issues
63+
//! caused by this, the symbol names must augmented with crate-specific metadata
64+
//! (e.g., `#[export_name = "r3_port_riscv-1.2.3*/ 42 /*"]`).
6265
use core::arch::asm;
66+
use macro_find_and_replace::replace_token_sequence as replace;
6367
use unstringify::unstringify;
6468

6569
#[derive(Clone, Copy)]
@@ -148,11 +152,12 @@ macro_rules! define_set {
148152
(
149153
impl<Traits: super::ThreadingOptions> CsrSet<Traits> {}
150154

155+
#[substitute($S:tt $PRIV:ident => PRIV)]
151156
$( #[$csrexpr_meta:meta] )*
152157
macro csrexpr {
153158
$(
154159
$( #[$const_meta:meta] )*
155-
($CONST:ident) => { $const_value:literal }
160+
($CONST:ident) => { $($const_value:tt)* }
156161
),*
157162
$(,)?
158163
}
@@ -183,17 +188,21 @@ macro_rules! define_set {
183188

184189
$(
185190
pub const $CONST: usize = {
186-
#[allow(non_snake_case)]
187-
let PRIV = Self::PRIV; // provide a value for `{PRIV}` in `$const_value`
188-
unstringify!($const_value)
191+
// `unstringify!` is unable to expand `replace!`. Replace
192+
// `$PRIV` using a macro-by-example and then pass the
193+
// resultant tokens to `unstringify!`.
194+
macro m($$ $PRIV:tt) { unstringify!($($const_value)*) }
195+
m!("Self::PRIV")
189196
};
190197
)*
191198
}
192199

193200
$( #[$csrexpr_meta] )*
194201
pub(crate) macro csrexpr {
195202
$(
196-
($CONST) => { $const_value }
203+
// An eager-expanded string literal is expected in the output.
204+
// Use `replace!` and let `asm!` expand it.
205+
($CONST) => { replace!([$$ $PRIV], ["(/*{PRIV}*/)"], $($const_value)*) }
197206
),*
198207
}
199208

@@ -257,40 +266,42 @@ pub const XSTATUS_FS_1: usize = 1 << 14;
257266
pub const XCAUSE_INTERRUPT: usize = usize::MAX - usize::MAX / 2;
258267
pub const XCAUSE_EXCEPTIONCODE_MASK: usize = usize::MAX / 2;
259268

269+
#[macropol::macropol(concat = "concat!($parts_comma_sep)")]
260270
define_set! {
261271
impl<Traits: super::ThreadingOptions> CsrSet<Traits> {
262272
/* `csrexpr!` is also exposed as `const`s here */
263273
}
264274

275+
#[substitute($PRIV => PRIV)]
265276
/// Create an assembly expression that evaluates to a CSR number or value.
266277
/// Assumes the presence of an operand `PRIV = sym Traits::Priv::value`.
267278
macro csrexpr {
268279
// CSRs
269-
(XSTATUS) => { "{PRIV} * 0x100" },
270-
(XIE) => { "{PRIV} * 0x100 + 0x04" },
271-
(XEPC) => { "{PRIV} * 0x100 + 0x41" },
272-
(XCAUSE) => { "{PRIV} * 0x100 + 0x42" },
273-
(XIP) => { "{PRIV} * 0x100 + 0x44" },
280+
(XSTATUS) => { "$PRIV * 0x100" },
281+
(XIE) => { "$PRIV * 0x100 + 0x04" },
282+
(XEPC) => { "$PRIV * 0x100 + 0x41" },
283+
(XCAUSE) => { "$PRIV * 0x100 + 0x42" },
284+
(XIP) => { "$PRIV * 0x100 + 0x44" },
274285

275286
// CSR values
276287
// Machine/Supervisor/... Interrupt Enable
277-
(XSTATUS_XIE) => { "1 << ({PRIV})" },
288+
(XSTATUS_XIE) => { "1 << 0 << $PRIV" },
278289
// Machine/Supervisor/... Previous Interrupt Enable
279-
(XSTATUS_XPIE) => { "1 << ({PRIV} + 4)" },
290+
(XSTATUS_XPIE) => { "1 << 4 << $PRIV" },
280291

281292
/// Machine/Supervisor/... Software Interrupt Enable
282-
(XIE_XSIE) => { "1 << ({PRIV})" },
293+
(XIE_XSIE) => { "1 << 0 << $PRIV" },
283294
/// Machine/Supervisor/... Timer Interrupt Enable
284-
(XIE_XTIE) => { "1 << ({PRIV} + 4)" },
295+
(XIE_XTIE) => { "1 << 4 << $PRIV" },
285296
/// Machine/Supervisor/... External Interrupt Enable
286-
(XIE_XEIE) => { "1 << ({PRIV} + 8)" },
297+
(XIE_XEIE) => { "1 << 8 << $PRIV" },
287298

288299
/// Machine/Supervisor/... Software Interrupt Pending
289-
(XIP_XSIP) => { "1 << ({PRIV})" },
300+
(XIP_XSIP) => { "1 << 0 << $PRIV" },
290301
/// Machine/Supervisor/... Timer Interrupt Pending
291-
(XIP_XTIP) => { "1 << ({PRIV} + 4)" },
302+
(XIP_XTIP) => { "1 << 4 << $PRIV" },
292303
/// Machine/Supervisor/... External Interrupt Pending
293-
(XIP_XEIP) => { "1 << ({PRIV} + 8)" },
304+
(XIP_XEIP) => { "1 << 8 << $PRIV" },
294305
}
295306

296307
impl<Traits> CsrSetAccess for CsrSet<Traits> {
@@ -338,10 +349,11 @@ pub trait Num {
338349
/// `<NumTy<N> as Num>::value` has a symbol name `N`.
339350
pub struct NumTy<const N: usize>;
340351

352+
#[macropol::macropol]
341353
seq_macro::seq!(N in 0..4 {
342354
#[doc(hidden)]
343355
impl Num for NumTy<N> {
344-
#[export_name = stringify!(N)]
356+
#[export_name = r#"r3_port_riscv-${env!("CARGO_PKG_VERSION")}*/$&{N}/*"#]
345357
fn value() {}
346358
}
347359
});

0 commit comments

Comments
 (0)