diff --git a/lib/error_highlight/core_ext.rb b/lib/error_highlight/core_ext.rb index c3354f46cdf5dc..c82857f35c93a7 100644 --- a/lib/error_highlight/core_ext.rb +++ b/lib/error_highlight/core_ext.rb @@ -5,7 +5,7 @@ module CoreExt private def generate_snippet if ArgumentError === self && message =~ /\A(?:wrong number of arguments|missing keyword[s]?|unknown keyword[s]?|no keywords accepted)\b/ locs = self.backtrace_locations - return "" if locs.size < 2 + return "" if locs.nil? || locs.size < 2 callee_loc, caller_loc = locs callee_spot = ErrorHighlight.spot(self, backtrace_location: callee_loc, point_type: :name) caller_spot = ErrorHighlight.spot(self, backtrace_location: caller_loc, point_type: :name) diff --git a/test/error_highlight/test_error_highlight.rb b/test/error_highlight/test_error_highlight.rb index 5de70f59c0cc76..76db1035e881e7 100644 --- a/test/error_highlight/test_error_highlight.rb +++ b/test/error_highlight/test_error_highlight.rb @@ -1815,6 +1815,30 @@ def test_detailed_message_does_not_raise_when_argument_error_is_rewrapped end end + def test_detailed_message_does_not_raise_when_backtrace_locations_is_nil + # This reproduces a real-world crash: when an exception crosses a process + # boundary via Marshal, `backtrace` survives as strings but + # `backtrace_locations` becomes nil. A keyword ArgumentError then crashed + # CoreExt#generate_snippet with `undefined method 'size' for nil`. + begin + def_with_required_keyword + rescue ArgumentError => original + exc = Marshal.load(Marshal.dump(original)) + end + + assert_nil exc.backtrace_locations + + msg = nil + assert_nothing_raised do + msg = exc.detailed_message(highlight: false) + end + assert_match("missing keyword", msg) + + assert_nothing_raised do + exc.full_message(highlight: false) + end + end + private def find_node_by_id(node, node_id) diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 07bad0df572326..4d3aea54baca7c 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -57,6 +57,12 @@ impl From for usize { } } +impl From for InsnId { + fn from(val: usize) -> Self { + InsnId(val) + } +} + impl std::fmt::Display for InsnId { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { write!(f, "v{}", self.0) @@ -2404,28 +2410,28 @@ impl<'a> FunctionPrinter<'a> { /// in missing optimizations. #[derive(Debug)] struct UnionFind> { - forwarded: Vec>, + forwarded: Vec, } -impl + PartialEq> UnionFind { +impl + PartialEq + std::convert::From> UnionFind { fn new() -> UnionFind { UnionFind { forwarded: vec![] } } /// Private. Return the internal representation of the forwarding pointer for a given element. - fn at(&self, idx: T) -> Option { - self.forwarded.get(idx.into()).copied().flatten() + fn at(&self, idx: T) -> T { + self.forwarded.get(idx.into()).unwrap_or(&idx).to_owned() } /// Private. Set the internal representation of the forwarding pointer for the given element /// `idx`. Extend the internal vector if necessary. fn set(&mut self, idx: T, value: T) { if idx.into() >= self.forwarded.len() { - self.forwarded.resize(idx.into()+1, None); - } - if idx != value { - self.forwarded[idx.into()] = Some(value); + for i in self.forwarded.len()..=idx.into() { + self.forwarded.push(i.into()); + } } + self.forwarded[idx.into()] = value; } /// Find the set representative for `insn`. Perform path compression at the same time to speed @@ -2452,21 +2458,18 @@ impl + PartialEq> UnionFind { fn find_const(&self, insn: T) -> T { let mut result = insn; loop { - match self.at(result) { - None => return result, - Some(insn) => { - assert!(result != insn, "cycle detected"); - result = insn; - } - } + let found = self.at(result); + if found == result { return found; } + result = found; } } /// Union the two sets containing `insn` and `target` such that every element in `insn`s set is /// now part of `target`'s. Neither argument must be the representative in its set. pub fn make_equal_to(&mut self, insn: T, target: T) { - let found = self.find(insn); - self.set(found, target); + let insn = self.find(insn); + let target = self.find(target); + self.set(insn, target); } } @@ -9661,6 +9664,12 @@ mod union_find_tests { assert_eq!(uf.find(3usize), 4); } + #[test] + fn test_find_with_unknown_element_returns_self() { + let mut uf = UnionFind::new(); + assert_eq!(uf.find(10usize), 10); + } + #[test] fn test_find_halts_with_identity_make_equal_to() { let mut uf = UnionFind::::new(); @@ -9682,9 +9691,20 @@ mod union_find_tests { let mut uf = UnionFind::new(); uf.make_equal_to(3, 4); uf.make_equal_to(4, 5); - assert_eq!(uf.at(3usize), Some(4)); + assert_eq!(uf.at(3usize), 4); assert_eq!(uf.find(3usize), 5); - assert_eq!(uf.at(3usize), Some(5)); + assert_eq!(uf.at(3usize), 5); + } + + #[test] + fn test_make_equal_to_does_not_create_cycles() { + let mut uf = UnionFind::new(); + uf.make_equal_to(3, 4); + uf.make_equal_to(4, 5); + uf.make_equal_to(5, 3); + assert_eq!(uf.find(3usize), 5); + assert_eq!(uf.find(4usize), 5); + assert_eq!(uf.find(5usize), 5); } }