Rollup merge of #67033 - cuviper:ValueName2, r=rkruppe

Migrate to LLVM{Get,Set}ValueName2 The deprecated `LLVM{Get,Set}ValueName` only work with NUL-terminated strings, but the `2` variants use explicit lengths, which fits better with Rust strings and slices. We now use these in new helper functions `llvm::{get,set}_value_name` that convert to/from `&[u8]`. Closes #64223. r? @rkruppe
rust-lang · Dec 6, 2019 · 0b471bf · 0b471bf
2 parents fd4cec0 + 16d2178
commit 0b471bf
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 39 deletions.
diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs
@@ -22,7 +22,7 @@ use rustc_fs_util::{path_to_c_string, link_or_copy};
 use rustc_data_structures::small_c_str::SmallCStr;
 use errors::{Handler, FatalError};
 
-use std::ffi::{CString, CStr};
+use std::ffi::CString;
 use std::fs;
 use std::io::{self, Write};
 use std::path::{Path, PathBuf};
@@ -833,16 +833,16 @@ fn create_msvc_imps(
             })
             .filter_map(|val| {
                 // Exclude some symbols that we know are not Rust symbols.
-                let name = CStr::from_ptr(llvm::LLVMGetValueName(val));
-                if ignored(name.to_bytes()) {
+                let name = llvm::get_value_name(val);
+                if ignored(name) {
                     None
                 } else {
                     Some((val, name))
                 }
             })
             .map(move |(val, name)| {
                 let mut imp_name = prefix.as_bytes().to_vec();
-                imp_name.extend(name.to_bytes());
+                imp_name.extend(name);
                 let imp_name = CString::new(imp_name).unwrap();
                 (imp_name, val)
             })

diff --git a/src/librustc_codegen_llvm/consts.rs b/src/librustc_codegen_llvm/consts.rs
@@ -21,7 +21,7 @@ use rustc::ty::layout::{self, Size, Align, LayoutOf};
 
 use rustc::hir::{self, CodegenFnAttrs, CodegenFnAttrFlags};
 
-use std::ffi::{CStr, CString};
+use std::ffi::CStr;
 
 pub fn const_alloc_to_llvm(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) -> &'ll Value {
     let mut llvals = Vec::with_capacity(alloc.relocations().len() + 1);
@@ -392,16 +392,14 @@ impl StaticMethods for CodegenCx<'ll, 'tcx> {
             } else {
                 // If we created the global with the wrong type,
                 // correct the type.
-                let empty_string = const_cstr!("");
-                let name_str_ref = CStr::from_ptr(llvm::LLVMGetValueName(g));
-                let name_string = CString::new(name_str_ref.to_bytes()).unwrap();
-                llvm::LLVMSetValueName(g, empty_string.as_ptr());
+                let name = llvm::get_value_name(g).to_vec();
+                llvm::set_value_name(g, b"");
 
                 let linkage = llvm::LLVMRustGetLinkage(g);
                 let visibility = llvm::LLVMRustGetVisibility(g);
 
                 let new_g = llvm::LLVMRustGetOrInsertGlobal(
-                    self.llmod, name_string.as_ptr(), val_llty);
+                    self.llmod, name.as_ptr().cast(), name.len(), val_llty);
 
                 llvm::LLVMRustSetLinkage(new_g, linkage);
                 llvm::LLVMRustSetVisibility(new_g, visibility);

diff --git a/src/librustc_codegen_llvm/debuginfo/mod.rs b/src/librustc_codegen_llvm/debuginfo/mod.rs
@@ -32,7 +32,7 @@ use rustc_codegen_ssa::mir::debuginfo::{FunctionDebugContext, DebugScope,
 
 use libc::c_uint;
 use std::cell::RefCell;
-use std::ffi::{CStr, CString};
+use std::ffi::CString;
 
 use smallvec::SmallVec;
 use syntax_pos::{self, BytePos, Span, Pos};
@@ -255,23 +255,11 @@ impl DebugInfoBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> {
             return;
         }
 
-        let old_name = unsafe {
-            CStr::from_ptr(llvm::LLVMGetValueName(value))
-        };
-        match old_name.to_str() {
-            Ok("") => {}
-            Ok(_) => {
-                // Avoid replacing the name if it already exists.
-                // While we could combine the names somehow, it'd
-                // get noisy quick, and the usefulness is dubious.
-                return;
-            }
-            Err(_) => return,
-        }
-
-        let cname = SmallCStr::new(name);
-        unsafe {
-            llvm::LLVMSetValueName(value, cname.as_ptr());
+        // Avoid replacing the name if it already exists.
+        // While we could combine the names somehow, it'd
+        // get noisy quick, and the usefulness is dubious.
+        if llvm::get_value_name(value).is_empty() {
+            llvm::set_value_name(value, name.as_bytes());
         }
     }
 }

diff --git a/src/librustc_codegen_llvm/declare.rs b/src/librustc_codegen_llvm/declare.rs
@@ -76,9 +76,8 @@ impl DeclareMethods<'tcx> for CodegenCx<'ll, 'tcx> {
         name: &str, ty: &'ll Type
     ) -> &'ll Value {
         debug!("declare_global(name={:?})", name);
-        let namebuf = SmallCStr::new(name);
         unsafe {
-            llvm::LLVMRustGetOrInsertGlobal(self.llmod, namebuf.as_ptr(), ty)
+            llvm::LLVMRustGetOrInsertGlobal(self.llmod, name.as_ptr().cast(), name.len(), ty)
         }
     }
 

diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs
@@ -701,8 +701,8 @@ extern "C" {
 
     // Operations on all values
     pub fn LLVMTypeOf(Val: &Value) -> &Type;
-    pub fn LLVMGetValueName(Val: &Value) -> *const c_char;
-    pub fn LLVMSetValueName(Val: &Value, Name: *const c_char);
+    pub fn LLVMGetValueName2(Val: &Value, Length: *mut size_t) -> *const c_char;
+    pub fn LLVMSetValueName2(Val: &Value, Name: *const c_char, NameLen: size_t);
     pub fn LLVMReplaceAllUsesWith(OldVal: &'a Value, NewVal: &'a Value);
     pub fn LLVMSetMetadata(Val: &'a Value, KindID: c_uint, Node: &'a Value);
 
@@ -774,7 +774,8 @@ extern "C" {
     pub fn LLVMIsAGlobalVariable(GlobalVar: &Value) -> Option<&Value>;
     pub fn LLVMAddGlobal(M: &'a Module, Ty: &'a Type, Name: *const c_char) -> &'a Value;
     pub fn LLVMGetNamedGlobal(M: &Module, Name: *const c_char) -> Option<&Value>;
-    pub fn LLVMRustGetOrInsertGlobal(M: &'a Module, Name: *const c_char, T: &'a Type) -> &'a Value;
+    pub fn LLVMRustGetOrInsertGlobal(M: &'a Module, Name: *const c_char, NameLen: size_t,
+                                     T: &'a Type) -> &'a Value;
     pub fn LLVMRustInsertPrivateGlobal(M: &'a Module, T: &'a Type) -> &'a Value;
     pub fn LLVMGetFirstGlobal(M: &Module) -> Option<&Value>;
     pub fn LLVMGetNextGlobal(GlobalVar: &Value) -> Option<&Value>;
@@ -1811,7 +1812,7 @@ extern "C" {
 
     pub fn LLVMRustPositionBuilderAtStart(B: &Builder<'a>, BB: &'a BasicBlock);
 
-    pub fn LLVMRustSetComdat(M: &'a Module, V: &'a Value, Name: *const c_char);
+    pub fn LLVMRustSetComdat(M: &'a Module, V: &'a Value, Name: *const c_char, NameLen: size_t);
     pub fn LLVMRustUnsetComdat(V: &Value);
     pub fn LLVMRustSetModulePICLevel(M: &Module);
     pub fn LLVMRustSetModulePIELevel(M: &Module);

diff --git a/src/librustc_codegen_llvm/llvm/mod.rs b/src/librustc_codegen_llvm/llvm/mod.rs
@@ -115,7 +115,8 @@ pub fn SetFunctionCallConv(fn_: &'a Value, cc: CallConv) {
 // For more details on COMDAT sections see e.g., http://www.airs.com/blog/archives/52
 pub fn SetUniqueComdat(llmod: &Module, val: &'a Value) {
     unsafe {
-        LLVMRustSetComdat(llmod, val, LLVMGetValueName(val));
+        let name = get_value_name(val);
+        LLVMRustSetComdat(llmod, val, name.as_ptr().cast(), name.len());
     }
 }
 
@@ -217,6 +218,23 @@ pub fn get_param(llfn: &'a Value, index: c_uint) -> &'a Value {
     }
 }
 
+/// Safe wrapper for `LLVMGetValueName2` into a byte slice
+pub fn get_value_name(value: &'a Value) -> &'a [u8] {
+    unsafe {
+        let mut len = 0;
+        let data = LLVMGetValueName2(value, &mut len);
+        std::slice::from_raw_parts(data.cast(), len)
+    }
+}
+
+/// Safe wrapper for `LLVMSetValueName2` from a byte slice
+pub fn set_value_name(value: &Value, name: &[u8]) {
+    unsafe {
+        let data = name.as_ptr().cast();
+        LLVMSetValueName2(value, data, name.len());
+    }
+}
+
 pub fn build_string(f: impl FnOnce(&RustString)) -> Result<String, FromUtf8Error> {
     let sr = RustString {
         bytes: RefCell::new(Vec::new()),

diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp
@@ -129,8 +129,9 @@ extern "C" LLVMValueRef LLVMRustGetOrInsertFunction(LLVMModuleRef M,
 }
 
 extern "C" LLVMValueRef
-LLVMRustGetOrInsertGlobal(LLVMModuleRef M, const char *Name, LLVMTypeRef Ty) {
-  return wrap(unwrap(M)->getOrInsertGlobal(Name, unwrap(Ty)));
+LLVMRustGetOrInsertGlobal(LLVMModuleRef M, const char *Name, size_t NameLen, LLVMTypeRef Ty) {
+  StringRef NameRef(Name, NameLen);
+  return wrap(unwrap(M)->getOrInsertGlobal(NameRef, unwrap(Ty)));
 }
 
 extern "C" LLVMValueRef
@@ -1287,11 +1288,12 @@ extern "C" void LLVMRustPositionBuilderAtStart(LLVMBuilderRef B,
 }
 
 extern "C" void LLVMRustSetComdat(LLVMModuleRef M, LLVMValueRef V,
-                                  const char *Name) {
+                                  const char *Name, size_t NameLen) {
   Triple TargetTriple(unwrap(M)->getTargetTriple());
   GlobalObject *GV = unwrap<GlobalObject>(V);
   if (!TargetTriple.isOSBinFormatMachO()) {
-    GV->setComdat(unwrap(M)->getOrInsertComdat(Name));
+    StringRef NameRef(Name, NameLen);
+    GV->setComdat(unwrap(M)->getOrInsertComdat(NameRef));
   }
 }