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

Absolute paths are embedded in produced binaries #1000

Closed
kayasoze opened this issue Mar 2, 2016 · 30 comments
Closed

Absolute paths are embedded in produced binaries #1000

kayasoze opened this issue Mar 2, 2016 · 30 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-CPP Issues for C++ rules type: bug

Comments

@kayasoze
Copy link

kayasoze commented Mar 2, 2016

It would be useful to allow -fdebug-prefix-map to be used to create debug info that references a workspace's source files (e.g. /home/<user>/workspace/foo/bar.cc) rather than symlinked working tree sources (e.g. /private/var/tmp/<_bazel_<user>/4dfa01e59a69e8a99b4743b0270c4ad8/workspace/foo/bar.cc). This may require a CROSSTOOL substitution that referenced the current working tree, such that one could write:

compiler_flag: "-fdebug-prefix-map=%working_tree%=%workspace%"

Alternatively, Bazel could unconditionally add -fdebug-prefix-map to the list of compile options via its own special magic, since it seems a good idea to always reference the original sources in debug info.

Rationale

Even when invoked with relative paths, Clang's debug information will contain absolute paths to sources deep within the working tree, as above, even though those sources are, in fact, just symlinks back into the workspace. This confuses some tools, such as CLion, which attempt, and fail, to set breakpoints in lldb using workspace paths, not working tree paths. lldb only knows about the working tree sources, and doesn't recognize that these paths actually reference the same files, and thus ignores the breakpoint.

Newer versions of clang appear to support GCC's -fdebug-prefix-map, which will presumably solve this problem.

@ulfjack
Copy link
Contributor

ulfjack commented Mar 3, 2016

That's non-hermetic, and prevents any cross-user caching of the results. All the paths embedded into outputs should be relative; if that's not happening, that's a bug.

@kayasoze
Copy link
Author

kayasoze commented Mar 3, 2016

Certainly for genfile sources, at least, absolute paths into the working tree are clearly visible by running strings or symbols -fullSourcePath on an OX clang debug binary. It's possible there are multiple issues here.

@dslomov dslomov added P2 We'll consider working on this in future. (Assignee optional) type: feature request labels Mar 14, 2016
@ulfjack ulfjack added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed P2 We'll consider working on this in future. (Assignee optional) labels Jun 15, 2016
@ulfjack ulfjack added this to the 1.0 milestone Jun 15, 2016
@ulfjack ulfjack removed their assignment Jun 15, 2016
@philbinj
Copy link

Any update here? This is breaking build caching for us.

@hlopko
Copy link
Member

hlopko commented Dec 2, 2016

Hi James,

do you mean the absolute paths in the binaries? Could you open a new issue for that, maybe with a concrete repro instructions, so we can give it the attention it deserves?

Thanks!

@kayasoze kayasoze changed the title Support -fdebug-prefix-map Absolute paths are embedded in produced binaries Dec 2, 2016
@kayasoze
Copy link
Author

kayasoze commented Dec 2, 2016

I've renamed this bug, as relative paths would fix both the original issue and any extant caching issues.

@glinscott
Copy link

How does caching work at all without this? I guess all the machines that things are being built on have the same directory structure? This is definitely not the case for most users :).

@glinscott
Copy link

