Don't assume llvm::StringRef is null terminated

StringRefs have a length and their contents are not usually null-terminated. The solution is to either copy the string data (in rustc_llvm::diagnostic) or take the size into account (in LLVMRustPrintPasses). I couldn't trigger a bug caused by this (apparently all the strings returned in practice are actually null-terminated) but this is more correct and more future-proof.
rust-lang · Nov 28, 2016 · 85dc08e · shepmaster · Dec 7, 2016 · hanna-kruppe
1 parent c7ddb89
commit 85dc08e
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 28 deletions.
diff --git a/src/librustc_llvm/diagnostic.rs b/src/librustc_llvm/diagnostic.rs
@@ -13,7 +13,7 @@
 pub use self::OptimizationDiagnosticKind::*;
 pub use self::Diagnostic::*;
 
-use libc::{c_char, c_uint};
+use libc::c_uint;
 use std::ptr;
 
 use {DiagnosticInfoRef, TwineRef, ValueRef};
@@ -45,7 +45,7 @@ impl OptimizationDiagnosticKind {
 
 pub struct OptimizationDiagnostic {
     pub kind: OptimizationDiagnosticKind,
-    pub pass_name: *const c_char,
+    pub pass_name: String,
     pub function: ValueRef,
     pub debug_loc: DebugLocRef,
     pub message: String,
@@ -55,21 +55,23 @@ impl OptimizationDiagnostic {
     unsafe fn unpack(kind: OptimizationDiagnosticKind,
                      di: DiagnosticInfoRef)
                      -> OptimizationDiagnostic {
-        let mut pass_name = ptr::null();
         let mut function = ptr::null_mut();
         let mut debug_loc = ptr::null_mut();
 
-        let message = super::build_string(|message|
-            super::LLVMRustUnpackOptimizationDiagnostic(di,
-                                                        &mut pass_name,
-                                                        &mut function,
-                                                        &mut debug_loc,
-                                                        message)
+        let mut message = None;
+        let pass_name = super::build_string(|pass_name|
+            message = super::build_string(|message|
+                super::LLVMRustUnpackOptimizationDiagnostic(di,
+                                                            pass_name,
+                                                            &mut function,
+                                                            &mut debug_loc,
+                                                            message)
+            )
         );
 
         OptimizationDiagnostic {
             kind: kind,
-            pass_name: pass_name,
+            pass_name: pass_name.expect("got a non-UTF8 pass name from LLVM"),
             function: function,
             debug_loc: debug_loc,
             message: message.expect("got a non-UTF8 OptimizationDiagnostic message from LLVM")

diff --git a/src/librustc_llvm/ffi.rs b/src/librustc_llvm/ffi.rs
@@ -1820,7 +1820,7 @@ extern "C" {
                                            DiagnosticContext: *mut c_void);
 
     pub fn LLVMRustUnpackOptimizationDiagnostic(DI: DiagnosticInfoRef,
-                                                pass_name_out: *mut *const c_char,
+                                                pass_name_out: RustStringRef,
                                                 function_out: *mut ValueRef,
                                                 debugloc_out: *mut DebugLocRef,
                                                 message_out: RustStringRef);

diff --git a/src/librustc_trans/back/write.rs b/src/librustc_trans/back/write.rs
@@ -26,7 +26,7 @@ use errors::emitter::Emitter;
 use syntax_pos::MultiSpan;
 use context::{is_pie_binary, get_reloc_model};
 
-use std::ffi::{CStr, CString};
+use std::ffi::CString;
 use std::fs;
 use std::path::{Path, PathBuf};
 use std::str;
@@ -403,19 +403,16 @@ unsafe extern "C" fn diagnostic_handler(info: DiagnosticInfoRef, user: *mut c_vo
         }
 
         llvm::diagnostic::Optimization(opt) => {
-            let pass_name = str::from_utf8(CStr::from_ptr(opt.pass_name).to_bytes())
-                                .ok()
-                                .expect("got a non-UTF8 pass name from LLVM");
             let enabled = match cgcx.remark {
                 AllPasses => true,
-                SomePasses(ref v) => v.iter().any(|s| *s == pass_name),
+                SomePasses(ref v) => v.iter().any(|s| *s == opt.pass_name),
             };
 
             if enabled {
                 let loc = llvm::debug_loc_to_string(llcx, opt.debug_loc);
                 cgcx.handler.note_without_error(&format!("optimization {} for {} at {}: {}",
                                                 opt.kind.describe(),
-                                                pass_name,
+                                                opt.pass_name,
                                                 if loc.is_empty() { "[unknown]" } else { &*loc },
                                                 opt.message));
             }

diff --git a/src/rustllvm/PassWrapper.cpp b/src/rustllvm/PassWrapper.cpp
@@ -530,9 +530,11 @@ LLVMRustPrintPasses() {
     struct MyListener : PassRegistrationListener {
         void passEnumerate(const PassInfo *info) {
 #if LLVM_VERSION_GE(4, 0)
-            if (!info->getPassArgument().empty()) {
-                printf("%15s - %s\n", info->getPassArgument().data(),
-                       info->getPassName().data());
+            StringRef PassArg = info->getPassArgument();
+            StringRef PassName = info->getPassName();
+            if (!PassArg.empty()) {
+                printf("%15.*s - %.*s\n", PassArg.size(), PassArg.data(),
+                       PassName.size(), PassName.data());
             }
 #else
             if (info->getPassArgument() && *info->getPassArgument()) {

diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp
@@ -872,7 +872,7 @@ LLVMRustWriteTwineToString(LLVMTwineRef T, RustStringRef str) {
 extern "C" void
 LLVMRustUnpackOptimizationDiagnostic(
     LLVMDiagnosticInfoRef di,
-    const char **pass_name_out,
+    RustStringRef pass_name_out,
     LLVMValueRef *function_out,
     LLVMDebugLocRef *debugloc_out,
     RustStringRef message_out)
@@ -881,15 +881,12 @@ LLVMRustUnpackOptimizationDiagnostic(
     llvm::DiagnosticInfoOptimizationBase *opt
         = static_cast<llvm::DiagnosticInfoOptimizationBase*>(unwrap(di));
 
-#if LLVM_VERSION_GE(4, 0)
-    *pass_name_out = opt->getPassName().data();
-#else
-    *pass_name_out = opt->getPassName();
-#endif
+    raw_rust_string_ostream pass_name_os(pass_name_out);
+    pass_name_os << opt->getPassName();
     *function_out = wrap(&opt->getFunction());
     *debugloc_out = wrap(&opt->getDebugLoc());
-    raw_rust_string_ostream os(message_out);
-    os << opt->getMsg();
+    raw_rust_string_ostream message_os(message_out);
+    message_os << opt->getMsg();
 }
 
 extern "C" void