From d84a0a89bc42bb3b8fbc8865784d8d4ab518b838 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 3 Mar 2020 15:21:24 +1100 Subject: [PATCH 1/2] Fix the tests involving file errors. They currently don't test file errors because the regex doesn't match! --- src/tests.rs | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/src/tests.rs b/src/tests.rs index c0699d8..d420ece 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -280,11 +280,6 @@ fn test_regex() { unchanged("#00: ???[tests/example-linux +1130]"); // Missing the '0x`. unchanged("#00: ???(tests/example-linux +0x1130)"); // Wrong parentheses. - // An error message is also printed to stderr for file errors, but we don't - // test for that. - unchanged("#00: ??? (tests/no-such-file)"); // No such file. - unchanged("#00: ??? (src/main.rs)"); // File exists, but has wrong format. - // Test various different changed line forms that do match the regex. let mut changed = |line1: &str, line2_expected| { let line2_actual = fixer.fix(line1.to_string()); @@ -303,3 +298,31 @@ fn test_regex() { "#01: main (/home/njn/moz/fix-stacks/tests/example.c:24)", ); } + +#[test] +fn test_files() { + let mut fixer = Fixer::new(JsonEscaping::Yes); + + // Test various different file errors. An error message is also printed to + // stderr for each one, but we don't test for that. + let mut file_error = |line1: &str, line2_expected| { + let line2_actual = fixer.fix(line1.to_string()); + assert_eq!(line2_expected, line2_actual); + }; + // No such file. + file_error( + "#00: ???[tests/no-such-file +0x0]", + "#00: ??? (tests/no-such-file +0x0)", + ); + // No such file, with backslashes (which tests JSON escaping). + // XXX: the output is currently incorrect, and will be fixed shortly. + file_error( + "#00: ???[tests\\no-such-dir\\\\no-such-file +0x0]", + "#00: ??? (tests\\\\no-such-dir\\\\\\\\no-such-file +0x0)", + ); + // File exists, but has the wrong format. + file_error( + "#00: ???[src/main.rs +0x0]", + "#00: ??? (src/main.rs +0x0)", + ); +} From 7d887fb1e06525dfdb1805b2b424ed1003eeae8a Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 3 Mar 2020 14:44:22 +1100 Subject: [PATCH 2/2] Fix JSON handling. When `-j` is specified, any new strings produced by `fix-stacks` need JSON escaping. However, any strings that come from the input and are reused will already be escaped, and so should not be re-escaped. This commit removes some erroneous escaping of input strings. As a result, some Windows paths no longer get their backslashes escaped twice. --- src/main.rs | 67 ++++++++++++++++++++++++++++++++-------------------- src/tests.rs | 3 +-- 2 files changed, 42 insertions(+), 28 deletions(-) diff --git a/src/main.rs b/src/main.rs index 3f0e849..4ae946f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -542,16 +542,16 @@ impl Fixer { }; let before = &captures[1]; - let func_name = &captures[2]; - let file_name = &captures[3]; + let in_func_name = &captures[2]; + let in_file_name = &captures[3]; let address = u64::from_str_radix(&captures[4], 16).unwrap(); let after = &captures[5]; // If we haven't seen this file yet, parse and record its contents, for // this lookup and any future lookups. - let file_info = match self.file_infos.entry(file_name.to_string()) { + let file_info = match self.file_infos.entry(in_file_name.to_string()) { Entry::Occupied(o) => o.into_mut(), - Entry::Vacant(v) => match Fixer::build_file_info(file_name) { + Entry::Vacant(v) => match Fixer::build_file_info(in_file_name) { Ok(file_info) => v.insert(file_info), Err(op) => { // Print an error message and then set up an empty @@ -561,42 +561,57 @@ impl Fixer { // first occurrence. // - The line will still receive some transformation, using // the "no symbols or debug info" case below. - eprintln!("fix-stacks error: failed to {} `{}`", op, file_name); + eprintln!("fix-stacks error: failed to {} `{}`", op, in_file_name); v.insert(FileInfo::default()) } }, }; - let mut func_name_and_locn = if let Some(func_info) = file_info.func_info(address) { + // If JSON escaping is enabled, we need to escape any new strings we + // produce. However, strings that came in from the text (i.e. + // `in_func_name` and `in_file_name`), will already be escaped, so if + // they become part of the output they shouldn't be escaped. + if let Some(func_info) = file_info.func_info(address) { + let raw_func_name = func_info.demangled_name(); + let out_func_name = if let JsonEscaping::Yes = self.json_escaping { + Fixer::json_escape(&raw_func_name) + } else { + raw_func_name + }; + if let Some(line_info) = func_info.line_info(address) { - // We have the filename and line number from the debug info. + // We have the function name, filename, and line number from + // the debug info. + let raw_file_name = file_info.interner.get(line_info.path); + let out_file_name = if let JsonEscaping::Yes = self.json_escaping { + Fixer::json_escape(&raw_file_name) + } else { + raw_file_name.to_string() + }; + format!( - "{} ({}:{})", - func_info.demangled_name(), - file_info.interner.get(line_info.path), - line_info.line + "{}{} ({}:{}){}", + before, out_func_name, out_file_name, line_info.line, after ) } else { - // We have the filename from the debug info, but no line number. + // We have the function name from the debug info, but no file + // name or line number. Use the file name and address from the + // original input. format!( - "{} ({} +0x{:x})", - func_info.demangled_name(), - file_name, - address + "{}{} ({} +0x{:x}){}", + before, out_func_name, in_file_name, address, after ) } } else { - // We have nothing from the symbols or debug info. Use the file name - // from original input, which is probably "???". The end result is the - // same as the original line, but with the address removed and slightly - // different formatting. - format!("{} ({} +0x{:x})", func_name, file_name, address) - }; - - if let JsonEscaping::Yes = self.json_escaping { - func_name_and_locn = Fixer::json_escape(&func_name_and_locn); + // We have nothing from the debug info. Use the function name, file + // name, and address from the original input. The end result is the + // same as the original line, but with slightly different + // formatting. + format!( + "{}{} ({} +0x{:x}{})", + before, in_func_name, in_file_name, address, after + ) } - format!("{}{}{}", before, func_name_and_locn, after) } } diff --git a/src/tests.rs b/src/tests.rs index d420ece..12e1b54 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -315,10 +315,9 @@ fn test_files() { "#00: ??? (tests/no-such-file +0x0)", ); // No such file, with backslashes (which tests JSON escaping). - // XXX: the output is currently incorrect, and will be fixed shortly. file_error( "#00: ???[tests\\no-such-dir\\\\no-such-file +0x0]", - "#00: ??? (tests\\\\no-such-dir\\\\\\\\no-such-file +0x0)", + "#00: ??? (tests\\no-such-dir\\\\no-such-file +0x0)", ); // File exists, but has the wrong format. file_error(