auto merge of #17871 : michaelwoerister/rust/lldb-versioning, r=alexc…

…richton Apart from making the build system determine the LLDB version, this PR also fixes an issue with enums in LLDB pretty printers. In order for GDB's pretty printers to know for sure if a field of some value is an enum discriminant, I had rustc mark discriminant fields with the `artificial` DWARF tag. This worked out nicely for GDB but it turns out that one can't access artificial fields from LLDB. So I changed the debuginfo representation so that enum discriminants are marked by the special field name `RUST$ENUM$DISR` instead, which works in both cases. The PR does not activate the LLDB test suite yet.
rust-lang · Oct 9, 2014 · d569dfe · d569dfe
2 parents 63fe80e + 98a0f91
commit d569dfe
Show file tree

Hide file tree

Showing 19 changed files with 152 additions and 48 deletions.
diff --git a/configure b/configure
@@ -535,13 +535,17 @@ probe CFG_LLDB             lldb
 
 if [ ! -z "$CFG_GDB" ]
 then
-    # Extract the version
+    # Store GDB's version
     CFG_GDB_VERSION=$($CFG_GDB --version 2>/dev/null | head -1)
     putvar CFG_GDB_VERSION
 fi
 
 if [ ! -z "$CFG_LLDB" ]
 then
+    # Store LLDB's version
+    CFG_LLDB_VERSION=$($CFG_LLDB --version 2>/dev/null | head -1)
+    putvar CFG_LLDB_VERSION
+
     # If CFG_LLDB_PYTHON_DIR is not already set from the outside and valid, try to read it from
     # LLDB via the -P commandline options.
     if [ -z "$CFG_LLDB_PYTHON_DIR" ] || [ ! -d "$CFG_LLDB_PYTHON_DIR" ]

diff --git a/mk/tests.mk b/mk/tests.mk
@@ -625,6 +625,7 @@ CTEST_COMMON_ARGS$(1)-T-$(2)-H-$(3) := \
         --target $(2) \
         --host $(3) \
         --gdb-version="$(CFG_GDB_VERSION)" \
+        --lldb-version="$(CFG_LLDB_VERSION)" \
         --android-cross-path=$(CFG_ANDROID_CROSS_PATH) \
         --adb-path=$(CFG_ADB) \
         --adb-test-dir=$(CFG_ADB_TEST_DIR) \

