[cfe-dev] Absolute paths in code coverage info

Fri Jul 3 22:31:45 PDT 2020

Here's the diff, please let me know what you think!
https://reviews.llvm.org/D83154
--
Keith Smiley

On Fri, Jun 26, 2020 at 3:11 PM Max Moroz <mmoroz at google.com> wrote:

> No objections from me!
>
> On Thu, Jun 25, 2020 at 11:42 AM Vedant Kumar <vsk at apple.com> wrote:
>
>> Thanks Keith, that sounds great to me!
>>
>> vedant
>>
>> On Jun 24, 2020, at 6:11 PM, Keith Smiley <keithbsmiley at gmail.com> wrote:
>>
>> Yes. I'm happy to implement the plan as I it understand so far. Please
>> correct me if I'm wrong.
>>
>> 1. Add the new -coverage-prefix-map flag
>> 2. Make -ffile-prefix-map imply this new flag.
>> 3. No new *dir flags for now
>> 4. No changes to -fdebug-prefix-map
>>
>> --
>> Keith Smiley
>>
>>
>> On Wed, Jun 24, 2020 at 4:10 PM Max Moroz <mmoroz at chromium.org> wrote:
>>
>>> Sorry for the late reply (I was on leave). Is this still relevant?
>>>
>>> On Tue, Jun 9, 2020 at 5:50 PM Vedant Kumar <vsk at apple.com> wrote:
>>>
>>>> Any objections to starting with having -ffile-prefix-map imply
>>>> “relative paths for coverage mappings”? I think this would work for both
>>>> Petr and Keith’s use cases.
>>>>
>>>> vedant
>>>>
>>>> On Jun 5, 2020, at 11:04 AM, Vedant Kumar <vsk at apple.com> wrote:
>>>>
>>>>
>>>> On Jun 4, 2020, at 4:17 PM, Dan McGregor <danismostlikely at gmail.com>
>>>> wrote:
>>>>
>>>> I like Vendant and Petr's proposals. -ffile-prefix-map was really
>>>> intended to be a union of -fdebug-prefix-map and -fmacro-prefix-map.
>>>> If a coverage-prefix-map is added I think it makes sense to add it to
>>>> file-prefix-map.
>>>>
>>>>
>>>> Thanks Dan. This part sounds good to me. If I’ve understood the
>>>> motivation for https://reviews.llvm.org/D68733, and given Petr’s
>>>> plans, it sounds there’s interest in both the coverage-prefix-map and the
>>>> coverage-compilation-dir options. Is that a fair summary?
>>>>
>>>> Likewise for debug-compilation-dir and
>>>> coverage-compilation-dir, and any hypothetical users of
>>>> macro-compilation-dir, though I don't think the compilation directory
>>>> is exposed to the preprocessor at all..
>>>>
>>>>
>>>> Is your preference for -coverage-compilation-dir being set by
>>>> -file-prefix-map, or for a new union flag that sets a relative compilation
>>>> dir (like -ffile-compilation-dir)? I’m assuming the latter, since the
>>>> summary from https://reviews.llvm.org/D63387 states that a downside of
>>>> the -fdebug-prefix-map=old=new syntax is that it "requires putting the
>>>> absolute path to the build directory on the build command line”, which I
>>>> suppose we’d want to avoid for any *-compilation-dir flag. I’d be
>>>> interested in hearing what others think as well.
>>>>
>>>> vedant
>>>>
>>>>
>>>> On Thu, 4 Jun 2020 at 15:08, Keith Smiley <keithbsmiley at gmail.com>
>>>> wrote:
>>>>
>>>>
>>>> I don't have a ton of context on the history of all these flags, but
>>>> I'm happy to implement either of those solutions once we have consensus!
>>>> --
>>>> Keith Smiley
>>>>
>>>>
>>>> On Thu, Jun 4, 2020 at 12:05 PM Vedant Kumar <vsk at apple.com> wrote:
>>>>
>>>>
>>>>
>>>> On Jun 3, 2020, at 2:38 PM, Petr Hosek <phosek at chromium.org> wrote:
>>>>
>>>> Would there be any opposition against supporting -ffile-prefix-map in
>>>> coverage mappings in addition to -fdebug-compilation-dir? We hit this issue
>>>> recently as well, and I was thinking about implementing a similar change
>>>> for -ffile-prefix-map.
>>>>
>>>>
>>>> I think it’s a good idea.
>>>>
>>>> One potential issue is that -ffile-prefix-map isn't currently passed to
>>>> cc1, rather it implies --debug-prefix-map but I'm not sure if we want to
>>>> make change semantics of that flag to apply to coverage as well which would
>>>> affect existing users of -fdebug-prefix-map,
>>>>
>>>>
>>>> Thanks for flagging this. You’re right, changing the absolute path
>>>> behavior under -fdebug-prefix-map might break llvm-cov workflows which
>>>> aren’t using -path-equivalence. -ffile-prefix-map seems relatively new, and
>>>> also its purpose is to be a ‘union’ of other *prefix-map options, so having
>>>> this imply —coverage-prefix-map makes sense to me.
>>>>
>>>> maybe we should introduce a new cc1 flag, e.g. --coverage-prefix-map,
>>>> which would be also implied by -ffile-prefix-map.
>>>>
>>>>
>>>> Sounds good to me. But for consistency, maybe we should rethink how
>>>> -fdebug-compilation-dir <relpath> is handled. A couple options:
>>>>
>>>> - Have `-fdebug-compilation-dir <relpath>` (driver flag) imply
>>>> `—coverage-prefix-map=$(abspath <relpath>)=./` (cc1 flag).
>>>>
>>>> The absolute path is hidden from the driver invocation, so this can
>>>> still be used by a caching build system. I’m assuming we don’t embed the
>>>> cc1 flags anywhere, e.g. not in the DW_AT_APPLE_flags. This is the closest
>>>> to what https://reviews.llvm.org/D81122 is currently doing.
>>>>
>>>> - Introduce `-ffile-compilation-dir <relpath>` (driver flag), which
>>>> implies `-fdebug-compilation-dir <relpath>` (cc1 flag) and a new
>>>> `-fcoverage-compilation-dir <relpath>`
>>>>
>>>> Essentially, make -ffile-compilation-dir analogous to
>>>> -ffile-prefix-map, a union of *compilation-dir options.
>>>>
>>>> vedant
>>>>
>>>>
>>>> On Wed, Jun 3, 2020 at 11:09 AM Vedant Kumar via cfe-dev <
>>>> cfe-dev at lists.llvm.org> wrote:
>>>>
>>>>
>>>>
>>>> On Jun 2, 2020, at 5:17 PM, Keith Smiley <keithbsmiley at gmail.com>
>>>> wrote:
>>>>
>>>> FWIW after updating this patch I've verified that llvm-cov in the
>>>> source directory with no `-path-equivalence` works fine, and also using
>>>> `-path-equivalence=,$SRCROOT` works if you want to run it not from the
>>>> source root.
>>>>
>>>>
>>>> That’s great to hear. I’ve cc’d Reid and Yuke who may have more context
>>>> on this patch and any potential pitfalls with it.
>>>>
>>>> The latter might be a bit unexpected since folks may prefer
>>>> `-path-equivalence=.,$SRCROOT` which I'm sure we could implement if that
>>>> was the missing piece.
>>>>
>>>>
>>>> It might be sufficient to add a section to the llvm-cov command guide
>>>> explaining how to use -fdebug-compilation-dir and -path-equivalence to get
>>>> remote builds working.
>>>>
>>>> --
>>>> Keith Smiley
>>>>
>>>>
>>>> On Tue, Jun 2, 2020 at 2:49 PM Keith Smiley <keithbsmiley at gmail.com>
>>>> wrote:
>>>>
>>>>
>>>> Ah actually it looks like that issue was resolved, but it was reverted
>>>> a second time for:
>>>>
>>>> There seem to be bugs in llvm-cov --path-equivalence that are causing
>>>> Chromium problems. Revert this until they are understood or fixed.
>>>>
>>>>
>>>>
>>>> https://github.com/llvm/llvm-project/commit/7cd595df96d5929488063d8ff5cc3b5d800386da
>>>>
>>>> Does anyone have more context on those?
>>>> --
>>>> Keith Smiley
>>>>
>>>>
>>>> On Tue, Jun 2, 2020 at 2:27 PM Keith Smiley <keithbsmiley at gmail.com>
>>>> wrote:
>>>>
>>>>
>>>> Thanks for the context! I found the revert
>>>> https://github.com/llvm/llvm-project/commit/62808631acceaa8b78f8ab9b407eb6b943ff5f77
>>>> and it looks like it was caused by a small test issue. I'm a bit surprised
>>>> by the justification for it since I would expect relying on the specific
>>>> directory of the test to be safe, but I think I can make it work and
>>>> re-submit.
>>>> --
>>>> Keith Smiley
>>>>
>>>>
>>>> On Tue, Jun 2, 2020 at 10:44 AM Vedant Kumar <vsk at apple.com> wrote:
>>>>
>>>>
>>>> A problem that absolute paths solve in local builds is dealing with a
>>>> changing compilation directory - this can result in two different files
>>>> being referenced by the same relative path.
>>>>
>>>> There was a promising attempt to make this work with remote builds. The
>>>> idea was to have the coverage mapping logic respect a fixed compilation
>>>> directory option (https://reviews.llvm.org/D68733), i.e. the paths
>>>> embedded in the coverage mapping should be rooted at the
>>>> -fdebug-compilation-dir <path>. It looks like the patch was reverted, but
>>>> (as far as I know) there aren’t any fundamental issues with it.
>>>>
>>>> On Jun 2, 2020, at 9:57 AM, Keith Smiley via cfe-dev <
>>>> cfe-dev at lists.llvm.org> wrote:
>>>>
>>>> Hey everyone,
>>>>
>>>> Currently when generating code coverage by passing
>>>> `-fprofile-instr-generate -fcoverage-mapping` to clang, the __LLVM_COV /
>>>> __llvm_covmap section ends up containing absolute paths to the source files
>>>> being compiled. This causes issues when producing coverage info with remote
>>>> builds where the absolute paths to the source files may differ between
>>>> machines.
>>>>
>>>> llvm-cov has a `-path-equivalence` flag in order for you to remap a
>>>> single absolute path from the coverage info which definitely helps, but it
>>>> doesn't solve this entirely for the cases where you have multiple paths
>>>> that need remapping, or you're using another tool such as, Xcode's code
>>>> coverage UI, that doesn't support this kind of path remapping.
>>>>
>>>> I'm wondering if it has been discussed, or how feasible it would be,
>>>> for me to remove the necessity for absolute paths in this info.
>>>>
>>>> Thanks!
>>>> --
>>>> Keith Smiley
>>>> _______________________________________________
>>>> cfe-dev mailing list
>>>> cfe-dev at lists.llvm.org
>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> cfe-dev mailing list
>>>> cfe-dev at lists.llvm.org
>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> --
>> --
>> Keith Smiley
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20200703/0820c3b6/attachment-0001.html>