Agree with all your comments here.  Lgtm, but give others a chance to comment more before committing <br><div class="gmail_quote">On Tue, Feb 24, 2015 at 1:43 AM Pavel Labath <<a href="mailto:labath@google.com">labath@google.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I have added an explanation about the test to the commit message. As for where the equivalent call should go to, I very much like the idea of the separation of responsibilities, as in std::path and std::filesystem. However, this seems out of scope of this patch. I think the next best solution is to leave the bare call to equivalent in Symbols.cpp, as it is now. I don't think of it as system-dependent, as all of that should be hidden deep in llvm.<br>
<br>
I'll wait for additional comments, and commit it in the present form if there is no pushback.<br>
<br>
<br>
================<br>
Comment at: test/functionalities/unwind/<u></u>noreturn/TestNoreturnUnwind.<u></u>py:47-49<br>
@@ -46,3 +46,5 @@<br>
         for f in thread.frames:<br>
-            if f.GetFunctionName() == "abort":<br>
+            # We use endswith() to look for abort() since some C libraries mangle the symbol into<br>
+            # __GI_abort or similar.<br>
+            if f.GetFunctionName().endswith("<u></u>abort"):<br>
                 break<br>
----------------<br>
zturner wrote:<br>
> :(  Really dislike seeing assumptions about mangling schemes hardcoded into string comparisons.  Although admittedly, I don't think this is worse than before, and even worse, there's no good alternative.  Mostly just a drive by comment.  That said, this will detect any function that ends with abort.<br>
><br>
> We don't have a perfect solution at this point, but checking against an explicit list of known manglings would generate fewer false positives.  For example, the algorithm here would catch a function called "completely_unrelated_abort".  The comment says some C libraries mangle into __GI_abort "or similar".  Is the list of similar manglings long?<br>
This is not actually "mangling" as the mangling of C++ names into strings. This is just libc declaring it's function as "__GI_abort" (a completely arbitrary string), and then linking the name "abort" to it using some elf symbol reference mechanism (or something, I don't know much about these things). But basically, since this is an arbitrary string and an implementation detail of the library, we would need to go through all c libraries and check how they resolve "abort". The list probably wouldn't be long, but it would never be definitive, as these things can change at any time.<br>
<br>
As I say in the commit message it would better to teach lldb to prefer public names for a symbol, but this is an unrelated issue, which this commit merely exposed. For a test, I think checking for endswith("abort") is fine as the danger of false positives is quite minimal.<br>
<br>
<a href="http://reviews.llvm.org/D7836" target="_blank">http://reviews.llvm.org/D7836</a><br>
<br>
EMAIL PREFERENCES<br>
  <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/<u></u>settings/panel/<u></u>emailpreferences/</a><br>
<br>
<br>
</blockquote></div>