[Lldb-commits] [PATCH] Skip symlinks to the original file when searching for debug info

[Zachary Turner]

Agree with all your comments here. Lgtm, but give others a chance to
comment more before committing
On Tue, Feb 24, 2015 at 1:43 AM Pavel Labath <labath at google.com> wrote:

> 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.
>
> I'll wait for additional comments, and commit it in the present form if
> there is no pushback.
>
>
> ================
> Comment at: test/functionalities/unwind/noreturn/TestNoreturnUnwind.
> py:47-49
> @@ -46,3 +46,5 @@
>          for f in thread.frames:
> -            if f.GetFunctionName() == "abort":
> +            # We use endswith() to look for abort() since some C
> libraries mangle the symbol into
> +            # __GI_abort or similar.
> +            if f.GetFunctionName().endswith("abort"):
>                  break
> ----------------
> zturner wrote:
> > :(  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.
> >
> > 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?
> 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.
>
> 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.
>
> http://reviews.llvm.org/D7836
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20150224/642cadeb/attachment.html>