Skip to content

Commit 943d7c7

Browse files
Clarify ALTREP array boundary semantics
1 parent 2db8654 commit 943d7c7

13 files changed

Lines changed: 88 additions & 40 deletions

NEWS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# Rtinycc 0.1.11
22

33
- Add `tcc_list_symbols()` to inspect global symbol names and resolved hexadecimal addresses known to a libtcc state.
4+
- Improve ALTREP-aware copy-in paths by using `RAW_GET_REGION()` when copying raw vectors into native memory and scalar accessors for callback/struct scalar conversions. Clarify that mutable array FFI inputs can materialize ALTREP vectors when R exposes writable C storage.
45
- Fix `PROTECT` and memory-balance hygiene bugs in internal C state initialization (`RC_libtcc_state_new`, `RC_libtcc_get_symbol`, and callback registration paths). Previously, class string attributes could be inadvertently allocated without proper protection.
56
- Fix a stack-depth and protection imbalance bug in `RC_invoke_callback_internal` where callback execution could unexpectedly unprotect the `call` object prior to evaluation.
67

R/aaa_ffi_codegen_matrix.R

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,13 @@ RTINYCC_FFI_SEMANTICS <- list(
580580
ownership = "R",
581581
r_storage = "raw",
582582
checks = character(),
583-
notes = "Zero-copy borrow of RAW(x); sound only while no fresh R allocations are inserted before the C call."
583+
notes = paste(
584+
"Mutable pointer input through RAW(x); for ordinary materialized vectors",
585+
"no extra buffer is allocated. ALTREP vectors follow R's writable pointer",
586+
"materialization path. ALTREP-specific read-only or temp-buffer behavior",
587+
"needs a separate access-mode contract because this type permits mutation",
588+
"and pointer aliasing."
589+
)
584590
),
585591
return = list(
586592
mode = "copy_array",
@@ -601,7 +607,13 @@ RTINYCC_FFI_SEMANTICS <- list(
601607
ownership = "R",
602608
r_storage = "integer",
603609
checks = character(),
604-
notes = "Zero-copy borrow of INTEGER(x); sound only while no fresh R allocations are inserted before the C call."
610+
notes = paste(
611+
"Mutable pointer input through INTEGER(x); for ordinary materialized",
612+
"vectors no extra buffer is allocated. ALTREP vectors follow R's writable",
613+
"pointer materialization path. ALTREP-specific read-only or temp-buffer",
614+
"behavior needs a separate access-mode contract because this type permits",
615+
"mutation and pointer aliasing."
616+
)
605617
),
606618
return = list(
607619
mode = "copy_array",
@@ -622,7 +634,13 @@ RTINYCC_FFI_SEMANTICS <- list(
622634
ownership = "R",
623635
r_storage = "numeric",
624636
checks = character(),
625-
notes = "Zero-copy borrow of REAL(x); sound only while no fresh R allocations are inserted before the C call."
637+
notes = paste(
638+
"Mutable pointer input through REAL(x); for ordinary materialized vectors",
639+
"no extra buffer is allocated. ALTREP vectors follow R's writable pointer",
640+
"materialization path. ALTREP-specific read-only or temp-buffer behavior",
641+
"needs a separate access-mode contract because this type permits mutation",
642+
"and pointer aliasing."
643+
)
626644
),
627645
return = list(
628646
mode = "copy_array",
@@ -643,7 +661,13 @@ RTINYCC_FFI_SEMANTICS <- list(
643661
ownership = "R",
644662
r_storage = "logical",
645663
checks = character(),
646-
notes = "Zero-copy borrow of LOGICAL(x); sound only while no fresh R allocations are inserted before the C call."
664+
notes = paste(
665+
"Mutable pointer input through LOGICAL(x); for ordinary materialized",
666+
"vectors no extra buffer is allocated. ALTREP vectors follow R's writable",
667+
"pointer materialization path. ALTREP-specific read-only or temp-buffer",
668+
"behavior needs a separate access-mode contract because this type permits",
669+
"mutation and pointer aliasing."
670+
)
647671
),
648672
return = list(
649673
mode = "copy_array",
@@ -1083,8 +1107,9 @@ RTINYCC_COMPOSITE_SEMANTICS <- list(
10831107
write_copy = TRUE,
10841108
ownership = "struct-owned-storage",
10851109
notes = paste(
1086-
"Raw struct helpers copy bytes out to a fresh RAWSXP or copy bytes from",
1087-
"a RAWSXP back into the struct buffer with memcpy()."
1110+
"Raw struct helpers copy bytes out to a fresh RAWSXP with memcpy() and",
1111+
"copy bytes from a RAWSXP back into the struct buffer with RAW_GET_REGION(),",
1112+
"which avoids asking R for a writable raw-vector data pointer on copy-in."
10881113
)
10891114
),
10901115
struct_array_field = list(
@@ -1253,7 +1278,7 @@ RTINYCC_COMPOSITE_CODEGEN_SPECS <- list(
12531278
),
12541279
list(
12551280
name = "struct_raw_access_copy",
1256-
info = "struct raw access helpers use memcpy copy paths",
1281+
info = "struct raw access helpers use explicit copy paths",
12571282
generate_args = list(
12581283
symbols = list(),
12591284
c_code = "struct packet { unsigned char data[8]; };",
@@ -1276,7 +1301,7 @@ RTINYCC_COMPOSITE_CODEGEN_SPECS <- list(
12761301
fixed = TRUE
12771302
),
12781303
list(
1279-
pattern = "memcpy(p, RAW(raw),",
1304+
pattern = "RAW_GET_REGION(raw, 0, n_copy, (Rbyte*)p)",
12801305
fixed = TRUE
12811306
)
12821307
)

R/aaa_ffi_codegen_rules.R

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ struct_array_field_setter_rule("i32", field_name) %as%
537537

538538
struct_array_field_setter_rule("i64", field_name) %as%
539539
{
540-
sprintf("p->%s[idx] = (int64_t)REAL(val)[0];", field_name)
540+
sprintf("p->%s[idx] = (int64_t)asReal(val);", field_name)
541541
}
542542

543543
struct_array_field_setter_rule("u8", field_name) %as%
@@ -552,22 +552,22 @@ struct_array_field_setter_rule("u16", field_name) %as%
552552

553553
struct_array_field_setter_rule("u32", field_name) %as%
554554
{
555-
sprintf("p->%s[idx] = (uint32_t)REAL(val)[0];", field_name)
555+
sprintf("p->%s[idx] = (uint32_t)asReal(val);", field_name)
556556
}
557557

558558
struct_array_field_setter_rule("u64", field_name) %as%
559559
{
560-
sprintf("p->%s[idx] = (uint64_t)REAL(val)[0];", field_name)
560+
sprintf("p->%s[idx] = (uint64_t)asReal(val);", field_name)
561561
}
562562

563563
struct_array_field_setter_rule("f32", field_name) %as%
564564
{
565-
sprintf("p->%s[idx] = (float)REAL(val)[0];", field_name)
565+
sprintf("p->%s[idx] = (float)asReal(val);", field_name)
566566
}
567567

568568
struct_array_field_setter_rule("f64", field_name) %as%
569569
{
570-
sprintf("p->%s[idx] = REAL(val)[0];", field_name)
570+
sprintf("p->%s[idx] = asReal(val);", field_name)
571571
}
572572

573573
struct_array_field_setter_rule("bool", field_name) %as%
@@ -604,7 +604,7 @@ struct_field_setter_rule("i32", field_name, size) %as%
604604

605605
struct_field_setter_rule("i64", field_name, size) %as%
606606
{
607-
sprintf("p->%s = (int64_t)REAL(val)[0];", field_name)
607+
sprintf("p->%s = (int64_t)asReal(val);", field_name)
608608
}
609609

610610
struct_field_setter_rule("u8", field_name, size) %as%
@@ -619,12 +619,12 @@ struct_field_setter_rule("u16", field_name, size) %as%
619619

620620
struct_field_setter_rule("u32", field_name, size) %as%
621621
{
622-
sprintf("p->%s = (uint32_t)REAL(val)[0];", field_name)
622+
sprintf("p->%s = (uint32_t)asReal(val);", field_name)
623623
}
624624

625625
struct_field_setter_rule("u64", field_name, size) %as%
626626
{
627-
sprintf("p->%s = (uint64_t)REAL(val)[0];", field_name)
627+
sprintf("p->%s = (uint64_t)asReal(val);", field_name)
628628
}
629629

630630
struct_field_setter_rule("f32", field_name, size) %as%

R/ffi_codegen.R

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -921,12 +921,14 @@ generate_struct_raw_access <- function(struct_name) {
921921
sprintf("SEXP R_wrap_struct_%s_set_raw(SEXP ext, SEXP raw) {", struct_name),
922922
sprintf(" struct %s *p = R_ExternalPtrAddr(ext);", struct_name),
923923
sprintf(" if (!p) Rf_error(\"Null pointer\");"),
924-
" int n = LENGTH(raw);",
924+
" if (TYPEOF(raw) != RAWSXP) Rf_error(\"expected raw vector\");",
925+
" R_xlen_t n = XLENGTH(raw);",
925926
sprintf(
926-
" memcpy(p, RAW(raw), (n < sizeof(struct %s)) ? n : sizeof(struct %s));",
927+
" R_xlen_t n_copy = (n < (R_xlen_t)sizeof(struct %s)) ? n : (R_xlen_t)sizeof(struct %s);",
927928
struct_name,
928929
struct_name
929930
),
931+
" if (n_copy > 0 && RAW_GET_REGION(raw, 0, n_copy, (Rbyte*)p) != n_copy) Rf_error(\"failed to read raw vector\");",
930932
" return R_NilValue;",
931933
"}",
932934
""

R/ffi_types.R

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@
1111
# - numeric() → double* (via REAL())
1212
# - logical() → int* (via LOGICAL())
1313
#
14-
# We support both scalar types (with coercion) and array types (zero-copy)
14+
# We support both scalar types (with coercion) and array types. Array inputs
15+
# borrow writable R vector storage for materialized vectors; ALTREP vectors may
16+
# be materialized by R when the wrapper asks for a C data pointer.
1517

1618
new_rtinycc_ffi_type <- function(name, ..., family = name) {
1719
fields <- list(name = name, family = family, ...)
@@ -101,7 +103,8 @@ FFI_TYPES <- list(
101103
kind = "scalar"
102104
),
103105

104-
# Array types - R native vector types (zero-copy)
106+
# Array types - R native vector types. These are writable pointer borrows;
107+
# ALTREP inputs may materialize when R exposes the C data pointer.
105108
# R raw vector → uint8_t* (byte buffer)
106109
raw = list(
107110
c_type = "uint8_t*",
@@ -112,7 +115,7 @@ FFI_TYPES <- list(
112115
c_element = "uint8_t"
113116
),
114117

115-
# R integer vector → int32_t* (zero-copy via INTEGER())
118+
# R integer vector → int32_t* (writable borrow via INTEGER())
116119
integer_array = list(
117120
c_type = "int32_t*",
118121
r_type = "integer",
@@ -122,7 +125,7 @@ FFI_TYPES <- list(
122125
c_element = "int32_t"
123126
),
124127

125-
# R numeric vector → double* (zero-copy via REAL())
128+
# R numeric vector → double* (writable borrow via REAL())
126129
numeric_array = list(
127130
c_type = "double*",
128131
r_type = "numeric",
@@ -132,7 +135,7 @@ FFI_TYPES <- list(
132135
c_element = "double"
133136
),
134137

135-
# R logical vector → int* (zero-copy via LOGICAL())
138+
# R logical vector → int* (writable borrow via LOGICAL())
136139
logical_array = list(
137140
c_type = "int*",
138141
r_type = "logical",

README.Rmd

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1104,8 +1104,8 @@ In practice, the usual pattern is:
11041104

11051105
- `Rtinycc` compiles tiny modules very quickly
11061106
- a regular `.Call()` module can have lower minimal per-call overhead
1107-
- array-oriented zero-copy inputs are a much better fit than many tiny scalar
1108-
crossings
1107+
- array-oriented inputs can be zero-copy for already-materialized R vectors,
1108+
but ALTREP inputs may materialize when C pointer access is requested
11091109
- return paths that copy native buffers back into fresh R vectors make that
11101110
copy cost visible
11111111

README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1979,8 +1979,9 @@ In practice, the usual pattern is:
19791979

19801980
- `Rtinycc` compiles tiny modules very quickly
19811981
- a regular `.Call()` module can have lower minimal per-call overhead
1982-
- array-oriented zero-copy inputs are a much better fit than many tiny
1983-
scalar crossings
1982+
- array-oriented inputs can be zero-copy for already-materialized R
1983+
vectors, but ALTREP inputs may materialize when C pointer access is
1984+
requested
19841985
- return paths that copy native buffers back into fresh R vectors make
19851986
that copy cost visible
19861987

TODO

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
- design explicit ALTREP-aware FFI array semantics before changing mutable array inputs; decide whether to add read-only array types, inout copy-back modes, aliasing rules, and when to use *_GET_REGION(), DATAPTR_RO(), or writable materialization
12
- add simple input types for tcc_call_symbol
23
- add simple more types
34
- setjump support for tcc_call_symbol

src/RC_libtcc.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1138,7 +1138,10 @@ SEXP RC_write_bytes(SEXP ptr, SEXP raw) {
11381138
}
11391139
R_xlen_t n = XLENGTH(raw);
11401140
if (n > 0) {
1141-
memcpy(data, RAW(raw), (size_t)n);
1141+
R_xlen_t copied = RAW_GET_REGION(raw, 0, n, (Rbyte*)data);
1142+
if (copied != n) {
1143+
Rf_error("failed to read raw vector");
1144+
}
11421145
}
11431146
return R_NilValue;
11441147
}

src/platform_async.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,13 @@ static cb_result_t cb_result_from_sexp(SEXP s) {
1616
if (s == R_NilValue) return r;
1717
if (Rf_isInteger(s) && XLENGTH(s) >= 1) {
1818
r.kind = CB_RESULT_INT;
19-
r.v.i = INTEGER(s)[0];
19+
r.v.i = Rf_asInteger(s);
2020
} else if (Rf_isReal(s) && XLENGTH(s) >= 1) {
2121
r.kind = CB_RESULT_REAL;
22-
r.v.d = REAL(s)[0];
22+
r.v.d = Rf_asReal(s);
2323
} else if (Rf_isLogical(s) && XLENGTH(s) >= 1) {
2424
r.kind = CB_RESULT_LOGICAL;
25-
r.v.i = LOGICAL(s)[0];
25+
r.v.i = Rf_asLogical(s);
2626
} else if (TYPEOF(s) == EXTPTRSXP) {
2727
r.kind = CB_RESULT_PTR;
2828
r.v.p = R_ExternalPtrAddr(s);

0 commit comments

Comments
 (0)