Skip to content

Commit d48fc0a

Browse files
fix(windows): address PR #470 review comments
- Remove unsafe `RAC_ORT_PATH` macro from rac_platform_compat.h — it returned `const wchar_t*` into a destroyed temporary `std::wstring`, UB at every call site. Replaced with a doc comment pinning the safe named-local pattern. Macro was never invoked; no code changes needed. (greptile #3081984814, coderabbit #3082012163) - Guard `ENABLE_LIBGCC` with `if(MSVC)` so Linux+GCC keeps the default OFF behaviour; only MSVC forces it ON. Add rationale comment. (greptile #3081984920) - Drop stale "Windows" from the backtrace-not-supported fallback comment in rac_structured_error.cpp — Windows has its own `CaptureStackBackTrace` branch now. (greptile #3081985002) - Fix malformed MSVC generator expression in RAG CMakeLists.txt: use semicolon-separated flag list instead of space-separated so CMake parses all three `/wd` warning suppressions correctly. (coderabbit #3082012173) macOS verification: `cmake --build` 100% clean, 51/51 tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 0c7e4fa commit d48fc0a

5 files changed

Lines changed: 19 additions & 16 deletions

File tree

sdk/runanywhere-commons/CMakeLists.txt

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,16 @@ set(ENABLE_LIBXML2 OFF CACHE BOOL "" FORCE)
247247
set(ENABLE_EXPAT OFF CACHE BOOL "" FORCE)
248248
set(ENABLE_PCREPOSIX OFF CACHE BOOL "" FORCE)
249249
set(ENABLE_PCRE2POSIX OFF CACHE BOOL "" FORCE)
250-
set(ENABLE_LIBGCC ON CACHE BOOL "" FORCE)
250+
# libarchive's ENABLE_LIBGCC gates a `find_library(GCC_LIBRARY gcc)` / `-lgcc`
251+
# link. Needed on MSVC for the bundled runtime; a no-op on Apple/Clang (no
252+
# libgcc); on Linux+GCC keep it OFF so we don't add an explicit -lgcc to the
253+
# link line (it's pulled in implicitly and adding it can cause duplicate-symbol
254+
# warnings in static configurations).
255+
if(MSVC)
256+
set(ENABLE_LIBGCC ON CACHE BOOL "" FORCE)
257+
else()
258+
set(ENABLE_LIBGCC OFF CACHE BOOL "" FORCE)
259+
endif()
251260
set(ENABLE_CNG OFF CACHE BOOL "" FORCE)
252261
set(ENABLE_TAR OFF CACHE BOOL "" FORCE) # Don't build bsdtar binary
253262
set(ENABLE_CPIO OFF CACHE BOOL "" FORCE) # Don't build bsdcpio binary

sdk/runanywhere-commons/include/rac/core/rac_platform_compat.h

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -188,19 +188,13 @@ inline std::wstring rac_to_wstring(const char* s) {
188188
if (!s || !*s) return {};
189189
return rac_to_wstring(std::string(s));
190190
}
191-
/**
192-
* On Windows, ONNX Runtime expects wchar_t* paths.
193-
* NOTE: The macro returns a pointer into a temporary std::wstring. Callers MUST
194-
* store the result in a named local before calling .c_str(), otherwise the
195-
* pointer dangles at the end of the full-expression:
196-
* std::wstring wp = rac_to_wstring(p);
197-
* Ort::Session s(env, wp.c_str(), options);
198-
*/
199-
#define RAC_ORT_PATH(p) rac_to_wstring(p).c_str()
200-
#else
201-
/** On non-Windows, ONNX Runtime expects char* paths */
202-
#define RAC_ORT_PATH(p) (p)
203191
#endif
192+
// ONNX Runtime path handling:
193+
// - On Windows, use `std::wstring wp = rac_to_wstring(p); session(env, wp.c_str(), opts);`
194+
// - On non-Windows, pass the `const char*` path directly.
195+
// No macro is provided on purpose: a macro returning `rac_to_wstring(p).c_str()`
196+
// would dangle at the end of the full-expression (the temporary wstring is
197+
// destroyed before Ort::Session reads from the pointer).
204198

205199
#endif /* __cplusplus */
206200

sdk/runanywhere-commons/src/backends/onnx/wakeword_onnx.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,7 @@ RAC_ONNX_API rac_result_t rac_wakeword_onnx_init_shared_models(
567567
try {
568568
// Load melspectrogram model (required for proper pipeline).
569569
// Store the wstring in a named local so the wchar_t* outlives the ctor call
570-
// (RAC_ORT_PATH returns a pointer into a temporary on Windows).
570+
// (passing `rac_to_wstring(p).c_str()` directly would dangle).
571571
if (melspec_model_path) {
572572
#ifdef _WIN32
573573
std::wstring melspec_wpath = rac_to_wstring(melspec_model_path);

sdk/runanywhere-commons/src/core/rac_structured_error.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ int32_t rac_error_capture_stack_trace(rac_error_t* error) {
246246
error->stack_frame_count = captured;
247247
return captured;
248248
#else
249-
// Platform doesn't support backtrace (Android, Windows, etc.)
249+
// Platform doesn't support backtrace (Android NDK, etc.)
250250
error->stack_frame_count = 0;
251251
return 0;
252252
#endif

sdk/runanywhere-commons/src/features/rag/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ set_target_properties(rac_backend_rag PROPERTIES
103103
target_compile_options(rac_backend_rag PRIVATE
104104
$<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-unused-parameter>
105105
$<$<CXX_COMPILER_ID:GNU,Clang,AppleClang>:-Wno-missing-field-initializers>
106-
$<$<CXX_COMPILER_ID:MSVC>:/wd4244 /wd4267 /wd4996>
106+
$<$<CXX_COMPILER_ID:MSVC>:/wd4244;/wd4267;/wd4996>
107107
)
108108

109109
target_compile_definitions(rac_backend_rag PRIVATE

0 commit comments

Comments
 (0)