Skip to content

Commit

Permalink
Remove handling of outdated 'sr' expressions that interfere with mode…
Browse files Browse the repository at this point in the history 
…rn encoding.

How we got here: In late 2017 (PR #102) I wrote f6bcf4c. The example symbol for that change was `_ZN4base8internal14CheckedSubImplIlEENSt9enable_ifIXsrSt14numeric_limitsIT_E10is_integerEbE4typeES4_S4_PS4_`. I now believe that symbol is invalid and that its production is either a GNU bug or a nonstandard extension.

The latest cxxabi defines `expression` at http://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangle.expression. It does not have any special casing of `sr` outside of the `unresolved-name` production. These changes were made in 2010 (http://sourcery.mentor.com/archives/cxx-abi-dev/msg02279.html) but GCC and binutils don't seem to have caught up.

If we remove our handling of GCC's outdated behavior, then the symbol `_Z6IsNullIiEN1EIXsr1FIT_EE1nEE4typeES1_` from our tests now demangles to `E<F<int>::n>::type IsNull<int>(int)` which is both more sensible and matches llvm-cxxfilt.

The symbol `_ZN4base8internal14CheckedSubImplIlEENSt9enable_ifIXsrSt14numeric_limitsIT_E10is_integerEbE4typeES4_S4_PS4_` is no longer valid. The expression (starting with the 'X' after enable_if) needs to be read as

`sr <unresolved-qualifier-level>+ E <base-unresolved-name>`

Unlike the `type` production which allows (among others a well-known) substitution, `unresolved-qualifier-level` is `simple-id`, so `srSt` must become `sr3std`. The `unresolved-qualifier-level` list must be terminated with an 'E', which must be inserted before '10is_integer'. This also drops std::numeric_limits from the substitution table (it was previously at slot 3), so we need to adjust the substitution indexes at the end. That produces a corrected symbol: `_ZN4base8internal14CheckedSubImplIlEENSt9enable_ifIXsr3std14numeric_limitsIT_EE10is_integerEbE4typeES3_S3_PS3_`, which both llvm-cxxfilt and us agree is demangled to what we were previously demangling it to.

roc added `_ZN7mozilla6detail12ListenerImplINS_14AbstractThreadEZNS_20MediaEventSourceImplILNS_14ListenerPolicyE0EJNS_13TimedMetadataEEE15ConnectInternalIS2_NS_12MediaDecoderEMS8_FvOS5_EEENS_8EnableIfIXsr8TakeArgsIT1_EE5valueENS_18MediaEventListenerEE4TypeEPT_PT0_SD_EUlS9_E_JS5_EE17ApplyWithArgsImplISL_EENSC_IXsr8TakeArgsISH_EE5valueEvE4TypeERKSH_S9_` as a symbol that loops infinitely, but it now demangles. It also demangled previously in llvm-cxxfilt. Our demanglings don't match, but I don't propose to solve that today.

Finally, we come to the symbol that actually motivated this: `_ZSt3powIdiEN9__gnu_cxx11__promote_2IT_T0_NS0_9__promoteIS2_Xsr3std12__is_integerIS2_EE7__valueEE6__typeENS4_IS3_Xsr3std12__is_integerIS3_EE7__valueEE6__typeEE6__typeES2_S3_`. This is pretty similar to the first symbol, after fixing it up. Here, after the 'X' `expression` production, there is another template argument. The second 'E' (which is required to terminate the `unresolved-qualifier-level` list) gets misinterpreted by the existing code as terminating the containing template production without the next template argument. Parsing then goes off the rails from there. Thus, the correct standards-compliant behavior is incompatible with the outdated output of gcc.
  • Loading branch information
@khuey
khuey committed Apr 5, 2019
1 parent 7f65e94 commit 97c4c47
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 68 deletions.
125 changes: 60 additions & 65 deletions src/ast.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5045,8 +5045,6 @@ where
/// ::= sZ <function-param> # sizeof...(parameter), size of a function parameter pack
/// ::= sP <template-arg>* E # sizeof...(T), size of a captured template parameter pack from an alias template
/// ::= sp <expression> # expression..., pack expansion
/// ::= sr <type> <unqualified-name>
/// ::= sr <type> <unqualified-name> <template-args>
/// ::= tw <expression> # throw expression
/// ::= tr # throw with no operand (rethrow)
/// ::= <unresolved-name> # f(p), N::f(p), ::f(p),
Expand Down Expand Up @@ -5176,12 +5174,6 @@ pub enum Expression {
/// `expression...`, pack expansion.
PackExpansion(Box<Expression>),

/// `type` `unqualified_name` expression.
TypeUnqualifiedName(TypeHandle, UnqualifiedName),

/// `type` `unqualified_name` `template-args` expression.
TypeUnqualifiedNameTemplateArgs(TypeHandle, UnqualifiedName, TemplateArgs),

/// `throw expression`
Throw(Box<Expression>),

Expand Down Expand Up @@ -5364,24 +5356,6 @@ impl Parse for Expression {
return Ok((expr, tail));
}
}
b"sr" => {
// If we can't parse <type> <unqualified-name> here,
// keep going because we might need to parse as <unresolved-name>
if let Ok((ty, tail)) = TypeHandle::parse(ctx, subs, tail) {
if let Ok((name, tail)) = UnqualifiedName::parse(ctx, subs, tail) {
let (expr, tail) = if tail.peek() == Some(b'I') {
let (template_args, tail) = TemplateArgs::parse(ctx, subs, tail)?;
(
Expression::TypeUnqualifiedNameTemplateArgs(ty, name, template_args),
tail,
)
} else {
(Expression::TypeUnqualifiedName(ty, name), tail)
};
return Ok((expr, tail));
}
}
}
_ => {}
}
}
Expand Down Expand Up @@ -5836,17 +5810,6 @@ where
write!(ctx, "...")?;
Ok(())
}
Expression::TypeUnqualifiedName(ref ty, ref name) => {
ty.demangle(ctx, scope)?;
write!(ctx, "::")?;
name.demangle(ctx, scope)
}
Expression::TypeUnqualifiedNameTemplateArgs(ref ty, ref name, ref template_args) => {
ty.demangle(ctx, scope)?;
write!(ctx, "::")?;
name.demangle(ctx, scope)?;
template_args.demangle(ctx, scope)
}
Expression::Throw(ref expr) => {
write!(ctx, "throw ")?;
expr.demangle(ctx, scope)
Expand Down Expand Up @@ -8790,30 +8753,55 @@ mod tests {
}
b"XsrS_1QE..." => {
TemplateArg::Expression(
Expression::TypeUnqualifiedName(
TypeHandle::BackReference(0),
UnqualifiedName::Source(
SourceName(Identifier {
start: 6,
end: 7,
})))),
Expression::UnresolvedName(
UnresolvedName::Nested1(
UnresolvedTypeHandle::BackReference(0),
vec![],
BaseUnresolvedName::Name(
SimpleId(
SourceName(Identifier {
start: 6,
end: 7
}),
None
)
)
)
)
),
b"...",
[]
}
b"XsrS_1QIlEE..." => {
TemplateArg::Expression(
Expression::TypeUnqualifiedNameTemplateArgs(
TypeHandle::BackReference(0),
UnqualifiedName::Source(
SourceName(Identifier {
start: 6,
end: 7,
})),
TemplateArgs(vec![
TemplateArg::Type(
TypeHandle::Builtin(
BuiltinType::Standard(StandardBuiltinType::Long))),
]))),
Expression::UnresolvedName(
UnresolvedName::Nested1(
UnresolvedTypeHandle::BackReference(0),
vec![],
BaseUnresolvedName::Name(
SimpleId(
SourceName(Identifier {
start: 6,
end: 7
}),
Some(
TemplateArgs(
vec![
TemplateArg::Type(
TypeHandle::Builtin(
BuiltinType::Standard(
StandardBuiltinType::Long
)
)
)
]
)
)
)
)
)
)
),
b"...",
[]
}
Expand Down Expand Up @@ -9404,18 +9392,25 @@ mod tests {
)
)),
vec![Expression::Unary(OperatorName::Simple(SimpleOperatorName::AddressOf),
Box::new(Expression::TypeUnqualifiedName(
TypeHandle::BackReference(1),
UnqualifiedName::Source(
SourceName(Identifier {
start: 21,
end: 26,
})))))]

Box::new(Expression::UnresolvedName(
UnresolvedName::Nested1(
UnresolvedTypeHandle::BackReference(1),
vec![],
BaseUnresolvedName::Name(
SimpleId(
SourceName(Identifier {
start: 21,
end: 26
}
),
None
)
)
))))]
),
b"...",
[
Substitutable::Type(Type::TemplateParam(TemplateParam(0)))
Substitutable::UnresolvedType(UnresolvedType::Template(TemplateParam(0), None))
]
}
}
Expand Down
11 changes: 8 additions & 3 deletions tests/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,12 @@ does_not_parse!(close);

// Test some potential stack-overflows due to cyclic template parameter references
does_not_demangle!(_Z1fIT_EvT_);
does_not_demangle!(_ZN7mozilla6detail12ListenerImplINS_14AbstractThreadEZNS_20MediaEventSourceImplILNS_14ListenerPolicyE0EJNS_13TimedMetadataEEE15ConnectInternalIS2_NS_12MediaDecoderEMS8_FvOS5_EEENS_8EnableIfIXsr8TakeArgsIT1_EE5valueENS_18MediaEventListenerEE4TypeEPT_PT0_SD_EUlS9_E_JS5_EE17ApplyWithArgsImplISL_EENSC_IXsr8TakeArgsISH_EE5valueEvE4TypeERKSH_S9_);

demangles!(
_ZN7mozilla6detail12ListenerImplINS_14AbstractThreadEZNS_20MediaEventSourceImplILNS_14ListenerPolicyE0EJNS_13TimedMetadataEEE15ConnectInternalIS2_NS_12MediaDecoderEMS8_FvOS5_EEENS_8EnableIfIXsr8TakeArgsIT1_EE5valueENS_18MediaEventListenerEE4TypeEPT_PT0_SD_EUlS9_E_JS5_EE17ApplyWithArgsImplISL_EENSC_IXsr8TakeArgsISH_EE5valueEvE4TypeERKSH_S9_,
// This does not match llvm-cxxfilt
"mozilla::EnableIf<TakeArgs<mozilla::EnableIf<TakeArgs<void (mozilla::MediaDecoder::*)(mozilla::TimedMetadata&&)>::value, mozilla::MediaEventListener>::Type mozilla::MediaEventSourceImpl<(mozilla::ListenerPolicy)0, mozilla::TimedMetadata>::ConnectInternal<mozilla::AbstractThread, mozilla::MediaDecoder, void (mozilla::MediaDecoder::*)(mozilla::TimedMetadata&&)>(mozilla::AbstractThread*, mozilla::MediaDecoder*, void (mozilla::MediaDecoder::*)(mozilla::TimedMetadata&&))::{lambda(mozilla::TimedMetadata&&)#1}>::value, void>::Type mozilla::detail::ListenerImpl<mozilla::AbstractThread, mozilla::EnableIf<TakeArgs<void (mozilla::MediaDecoder::*)(mozilla::TimedMetadata&&)>::value, mozilla::MediaEventListener>::Type mozilla::MediaEventSourceImpl<(mozilla::ListenerPolicy)0, mozilla::TimedMetadata>::ConnectInternal<mozilla::AbstractThread, mozilla::MediaDecoder, void (mozilla::MediaDecoder::*)(mozilla::TimedMetadata&&)>(mozilla::AbstractThread*, mozilla::MediaDecoder*, void (mozilla::MediaDecoder::*)(mozilla::TimedMetadata&&))::{lambda(mozilla::TimedMetadata&&)#1}, mozilla::TimedMetadata>::ApplyWithArgsImpl<mozilla::EnableIf<TakeArgs<void (mozilla::MediaDecoder::*)(mozilla::TimedMetadata&&)>::value, mozilla::MediaEventListener>::Type mozilla::MediaEventSourceImpl<(mozilla::ListenerPolicy)0, mozilla::TimedMetadata>::ConnectInternal<mozilla::AbstractThread, mozilla::MediaDecoder, void (mozilla::MediaDecoder::*)(mozilla::TimedMetadata&&)>(mozilla::AbstractThread*, mozilla::MediaDecoder*, void (mozilla::MediaDecoder::*)(mozilla::TimedMetadata&&))::{lambda(mozilla::TimedMetadata&&)#1}>(mozilla::EnableIf<TakeArgs<void (mozilla::MediaDecoder::*)(mozilla::TimedMetadata&&)>::value, mozilla::MediaEventListener>::Type mozilla::MediaEventSourceImpl<(mozilla::ListenerPolicy)0, mozilla::TimedMetadata>::ConnectInternal<mozilla::AbstractThread, mozilla::MediaDecoder, void (mozilla::MediaDecoder::*)(mozilla::TimedMetadata&&)>(mozilla::AbstractThread*, mozilla::MediaDecoder*, void (mozilla::MediaDecoder::*)(mozilla::TimedMetadata&&))::{lambda(mozilla::TimedMetadata&&)#1} const&, mozilla::TimedMetadata&&)"
);

demangles!(
_Z20instantiate_with_intI3FooET_IiEv,
Expand Down Expand Up @@ -171,7 +176,7 @@ demangles!(
demangles!(_ZNK1fB5cxx11Ev, "f[abi:cxx11]() const");

demangles!(
_ZN4base8internal14CheckedSubImplIlEENSt9enable_ifIXsrSt14numeric_limitsIT_E10is_integerEbE4typeES4_S4_PS4_,
_ZN4base8internal14CheckedSubImplIlEENSt9enable_ifIXsr3std14numeric_limitsIT_EE10is_integerEbE4typeES3_S3_PS3_,
"std::enable_if<std::numeric_limits<long>::is_integer, bool>::type base::internal::CheckedSubImpl<long>(long, long, long*)"
);

Expand Down Expand Up @@ -401,7 +406,7 @@ demangles!(

demangles!(
_Z6IsNullIiEN1EIXsr1FIT_EE1nEE4typeES1_,
"E<F<int>::n>::type IsNull<int>(F)"
"E<F<int>::n>::type IsNull<int>(int)"
);
demangles!(
_Z6IsNullIiEN1EIXgssr1FIT_EE1nEE4typeEv,
Expand Down

0 comments on commit 97c4c47

Please sign in to comment.