symbols versions are not mangled correctly

[brson]

As part of #2137 and #456 we have added the crate version to every exported symbol, but not quite in the correct way.

Right now the final part of the path used for the symbol ends with "V" + vers, which gets passed through the mangler. The original design called for @vers but the @ isn't a valid symbol on all platforms (I think?) so gets mangled to _sbox_, which is a bizarre way to indicate the version.

Mangling also makes any dots in the version go away so a version "0.2" symbol will end with "V02".

There is some additional discussion here about how this is supposed to work that I haven't read.

[lht]

Patch pasted below works on FreeBSD, Windows bots and my Linux machine (x64) with gcc-4.6.3, ld-2.22, but failed on the Linux bots with error:

note: /usr/bin/ld: x86_64-unknown-linux-gnu/stage2/lib/rustc/x86_64-unknown-linux-gnu/bin/fuzzer: No symbol version section for versioned symbol `replace_ty_in_crate::_3856bdcd8148cb38@VERS_0.0'
/usr/bin/ld: final link failed: Nonrepresentable section on output
collect2: ld returned 1 exit status

diff --git a/src/rustc/back/link.rs b/src/rustc/back/link.rs
index 114f02f..1a20084 100644
--- a/src/rustc/back/link.rs
+++ b/src/rustc/back/link.rs
@@ -13,6 +13,7 @@ import syntax::print::pprust;
 import lib::llvm::{ModuleRef, mk_pass_manager, mk_target_data, True, False};
 import util::filesearch;
 import middle::ast_map::{path, path_mod, path_name};
+import io::writer_util;

 enum output_type {
     output_type_none,
@@ -448,7 +449,6 @@ fn sanitize(s: str) -> str {
     let mut result = "";
     str::chars_iter(s) {|c|
         alt c {
-          '@' { result += "_sbox_"; }
           '~' { result += "_ubox_"; }
           '*' { result += "_ptr_"; }
           '&' { result += "_ref_"; }
@@ -458,7 +458,7 @@ fn sanitize(s: str) -> str {
           'a' to 'z'
           | 'A' to 'Z'
           | '0' to '9'
-          | '_' { str::push_char(result,c); }
+          | '_' | '@' | '.' { str::push_char(result,c); }
           _ {
             if c > 'z' && char::is_XID_continue(c) {
                 str::push_char(result,c);
@@ -485,12 +485,13 @@ fn mangle(ss: path) -> str {
 }

 fn exported_name(path: path, hash: str, vers: str) -> str {
-    ret mangle(path + [path_name(hash)] + [path_name(vers)]);
+    ret mangle(path + [path_name(hash)]) + "@VERS_" + sanitize(vers);
 }

 fn mangle_exported_name(ccx: @crate_ctxt, path: path, t: ty::t) -> str {
     let hash = get_symbol_hash(ccx, t);
-    ret exported_name(path, hash, ccx.link_meta.vers);
+    let exp = exported_name(path, hash, ccx.link_meta.vers);
+    ret exp;
 }

 fn mangle_internal_name_by_type_only(ccx: @crate_ctxt, t: ty::t, name: str) ->
@@ -602,6 +603,18 @@ fn link_binary(sess: session,
     if sess.building_library {
         cc_args += [lib_cmd];

+        if sess.targ_cfg.os != session::os_macos {
+            let version_script = out_filename + ".verscript";
+            #debug["version_script: %s", version_script];
+            alt io::file_writer(version_script, [io::create]) {
+                result::ok(w) {
+                    w.write_line("VERS_" + lm.vers + " { global: *; };");
+                }
+                _ { fail }
+            };
+            cc_args += ["-Wl,--version-script=" + version_script];
+        }
+
         // On mac we need to tell the linker to let this library
         // be rpathed
         if sess.targ_cfg.os == session::os_macos {

[graydon]

This is great work. Thanks for digging into it.

[brson]

Removing this from 0.3 since it's just a completeness issue.

[graydon]

Version strings (as of 1c1af99) are now appended with a trailing _NN so as not to break the name manger. But I'm thinking about this more and wondering if it's actually necessary -- or even important at all -- to try to support the symbol-versioning thing the GNU linker does. Increasingly I think not. That's for integration with C symbols, where the C language has no real concept of a versioned symbol. We do: our symbols are always resolved in the context of a specific crate-linkage, which has a crate-version associated.

Now that I consider it, I think maybe it'll work best if we don't mangle the version into the filename, but do suffix the symbols. Then we can simulate what the gnu linker does, but on all platforms: tell the loader which library to request by name+hash, and then support multiple versions of each symbol in a crate, say via a #[vers="x.y"] attribute that you can put on any item you like, to keep it around for backward-compatibility.

Thoughts?

[lkuper]

By the way, I tried running some mangled Rust symbols through c++filt, and it didn't seem to know how to unmangle them. It would be nice if whatever we do can be unmangled by existing tools, if that's possible.

[graydon]

The c++filt thing has to do with us making duplicate symbols (by accident) and LLVM disambiguating them by appending numbers, breaking our (otherwise c++filt-compatible) mangling. This keeps happening. it's very uncool. I put in some hacks to try to notice when it's happening already but apparently some slipped by. I'll try to track down the current culprits.

[catamorphism]

Longer-term issue, de-milestoning

[bstrie]

@graydon, perhaps bring this up on the mailing list if you have questions as to the design here?

Nominating for Maturity 2.

[graydon]

accepted for production-ready milestone

[catamorphism]

Low, not 1.0

[steveklabnik]

Is this issue still relevant today?

[arielb1]

This was fixed when we started hashing crate contents.