Page MenuHomePhabricator

[DebugInfo] Don't use realpath when looking up debug binary locations.
ClosedPublic

Authored by rupprecht on Feb 1 2019, 11:58 AM.

Details

Summary

Using realpath makes assumptions about build systems that do not always hold true. The debug binary referred to from the .gnu_debuglink should exist in the same directory (or in a .debug directory, etc.), but the files may only exist as symlinks to a differently named files elsewhere, and using realpath causes that lookup to fail.

This was added in r189250, and this is basically a revert + regression test case.

Diff Detail

Repository
rL LLVM

Event Timeline

rupprecht created this revision.Feb 1 2019, 11:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 1 2019, 11:58 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
dblaikie accepted this revision.Feb 1 2019, 12:10 PM

In the absence of further info about the original commit/intended functionality - sure. If it breaks, we'll at least know why it was this way in the first place. :)

llvm/test/DebugInfo/symbolize-gnu-debuglink-no-realpath.test
5–6 ↗(On Diff #184807)

Perhaps name the directories "real" and "sym" so they're more self explanatory?

This revision is now accepted and ready to land.Feb 1 2019, 12:10 PM
rupprecht updated this revision to Diff 184824.Feb 1 2019, 12:53 PM
rupprecht marked an inline comment as done.
  • Use real/syms instead of objs/runfiles
This revision was automatically updated to reflect the committed changes.

Ugh, too bad the original commit didn't provide any rationale, but here's my guess based on reading the source code:

Note that llvm-symbolizer (or another tool that uses this library) can be invoked with the relative path (e.g. cd /tmp/dir; llvm-symbolizer foo).

Then, if you can't find the .debuglink file at following locations: foo.debuglink (absolute path /tmp/dir/foo.debuglink), or .debug/foo.debuglink (absolute path /tmp/dir/.debuglink/foo.debuglink),
which can often happen if you're symbolizing the code from standard library, you have to look for this file in /usr/lib/... or /usr/libdata/... for NetBSD. That's when you need the realpath for the file in question,
so that you try to open:

/usr/lib/debug/tmp/dir/foo.debuglink

instead of

/usr/lib/debug/foo.debuglink

Ugh, too bad the original commit didn't provide any rationale, but here's my guess based on reading the source code:

Note that llvm-symbolizer (or another tool that uses this library) can be invoked with the relative path (e.g. cd /tmp/dir; llvm-symbolizer foo).

Then, if you can't find the .debuglink file at following locations: foo.debuglink (absolute path /tmp/dir/foo.debuglink), or .debug/foo.debuglink (absolute path /tmp/dir/.debuglink/foo.debuglink),
which can often happen if you're symbolizing the code from standard library, you have to look for this file in /usr/lib/... or /usr/libdata/... for NetBSD. That's when you need the realpath for the file in question,
so that you try to open:

/usr/lib/debug/tmp/dir/foo.debuglink

instead of

/usr/lib/debug/foo.debuglink

I think the absolute path (not realpath) would suffice, then.

Also, using the full path can be delayed to when checking in /usr/lib/debug.

I'll restore this patch with those modifications. No idea how I'll create a test for it though -- writing to /usr/lib/debug probably doesn't work on buildbots.

Ugh, too bad the original commit didn't provide any rationale, but here's my guess based on reading the source code:

Note that llvm-symbolizer (or another tool that uses this library) can be invoked with the relative path (e.g. cd /tmp/dir; llvm-symbolizer foo).

Then, if you can't find the .debuglink file at following locations: foo.debuglink (absolute path /tmp/dir/foo.debuglink), or .debug/foo.debuglink (absolute path /tmp/dir/.debuglink/foo.debuglink),
which can often happen if you're symbolizing the code from standard library, you have to look for this file in /usr/lib/... or /usr/libdata/... for NetBSD. That's when you need the realpath for the file in question,
so that you try to open:

/usr/lib/debug/tmp/dir/foo.debuglink

instead of

/usr/lib/debug/foo.debuglink

I think the absolute path (not realpath) would suffice, then.

Also, using the full path can be delayed to when checking in /usr/lib/debug.

I'll restore this patch with those modifications. No idea how I'll create a test for it though -- writing to /usr/lib/debug probably doesn't work on buildbots.

Mailed D57916