diff --git a/src/compiletest/common.rs b/src/compiletest/common.rs
@@ -133,6 +133,9 @@ pub struct Config {
     // Version of GDB
     pub gdb_version: Option<String>,
 
+    // Version of LLDB
+    pub lldb_version: Option<String>,
+
     // Path to the android tools
     pub android_cross_path: Path,
 

diff --git a/src/compiletest/compiletest.rs b/src/compiletest/compiletest.rs
@@ -71,7 +71,8 @@ pub fn parse_config(args: Vec<String> ) -> Config {
           optflag("", "jit", "run tests under the JIT"),
           optopt("", "target", "the target to build for", "TARGET"),
           optopt("", "host", "the host to build for", "HOST"),
-          optopt("", "gdb-version", "the version of GDB used", "MAJOR.MINOR"),
+          optopt("", "gdb-version", "the version of GDB used", "VERSION STRING"),
+          optopt("", "lldb-version", "the version of LLDB used", "VERSION STRING"),
           optopt("", "android-cross-path", "Android NDK standalone path", "PATH"),
           optopt("", "adb-path", "path to the android debugger", "PATH"),
           optopt("", "adb-test-dir", "path to tests for the android debugger", "PATH"),
@@ -149,6 +150,7 @@ pub fn parse_config(args: Vec<String> ) -> Config {
         target: opt_str2(matches.opt_str("target")),
         host: opt_str2(matches.opt_str("host")),
         gdb_version: extract_gdb_version(matches.opt_str("gdb-version")),
+        lldb_version: extract_lldb_version(matches.opt_str("lldb-version")),
         android_cross_path: opt_path(matches, "android-cross-path"),
         adb_path: opt_str2(matches.opt_str("adb-path")),
         adb_test_dir: opt_str2(matches.opt_str("adb-test-dir")),
@@ -391,3 +393,37 @@ fn extract_gdb_version(full_version_line: Option<String>) -> Option<String> {
         _ => None
     }
 }
+
+fn extract_lldb_version(full_version_line: Option<String>) -> Option<String> {
+    // Extract the major LLDB version from the given version string.
+    // LLDB version strings are different for Apple and non-Apple platforms.
+    // At the moment, this function only supports the Apple variant, which looks
+    // like this:
+    //
+    // LLDB-179.5 (older versions)
+    // lldb-300.2.51 (new versions)
+    //
+    // We are only interested in the major version number, so this function
+    // will return `Some("179")` and `Some("300")` respectively.
+
+    match full_version_line {
+        Some(ref full_version_line)
+          if full_version_line.as_slice().trim().len() > 0 => {
+            let full_version_line = full_version_line.as_slice().trim();
+
+            let re = Regex::new(r"[Ll][Ll][Dd][Bb]-([0-9]+)").unwrap();
+
+            match re.captures(full_version_line) {
+                Some(captures) => {
+                    Some(captures.at(1).to_string())
+                }
+                None => {
+                    println!("Could not extract LLDB version from line '{}'",
+                             full_version_line);
+                    None
+                }
+            }
+        },
+        _ => None
+    }
+}
diff --git a/src/compiletest/header.rs b/src/compiletest/header.rs
@@ -181,14 +181,42 @@ pub fn is_test_ignored(config: &Config, testfile: &Path) -> bool {
         }
     }
 
+    fn ignore_lldb(config: &Config, line: &str) -> bool {
+        if config.mode != common::DebugInfoLldb {
+            return false;
+        }
+
+        if parse_name_directive(line, "ignore-lldb") {
+            return true;
+        }
+
+        match config.lldb_version {
+            Some(ref actual_version) => {
+                if line.contains("min-lldb-version") {
+                    let min_version = line.trim()
+                                          .split(' ')
+                                          .last()
+                                          .expect("Malformed lldb version directive");
+                    // Ignore if actual version is smaller the minimum required
+                    // version
+                    lldb_version_to_int(actual_version.as_slice()) <
+                        lldb_version_to_int(min_version.as_slice())
+                } else {
+                    false
+                }
+            }
+            None => false
+        }
+    }
+
     let val = iter_header(testfile, |ln| {
         !parse_name_directive(ln, "ignore-test") &&
         !parse_name_directive(ln, ignore_target(config).as_slice()) &&
         !parse_name_directive(ln, ignore_stage(config).as_slice()) &&
         !(config.mode == common::Pretty && parse_name_directive(ln, "ignore-pretty")) &&
         !(config.target != config.host && parse_name_directive(ln, "ignore-cross-compile")) &&
         !ignore_gdb(config, ln) &&
-        !(config.mode == common::DebugInfoLldb && parse_name_directive(ln, "ignore-lldb"))
+        !ignore_lldb(config, ln)
     });
 
     !val
@@ -330,3 +358,12 @@ pub fn gdb_version_to_int(version_string: &str) -> int {
 
     return major * 1000 + minor;
 }
+
+pub fn lldb_version_to_int(version_string: &str) -> int {
+    let error_string = format!(
+        "Encountered LLDB version string with unexpected format: {}",
+        version_string);
+    let error_string = error_string.as_slice();
+    let major: int = FromStr::from_str(version_string).expect(error_string);
+    return major;
+}
diff --git a/src/compiletest/runtest.rs b/src/compiletest/runtest.rs
@@ -626,6 +626,17 @@ fn run_debuginfo_lldb_test(config: &Config, props: &TestProps, testfile: &Path)
 
     let exe_file = make_exe_name(config, testfile);
 
+    match config.lldb_version {
+        Some(ref version) => {
+            println!("NOTE: compiletest thinks it is using LLDB version {}",
+                     version.as_slice());
+        }
+        _ => {
+            println!("NOTE: compiletest does not know which version of \
+                      LLDB it is using");
+        }
+    }
+
     // Parse debugger commands etc from test files
     let DebuggerCommands {
         commands,

diff --git a/src/etc/gdb_rust_pretty_printing.py b/src/etc/gdb_rust_pretty_printing.py
@@ -196,7 +196,7 @@ def classify_struct(type):
   if field_count == 0:
     return STRUCT_KIND_REGULAR_STRUCT
 
-  if fields[0].artificial:
+  if fields[0].name == "RUST$ENUM$DISR":
     if field_count == 1:
       return STRUCT_KIND_CSTYLE_VARIANT
     elif fields[1].name == None:

diff --git a/src/etc/lldb_rust_formatters.py b/src/etc/lldb_rust_formatters.py
@@ -117,11 +117,19 @@ def print_enum_val(val, internal_dict):
 
   assert val.GetType().GetTypeClass() == lldb.eTypeClassUnion
 
+
   if val.num_children == 1:
+    # This is either an enum with just one variant, or it is an Option-like enum
+    # where the discriminant is encoded in a non-nullable pointer field. We find
+    # out which one it is by looking at the member name of the sole union
+    # variant. If it starts with "RUST$ENCODED$ENUM$" then we have an
+    # Option-like enum.
     first_variant_name = val.GetChildAtIndex(0).GetName()
     if first_variant_name and first_variant_name.startswith("RUST$ENCODED$ENUM$"):
-      # Try to extract the
 
+      # This is an Option-like enum. The position of the discriminator field is
+      # encoded in the name which has the format:
+      #  RUST$ENCODED$ENUM$<index of discriminator field>$<name of null variant>
       last_separator_index = first_variant_name.rfind("$")
       if last_separator_index == -1:
         return "<invalid enum encoding: %s>" % first_variant_name
@@ -130,25 +138,30 @@ def print_enum_val(val, internal_dict):
       if second_last_separator_index == -1:
         return "<invalid enum encoding: %s>" % first_variant_name
 
+      # Extract index of the discriminator field
       try:
         disr_field_index = first_variant_name[second_last_separator_index + 1 :
                                               last_separator_index]
         disr_field_index = int(disr_field_index)
       except:
         return "<invalid enum encoding: %s>" % first_variant_name
 
+      # Read the discriminant
       disr_val = val.GetChildAtIndex(0).GetChildAtIndex(disr_field_index).GetValueAsUnsigned()
 
       if disr_val == 0:
+        # Null case: Print the name of the null-variant
         null_variant_name = first_variant_name[last_separator_index + 1:]
         return null_variant_name
       else:
+        # Non-null case: Interpret the data as a value of the non-null variant type
         return print_struct_val_starting_from(0, val.GetChildAtIndex(0), internal_dict)
     else:
+      # This is just a regular uni-variant enum without discriminator field
       return print_struct_val_starting_from(0, val.GetChildAtIndex(0), internal_dict)
 
-  # extract the discriminator value by
-  disr_val = val.GetChildAtIndex(0).GetChildAtIndex(0)
+  # If we are here, this is a regular enum with more than one variant
+  disr_val = val.GetChildAtIndex(0).GetChildMemberWithName("RUST$ENUM$DISR")
   disr_type = disr_val.GetType()
 
   if disr_type.GetTypeClass() != lldb.eTypeClassEnumeration:

diff --git a/src/librustc/middle/trans/debuginfo.rs b/src/librustc/middle/trans/debuginfo.rs
@@ -242,7 +242,6 @@ static UNKNOWN_FILE_METADATA: DIFile = (0 as DIFile);
 static UNKNOWN_SCOPE_METADATA: DIScope = (0 as DIScope);
 
 static FLAGS_NONE: c_uint = 0;
-static FLAGS_ARTIFICAL: c_uint = llvm::debuginfo::FlagArtificial as c_uint;
 
 //=-----------------------------------------------------------------------------
 //  Public Interface of debuginfo module
@@ -2276,11 +2275,7 @@ impl VariantMemberDescriptionFactory {
                     _ => type_metadata(cx, ty, self.span)
                 },
                 offset: ComputedMemberOffset,
-                flags: if self.discriminant_type_metadata.is_some() &&  i == 0 {
-                    FLAGS_ARTIFICAL
-                } else {
-                    FLAGS_NONE
-                }
+                flags: FLAGS_NONE
             }
         }).collect()
     }
