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

--fission=yes alone does not generate real .dwp files #109

Open
4 tasks
rrbutani opened this issue Sep 27, 2021 · 2 comments
Open
4 tasks

--fission=yes alone does not generate real .dwp files #109

rrbutani opened this issue Sep 27, 2021 · 2 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@rrbutani
Copy link
Collaborator

rrbutani commented Sep 27, 2021

My understanding is that --fission is supposed to be a cross-platform way to generate separate debug info files for C/C++ rules (this is definitely stretching it a bit; I think --fission is intended to be .dwo/.dwp specific – that is, it's not supposed to generate .dSYMs on macOS and .pdbs on Windows).

In practice, --fission=yes alone does not generate non-empty .dwp files; --features=per_object_debug_info must also be specified.

This is because unix_cc_toolchain_config.bzl's cc_toolchain_config does not enable the per_object_debug_info feature by default.

Passing in --features=per_object_debug_info to enable it manually (i.e. in a .bazelrc) is sub-optimal because this breaks compilation on other platforms like macOS where -gsplit-dwarf has different behavior and will not produce .dwo files (on macOS debug info lives in the .o files – as in there is no .dwo equivalent – and running with -gsplit-dwarf when producing a binary will produce a .dSYM file, the .dwp equivalent).

It seems better to have the toolchain – which knows the platform it targets and has enough information to choose to enable this feature – handle this rather than to foist this onto users (afaik there isn't a create way to deal with this outside of the toolchain anyways; best I can come up with is build:linux --features=per_object_debug_info and then requiring --config=linux when building for Linux, etc.).

I think it's actually safe for us to enable the per_object_debug_info feature by default on Linux because it's effectively gated on fission:

steps

While we're still using unix_cc_toolchain_config.bzl there's not much we can do about this. These are notes for what to do when we eventually vendor in a version of unix_cc_toolchain_config.bzl (and potentially macOS and Windows host counterparts as well).

It's not clear yet whether this is doable/sensible but it'd be nice to have --fission (or some flag like it) cause rules_cc to generate split debug info for all the targets that support it. This would mean:

All in all, probably not pursuing. We should do the first thing though.

open questions

  • I'm confused about what the features associated with a cc_toolchain actually do.
    • they definitely don't enable those features by default for the associated toolchain
    • they don't seem to indicate what features a toolchain supports (i.e. the per_object_debug_info feature is not associated with the toolchains unix_cc_toolchain_config.bzl produces for macOS but if I enable the feature manually with --features=per_object_debug_info I can see that feature's flags being added to the commands executed)
    • they definitely do not influence toolchain resolution
    • cc_common.create_cc_toolchain_config_info ultimately is backed by ccToolchainConfigInfoFromStarlark but other than getting stored here I haven't really been able to tell what the features that are passed in get used for. I'll look into this more later.
rrbutani added a commit to fishcakez/bazel-toolchain that referenced this issue Sep 28, 2021
This has some things we'll eventually want to remove; see bazel-contrib#109 for some context.
rrbutani added a commit to fishcakez/bazel-toolchain that referenced this issue Sep 28, 2021
…ated with clang-12+

Issue bazel-contrib#109 and [this comment](bazel-contrib#108 (comment))
have some context; the short version is that `-gsplit-dwarf` no longer implies `-g2` which means under
`-c fastbuild` (implies `-g0`) no `.dwo` files are created.

There's a patch to `unix_cc_toolchain_config.bzl` but for now we'll just use `-c dbg` for this one
target using a transition.
rrbutani added a commit that referenced this issue Sep 28, 2021
* Fix including symlink extra files in dwp/objcopy

* Add strip filegroups to support strip inside sandbox

* tests: add a `.stripped` test

* tests: test that `.dwp` targets can be built

This has some things we'll eventually want to remove; see #109 for some context.

* tests: have `dwp_file` transition to `-c dbg` so .dwo files are generated with clang-12+

Issue #109 and [this comment](#108 (comment))
have some context; the short version is that `-gsplit-dwarf` no longer implies `-g2` which means under
`-c fastbuild` (implies `-g0`) no `.dwo` files are created.

There's a patch to `unix_cc_toolchain_config.bzl` but for now we'll just use `-c dbg` for this one
target using a transition.

* tests: migration fixes

Co-authored-by: Rahul Butani <rr.butani@gmail.com>
@keith
Copy link
Member

keith commented Feb 9, 2022

I submitted bazelbuild/bazel#14777 which solves one of the concerns above

@keith
Copy link
Member

keith commented Feb 9, 2022

Thanks for this write up! It was super useful when tracing back android fission support which I'm working on here bazelbuild/bazel#14765

@siddharthab siddharthab added bug Something isn't working help wanted Extra attention is needed labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants