1805058 - [hazards] malformed function and type names

Status: ASSIGNED

(Core :: JavaScript: GC, task, P1)

Description

[Steve Fink [:sfink] [:s:]]

The hazard analysis generates C++ function names in the form "<mangled>$<readable>": it appends together the C++ mangled form of the name, followed by an expanded form of the name that sixgill generates (and occasionally contains some extra info to distinguish it from other functions), and separates them with '$'.

Later parts of the analysis pipeline split the two apart on the '$' sign.

C functions (and a few other cases) do not have a separate mangled version, so they produce a name without '$'. Various things canonicalize them by turning foo into foo$foo just to treat things the same. That canonicalization checks for the existence of '$' and duplicates the name based on whether it was found.

The problem arises when an unmangled name contains '$'. This can happen when a type name is decorated with a context. For example, if you declare a type within a function, then that type must be local to the function. Ordinarily it doesn't really matter since nothing of interest to the analysis uses that type, but if for example a template is specialized on that type, it can show up.

The shortest example I see in the JS tree is:

_ZZ8TestMovevEN5ThingC1EOS_$TestSPSCQueue.cpp:void TestMove(_Z8TestMovev$void TestMove()::Thing*)::Thing::Thing(TestMove()::Thing&&)

The mangled name is _ZZ8TestMovevEN5ThingC1EOS_. The readable name is TestSPSCQueue.cpp:void TestMove(_Z8TestMovev$void TestMove()::Thing*)::Thing::Thing(TestMove()::Thing&&). The TestSPSCQueue.cpp: prefix is because it's a static function. The probalem comes in the first parameter, which uses a local type. It is given the type _Z8TestMovev$void TestMove()::Thing*)::Thing::Thing(TestMove()::Thing&&, which is qualified with the mangled name of the function TestMove().

A better name for the type would be _Z8TestMovev::Thing*)::Thing::Thing(TestMove()::Thing&& -- namely, qualify the type with just the mangled name.

The actual example I ran into was much, much worse.

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1805058 - Use mangled names only when adding function context to a name

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:sfink, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.