aras-p / ClangBuildAnalyzer

Clang build analysis tool using -ftime-trace
The Unlicense
970 stars 61 forks source link

Exception on less-than operator in template argument #21

Closed aaronpuchert closed 4 years ago

aaronpuchert commented 4 years ago

With code like

template<typename T, unsigned N>
void foo(typename std::enable_if<(N < sizeof(T)), T>::type argument) {}

void bar() { foo<int, 2>(3); }

the parsing in collapseName breaks down, because < and > no longer match up. Then we go into retval.append(pos, new_pos); with pos > new_pos, which causes a std::length_error to be thrown in libstdc++.

Note that while C++11 syntax requires parantheses around comparisons in template arguments (like in the code above), they don't seem to appear in the string that we have here, which looks like std::enable_if<(2u) < (sizeof(int)), int>::type.

ben-craig commented 4 years ago

It's amazing what gets put into symbols these days... Also, parsing C++ is hard.

I'll stew on this and see what I can come up with. The potential upside is that maybe I'll be able to deal with op<< and op>> as well.

ben-craig commented 4 years ago

I have a band-aid fix for this that I'm not ready to post yet, but it basically does an if (nasty case) return uncollapsedName; That isn't a real fix, as you can still get into some situations where my trivial matching code matches things that it isn't supposed to, and gives misleading results. That situation is probably preferable to where we are right now though.

aaronpuchert commented 4 years ago

I have a band-aid fix for this that I'm not ready to post yet, but it basically does an if (nasty case) return uncollapsedName;

I've done something similar for now, because effectively it shouldn't change the statistic.

diff --git a/src/Analysis.cpp b/src/Analysis.cpp
index a556b0a..f969a73 100755
--- a/src/Analysis.cpp
+++ b/src/Analysis.cpp
@@ -278,6 +278,8 @@ std::string collapseName(const std::string &elt)
         // pos is now pointing at a close angle, or it is at the end of the string
     }
     // append the footer
+    if (pos > new_pos)
+        pos = new_pos;
     retval.append(pos, new_pos);
     return retval;
 }

That isn't a real fix, as you can still get into some situations where my trivial matching code matches things that it isn't supposed to, and gives misleading results. That situation is probably preferable to where we are right now though.

I think this is a pretty hard problem, because the parentheses around the comparison expression that one would expect are no longer there. Maybe something needs to be changed in the demangling code.

aras-p commented 4 years ago

Should be fixed by #24