clayborg added a comment.
In D133042#3791146 <https://reviews.llvm.org/D133042#3791146>, @yinghuitan
wrote:
> @clayborg , it is my intention to make `target.auto-deduce-source-map`
> boolean flag ultimately working for both relative paths and two full paths
> (two full paths are really important for off build host debugging, e.g. dump
> or production debugging). In this patch, I focuses on getting relative path
> working because that's the default behavior; a follow-up patch can get two
> full paths working.
> In my opinion boolean flag setting is dead simple to use (to enable both) and
> an enumeration setting would add extra barrier for adoption.
So I think we should have two settings if this is the case. Why? Because I
think the default values for "target.auto-deduce-source-map" should be true for
relative paths in debug info, but not true by default for two different full
paths in the debug info. We have no way to doing a great job at trying to match
up two different full paths. What if we have "/a/b/c/foo.cpp" and
"/d/e/f/foo.cpp". If this setting is enabled, I don't expect us to make an auto
source map between "/a/b/c" -> "/d/e/f". We would need to specify a minimum
directory depth and that gets really messy and involved more settings, and I
don't think the full path stuff should be enabled by default.
> I can add description to `target.auto-deduce-source-map` to explain it only
> works for relative paths.
So not sure if we need to rename this setting to something like
"target.auto-source-map-relative" to make this clear, and then this can and
should default to true. Then if we later add full path remapping we can use
"target.auto-source-map-absolute" and default it to false, and there will need
to be many other settings for directory depth and other things. I just think
there is too much that can go wrong with the auto remapping of full paths to
group them both into the same bucket. I would also rather teach the production
build toolchains to emit relative debug info and then never have to add the
really hard auto source maps for different paths. That is unless we expose the
DW_AT_comp_dir value in lldb_private::CompileUnit and can make a rock solid
auto path mapping tool from those settings.
================
Comment at: lldb/source/Breakpoint/BreakpointResolverFileLine.cpp:225
+ const bool case_sensitive = request_file.IsCaseSensitive();
+ const bool full = !request_file.GetDirectory().IsEmpty();
+ for (uint32_t i = 0; i < sc_list.GetSize(); ++i) {
----------------
If the directory is empty on the requested file, should we be doing anything
here? Can we early return?
================
Comment at: lldb/source/Breakpoint/BreakpointResolverFileLine.cpp:307
+ if (GetBreakpoint()->GetTarget().GetAutoDeduceSourceMap())
+ DeduceSourceMapping(sc_list);
----------------
See my inline comment below, but the BreakpointResolverFileLine is created with
a NULL for the breakpoint. Does the breakpoint get set properly prior any of
these calls? I am sure it must?
================
Comment at: lldb/source/Target/Target.cpp:405
BreakpointResolverSP resolver_sp(new BreakpointResolverFileLine(
- nullptr, offset, skip_prologue, location_spec));
+ nullptr, offset, skip_prologue, location_spec, removed_prefix_opt));
return CreateBreakpoint(filter_sp, resolver_sp, internal, hardware, true);
----------------
The breakpoint is initialized with NULL here. Does it get set to something
valid before we try to use it somehow? I am worried we won't be able to get a
target from the BreakpointResolver's stored breakpoint????
================
Comment at: lldb/source/Target/TargetProperties.td:40
Desc<"Source path remappings apply substitutions to the paths of source
files, typically needed to debug from a different host than the one that built
the target. The source-map property consists of an array of pairs, the first
element is a path prefix, and the second is its replacement. The syntax is
`prefix1 replacement1 prefix2 replacement2...`. The pairs are checked in
order, the first prefix that matches is used, and that prefix is substituted
with the replacement. A common pattern is to use source-map in conjunction
with the clang -fdebug-prefix-map flag. In the build, use
`-fdebug-prefix-map=/path/to/build_dir=.` to rewrite the host specific build
directory to `.`. Then for debugging, use `settings set target.source-map .
/path/to/local_dir` to convert `.` to a valid local path.">;
+ def AutoDeduceSourceMap: Property<"auto-deduce-source-map", "Boolean">,
+ DefaultFalse,
----------------
================
Comment at: lldb/source/Target/TargetProperties.td:41
+ def AutoDeduceSourceMap: Property<"auto-deduce-source-map", "Boolean">,
+ DefaultFalse,
+ Desc<"Automatically deduce source path mappings based on source file
breakpoint resolution. It only deduces source mapping if source file breakpoint
request is using full path.">;
----------------
================
Comment at: lldb/source/Target/TargetProperties.td:42
+ DefaultFalse,
+ Desc<"Automatically deduce source path mappings based on source file
breakpoint resolution. It only deduces source mapping if source file breakpoint
request is using full path.">;
def ExecutableSearchPaths: Property<"exec-search-paths", "FileSpecList">,
----------------
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133042/new/
https://reviews.llvm.org/D133042
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