@@ -2339,9 +2334,9 @@ fn describe_enum_variant(cx: &CrateContext,
         None => variant_info.args.iter().map(|_| "".to_string()).collect()
     };
 
-    // If this is not a univariant enum, there is also the (unnamed) discriminant field.
+    // If this is not a univariant enum, there is also the discriminant field.
     match discriminant_info {
-        RegularDiscriminant(_) => arg_names.insert(0, "".to_string()),
+        RegularDiscriminant(_) => arg_names.insert(0, "RUST$ENUM$DISR".to_string()),
         _ => { /* do nothing */ }
     };
 
@@ -2713,14 +2708,14 @@ fn vec_slice_metadata(cx: &CrateContext,
             llvm_type: *member_llvm_types.get(0),
             type_metadata: element_type_metadata,
             offset: ComputedMemberOffset,
-            flags: FLAGS_ARTIFICAL
+            flags: FLAGS_NONE
         },
         MemberDescription {
             name: "length".to_string(),
             llvm_type: *member_llvm_types.get(1),
             type_metadata: type_metadata(cx, ty::mk_uint(), span),
             offset: ComputedMemberOffset,
-            flags: FLAGS_ARTIFICAL
+            flags: FLAGS_NONE
         },
     ];
 

diff --git a/src/test/debuginfo/borrowed-enum.rs b/src/test/debuginfo/borrowed-enum.rs
@@ -8,6 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+// ignore-tidy-linelength
 // ignore-android: FIXME(#10381)
 
 // compile-flags:-g
