Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide means for opting out of having "_virtual_includes" mentioned in C/C++ binaries #11874

Open
nacl opened this issue Jul 30, 2020 · 12 comments
Labels
not stale Issues or PRs that are inactive but not considered stale P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules type: feature request

Comments

@nacl
Copy link

nacl commented Jul 30, 2020

Description of the problem / feature request:

Whenever include_prefix or strip_include_prefix are provided to cc_library, bazel creates a hierarchy in the execution root that starts with something like this:

path/to/$PACKAGE/_virtual_includes/$TARGET

Which are passed in to the C/C++ compiler via -I. This results in the debuginfo values in the compiled files specifying that said headers can be found entirely under execution root, which is only valid if Bazel has been run before. Further, the _virtual_includes path hides exactly where the files in question are located, and requires additional steps to be performed to determine the location the actual header file for editing purposes.

We have a workflow in which engineers do debugging of coredumps provided by customers and they must (at least initially) be able to operate in environments where a build was not necessarily done previously. It is understood and expected that debugging generated code or external repositories will require interaction with the build system.

Our current thoughts on this is that it should be possible to create a toolchain feature that can create this mapping using compiler flags like gcc's -fdebug-prefix-map or -ffile-prefix-map.

See also our post on this on bazel-discuss: https://groups.google.com/forum/#!topic/bazel-discuss/prVSDuifmHY

Feature requests: what underlying problem are you trying to solve with this feature?

Debuginfos generated by gcc in Bazel may specify headers relative to a _virtual_includes path instead of where they actually may be found in the source repository. An option should be available to allow users to opt-out of this feature.

This problem, in one form or another, impacts things like emitting coverage instrumentation (bazel build --collect-code-coverage ...) and potentially compilation databases.

What operating system are you running Bazel on?

CentOS Linux release 7.7.1908 (Core)

What's the output of bazel info release?

release 3.3.0-vmware

If bazel info release returns "development version" or "(@non-git)", tell us how you built Bazel.

N/A

What's the output of git remote get-url origin ; git rev-parse master ; git rev-parse HEAD ?

N/A

Have you found anything relevant by searching the web?

Similar issues/commits found:

  • Commit 3f46dd0 adds a feature for the macOS CROSSTOOL that uses -fdebug-prefix-map. It, however, is only really useful for macOS builds.

  • Absolute paths are embedded in produced binaries #1000 is similar to this issue, but is mostly related to macOS and the embedding of absolute paths within the binaries. This ticket is more interested in remapping paths from within the macOS sandbox; the Linux sandboxes all seem to use paths relative to the WORKSPACE root.

  • Output the actual path of virtual include headers in the coverage report #6254 fixes a similar problem was encountered by coverage builds (as run via bazel coverage). Unfortunately, while we could use the dataset that is assembled there, it, unfortunately, is far too large to be used on compiler command lines, nearing 6000+ entries in some cases.

Any other information, logs, or outputs that you want to share?

We have written a internal change to bazel that creates a CcToolchainConfigInfo build variable containing the necessary mapping information that can be used to write a toolchain feature that can solve this problem by using gcc's -fdebug-prefix-map. We can look into publishing it here, but it needs a little more refinement before it would be ready.

@burkpojken
Copy link

For our company it is also a big problem that even if you have all source code you cannot use the debug information without actually building the binaries your self. We have very much the same use case as stated in the request with engineers that do debugging of coredumps provided by customers.
So for us it would be a very valuable improvement if this was solved.

@c-mita c-mita added P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels Aug 28, 2020
@oquenchil oquenchil added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) and removed P2 We'll consider working on this in future. (Assignee optional) labels Nov 19, 2020
@github-actions
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 2+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Apr 12, 2023
@github-actions
Copy link

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team (@bazelbuild/triage). Thanks!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 27, 2023
@kolrami
Copy link

kolrami commented Sep 27, 2023

Hi @nacl and @burkpojken, I jus discovered this issue because I have the same situation for me. Is there some solution for this?

@burkpojken
Copy link

We stopped using strip_include_prefix and use includes instead, https://bazel.build/reference/be/c-cpp#cc_library.includes

@eguiraud
Copy link

This should really be reopened: afaict there is still no good way to generate a compile_commands.json that does not contain non-existing virtual_include paths to third-party dependency headers.

@eguiraud
Copy link

@bazelbuild/triage gentle ping

@jwnimmer-tri
Copy link
Contributor

... afaict there is still no good way ...

To be clear, the immediately prior message shows a solution for strip_include_prefix: use includes = ["foo"] instead of strip_include_prefix = "foo". It would help steer the discussion if you could explain what about that solution is "no good". That would help sharpen what the remaining problem(s) are here.

@eguiraud
Copy link

Sorry for the high latency -- the problem I was seeing turns out to be a bit more convoluted (more details here) and it is not strictly the fault of virtual_includes -- it's that when generating the compilation database the execroot directory in which those includes live is deleted/overwritten.

About using includes instead of strip_include_prefix: in my case the problem is with external dependencies and I am not sure what to put in the includes in that case. For example, what's the path to a googletest include directory when it is pulled in as a dependency?

@cpsauer
Copy link
Contributor

cpsauer commented Dec 14, 2023

^ indeed, not every _virtual_includes has an includes equivalent, sadly. Consider the case with just an include_prefix.

@ghost
Copy link

ghost commented Dec 19, 2023

One drawback with includes vs. {,strip_}include_prefix is that, with includes, headers are included with -isystem. GCC suppresses warnings when processing system headers.

For another, suppose the following:

  • Header is foo/bar/baz.h.
  • Bazel package is //foo/bar.
  • Canonical include statement is #include "bar/baz.h".

I can either use include_prefix = "bar" or includes = [".."]. The latter form is (a) unintuitive and (b) given past experience, is a little risky. (Misc Bazel impl details tend to assume the absence of, if not outright forbid, stray ..s.)

Given that, @nacl whipped up a patch for this that we've been using for a few years, and I'm currently rebasing it against Bazel 7. I'd be happy to turn this into a PR.

rbeasley added a commit to rbeasley/bazel that referenced this issue Jan 31, 2024
…lchain subsystem

Whenever `prefix` and/or `strip_include_prefix` is passed into
`cc_library` rules, Bazel copies the headers into a special
"_virtual_includes" directory that allows for such path transformations
to occur.  Unfortunately, this also has the side-effect of changing how
source files are designated within the source binaries: they will refer
to the file in the `_virtual_includes` sandbox directory instead of the
actual files in the build tree.

This change allows us to work around this problem by providing a mapping between
the "virtual" and where they are in the filesystem as a
[CcToolchainConfigInfo variable][1], `virtual_to_original_dirs`, which ends up
being a list of `<virtual>=<original>` mappings that can be used in rules_cc
toolchain features, specifically using [-ffile-prefix-map][2].

Questions
=========
- Is `<virtual>=<original>` okay, or do we need a more sophisticated lazy
  expansion involving subvariables like what `libraries_to_link` has?
  E.g., do we imagine another compiler command-line flag format that'd
  require `A:B` instead of `A=B`?  (Then again, considering MSVC's ASan
  is implemented w/ `/fsanitize=address`, maybe they're on the `A=B`
  train now?)

Notes
=====
- We mostly mimicked coverage's `virtual_to_original_headers`.

Fixes upstream bazelbuild#11874.

[1]: https://docs.bazel.build/versions/master/cc-toolchain-config-reference.html#cctoolchainconfiginfo-build-variables
[2]: https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html#index-ffile-prefix-map

Co-authored-by: Andrew Psaltis <apsaltis@vmware.com>
rbeasley added a commit to rbeasley/bazel that referenced this issue Jan 31, 2024
Whenever `prefix` and/or `strip_include_prefix` is passed into
`cc_library` rules, Bazel copies the headers into a special
"_virtual_includes" directory that allows for such path transformations
to occur.  Unfortunately, this also has the side-effect of changing how
source files are designated within the source binaries: they will refer
to the file in the `_virtual_includes` sandbox directory instead of the
actual files in the build tree.

This change allows us to work around this problem by providing a mapping between
the "virtual" and where they are in the filesystem as a
[CcToolchainConfigInfo variable][1], `virtual_to_original_dirs`, which ends up
being a list of `<virtual>=<original>` mappings that can be used in rules_cc
toolchain features, specifically using [-ffile-prefix-map][2].

Questions
=========
- Is `<virtual>=<original>` okay, or do we need a more sophisticated lazy
  expansion involving subvariables like what `libraries_to_link` has?
  E.g., do we imagine another compiler command-line flag format that'd
  require `A:B` instead of `A=B`?  (Then again, considering MSVC's ASan
  is implemented w/ `/fsanitize=address`, maybe they're on the `A=B`
  train now?)

Notes
=====
- We mostly mimicked coverage's `virtual_to_original_headers`.

Fixes upstream bazelbuild#11874.

[1]: https://docs.bazel.build/versions/master/cc-toolchain-config-reference.html#cctoolchainconfiginfo-build-variables
[2]: https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html#index-ffile-prefix-map

Co-authored-by: Andrew Psaltis <apsaltis@vmware.com>
rbeasley added a commit to rbeasley/bazel that referenced this issue Jan 31, 2024
Whenever `prefix` and/or `strip_include_prefix` is passed into
`cc_library` rules, Bazel copies the headers into a special
"_virtual_includes" directory that allows for such path transformations
to occur.  Unfortunately, this also has the side-effect of changing how
source files are designated within the source binaries: they will refer
to the file in the `_virtual_includes` sandbox directory instead of the
actual files in the build tree.

This change allows us to work around this problem by providing a mapping between
the "virtual" and where they are in the filesystem as a
[CcToolchainConfigInfo variable][1], `virtual_to_original_dirs`, which ends up
being a list of `<virtual>=<original>` mappings that can be used in rules_cc
toolchain features, specifically using [-ffile-prefix-map][2].

Questions
=========
- Is `<virtual>=<original>` okay, or do we need a more sophisticated lazy
  expansion involving subvariables like what `libraries_to_link` has?
  E.g., do we imagine another compiler command-line flag format that'd
  require `A:B` instead of `A=B`?  (Then again, considering MSVC's ASan
  is implemented w/ `/fsanitize=address`, maybe they're on the `A=B`
  train now?)

Notes
=====
- We mostly mimicked coverage's `virtual_to_original_headers`.

Fixes upstream bazelbuild#11874.

[1]: https://docs.bazel.build/versions/master/cc-toolchain-config-reference.html#cctoolchainconfiginfo-build-variables
[2]: https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html#index-ffile-prefix-map

Co-authored-by: Andrew Psaltis <apsaltis@vmware.com>
@bgianfo
Copy link

bgianfo commented Apr 28, 2024

@bazelbuild/triage does it make sense to re-open this, as there is an active PR trying to fix it?

@fmeum fmeum added not stale Issues or PRs that are inactive but not considered stale and removed stale Issues or PRs that are stale (no activity for 30 days) labels Apr 28, 2024
@fmeum fmeum reopened this Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not stale Issues or PRs that are inactive but not considered stale P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules type: feature request
Projects
None yet
Development

No branches or pull requests