I have a prototype of this working now, it did require a small change to bazel though:

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
index 2304de3..bb8a721 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
@@ -369,6 +369,7 @@ public final class CppModel {
     CppCompilationContext builderContext = builder.getContext();
     CppModuleMap cppModuleMap = builderContext.getCppModuleMap();
     buildVariables.addVariable("output_file", builder.getOutputFile().getExecPathString());
+    buildVariables.addVariable("execution_root", this.configuration.getOutputDirectory().getPath().getParentDirectory().getParentDirectory().getPathString());
 
     if (featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAPS) && cppModuleMap != null) {
       // If the feature is enabled and cppModuleMap is null, we are about to fail during analysis

Then, I added this into my CROSSTOOL:

  # TODO(gary): WORKAROUND
  # Because we don't know how to apply a feature by default
  # other than the bazel source, replicate one that gets turned on by default
  # (the random_seed option), and then use the implies feature to ensure our
  # remove_debug_prefix is set.
  feature {
    name: 'random_seed'
    flag_set {
      action: 'preprocess-assemble'
      action: 'assemble'
      action: 'c-compile'
      action: 'c++-compile'
      action: 'c++-module-compile'
      flag_group {
        flag: '-frandom-seed=%{output_file}'
      }
    }
    implies: 'remove_debug_prefix'
  }

  feature {
    name: 'remove_debug_prefix'
    flag_set {
      action: 'preprocess-assemble'
      action: 'assemble'
      action: 'c-compile'
      action: 'c++-compile'
      action: 'c++-module-compile'
      flag_group {
        flag: '-fdebug-prefix-map=%{execution_root}=.'
        flag: '-gno-record-gcc-switches'
      }
    }
}

@lberki
Copy link
Contributor

lberki commented Jan 2, 2017

I concur with @ulfjack : %{execution_root} is not a good idea, because it is only meaningful on the system where Bazel is running, so if any actions are executed remotely, it breaks horribly because it'll be different for every user and also becuse the remote paths are probably not the same as the paths on the box where Bazel is running.

Unfortunately, the trivial fix, -fdebug-prefix-map=. doesn't work (I checked). So I don't really know how this could be fixed without adding a wrapper script around the compiler invocation :(

@glinscott
Copy link

I believe the wrapper script wouldn't be sufficient, as it would still need something similar to the execution root to be able to send into the debug-prefix-map flag. I think we'd need something similar to execution root that would work for remote workers. Is there any such concept?

@lberki
Copy link
Contributor

lberki commented Jan 4, 2017

Why do you need something like the execution root? If we map whatever the workspace root is to, say /workspace using -fdebug-prefix-map, that will produce identical binaries regardless of the location of the workspace root on the worker.

@kayasoze
Copy link
Author

kayasoze commented Jan 5, 2017

Indeed, it might be best to have Bazel add the appropriate flag value automatically, thus avoiding the need to expose the variable. If there is a need to disable this functionality, a field can be added to the CROSSTOOL proto.

@glinscott
Copy link

@lberki ah, okay. I think I misunderstood. Agreed, if we have bazel remap the workspace root automatically, that would be ideal.

@ulfjack
Copy link
Contributor

ulfjack commented Mar 9, 2017

I did some research to find out how we can fix this, and it does not look good. I checked ccache and distcc, and apparently they are rewriting the output files after compilation to remove the absolute paths:
http://jlebar.com/2010/3/21/ccache_3.0.html
https://lists.samba.org/archive/ccache/2011q1/000735.html
https://ccache.samba.org/manual.html

The tricky part about -fdebug-prefix-map is that we don't know the absolute path ahead of time in the case of remote execution. We can't use the local path, because then remote caching doesn't work. We could conceivably make it so remote execution runs in a fixed path, but then we can't cache between local and remote execution, unless we can (somehow) make local execution also use a fixed path, which isn't possible in all cases. Sandboxing locally helps, because we control the file system, but we can't rely on that at this point in time.

A wrapper script could rewrite the command line to pass the correct absolute path to -fdebug-prefix-map. Not nice, but I don't really see any other option. Am I missing something?

@lberki
Copy link
Contributor

lberki commented Mar 9, 2017

An alternative would be post-processing object files instead in a separate action?

I just told @mhlopko that I'd prefer post-processing because that doesn't assume that there is a bash shell wherever the compiler runs, but then again, post-processing has to know the format of the .o files, which is a bigger deal.

I guess a wrapper script is okay as long as we can come up with a way not to mandate it e.g. for Windows.

@ulfjack
Copy link
Contributor

ulfjack commented Mar 9, 2017

Note that this mainly affects MacOS right now, where we already use a wrapper script. Extending that to also handle -fdebug-prefix-map doesn't sound too bad. On Linux, we set PWD=/proc/self/cwd, which makes the outputs deterministic. On Windows, we need to check whether MSVC is even writing absolute paths, and whether it has options to suppress that. Or are you concerned about gcc / clang on Windows?

We could also ask whether upstream Clang could provide an option for this. It seems weird that multiple projects are working around it instead of getting a fix into upstream.

@lberki
Copy link
Contributor

lberki commented Mar 10, 2017

That's correct, I think.

@steeve
Copy link
Contributor

steeve commented Apr 17, 2018

I've opened a similar issue at #5031. We have a patch incoming for OSX binaries built on OSX (we patched wrapped_clang).

That said, this doesn't fix Android builds since this doesn't call wrapped_clang, so I believe the best approach may be to patch CppCompileAction and CppLinkAction.

While /proc/self/cwd does work when Bazel is run on Linux, when run on MacOS it doesn't work. See the new issue.

@steeve
Copy link
Contributor

steeve commented Apr 17, 2018

@steeve
Copy link
Contributor

steeve commented Nov 12, 2018

This is still a problem for Android builds on OSX.
Which a simple helloworld compiled with the android ndk:

$ bazel build //:main --crosstool_top=@androidndk//:toolchain-libcpp --cpu=arm64-v8a --host_crosstool_top=@bazel_tools//tools/cpp:toolchain -c dbg
$ dwarfdump bazel-bin/main
----------------------------------------------------------------------
 File: bazel-bin/main (183-1162626592)
----------------------------------------------------------------------
.debug_info contents:

0x00000000: Compile Unit: length = 0x00000047  version = 0x0004  abbr_offset = 0x00000000  addr_size = 0x08  (next CU at 0x0000004b)

0x0000000b: TAG_compile_unit [1] *
             AT_producer( "Android (4691093 based on r316199) clang version 6.0.2 (https://android.googlesource.com/toolchain/clang 183abd29fc496f55536e7d904e0abae47888fc7f) (https://android.googlesource.com/toolchain/llvm 34361f192e41ed6e4e8f9aca80a4ea7e9856f327) (based on LLVM 6.0.2svn)" )
             AT_language( DW_LANG_C99 )
             AT_name( "main.c" )
             AT_stmt_list( 0x00000000 )
    -------> AT_comp_dir( "/private/var/tmp/_bazel_steeve/b78b3b8a63bf1f81b6cb340ce2f2b555/sandbox/darwin-sandbox/1/execroot/__main__" )
            Unknown DW_AT constant: 0x2134( true )
             AT_low_pc( 0x00000000004005d8 )
             AT_high_pc( 0x0000000c )

0x0000002a:     TAG_subprogram [2]
                 AT_low_pc( 0x00000000004005d8 )
                 AT_high_pc( 0x0000000c )
                 AT_frame_base( reg31 )
                 AT_name( "main" )
        -------> AT_decl_file( "/Users/steeve/go/src/github.com/znly/bzl/main.c" )
                 AT_decl_line( 5 )
                 AT_type( {0x00000043} ( int ) )
                 AT_external( true )

0x00000043:     TAG_base_type [3]
                 AT_name( "int" )
                 AT_encoding( DW_ATE_signed )
                 AT_byte_size( 0x04 )

0x0000004a:     NULL

The first one is fixed by adding -fdebug-prefix-map, but somehow the second one isn't.

Also, it's preventing debugging from working on Android Studio + OSX.

@hlopko hlopko removed their assignment Nov 28, 2018
@dslomov dslomov removed the bazel 1.0 label Jul 24, 2019
@meisterT meisterT removed this from the 0.7 milestone May 12, 2020
@c-mita c-mita added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed P2 We'll consider working on this in future. (Assignee optional) labels Nov 24, 2020
@eagleshine
Copy link

eagleshine commented Apr 5, 2021

Encountered an issue recently with /proc/self/cwd in the source file path: the tool /usr/lib/rpm/debugedit from rpmbuild is not able to find the debug sources file list.

The relevant command is like: /usr/lib/rpm/debugedit -b /tmp/rpmbuild/BUILD -d /usr/src/debug -i -l /tmp/rpmbuild/BUILD/debugsources.list <path to the .so file>. Here debugedit is trying to replace the BUILD directory with /usr/src/debug and copy all sources files to /usr/src/debug.

  • /proc/self/cwd in the debug source file path leading to a 'source files not found' error
  • The original absolute path won't work either since it cannot be replaced by /usr/src/debug. The rpmbuild assumes that the source files reside in the BUILD directory of %_topdir.

So instead of fix the prefix to be /proc/self/cwd, can we provide a command-line option for this?

@ShreeM01 ShreeM01 added the stale Issues or PRs that are stale (no activity for 30 days) label Feb 16, 2023
@ShreeM01
Copy link
Contributor

Hi there! We're doing a clean up of old issues and will be closing this one. Please reopen if you’d like to discuss anything further. We’ll respond as soon as we have the bandwidth/resources to do so.

@ShreeM01 ShreeM01 closed this as not planned Won't fix, can't repro, duplicate, stale Feb 16, 2023
@aminya
Copy link

aminya commented May 30, 2023

Encountered an issue recently with /proc/self/cwd in the source file path: the tool /usr/lib/rpm/debugedit from rpmbuild is not able to find the debug sources file list.

The relevant command is like: /usr/lib/rpm/debugedit -b /tmp/rpmbuild/BUILD -d /usr/src/debug -i -l /tmp/rpmbuild/BUILD/debugsources.list <path to the .so file>. Here debugedit is trying to replace the BUILD directory with /usr/src/debug and copy all sources files to /usr/src/debug.

  • /proc/self/cwd in the debug source file path leading to a 'source files not found' error
  • The original absolute path won't work either since it cannot be replaced by /usr/src/debug. The rpmbuild assumes that the source files reside in the BUILD directory of %_topdir.

So instead of fix the prefix to be /proc/self/cwd, can we provide a command-line option for this?

I am having the same issue when using Boost stacktrce. The backtrace library prints /proc/self/cwd all over the place instead of the relative paths with ./. This breaks the go-to source code feature of IDEs like VsCode, which expect actually existing files instead of some /proc/self/cwd/file.

I'd also appreciate a CLI option that allows configuring this.
@kshyanashree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-CPP Issues for C++ rules type: bug
Projects
None yet
Development

No branches or pull requests