@@ -19,10 +20,10 @@
 // gdb-command:finish
 
 // gdb-command:print *the_a_ref
-// gdb-check:$1 = {{TheA, x = 0, y = 8970181431921507452}, {TheA, 0, 2088533116, 2088533116}}
+// gdb-check:$1 = {{RUST$ENUM$DISR = TheA, x = 0, y = 8970181431921507452}, {RUST$ENUM$DISR = TheA, 0, 2088533116, 2088533116}}
 
 // gdb-command:print *the_b_ref
-// gdb-check:$2 = {{TheB, x = 0, y = 1229782938247303441}, {TheB, 0, 286331153, 286331153}}
+// gdb-check:$2 = {{RUST$ENUM$DISR = TheB, x = 0, y = 1229782938247303441}, {RUST$ENUM$DISR = TheB, 0, 286331153, 286331153}}
 
 // gdb-command:print *univariant_ref
 // gdb-check:$3 = {{4820353753753434}}

diff --git a/src/test/debuginfo/by-value-non-immediate-argument.rs b/src/test/debuginfo/by-value-non-immediate-argument.rs
@@ -8,6 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+// ignore-tidy-linelength
 // ignore-android: FIXME(#10381)
 
 // compile-flags:-g
@@ -43,7 +44,7 @@
 
 // gdb-command:finish
 // gdb-command:print x
-// gdb-check:$7 = {{Case1, x = 0, y = 8970181431921507452}, {Case1, 0, 2088533116, 2088533116}}
+// gdb-check:$7 = {{RUST$ENUM$DISR = Case1, x = 0, y = 8970181431921507452}, {RUST$ENUM$DISR = Case1, 0, 2088533116, 2088533116}}
 // gdb-command:continue
 
 

