jankratochvil requested changes to this revision.
jankratochvil added inline comments.
This revision now requires changes to proceed.
================
Comment at: lldb/cmake/modules/FindDebuginfod.cmake:59
+endif()
\ No newline at end of file
----------------
"No newline at end of file", this is what saving this diff, git apply --index
and git diff says to me.
================
Comment at: lldb/include/lldb/Host/DebugInfoD.h:27
+// cached version of the file. If there was an error, we return that instead.
+llvm::Expected<std::string> findSource(const UUID &buildID,
+ const std::string &path);
----------------
Describe what does mean a returned `std::string("")` - that no error happened
but server does not know this UUID/path.
================
Comment at: lldb/source/Core/SourceManager.cpp:408
+ if ((!file_spec.GetDirectory() && file_spec.GetFilename()) ||
+ !FileSystem::Instance().Exists(m_file_spec)) {
// If this is just a file name, lets see if we can find it in the
----------------
I do not like this extra line as it changes behavior of LLDB unrelated to
`debuginfod`. IIUC if the source file with fully specified directory+filename
in DWARF does not exist but the same filename exists in a different directory
of the sourcetree LLDB will now quietly use the different file. That's a bug.
I think it is there as you needed to initialize `sc.module_sp`.
================
Comment at: lldb/source/Core/SourceManager.cpp:460
+ if (!FileSystem::Instance().Exists(m_file_spec) &&
+ debuginfod::isAvailable() && sc.module_sp) {
+ llvm::Expected<std::string> cache_path = debuginfod::findSource(
----------------
Make the `debuginfod::isAvailable()` check first as it is zero-cost,
`FileSystem::Instance().Exists` is expensive filesystem operation.
The problem with that `sc.module_sp` is it is initialized above with some side
effects. I think you should be fine without needing any `sc`. The following
code does not pass the testcase for me but I guess you may fix it better:
```
// Try finding the file using elfutils' debuginfod
if (!FileSystem::Instance().Exists(m_file_spec) &&
debuginfod::isAvailable())
target->GetImages().ForEach(
[&](const ModuleSP &module_sp) -> bool {
llvm::Expected<std::string> cache_path = debuginfod::findSource(
module_sp->GetUUID(), file_spec.GetCString());
if (!cache_path) {
module_sp->ReportWarning(
"An error occurred while finding the "
"source file %s using debuginfod for build ID %s: %s",
file_spec.GetCString(),
sc.module_sp->GetUUID().GetAsString("").c_str(),
llvm::toString(cache_path.takeError()).c_str());
} else if (!cache_path->empty()) {
m_file_spec = FileSpec(*cache_path);
m_mod_time =
FileSystem::Instance().GetModificationTime(*cache_path);
return false;
}
return true;
});
```
================
Comment at: lldb/source/Host/common/DebugInfoD.cpp:50
+ "invalid build ID: %s",
+ buildID.GetAsString("").c_str());
+
----------------
It should not be an error:
```
echo 'int main(void) { return 0; }' >/tmp/main2.c;gcc -o /tmp/main2
/tmp/main2.c -Wall -g -Wl,--build-id=none;rm
/tmp/main2.c;DEBUGINFOD_URLS=http://localhost:8002/ ./bin/lldb /tmp/main2 -o 'l
main' -o q
(lldb) target create "/tmp/main2"
Current executable set to '/tmp/main2' (x86_64).
(lldb) l main
warning: (x86_64) /tmp/main2 An error occurred while finding the source file
/tmp/main2.c using debuginfod for build ID A9C3D738: invalid build ID: A9C3D738
File: /tmp/main2.c
(lldb) q
```
================
Comment at: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp:103
+// TEST-3: File: /my/new/path/test.cpp
+// TEST-3: 123
+// TEST-3-NEXT: {{[0-9]+}} // Some context lines before
----------------
`s/123/{{[0-9]+}}/?`
================
Comment at: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp:136
+// the function.
\ No newline at end of file
----------------
"No newline at end of file", this is what saving this diff, git apply --index
and git diff says to me.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75750/new/
https://reviews.llvm.org/D75750
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