diff --git a/src/test/debuginfo/generic-struct-style-enum.rs b/src/test/debuginfo/generic-struct-style-enum.rs
@@ -18,13 +18,13 @@
 // gdb-command:finish
 
 // gdb-command:print case1
-// gdb-check:$1 = {{Case1, a = 0, b = 31868, c = 31868, d = 31868, e = 31868}, {Case1, a = 0, b = 2088533116, c = 2088533116}, {Case1, a = 0, b = 8970181431921507452}}
+// gdb-check:$1 = {{RUST$ENUM$DISR = Case1, a = 0, b = 31868, c = 31868, d = 31868, e = 31868}, {RUST$ENUM$DISR = Case1, a = 0, b = 2088533116, c = 2088533116}, {RUST$ENUM$DISR = Case1, a = 0, b = 8970181431921507452}}
 
 // gdb-command:print case2
-// gdb-check:$2 = {{Case2, a = 0, b = 4369, c = 4369, d = 4369, e = 4369}, {Case2, a = 0, b = 286331153, c = 286331153}, {Case2, a = 0, b = 1229782938247303441}}
+// gdb-check:$2 = {{RUST$ENUM$DISR = Case2, a = 0, b = 4369, c = 4369, d = 4369, e = 4369}, {RUST$ENUM$DISR = Case2, a = 0, b = 286331153, c = 286331153}, {RUST$ENUM$DISR = Case2, a = 0, b = 1229782938247303441}}
 
 // gdb-command:print case3
-// gdb-check:$3 = {{Case3, a = 0, b = 22873, c = 22873, d = 22873, e = 22873}, {Case3, a = 0, b = 1499027801, c = 1499027801}, {Case3, a = 0, b = 6438275382588823897}}
+// gdb-check:$3 = {{RUST$ENUM$DISR = Case3, a = 0, b = 22873, c = 22873, d = 22873, e = 22873}, {RUST$ENUM$DISR = Case3, a = 0, b = 1499027801, c = 1499027801}, {RUST$ENUM$DISR = Case3, a = 0, b = 6438275382588823897}}
 
 // gdb-command:print univariant
 // gdb-check:$4 = {{a = -1}}

diff --git a/src/test/debuginfo/generic-tuple-style-enum.rs b/src/test/debuginfo/generic-tuple-style-enum.rs
@@ -21,13 +21,13 @@
 // gdb-command:finish
 
 // gdb-command:print case1
-// gdb-check:$1 = {{Case1, 0, 31868, 31868, 31868, 31868}, {Case1, 0, 2088533116, 2088533116}, {Case1, 0, 8970181431921507452}}
+// gdb-check:$1 = {{RUST$ENUM$DISR = Case1, 0, 31868, 31868, 31868, 31868}, {RUST$ENUM$DISR = Case1, 0, 2088533116, 2088533116}, {RUST$ENUM$DISR = Case1, 0, 8970181431921507452}}
 
 // gdb-command:print case2
-// gdb-check:$2 = {{Case2, 0, 4369, 4369, 4369, 4369}, {Case2, 0, 286331153, 286331153}, {Case2, 0, 1229782938247303441}}
+// gdb-check:$2 = {{RUST$ENUM$DISR = Case2, 0, 4369, 4369, 4369, 4369}, {RUST$ENUM$DISR = Case2, 0, 286331153, 286331153}, {RUST$ENUM$DISR = Case2, 0, 1229782938247303441}}
 
 // gdb-command:print case3
-// gdb-check:$3 = {{Case3, 0, 22873, 22873, 22873, 22873}, {Case3, 0, 1499027801, 1499027801}, {Case3, 0, 6438275382588823897}}
+// gdb-check:$3 = {{RUST$ENUM$DISR = Case3, 0, 22873, 22873, 22873, 22873}, {RUST$ENUM$DISR = Case3, 0, 1499027801, 1499027801}, {RUST$ENUM$DISR = Case3, 0, 6438275382588823897}}
 
 // gdb-command:print univariant
 // gdb-check:$4 = {{-1}}